You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Adar Dembo (Code Review)" <ge...@cloudera.org> on 2019/03/26 06:17:24 UTC

[kudu-CR] ksck remote-test: deflake some more

Hello Will Berkeley, Alexey Serbin,

I'd like you to do a code review. Please visit

    http://gerrit.cloudera.org:8080/12853

to review the following change.


Change subject: ksck_remote-test: deflake some more
......................................................................

ksck_remote-test: deflake some more

Apparently fixing KUDU-2748 wasn't enough, and there are still some cases
where the consensus state isn't the same across all masters after they are
restarted. Maybe this will do the trick?

Change-Id: I2cc6c98f91703b52dbf099b1f544a2c70099f35b
---
M src/kudu/tools/ksck_remote-test.cc
1 file changed, 1 insertion(+), 0 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/12853/1
-- 
To view, visit http://gerrit.cloudera.org:8080/12853
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2cc6c98f91703b52dbf099b1f544a2c70099f35b
Gerrit-Change-Number: 12853
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] ksck remote-test: deflake some more

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Hello Will Berkeley, Alexey Serbin, Kudu Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/12853

to look at the new patch set (#2).

Change subject: ksck_remote-test: deflake some more
......................................................................

ksck_remote-test: deflake some more

Apparently fixing KUDU-2748 wasn't enough, and there are still some cases
where the consensus state isn't the same across all masters after they are
restarted, likely due to KUDU-2709. Maybe this will do the trick?

Change-Id: I2cc6c98f91703b52dbf099b1f544a2c70099f35b
---
M src/kudu/tools/ksck_remote-test.cc
1 file changed, 1 insertion(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/12853/2
-- 
To view, visit http://gerrit.cloudera.org:8080/12853
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2cc6c98f91703b52dbf099b1f544a2c70099f35b
Gerrit-Change-Number: 12853
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] ksck remote-test: deflake some more

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12853 )

Change subject: ksck_remote-test: deflake some more
......................................................................


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/12853/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/12853/1//COMMIT_MSG@9
PS1, Line 9: some cases
           : where the consensus state isn't the same across all masters after they are
           : restarted
> Do you have a consensus matrix describing how they differ?
Not a consensus matrix, but take a look at http://dist-test.cloudera.org:8080/diagnose?key=704dfc44-4fd0-11e9-8819-0242ac110002. Looks like the added tserver came up during a master election, so I can see why the masters would disagree on the cstate when queried. I don't see anything in the ksck code that retries cstate fetching during an election.

I believe this is basically KUDU-2709.


http://gerrit.cloudera.org:8080/#/c/12853/1//COMMIT_MSG@11
PS1, Line 11: Maybe this will do the trick?
> When I fixed 2748 my testing showed the flakiness went down to 6/2000 failu
I looped before and after. I only got 2 failures after, but also only 2 before, which isn't representative of the current flakiness rate as per the flaky test dashboard (nor, anecdotally, of the rate that we seem to still be seeing this failure in precommit).


http://gerrit.cloudera.org:8080/#/c/12853/1/src/kudu/tools/ksck_remote-test.cc
File src/kudu/tools/ksck_remote-test.cc:

http://gerrit.cloudera.org:8080/#/c/12853/1/src/kudu/tools/ksck_remote-test.cc@541
PS1, Line 541: n
> N
Done


http://gerrit.cloudera.org:8080/#/c/12853/1/src/kudu/tools/ksck_remote-test.cc@541
PS1, Line 541: cstate
> .
The Google Style Guide permits comments in a line of code to be less formal[1], which is why I opted for a fragment here. I'll add the capitalization and period, though.

1. https://google.github.io/styleguide/cppguide.html#Punctuation,_Spelling_and_Grammar



-- 
To view, visit http://gerrit.cloudera.org:8080/12853
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2cc6c98f91703b52dbf099b1f544a2c70099f35b
Gerrit-Change-Number: 12853
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Tue, 26 Mar 2019 20:51:14 +0000
Gerrit-HasComments: Yes

[kudu-CR] ksck remote-test: deflake some more

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12853 )

Change subject: ksck_remote-test: deflake some more
......................................................................

ksck_remote-test: deflake some more

Apparently fixing KUDU-2748 wasn't enough, and there are still some cases
where the consensus state isn't the same across all masters after they are
restarted, likely due to KUDU-2709. Maybe this will do the trick?

Change-Id: I2cc6c98f91703b52dbf099b1f544a2c70099f35b
Reviewed-on: http://gerrit.cloudera.org:8080/12853
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
M src/kudu/tools/ksck_remote-test.cc
1 file changed, 1 insertion(+), 0 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Alexey Serbin: Looks good to me, approved

-- 
To view, visit http://gerrit.cloudera.org:8080/12853
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I2cc6c98f91703b52dbf099b1f544a2c70099f35b
Gerrit-Change-Number: 12853
Gerrit-PatchSet: 3
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] ksck remote-test: deflake some more

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/12853 )

Change subject: ksck_remote-test: deflake some more
......................................................................


Patch Set 2: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/12853
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2cc6c98f91703b52dbf099b1f544a2c70099f35b
Gerrit-Change-Number: 12853
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Tue, 26 Mar 2019 21:27:13 +0000
Gerrit-HasComments: No

[kudu-CR] ksck remote-test: deflake some more

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/12853 )

Change subject: ksck_remote-test: deflake some more
......................................................................


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/12853/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/12853/1//COMMIT_MSG@11
PS1, Line 11: Maybe this will do the trick?
When I fixed 2748 my testing showed the flakiness went down to 6/2000 failures. Does this change reduce the flakiness more?


http://gerrit.cloudera.org:8080/#/c/12853/1//COMMIT_MSG@9
PS1, Line 9: some cases
           : where the consensus state isn't the same across all masters after they are
           : restarted
Do you have a consensus matrix describing how they differ?


http://gerrit.cloudera.org:8080/#/c/12853/1/src/kudu/tools/ksck_remote-test.cc
File src/kudu/tools/ksck_remote-test.cc:

http://gerrit.cloudera.org:8080/#/c/12853/1/src/kudu/tools/ksck_remote-test.cc@541
PS1, Line 541: cstate
.


http://gerrit.cloudera.org:8080/#/c/12853/1/src/kudu/tools/ksck_remote-test.cc@541
PS1, Line 541: n
N



-- 
To view, visit http://gerrit.cloudera.org:8080/12853
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2cc6c98f91703b52dbf099b1f544a2c70099f35b
Gerrit-Change-Number: 12853
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Tue, 26 Mar 2019 20:31:32 +0000
Gerrit-HasComments: Yes