You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2018/08/02 00:01:01 UTC
[kudu-CR] [tools] run rebalancer during 'election storm'
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11107
Change subject: [tools] run rebalancer during 'election storm'
......................................................................
[tools] run rebalancer during 'election storm'
Added an integration test to run the rebalancer tool in an 'election
storm' environment. The rebalancer should run with no issues
unless a tablet server is reported as unavailable.
Change-Id: Ic98684dbe55049bbc411513faa0b6bbaef20f434
---
M src/kudu/tools/kudu-admin-test.cc
1 file changed, 154 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/11107/1
--
To view, visit http://gerrit.cloudera.org:8080/11107
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic98684dbe55049bbc411513faa0b6bbaef20f434
Gerrit-Change-Number: 11107
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
[kudu-CR] [tools] run rebalancer during 'election storm'
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Will Berkeley, Tidy Bot, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11107
to look at the new patch set (#2).
Change subject: [tools] run rebalancer during 'election storm'
......................................................................
[tools] run rebalancer during 'election storm'
Added an integration test to run the rebalancer tool in an 'election
storm' environment. The rebalancer should run with no issues
unless a tablet server is reported as unavailable.
I ran the test 250 times in each build configuration and didn't spot any
failures.
Change-Id: Ic98684dbe55049bbc411513faa0b6bbaef20f434
---
M src/kudu/tools/kudu-admin-test.cc
1 file changed, 155 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/11107/2
--
To view, visit http://gerrit.cloudera.org:8080/11107
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic98684dbe55049bbc411513faa0b6bbaef20f434
Gerrit-Change-Number: 11107
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] [tools] run rebalancer during 'election storm'
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/11107 )
Change subject: [tools] run rebalancer during 'election storm'
......................................................................
Patch Set 1:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/11107/1/src/kudu/tools/kudu-admin-test.cc
File src/kudu/tools/kudu-admin-test.cc:
http://gerrit.cloudera.org:8080/#/c/11107/1/src/kudu/tools/kudu-admin-test.cc@2086
PS1, Line 2086: #if defined(ADDRESS_SANITIZER) || defined(THREAD_SANITIZER)
: const auto timeout = MonoDelta::FromSeconds(5);
: #else
: const auto timeout = MonoDelta::FromSeconds(10);
> The timeout here is a time-to-run for the stormy elector thread as well. S
(thumbsup)
http://gerrit.cloudera.org:8080/#/c/11107/1/src/kudu/tools/kudu-admin-test.cc@2093
PS1, Line 2093: auto max_sleep_ms = 2.0;
> It was the lowest possible to be used in module expression.
What do you mean by "module expression"?
http://gerrit.cloudera.org:8080/#/c/11107/1/src/kudu/tools/kudu-admin-test.cc@2109
PS1, Line 2109: for (const auto& tablet : tablets) {
> Yes, that's what I want -- no matter how, but initiate as much elections as
(thumbsup)
http://gerrit.cloudera.org:8080/#/c/11107/1/src/kudu/tools/kudu-admin-test.cc@2164
PS1, Line 2164: usually happens because GetConsensusState requests are dropped due to
: // backpressure
> We should, and as soon as it's done in a separate changelist, I'm thinking
(thumbsup)
--
To view, visit http://gerrit.cloudera.org:8080/11107
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic98684dbe55049bbc411513faa0b6bbaef20f434
Gerrit-Change-Number: 11107
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Thu, 02 Aug 2018 21:28:35 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] run rebalancer during 'election storm'
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11107 )
Change subject: [tools] run rebalancer during 'election storm'
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11107/1/src/kudu/tools/kudu-admin-test.cc
File src/kudu/tools/kudu-admin-test.cc:
http://gerrit.cloudera.org:8080/#/c/11107/1/src/kudu/tools/kudu-admin-test.cc@2093
PS1, Line 2093: auto max_sleep_ms = 2.0;
> What do you mean by "module expression"?
Oh, that was a typo. I meant 'modulo' :)
And then there is std::min(x * 1.1, 2000.0) step.
I added corresponding comment into the code.
--
To view, visit http://gerrit.cloudera.org:8080/11107
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic98684dbe55049bbc411513faa0b6bbaef20f434
Gerrit-Change-Number: 11107
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Thu, 02 Aug 2018 21:50:13 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] run rebalancer during 'election storm'
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11107 )
Change subject: [tools] run rebalancer during 'election storm'
......................................................................
[tools] run rebalancer during 'election storm'
Added an integration test to run the rebalancer tool in an 'election
storm' environment. The rebalancer should run with no issues
unless a tablet server is reported as unavailable.
I ran the test 250 times in each build configuration and didn't spot any
failures.
Change-Id: Ic98684dbe55049bbc411513faa0b6bbaef20f434
Reviewed-on: http://gerrit.cloudera.org:8080/11107
Tested-by: Kudu Jenkins
Reviewed-by: Will Berkeley <wd...@gmail.com>
---
M src/kudu/tools/kudu-admin-test.cc
1 file changed, 155 insertions(+), 0 deletions(-)
Approvals:
Kudu Jenkins: Verified
Will Berkeley: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/11107
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic98684dbe55049bbc411513faa0b6bbaef20f434
Gerrit-Change-Number: 11107
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] [tools] run rebalancer during 'election storm'
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/11107 )
Change subject: [tools] run rebalancer during 'election storm'
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11107
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic98684dbe55049bbc411513faa0b6bbaef20f434
Gerrit-Change-Number: 11107
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Thu, 02 Aug 2018 23:05:30 +0000
Gerrit-HasComments: No
[kudu-CR] [tools] run rebalancer during 'election storm'
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/11107 )
Change subject: [tools] run rebalancer during 'election storm'
......................................................................
Patch Set 1:
(4 comments)
I'm guessing you already did, but if you haven't can you run this a bunch of times with dist-test to check it's not flaky, and add some stats about that to the commit msg?
http://gerrit.cloudera.org:8080/#/c/11107/1/src/kudu/tools/kudu-admin-test.cc
File src/kudu/tools/kudu-admin-test.cc:
http://gerrit.cloudera.org:8080/#/c/11107/1/src/kudu/tools/kudu-admin-test.cc@2086
PS1, Line 2086: #if defined(ADDRESS_SANITIZER) || defined(THREAD_SANITIZER)
: const auto timeout = MonoDelta::FromSeconds(5);
: #else
: const auto timeout = MonoDelta::FromSeconds(10);
I think you swapped the timeouts here because the sanitizer builds should have longer timeouts.
http://gerrit.cloudera.org:8080/#/c/11107/1/src/kudu/tools/kudu-admin-test.cc@2093
PS1, Line 2093: auto max_sleep_ms = 2.0;
Can you explain where this number comes from? Obviously it's super low compared to the expected time to an election start from a leader stepdown in an idle cluster of about 1.25 seconds, but the Raft heartbeat times are changed for the tests. A couple sentences explaining the initial setting and backoff would be good.
http://gerrit.cloudera.org:8080/#/c/11107/1/src/kudu/tools/kudu-admin-test.cc@2109
PS1, Line 2109: for (const auto& tablet : tablets) {
With RF=3, running over each TS and replica will cause 3 elections to be called on each tablet. Is that what you want? Or do you want one per tablet per cycle through all the tablets?
http://gerrit.cloudera.org:8080/#/c/11107/1/src/kudu/tools/kudu-admin-test.cc@2164
PS1, Line 2164: usually happens because GetConsensusState requests are dropped due to
: // backpressure
As you mentioned to me in person, we should fix ksck to be resilient to this up to some timeout.
--
To view, visit http://gerrit.cloudera.org:8080/11107
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic98684dbe55049bbc411513faa0b6bbaef20f434
Gerrit-Change-Number: 11107
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Thu, 02 Aug 2018 18:15:39 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] run rebalancer during 'election storm'
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11107 )
Change subject: [tools] run rebalancer during 'election storm'
......................................................................
Patch Set 1:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/11107/1/src/kudu/tools/kudu-admin-test.cc
File src/kudu/tools/kudu-admin-test.cc:
http://gerrit.cloudera.org:8080/#/c/11107/1/src/kudu/tools/kudu-admin-test.cc@2072
PS1, Line 2072: : RebalancingTest() {
> warning: initializer for base class 'kudu::tools::RebalancingTest' is redun
Done
http://gerrit.cloudera.org:8080/#/c/11107/1/src/kudu/tools/kudu-admin-test.cc@2086
PS1, Line 2086: #if defined(ADDRESS_SANITIZER) || defined(THREAD_SANITIZER)
: const auto timeout = MonoDelta::FromSeconds(5);
: #else
: const auto timeout = MonoDelta::FromSeconds(10);
> I think you swapped the timeouts here because the sanitizer builds should h
The timeout here is a time-to-run for the stormy elector thread as well. So no, I didn't swapped those. Making longer timeout for workload in case of TSAN/ASAN is not needed -- the code is written to tolerate timeouts, if any: having everything generated written into the tables is not required.
http://gerrit.cloudera.org:8080/#/c/11107/1/src/kudu/tools/kudu-admin-test.cc@2093
PS1, Line 2093: auto max_sleep_ms = 2.0;
> Can you explain where this number comes from? Obviously it's super low comp
It was the lowest possible to be used in module expression.
http://gerrit.cloudera.org:8080/#/c/11107/1/src/kudu/tools/kudu-admin-test.cc@2109
PS1, Line 2109: for (const auto& tablet : tablets) {
> With RF=3, running over each TS and replica will cause 3 elections to be ca
Yes, that's what I want -- no matter how, but initiate as much elections as possible, but implemented with easy to follow code.
http://gerrit.cloudera.org:8080/#/c/11107/1/src/kudu/tools/kudu-admin-test.cc@2164
PS1, Line 2164: usually happens because GetConsensusState requests are dropped due to
: // backpressure
> As you mentioned to me in person, we should fix ksck to be resilient to thi
We should, and as soon as it's done in a separate changelist, I'm thinking to update this part.
--
To view, visit http://gerrit.cloudera.org:8080/11107
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic98684dbe55049bbc411513faa0b6bbaef20f434
Gerrit-Change-Number: 11107
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Thu, 02 Aug 2018 20:53:26 +0000
Gerrit-HasComments: Yes