Reporting: register a single ESQueue worker, simultaneous poll for all export types #32839
Conversation
tsullivan
added
v8.0.0
v7.2.0
review
(Deprecated) Feature:Reporting
tsullivan
force-pushed
the
fix/reporting/job-worker-polling
branch
from
e2b8128 to
169f791
Compare
| const conditions = get(body, conditionPath); | ||
| expect(conditions.must).to.eql({ term: { jobtype: jobtype } }); | ||
| }); | ||
|
|
There was a problem hiding this comment.
This test is no longer relevant, because each search should now turn up a waiting job for any type
| @@ -357,7 +362,6 @@ export class Worker extends events.EventEmitter { | |||
| filter: { | |||
| bool: { | |||
| minimum_should_match: 1, | |||
| must: { term: { jobtype: this.jobtype } }, | |||
There was a problem hiding this comment.
tsullivan
requested review from
joelgriffith,
kobelb,
stacey-gammon,
chrisdavies and
nreese
|
jenkins test this |
💚 Build Succeeded |
| * you may not use this file except in compliance with the Elastic License. | ||
| */ | ||
|
|
||
| // @ts-ignore |
There was a problem hiding this comment.
If you're ignoring due to the import being untyped, this should be:
// @ts-ignore untyped dependency
Otherwise, maybe add a comment about why these are ignored?
There was a problem hiding this comment.
Good to know! This is way more preferable, now it will complain if the module is not found.
I had a bad habit of this. I think I have a followup PR now to fix more of these :D
There was a problem hiding this comment.
oh I misunderstood: that isn't a syntax that causes just that one type of error to be ignored ts
| logger.debug(`Worker completed: (${res.job.id})`); | ||
| }); | ||
| worker.on(esqueueEvents.EVENT_WORKER_JOB_EXECUTION_ERROR, (res: any) => { | ||
| logger.debug(`Worker error: (${res.job.id})`); |
There was a problem hiding this comment.
I wonder if this should be higher than a debug message? Maybe not... could get spammy.
There was a problem hiding this comment.
Looks like these are redundant actually. It looks like the code usually logs a warning before firing one of the "bad" Esqueue events. We would probably not lose anything from removing these event handlers...
Probably best to leave them as debug for now, that was the original behavior since log was hardcoded to be debug in this file: https://github.com/elastic/kibana/blob/master/x-pack/plugins/reporting/server/lib/create_workers.js#L21
There was a problem hiding this comment.
yeah, there's a few events that get emitted/bound that no subscribers even listen to iirc.
💚 Build Succeeded |
| @@ -0,0 +1,99 @@ | |||
| /* | |||
There was a problem hiding this comment.
Thanks for going the distance on the tests!
…ll export types (elastic#32839) * Reporting: register a single ESQueue worker, simultaneous poll for all export types * more typescript * PLUGIN_ID constant * move down log / internal state * fix tests * jest test for createWorker * assert arguments to queue.registerWorker * logic move * make ts ignore specific * minor reversion to fix some esqueue worker tests
💔 Build Failed |
…ll export types (#32839) (#33339) * Reporting: register a single ESQueue worker, simultaneous poll for all export types * more typescript * PLUGIN_ID constant * move down log / internal state * fix tests * jest test for createWorker * assert arguments to queue.registerWorker * logic move * make ts ignore specific * minor reversion to fix some esqueue worker tests
Fixes #32089
Have a single ESQueue worker registered for Reporting. The single worker handles multiple export types by routing the claimed job details to the proper export type execute function, using a Map object (
jobExecutors)create_workers.jstocreate_worker.tsTODO: