-
Notifications
You must be signed in to change notification settings - Fork 870
feat: [shard-distributor]Send "draining" heartbeat on executer shutdown #7505
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
feat: [shard-distributor]Send "draining" heartbeat on executer shutdown #7505
Conversation
Signed-off-by: Gaziza Yestemirova <[email protected]>
Signed-off-by: Gaziza Yestemirova <[email protected]>
Signed-off-by: Gaziza Yestemirova <[email protected]>
| executorID: "test-executor-id", | ||
| timeSource: timeSource, | ||
| } | ||
| } |
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.
Nice!
| t.Fatal("stopC should not be closed") | ||
| default: | ||
| } | ||
| } |
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 think we want it to be the same. In hindsight we should have just used context cancellation to stop.
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.
makes sense! let me update the test
Signed-off-by: Gaziza Yestemirova <[email protected]>
Signed-off-by: Gaziza Yestemirova <[email protected]>
Signed-off-by: Gaziza Yestemirova <[email protected]>
Signed-off-by: Gaziza Yestemirova <[email protected]>
| } | ||
|
|
||
| // Send the request | ||
| fmt.Println("sending the heartbeat+++") |
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 think you forgot some debug code
| // Send the request | ||
| fmt.Println("sending the heartbeat+++") | ||
| response, err := e.shardDistributorClient.Heartbeat(ctx, request) | ||
| fmt.Println("response", response, err) |
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 think you forgot some debug code
Signed-off-by: Gaziza Yestemirova <[email protected]>
What changed?
Why?
Shard-distributor should be notified about the executors that are shutting down to reassign the shards and keep track of ownership.
How did you test it?
unit-tests and local testing with etcd
Potential risks
Release notes
Documentation Changes