[kudu-CR] [test] Make the kudu-admin-test more rubust by adding AssertEventually

2019-08-15 Thread Alexey Serbin (Code Review) 
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14070 )

Change subject: [test] Make the kudu-admin-test more rubust by adding 
AssertEventually
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/14070/1//COMMIT_MSG@9
PS1, Line 9: Sometimes it's possible the scan will not immediately retrieve the 
row
   : even though the write has already succeeded, therefore add 
AssertEventually
   : to assure the scan return the right result. It will make the test 
case more robust.
> Agreed when the scan follows a drop range partition, but we don't need it e
+1

Since CountTableRows() use LEADER_ONLY scan mode and the prior write operation 
succeeds, the rows should be there.  Adding ASSERT_EVENTUALLY in such cases 
would mask a bug (if any appears).



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ied2993acaf35eef9d177f7e3cd1cd62ea9a1cdfb
Gerrit-Change-Number: 14070
Gerrit-PatchSet: 1
Gerrit-Owner: honeyhexin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 15 Aug 2019 16:40:10 +
Gerrit-HasComments: Yes

[kudu-CR] [test] Make the kudu-admin-test more rubust by adding AssertEventually

2019-08-15 Thread Adar Dembo (Code Review) 
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14070 )

Change subject: [test] Make the kudu-admin-test more rubust by adding 
AssertEventually
..


Patch Set 1:

(7 comments)

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

http://gerrit.cloudera.org:8080/#/c/14070/1//COMMIT_MSG@7
PS1, Line 7: rubust
robust


http://gerrit.cloudera.org:8080/#/c/14070/1//COMMIT_MSG@9
PS1, Line 9: Sometimes it's possible the scan will not immediately retrieve the 
row
   : even though the write has already succeeded, therefore add 
AssertEventually
   : to assure the scan return the right result. It will make the test 
case more robust.
Agreed when the scan follows a drop range partition, but we don't need it 
everywhere. For example, after a bunch of inserts with a flush, the rows are 
guaranteed to be read by the next scan.

I've left comments in the code with the locations of AssertEventually calls 
that we don't need.


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

http://gerrit.cloudera.org:8080/#/c/14070/1/src/kudu/tools/kudu-admin-test.cc@2394
PS1, Line 2394:   ASSERT_EVENTUALLY([&]() {
Or here.


http://gerrit.cloudera.org:8080/#/c/14070/1/src/kudu/tools/kudu-admin-test.cc@2434
PS1, Line 2434:   ASSERT_EVENTUALLY([&]() {
Or here.


http://gerrit.cloudera.org:8080/#/c/14070/1/src/kudu/tools/kudu-admin-test.cc@2535
PS1, Line 2535: ASSERT_EVENTUALLY([&]() {
Or here.


http://gerrit.cloudera.org:8080/#/c/14070/1/src/kudu/tools/kudu-admin-test.cc@2564
PS1, Line 2564: ASSERT_EVENTUALLY([&]() {
Or here.


http://gerrit.cloudera.org:8080/#/c/14070/1/src/kudu/tools/kudu-admin-test.cc@2847
PS1, Line 2847:   ASSERT_EVENTUALLY([&]() {
Don't need it here.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ied2993acaf35eef9d177f7e3cd1cd62ea9a1cdfb
Gerrit-Change-Number: 14070
Gerrit-PatchSet: 1
Gerrit-Owner: honeyhexin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 15 Aug 2019 16:34:05 +
Gerrit-HasComments: Yes

[kudu-CR] [test] Make the kudu-admin-test more rubust by adding AssertEventually

2019-08-15 Thread honeyhexin (Code Review) 
honeyhexin has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/14070


Change subject: [test] Make the kudu-admin-test more rubust by adding 
AssertEventually
..

[test] Make the kudu-admin-test more rubust by adding AssertEventually

Sometimes it's possible the scan will not immediately retrieve the row
even though the write has already succeeded, therefore add AssertEventually
to assure the scan return the right result. It will make the test case more 
robust.

Change-Id: Ied2993acaf35eef9d177f7e3cd1cd62ea9a1cdfb
---
M src/kudu/tools/kudu-admin-test.cc
1 file changed, 38 insertions(+), 19 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ied2993acaf35eef9d177f7e3cd1cd62ea9a1cdfb
Gerrit-Change-Number: 14070
Gerrit-PatchSet: 1
Gerrit-Owner: honeyhexin