Skip to content
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

Handle leadership change and shutdown placement gracefully #2390

Merged
merged 13 commits into from
Nov 5, 2020
Merged

Handle leadership change and shutdown placement gracefully #2390

merged 13 commits into from
Nov 5, 2020

Conversation

youngbupark
Copy link
Contributor

@youngbupark youngbupark commented Nov 4, 2020

Description

This PR is to handle leadership establishment and revoke scenario properly and shutdown placement gracefully to reduce the impact to runtime and the other placement nodes.

  1. Add --cluster-bootstrap and --raft-logstore-path
  2. Handle leadership establishment safely
  3. Shutdown gRPC and Raft server gracefully
  4. log out all Raft logs as debug level

Run three placement nodes:

placement --port 50005 --id node0 --initial-cluster node0=127.0.0.1:15050,node1=127.0.0.1:15051,node2=127.0.0.1:15052 --raft-logstore-path log-node0 

placement --port 50006 --id node1 --initial-cluster node0=127.0.0.1:15050,node1=127.0.0.1:15051,node2=127.0.0.1:15052 --raft-logstore-path log-node1 --cluster-bootstrap=false

placement --port 50007 --id node2 --initial-cluster node0=127.0.0.1:15050,node1=127.0.0.1:15051,node2=127.0.0.1:15052 --raft-logstore-path log-node2 --cluster-bootstrap=false

Issue reference

We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.

Please reference the issue this PR will close: #2377

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

@youngbupark youngbupark changed the title Handling leadership change and shutdown placement gracefully [WIP] Handle leadership change and shutdown placement gracefully [WIP] Nov 4, 2020
@codecov
Copy link

codecov bot commented Nov 5, 2020

Codecov Report

Merging #2390 into master will increase coverage by 1.04%.
The diff coverage is 55.97%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2390      +/-   ##
==========================================
+ Coverage   55.55%   56.59%   +1.04%     
==========================================
  Files          79       80       +1     
  Lines        6698     6836     +138     
==========================================
+ Hits         3721     3869     +148     
+ Misses       2721     2709      -12     
- Partials      256      258       +2     
Impacted Files Coverage Δ
pkg/actors/actors.go 55.68% <0.00%> (+7.88%) ⬆️
pkg/actors/config.go 100.00% <ø> (ø)
pkg/placement/raft/logger.go 0.00% <0.00%> (ø)
pkg/placement/raft/server.go 0.00% <0.00%> (ø)
pkg/runtime/runtime.go 48.13% <0.00%> (-0.12%) ⬇️
pkg/placement/membership.go 43.58% <24.00%> (-31.85%) ⬇️
pkg/runtime/cli.go 3.84% <44.44%> (+3.84%) ⬆️
pkg/placement/placement.go 78.08% <65.00%> (-3.89%) ⬇️
pkg/actors/internal/placement.go 87.91% <87.91%> (ø)
pkg/runtime/config.go 100.00% <100.00%> (ø)
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 49e3e6f...a1ac031. Read the comment docs.

@youngbupark youngbupark changed the title Handle leadership change and shutdown placement gracefully [WIP] Handle leadership change and shutdown placement gracefully Nov 5, 2020
@youngbupark youngbupark marked this pull request as ready for review November 5, 2020 09:22
@youngbupark
Copy link
Contributor Author

/ok-to-test

@dapr-bot
Copy link
Collaborator

dapr-bot commented Nov 5, 2020

Found the available test cluster - dapr-aks-e2e-06 for linux. Please check the build status.

@dapr-bot
Copy link
Collaborator

dapr-bot commented Nov 5, 2020

Found the available test cluster - dapr-aks-e2e-08 for windows. Please check the build status.

@dapr-bot
Copy link
Collaborator

dapr-bot commented Nov 5, 2020

Congrats! All end-to-end tests have passed on linux. Thanks for your contribution!

@dapr-bot
Copy link
Collaborator

dapr-bot commented Nov 5, 2020

Congrats! All end-to-end tests have passed on windows. Thanks for your contribution!

@youngbupark youngbupark merged commit 6b40d3b into dapr:master Nov 5, 2020
@youngbupark youngbupark deleted the youngp/graceful-shutdown branch November 5, 2020 19:07
@@ -59,7 +61,9 @@ func newConfig() *config {

flag.StringVar(&cfg.raftID, "id", cfg.raftID, "Placement server ID.")
flag.StringVar(&cfg.raftPeerString, "initial-cluster", cfg.raftPeerString, "raft cluster peers")
flag.BoolVar(&cfg.raftBootStrap, "cluster-bootstrap", cfg.raftBootStrap, "raft cluster bootstrap required")
flag.BoolVar(&cfg.raftInMemEnabled, "inmem-store-enabled", cfg.raftInMemEnabled, "Enable in-memory log and snapshot store")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
flag.BoolVar(&cfg.raftInMemEnabled, "inmem-store-enabled", cfg.raftInMemEnabled, "Enable in-memory log and snapshot store")
flag.BoolVar(&cfg.raftInMemEnabled, "inmem-store-enabled", cfg.raftInMemEnabled, "Enable in-memory log and snapshot store. Can only work if --raftLogStorePath is unset.")

for {
select {
case isLeader := <-leaderCh:
switch {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will we anticipate more cases in this switch? Otherwise, change to if-else?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes will do that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Shutdown placement gracefully
4 participants