fix: Prevent Coordinator from stopping V1 executors #544
Conversation
morgsmccauley
changed the title
morgsmccauley
force-pushed
the
fix/coordinator-stops-v1-executor
branch
2 times, most recently
from
3c55ec6 to
0bdf08b
Compare
morgsmccauley
changed the title
morgsmccauley
force-pushed
the
fix/coordinator-stops-v1-executor
branch
from
0bdf08b to
9c8021b
Compare
| @@ -1,3 +1,5 @@ | |||
| #![cfg_attr(test, allow(dead_code))] | |||
There was a problem hiding this comment.
Since the impl is mocked in test builds, the actual implementation warns dead_code, as it isn't used with the tests. This suppresses these errors for test builds, code that is not used in normal builds will still warn.
| @@ -102,7 +102,7 @@ function getRunnerService (executors: Map<string, StreamHandler>, StreamHandlerT | |||
| config = { | |||
| account_id: accountId, | |||
| function_name: functionName, | |||
| version: -1, // Ensure Coordinator V2 sees version mismatch | |||
There was a problem hiding this comment.
As version is uint64 this ends up being a the largest possible 64 bit int.
There was a problem hiding this comment.
The changes look good to me!
We still need to add an allowlist block on Block Streamer, and also add logs to it (For when it successfully completes a start or stop command). We could do that in a separate PR but we just gotta be careful as once we deploy this PR and the triggers run, V2 coordinator will go and kick off every block streamer it can, which spikes Runner/Redis load. We would want to either implement the block streamer allowlist in this PR too, or be ready to action on dev. I'll leave it up to you!
Also, I haven't actually flushed the streams yet. Do you mind doing it? Sorry!
|
@darunrs Implementing the I'll add logging in a follow up PR :)
Yup I can do it :) |
As V1 executors are returned via Runner gRPC, Coordinator V2 will attempt to synchronise them. These executors should be completely ignored, within the synchronisation logic.