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 2021/02/25 06:10:59 UTC

[kudu-CR] [client-test] added WriteWhileRestartingMultipleTabletServers

Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17120


Change subject: [client-test] added WriteWhileRestartingMultipleTabletServers
......................................................................

[client-test] added WriteWhileRestartingMultipleTabletServers

While working on tests for multi-row transactions, I added a similar
scenario and found it a bit flaky.  So, I wanted to see if the same
is true for the non-transactional write operations.  It turns out
the latter is pretty stable, so I need to dig in to find the root
cause of the former.  Anyways, I think this is a good scenario to
add into client-test.cc, extending already existing scenario
ClientTest.TestWriteWhileRestarting to multi-replica case and going
through multiple restarts.  Another important detail is that the newly
added test scenario verifies the number of persisted rows in the end.

I also did a small touch-up of the code related to the utility method
ClientTest::CountRowsFromClient().

Change-Id: I95d1456dea2e6e2bb7d8b0c5d05e95798098710d
---
M src/kudu/client/client-test.cc
1 file changed, 105 insertions(+), 42 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I95d1456dea2e6e2bb7d8b0c5d05e95798098710d
Gerrit-Change-Number: 17120
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>

[kudu-CR] [client-test] added WriteWhileRestartingMultipleTabletServers

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has removed a vote on this change.

Change subject: [client-test] added WriteWhileRestartingMultipleTabletServers
......................................................................


Removed Verified-1 by Kudu Jenkins (120)
-- 
To view, visit http://gerrit.cloudera.org:8080/17120
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I95d1456dea2e6e2bb7d8b0c5d05e95798098710d
Gerrit-Change-Number: 17120
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] [client-test] added WriteWhileRestartingMultipleTabletServers

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

Change subject: [client-test] added WriteWhileRestartingMultipleTabletServers
......................................................................

[client-test] added WriteWhileRestartingMultipleTabletServers

While working on tests for multi-row transactions, I added a similar
scenario and found it a bit flaky.  So, I wanted to see if the same
is true for the non-transactional write operations.  It turns out
the latter is pretty stable, so I need to dig in to find the root
cause of the former.  Anyways, I think this is a good scenario to
add into client-test.cc, extending already existing scenario
ClientTest.TestWriteWhileRestarting to multi-replica case and going
through multiple restarts.  Another important detail is that the newly
added test scenario verifies the number of persisted rows in the end.

I also did a small touch-up of the code related to the utility method
ClientTest::CountRowsFromClient().

Change-Id: I95d1456dea2e6e2bb7d8b0c5d05e95798098710d
Reviewed-on: http://gerrit.cloudera.org:8080/17120
Tested-by: Alexey Serbin <as...@cloudera.com>
Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
M src/kudu/client/client-test.cc
1 file changed, 105 insertions(+), 42 deletions(-)

Approvals:
  Alexey Serbin: Verified
  Andrew Wong: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I95d1456dea2e6e2bb7d8b0c5d05e95798098710d
Gerrit-Change-Number: 17120
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] [client-test] added WriteWhileRestartingMultipleTabletServers

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

Change subject: [client-test] added WriteWhileRestartingMultipleTabletServers
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17120/1/src/kudu/client/client-test.cc
File src/kudu/client/client-test.cc:

http://gerrit.cloudera.org:8080/#/c/17120/1/src/kudu/client/client-test.cc@7912
PS1, Line 7912:   for (auto mode : {KuduScanner::READ_LATEST, KuduScanner::READ_YOUR_WRITES}) {
> At first I thought this could lead to flakiness because there could be a le
Yes, there isn't any flakiness: it's LEADER_ONLY replica selection.  If that's a former leader becomes stale, it has all the written rows because the same client is used to scan the rows as was use to write them.

As for the original txn-related issue mentioned in the description of this patch, I guess the issue should be fixed once https://gerrit.cloudera.org/#/c/17037/10/src/kudu/integration-tests/txn_write_ops-itest.cc@1319 passes (I hope https://gerrit.cloudera.org/#/c/17127/ should fix it).



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95d1456dea2e6e2bb7d8b0c5d05e95798098710d
Gerrit-Change-Number: 17120
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 26 Feb 2021 03:09:38 +0000
Gerrit-HasComments: Yes

[kudu-CR] [client-test] added WriteWhileRestartingMultipleTabletServers

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

Change subject: [client-test] added WriteWhileRestartingMultipleTabletServers
......................................................................


Patch Set 1: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17120/1/src/kudu/client/client-test.cc
File src/kudu/client/client-test.cc:

http://gerrit.cloudera.org:8080/#/c/17120/1/src/kudu/client/client-test.cc@7912
PS1, Line 7912:   for (auto mode : {KuduScanner::READ_LATEST, KuduScanner::READ_YOUR_WRITES}) {
At first I thought this could lead to flakiness because there could be a leadership change in between writing and reading, and we could end up scanning a stale leader. I don't think that's actually the case though, since if we're scanning a "stale follower", it must have changed leadership after having written all rows successfully, since we're using the same client that wrote the rows.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95d1456dea2e6e2bb7d8b0c5d05e95798098710d
Gerrit-Change-Number: 17120
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 26 Feb 2021 01:57:22 +0000
Gerrit-HasComments: Yes

[kudu-CR] [client-test] added WriteWhileRestartingMultipleTabletServers

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

Change subject: [client-test] added WriteWhileRestartingMultipleTabletServers
......................................................................


Patch Set 1: Verified+1

unrelated test failure in ExactlyOnceSemanticsITest.TestWritesWithExactlyOnceSemanticsWithCrashyNodes


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95d1456dea2e6e2bb7d8b0c5d05e95798098710d
Gerrit-Change-Number: 17120
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 25 Feb 2021 06:56:13 +0000
Gerrit-HasComments: No