Ghost icon indicating copy to clipboard operation
Ghost copied to clipboard

Added experimental background job queue

Open 9larsons opened this issue 1 year ago • 9 comments

ref https://linear.app/tryghost/issue/ENG-1556/

  • added background job queue behind config flags
  • when enabled, is only used for the member email analytics updates in order to speed up the parent job, and take load off of the main process that is serving requests

9larsons avatar Sep 12 '24 02:09 9larsons

It looks like this PR contains a migration 👀 Here's the checklist for reviewing migrations:

General requirements

  • [x] Satisfies idempotency requirement (both up() and down())
  • [x] Does not reference models
  • [x] Filename is in the correct format (and correctly ordered)
  • [x] Targets the next minor version
  • [x] All code paths have appropriate log messages
  • [x] Uses the correct utils
  • [x] Contains a minimal changeset
  • [x] Does not mix DDL/DML operations
  • [x] Tested in MySQL and SQLite

Schema changes

  • [x] Both schema change and related migration have been implemented
  • [x] For index changes: has been performance tested for large tables
  • [x] For new tables/columns: fields use the appropriate predefined field lengths
  • [x] For new tables/columns: field names follow the appropriate conventions
  • [x] Does not drop a non-alpha table outside of a major version

Data changes

  • [ ] Mass updates/inserts are batched appropriately
  • [ ] Does not loop over large tables/datasets
  • [ ] Defends against missing or invalid data
  • [ ] For settings updates: follows the appropriate guidelines

github-actions[bot] avatar Sep 12 '24 02:09 github-actions[bot]

Need to add a couple integration tests then we ought to be ok. Not sure what the failure for Ghost-CLI is.

9larsons avatar Sep 16 '24 20:09 9larsons

It looks like there's an error that's preventing the persisted logs from being printed 🙃 Here's the culprint:

Ghost was able to start, but errored during boot with: Cannot find module 'workerpool'

Looking at the lockfile changes, I'm not sure if they're intentional - there are a lot of new dependencies, which I think might stem from an older version of @tryghost/errors Also, the real issue is that it looks like workerpool wasn't added as a dependency?

vikaspotluri123 avatar Sep 17 '24 00:09 vikaspotluri123

Ok, clearly there's some kind of race condition with the config mocks and initializing the service. I will have to look into that when I can reprioritize this.

9larsons avatar Sep 20 '24 11:09 9larsons

@vikaspotluri123 Hey, it seems like the ghost-cli tests are hanging on restart with this. Do you have any suggestions on how to repro/troubleshoot that? Only thing I can guess is that the job queue service is hanging. It doesn't do that to me locally.

9larsons avatar Oct 17 '24 16:10 9larsons

Note to self: need to update migration to new version before merging.

9larsons avatar Oct 17 '24 16:10 9larsons

It took me a minute, but taking a look 👀

https://github.com/vikaspotluri123/Ghost/actions/runs/11393243539/job/31701108553?pr=14

vikaspotluri123 avatar Oct 17 '24 21:10 vikaspotluri123

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index eec2c09aeda..d68f13ce3a2 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -861,7 +861,8 @@ jobs:
 
       - name: Update from v4
         run: |
-          ghost update -f -d $V4_DIR --archive $(pwd)/ghost/core/ghost.tgz
+          ghost update -f -d $V4_DIR --archive $(pwd)/ghost/core/ghost.tgz --no-restart
+          ghost run -d $V4_DIR -D
 
       - name: Save Ghost CLI Debug Logs
         if: failure()
@@ -879,7 +880,8 @@ jobs:
         run: |
           DIR=$(mktemp -d)
           ghost install local -d $DIR
-          ghost update -d $DIR --archive $(pwd)/ghost/core/ghost.tgz
+          ghost update -d $DIR --archive $(pwd)/ghost/core/ghost.tgz --no-restart
+          ghost run -d $DIR -D
 
       - name: Print debug logs
         if: failure()

I hacked the job like that and found this

[2024-10-18 00:32:03] ERROR Unhandled rejection: insert into `jobs` (`created_at`, `finished_at`, `id`, `metadata`, `name`, `queue_entry`, `started_at`, `status`, `updated_at`) values ('2024-10-18 00:32:03', NULL, '6711ac8389d4880ed0fcad4e', NULL, 'members-migrations', NULL, NULL, 'queued', '2024-10-18 00:32:03') - SQLITE_ERROR: table jobs has no column named metadata

insert into `jobs` (`created_at`, `finished_at`, `id`, `metadata`, `name`, `queue_entry`, `started_at`, `status`, `updated_at`) values ('2024-10-18 00:32:03', NULL, '6711ac8389d4880ed0fcad4e', NULL, 'members-migrations', NULL, NULL, 'queued', '2024-10-18 00:32:03') - SQLITE_ERROR: table jobs has no column named metadata
Error Code: 
    SQLITE_ERROR
----------------------------------------
Error: insert into `jobs` (`created_at`, `finished_at`, `id`, `metadata`, `name`, `queue_entry`, `started_at`, `status`, `updated_at`) values ('2024-10-18 00:32:03', NULL, '6711ac8389d4880ed0fcad4e', NULL, 'members-migrations', NULL, NULL, 'queued', '2024-10-18 00:32:03') - SQLITE_ERROR: table jobs has no column named metadata

[2024-10-18 00:32:03] INFO Ghost URL Service Ready in 3.509s

vikaspotluri123 avatar Oct 18 '24 00:10 vikaspotluri123

Ah awesome, thank you. I suppose that's a good thing! I knew the migration needed updating but it's nice to know that it'll fail in CI. Should've thought of that.

9larsons avatar Oct 18 '24 21:10 9larsons