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 2017/09/01 01:27:34 UTC
[kudu-CR] [periodic-test] fix flaky PeriodicTimerTest.TestReset
Alexey Serbin has uploaded a new change for review.
http://gerrit.cloudera.org:8080/7932
Change subject: [periodic-test] fix flaky PeriodicTimerTest.TestReset
......................................................................
[periodic-test] fix flaky PeriodicTimerTest.TestReset
Fixed flake in PeriodicTimerTest.TestReset. The test was a little bit
flaky because it might happen that the main test thread slept much more
than requested 1 ms, sometimes longer than the period of the task. In
such cases, the test failed with messages like the following:
src/kudu/rpc/periodic-test.cc:116: Failure
Expected: 0
To be equal to: counter_
Which is: 1
This fix adds the code which verifies that the actual sleep time of the
main tests thread was not greater than the specified threshold (1/2 of
task period). Also, the period of the task in non-TSAN modes has been
bumped up to 50 ms.
Prior to this fix if running the DEBUG build with --stress-cpu-threads=16,
the was about 1 failure in every 1K run:
http://dist-test.cloudera.org//job?job_id=aserbin.1504229055.4431
After the fix, there are no failures anymore:
http://dist-test.cloudera.org//job?job_id=aserbin.1504228829.30928
Change-Id: I6848bee0e830075aba3ae5d8ed49f99038043063
---
M src/kudu/rpc/periodic-test.cc
1 file changed, 13 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/32/7932/1
--
To view, visit http://gerrit.cloudera.org:8080/7932
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6848bee0e830075aba3ae5d8ed49f99038043063
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
[kudu-CR] [periodic-test] fix flaky PeriodicTimerTest.TestReset
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7932
to look at the new patch set (#3).
Change subject: [periodic-test] fix flaky PeriodicTimerTest.TestReset
......................................................................
[periodic-test] fix flaky PeriodicTimerTest.TestReset
Fixed flake in PeriodicTimerTest.TestReset. The test was a little bit
flaky because sometimes the main test thread might have slept for much
longer than requested, sometimes longer than the period of the task.
If that happened, the test failed with messages like the following:
src/kudu/rpc/periodic-test.cc:116: Failure
Expected: 0
To be equal to: counter_
Which is: 1
This fix just increases the period of the task across the board.
Prior to this fix, if running a DEBUG or ASAN build with
--stress-cpu-threads=64, there were about 2 failures every 1K run:
http://dist-test.cloudera.org//job?job_id=aserbin.1504248200.5336
After the fix there are no failures anymore:
http://dist-test.cloudera.org//job?job_id=aserbin.1504299298.23673
Change-Id: I6848bee0e830075aba3ae5d8ed49f99038043063
---
M src/kudu/rpc/periodic-test.cc
1 file changed, 8 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/32/7932/3
--
To view, visit http://gerrit.cloudera.org:8080/7932
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6848bee0e830075aba3ae5d8ed49f99038043063
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [periodic-test] fix flaky PeriodicTimerTest.TestReset
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged.
Change subject: [periodic-test] fix flaky PeriodicTimerTest.TestReset
......................................................................
[periodic-test] fix flaky PeriodicTimerTest.TestReset
Fixed flake in PeriodicTimerTest.TestReset. The test was a little bit
flaky because sometimes the main test thread might have slept for much
longer than requested, sometimes longer than the period of the task.
If that happened, the test failed with messages like the following:
src/kudu/rpc/periodic-test.cc:116: Failure
Expected: 0
To be equal to: counter_
Which is: 1
This fix just increases the period of the task across the board.
Prior to this fix, if running a DEBUG or ASAN build with
--stress-cpu-threads=64, there were about 2 failures every 1K run:
http://dist-test.cloudera.org//job?job_id=aserbin.1504248200.5336
After the fix there are no failures anymore:
http://dist-test.cloudera.org//job?job_id=aserbin.1504299298.23673
Change-Id: I6848bee0e830075aba3ae5d8ed49f99038043063
Reviewed-on: http://gerrit.cloudera.org:8080/7932
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
---
M src/kudu/rpc/periodic-test.cc
1 file changed, 8 insertions(+), 7 deletions(-)
Approvals:
Adar Dembo: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/7932
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I6848bee0e830075aba3ae5d8ed49f99038043063
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [periodic-test] fix flaky PeriodicTimerTest.TestReset
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: [periodic-test] fix flaky PeriodicTimerTest.TestReset
......................................................................
Patch Set 3: Code-Review+2
Two questions:
1. The new period also deflakes the test when looped with stress threads?
2. How much slower is a regular run of periodic-test now?
--
To view, visit http://gerrit.cloudera.org:8080/7932
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6848bee0e830075aba3ae5d8ed49f99038043063
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No
[kudu-CR] [periodic-test] fix flaky PeriodicTimerTest.TestReset
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: [periodic-test] fix flaky PeriodicTimerTest.TestReset
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7932/2/src/kudu/rpc/periodic-test.cc
File src/kudu/rpc/periodic-test.cc:
Line 44: #ifdef THREAD_SANITIZER
The other option is to just increase the period across the board. Todd recommended that at first; I pushed back because I wanted the test to run quickly. But maybe it's the better move.
--
To view, visit http://gerrit.cloudera.org:8080/7932
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6848bee0e830075aba3ae5d8ed49f99038043063
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] [periodic-test] fix flaky PeriodicTimerTest.TestReset
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [periodic-test] fix flaky PeriodicTimerTest.TestReset
......................................................................
Patch Set 3:
Thank you for the review.
Please see below for the answers.
> Two questions:
> 1. The new period also deflakes the test when looped with stress
> threads?
Yes, it does -- I ran different build configurations (DEBUG, RELEASE, ASAN, TSAN) with --stress-cpu-threads=64 for multiples of 1K, and found no flakes in any of my runs.
> 2. How much slower is a regular run of periodic-test now?
Prior to this fix, the test ran about 1 second for a DEBUG build. After the fix, it runs about 9 seconds.
--
To view, visit http://gerrit.cloudera.org:8080/7932
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6848bee0e830075aba3ae5d8ed49f99038043063
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No
[kudu-CR] [periodic-test] fix flaky PeriodicTimerTest.TestReset
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7932
to look at the new patch set (#2).
Change subject: [periodic-test] fix flaky PeriodicTimerTest.TestReset
......................................................................
[periodic-test] fix flaky PeriodicTimerTest.TestReset
Fixed flake in PeriodicTimerTest.TestReset. The test was a little bit
flaky because sometimes the main test thread might have slept for much
longer than requested, sometimes longer than the period of the task.
If that happened, the test failed with messages like the following:
src/kudu/rpc/periodic-test.cc:116: Failure
Expected: 0
To be equal to: counter_
Which is: 1
This fix adds code which short-circuits the test if the actual sleep
time was too long.
Prior to this fix, if running a DEBUG or ASAN build with
--stress-cpu-threads=32, the was about 2 failures in every 1K run:
http://dist-test.cloudera.org//job?job_id=aserbin.1504248200.5336
After the fix there are no failures anymore:
http://dist-test.cloudera.org//job?job_id=aserbin.1504248606.11445
Change-Id: I6848bee0e830075aba3ae5d8ed49f99038043063
---
M src/kudu/rpc/periodic-test.cc
1 file changed, 17 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/32/7932/2
--
To view, visit http://gerrit.cloudera.org:8080/7932
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6848bee0e830075aba3ae5d8ed49f99038043063
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [periodic-test] fix flaky PeriodicTimerTest.TestReset
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [periodic-test] fix flaky PeriodicTimerTest.TestReset
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7932/2/src/kudu/rpc/periodic-test.cc
File src/kudu/rpc/periodic-test.cc:
Line 44: #ifdef THREAD_SANITIZER
> The other option is to just increase the period across the board. Todd reco
Done
--
To view, visit http://gerrit.cloudera.org:8080/7932
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6848bee0e830075aba3ae5d8ed49f99038043063
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes