Things to check that I actually implemented (from my notes):

• Nudge on new job notification.
• Must release jobs in the queue if held for greater than a certain period of time (default 5 minutes?)
• Must fetch the next batch even if current batch is ongoing if we know we cannot possibly get enough results
• getJob should return a defer if no jobs currently available
• Must perform polling only when no fetches are in progress and there are no jobs in the queue and there is at least one pending defer
• on pool release, defers must all be resolved to undefined
• When the last job is assigned, automatically fetch the next batch. Then set up polling. Polling should be interrupted if a new job announcement is received.
• Only refetch when the queue is emptied if the previous fetch was at max limit (i.e. 4 jobs for 4 slots).
• Do not fetch on "new job" announcement unless job cache is empty
• Timer is active if workers are waiting (deferreds)
• Reset timer after fetching jobs
• New job announcment triggers job fetch when workers waiting
• Timer is active if and only if queued jobs is zero
• localQueueSize
• localQueueTTL
• Rate limit getJob when the queue is empty to avoid overwhelming the database; make this configurable

Also:

• Make pro compatible
• Revisit ttlExpiredTimer - perhaps this should persist because some of the jobs may have been in the local queue for ages even if the odd worker has completed from time to time.
• Create tests for refetchDelay code

To enable feedback on this PR, I've released it as a canary; install via graphile-worker@canary. Please note that this PR does not have tests or documentation, so I would not recommend using it in a production environment - try it in staging and see if it fixes your performance issues (when configured to do so).

(Also judging by the sporadic CI failures, there's clearly some kind of bug/timing issue somewhere.)

hey, after some basic testing i decided to just swing it and deployed graphile worker canary to prod while closely monitoring. we are mainly exporting jobs and they are deduped so not a lot can go wrong.

results are really awesome. improvements pretty much all over the board:

• cpu usage of the db dropped from 40+ to 20
• time consumed by the job query is reduced from 97% to 49%
• job latency (run_at until its done executing) dropped from 3 seconds at peak to around 500ms

current settings, choose randomly really...

    preset: {
      extends: [WorkerProPreset],
      worker: {
        connectionString: DB_CONNECTION_STRING,
        schema: 'graphile_worker',
        sweepThreshold: 600000, // 10 minutes
        concurrentJobs: 24,
        // pollInterval is only relevant for scheduled jobs (after now()) and retries
        pollInterval: 120_000,
        maxPoolSize: 24,
        localQueueSize: 24,
        completeJobBatchDelay: 100, // milliseconds
        failJobBatchDelay: 100, // milliseconds
      },
    },

some visuals:

@benjie after two weeks we just had our first crash. here are the logs:

Failed to release job '17207078' after success\; committing seppukuCannot read properties of null (reading 'message')

Worker exited, but pool is in continuous mode, is active, and is not shutting down... Did something go wrong?

Worker exited with error: TypeError: Cannot read properties of null (reading 'message')

Failed to return jobs from local queue to database queue

Thanks for the report of the crash! There's clearly an issue preventing the tests from reliably passing too, I suspect some kind of race condition has slipped in.

Did these errors not have stack traces? If they have stack traces, that would significantly help in tracking down the issue!

This is the closest I can get to a stack trace:

Given the error is coming from here:

Could you just coalesce the entryResult.reason a few lines up:
entryResult.reason ?? 'No reason provided'
Or something similar, check if message in the e etc. The promise must have been rejected with null as the reason. Also if the promise is rejected with no reason it will also be undefined, so It's not really safe to assume e is an object.

Although this doesn't actually help find out the real reason the promise rejected. Is it possible your task executor is calling 'reject(null)' on the promise?

@hillac I've fixed that line by adding a ? - you're right that there's nothing forcing the rejection reason to be an object/error. (This may well be an issue that the task itself is rejecting with null.)

How did you determine that was the correct location?

We still seem to have an issue prevent CI passing reliably.

New release contains the ? fix:

[email protected]

@psteinroe's stack trace has the line number, worker.js:222:43. You can look in the node modules path the stack trace shows to see the js code. Or here.

stack trace has the line number, worker.js:222:43

I read worker.js as main.js again 🤦‍♂️ I definitely need more sleep! 😴

Hooray; seems that the instability in this PR was actually in the test framework rather than Worker itself. 4 passes in a row suggests I have stabilized this now, plus the tests are now faster (locally, not in CI) because they run in parallel, and they're independent of each other since they run on clean databases.

Latest canary released: [email protected]

I've extracted the following PRs from this PR so that it's simpler to review:

• New test framework, extracted from #474 #506
• Extract perfTest enhancements from #474 #507

The latest canary is out and it contains a number of fixes to other parts of Worker.

Error handling needs testing, in particular I'm not sure we have any tests that touch returnJobs

The latest canary is available, along with initial support for the pro plugin:

[email protected]
@graphile-pro/[email protected]