-
Notifications
You must be signed in to change notification settings - Fork 15
Add sequencer id to job payloads #339
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
8e112ff to
ddf7607
Compare
packages/sequencer/test/protocol/production/flow/ReductionTaskFlow.test.ts
Outdated
Show resolved
Hide resolved
removed .vscode/launch.json
fcd0764 to
19c7716
Compare
rpanic
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved given fix of the one new comment
|
|
||
| // Drain all task queues to clear stale tasks from previous sequencer instances | ||
| if (this.container.isRegistered("TaskQueue")) { | ||
| const taskQueue = this.container.resolve<TaskQueue>("TaskQueue"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought I added a comment here but apparently didn't (please say if we already talked about it).
But I'd much rather see this logic here implemented in the start() function that the TaskQueue implements (SequencerModule requires that anyways, and it is automatically called by the sequencer during startup)
6ba3389 to
045455f
Compare
This PR adds a unique
sequencer.idto a sequencer instance and includes it in all task payloads.So the Flow class can now filters out responses from old sequencer instances.
Fixes #314