preliminary support for a job queue #1212
Conversation
paulfitz
force-pushed
the
paulfitz/q-test
branch
from
74d8a0a
to
7de3438
Compare
Grist has needed a job queue for some time. This adds one, using BullMQ. BullMQ however requires Redis, meaning we couldn't use jobs for the large subset of Grist that needs to be runnable without Redis (e.g. for use on desktop, or on simple self-hosted sites). So simple immediate, delayed, and repeated jobs are supported also in a crude single-process form when Redis is not available. This code isn't ready for actual use since an important issue remains to be worked out, specifically how to handle draining the queue during deployments to avoid mixing versions (or - if allowing mixed versions - thinking through any extra support needed for the developer to avoid introducing hard-to-test code paths).
paulfitz
force-pushed
the
paulfitz/q-test
branch
from
47189c5
to
8e1301d
Compare
package.json
Outdated
| @@ -128,6 +128,7 @@ | |||
| "bootstrap": "3.4.1", | |||
| "bootstrap-datepicker": "1.9.0", | |||
| "bowser": "2.7.0", | |||
| "bullmq": "^5.8.7", | |||
There was a problem hiding this comment.
| "bullmq": "^5.8.7", | |
| "bullmq": "5.8.7", |
|
|
||
| // Clean up any jobs left over from previous round of tests. | ||
| beforeEach(async function() { | ||
| const jobs = new GristBullMQJobs(); |
There was a problem hiding this comment.
Can you add a comment here? I think this is needed only when we run it with redis, am I right? For in memory this doesn't do anything?
There was a problem hiding this comment.
That's right. Added to comment.
| } | ||
| }); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Do we need a test like this one? That checks what happens with long running tasks? Or async jobs that throw errors?
The callback we use is not awaited anywhere, maybe the BullMq library handles it properly, but I don't think that our in-memory implementation deals with it.
it('can stop when job is in the middle', async function() {
const jobs: GristJobs = new GristBullMQJobs();
const q = jobs.queue();
let started = false;
try {
q.handleDefault(async () => {
started = true;
await delay(2000);
started = false;
});
await q.add('', null, {delay: 500});
await delay(600);
assert.isTrue(started);
} finally {
await jobs.stop({obliterate: true});
assert.isFalse(started);
}
});There was a problem hiding this comment.
So long running tasks are a problem since BullMQ cancelation relies on non-open source features:
https://docs.bullmq.io/bullmq-pro/observables/cancelation
And for the Grist Labs SaaS, we are very restricted on how long we can wait for workers to shut down because of ECS/Fargate requirements.
This for me is related to the big operational question mark about how to properly drain queues as workers are replaced, which we will have to deal with. There will be more work to do here.
There was a problem hiding this comment.
I added a note about this in GristJobs.
Context
Grist has needed a job queue for some time. This adds one, using BullMQ. BullMQ however requires Redis, meaning we couldn't use jobs for the large subset of Grist that needs to be runnable without Redis (e.g. for use on desktop, or on simple self-hosted sites). So simple immediate, delayed, and repeated jobs are supported also in a crude single-process form when Redis is not available.
This code isn't ready for actual use since an important issue remains to be worked out, specifically how to handle draining the queue during deployments to avoid mixing versions (or - if allowing mixed versions - thinking through any extra support needed for the developer to avoid introducing hard-to-test code paths).
Has this been tested?