Adar Dembo has posted comments on this change.

Change subject: [tools] Implement a manual leader_step_down for a tablet

Patch Set 6:

File src/kudu/tools/

PS6, Line 219: MonoDelta::FromMilliseconds(timeout_ms)
Why do you need to override the default timeout value of 30s? timeout_ms is 
almost assuredly going to be less.

PS6, Line 222: while (current_leader == new_leader && --num_retries > 0);
There's no difference in the outcome of the test if we retry 10 times without a 
change in leader, or if the leader does change. Either way, the test will pass. 
Put another way, if there was a bug in leader_step_down that prevented the 
leader from actually  changing, this test would not catch that.

As such, I don't see the point of the retry loop. Just try the outer loop logic 
once, looping until the term changes, and then end the test.

The alternative would be to run the outer loop extensively and enforce that the 
leader really has changed (either by asserting that new_leader!=current_leader, 
or by asserting that there's no leader at all), but it didn't sound like you 
were comfortable doing that.

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia046a28a2008f4f5d1e955f57752a32a1ddc5ab8
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat <>
Gerrit-Reviewer: Adar Dembo <>
Gerrit-Reviewer: Dan Burkert <>
Gerrit-Reviewer: David Ribeiro Alves <>
Gerrit-Reviewer: Dinesh Bhat <>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <>
Gerrit-HasComments: Yes

Reply via email to