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 2022/04/30 00:42:22 UTC
[kudu-CR] [client] fix toggling fault-tolerance for a scanner
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18462
Change subject: [client] fix toggling fault-tolerance for a scanner
......................................................................
[client] fix toggling fault-tolerance for a scanner
Change-Id: Ic50f9d62623d8b9ced7d854ce0e80a0faaa60ad3
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AbstractKuduScannerBuilder.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestScanToken.java
M src/kudu/client/scan_configuration.cc
M src/kudu/client/scan_token-test.cc
4 files changed, 50 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/62/18462/1
--
To view, visit http://gerrit.cloudera.org:8080/18462
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic50f9d62623d8b9ced7d854ce0e80a0faaa60ad3
Gerrit-Change-Number: 18462
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <al...@apache.org>
[kudu-CR] [client] fix toggling fault-tolerance for a scanner
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/18462 )
Change subject: [client] fix toggling fault-tolerance for a scanner
......................................................................
[client] fix toggling fault-tolerance for a scanner
Change-Id: Ic50f9d62623d8b9ced7d854ce0e80a0faaa60ad3
Reviewed-on: http://gerrit.cloudera.org:8080/18462
Reviewed-by: Andrew Wong <aw...@cloudera.com>
Tested-by: Alexey Serbin <al...@apache.org>
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AbstractKuduScannerBuilder.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestScanToken.java
M src/kudu/client/scan_configuration.cc
M src/kudu/client/scan_token-test.cc
4 files changed, 57 insertions(+), 3 deletions(-)
Approvals:
Andrew Wong: Looks good to me, approved
Alexey Serbin: Verified
--
To view, visit http://gerrit.cloudera.org:8080/18462
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic50f9d62623d8b9ced7d854ce0e80a0faaa60ad3
Gerrit-Change-Number: 18462
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [client] fix toggling fault-tolerance for a scanner
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/18462 )
Change subject: [client] fix toggling fault-tolerance for a scanner
......................................................................
Patch Set 2: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18462/1/src/kudu/client/scan_token-test.cc
File src/kudu/client/scan_token-test.cc:
http://gerrit.cloudera.org:8080/#/c/18462/1/src/kudu/client/scan_token-test.cc@1085
PS1, Line 1085: ASSERT_EQ(KuduScanner::READ_AT_SNAPSHOT, sc.read_mode());
> It turns out the code in Java client assumes that switching to non-fault-to
Right, I might expect that kind of behavior in some ResetToDefault() call or somesuch, but it seemed a bit odd otherwise. Things like
scanner.SetReadMode(READ_AT_SNAPSHOT);
scanner.SetFaultTolerant(false);
would be have surprising results in that case IMO
--
To view, visit http://gerrit.cloudera.org:8080/18462
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic50f9d62623d8b9ced7d854ce0e80a0faaa60ad3
Gerrit-Change-Number: 18462
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sat, 30 Apr 2022 05:14:30 +0000
Gerrit-HasComments: Yes
[kudu-CR] [client] fix toggling fault-tolerance for a scanner
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18462 )
Change subject: [client] fix toggling fault-tolerance for a scanner
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18462/1/src/kudu/client/scan_token-test.cc
File src/kudu/client/scan_token-test.cc:
http://gerrit.cloudera.org:8080/#/c/18462/1/src/kudu/client/scan_token-test.cc@1085
PS1, Line 1085: ASSERT_EQ(KuduScanner::ReadMode::READ_LATEST, sc.read_mode());
> This seems a bit surprising -- why not have SetFaultTolerant(false) leave t
The idea was to return it back to the original state that ScanConfiguration was created with. I agree there might be different idea how it works: the current API is a bit clumsy for fault tolerance settings.
--
To view, visit http://gerrit.cloudera.org:8080/18462
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic50f9d62623d8b9ced7d854ce0e80a0faaa60ad3
Gerrit-Change-Number: 18462
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sat, 30 Apr 2022 02:02:18 +0000
Gerrit-HasComments: Yes
[kudu-CR] [client] fix toggling fault-tolerance for a scanner
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18462
to look at the new patch set (#2).
Change subject: [client] fix toggling fault-tolerance for a scanner
......................................................................
[client] fix toggling fault-tolerance for a scanner
Change-Id: Ic50f9d62623d8b9ced7d854ce0e80a0faaa60ad3
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AbstractKuduScannerBuilder.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestScanToken.java
M src/kudu/client/scan_configuration.cc
M src/kudu/client/scan_token-test.cc
4 files changed, 57 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/62/18462/2
--
To view, visit http://gerrit.cloudera.org:8080/18462
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic50f9d62623d8b9ced7d854ce0e80a0faaa60ad3
Gerrit-Change-Number: 18462
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [client] fix toggling fault-tolerance for a scanner
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has removed a vote on this change.
Change subject: [client] fix toggling fault-tolerance for a scanner
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/18462
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ic50f9d62623d8b9ced7d854ce0e80a0faaa60ad3
Gerrit-Change-Number: 18462
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [client] fix toggling fault-tolerance for a scanner
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18462 )
Change subject: [client] fix toggling fault-tolerance for a scanner
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18462/1/src/kudu/client/scan_token-test.cc
File src/kudu/client/scan_token-test.cc:
http://gerrit.cloudera.org:8080/#/c/18462/1/src/kudu/client/scan_token-test.cc@1085
PS1, Line 1085: ASSERT_EQ(KuduScanner::READ_AT_SNAPSHOT, sc.read_mode());
> The idea was to return it back to the original state that ScanConfiguration
It turns out the code in Java client assumes that switching to non-fault-tolerant scan keeps the read mode as READ_AT_SNAPSHOT. So, it seems that's a manifestation of the the Hyrum's law in this particular case :)
--
To view, visit http://gerrit.cloudera.org:8080/18462
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic50f9d62623d8b9ced7d854ce0e80a0faaa60ad3
Gerrit-Change-Number: 18462
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sat, 30 Apr 2022 02:26:15 +0000
Gerrit-HasComments: Yes
[kudu-CR] [client] fix toggling fault-tolerance for a scanner
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/18462 )
Change subject: [client] fix toggling fault-tolerance for a scanner
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18462/1/src/kudu/client/scan_token-test.cc
File src/kudu/client/scan_token-test.cc:
http://gerrit.cloudera.org:8080/#/c/18462/1/src/kudu/client/scan_token-test.cc@1085
PS1, Line 1085: ASSERT_EQ(KuduScanner::ReadMode::READ_LATEST, sc.read_mode());
This seems a bit surprising -- why not have SetFaultTolerant(false) leave the read mode as is?
--
To view, visit http://gerrit.cloudera.org:8080/18462
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic50f9d62623d8b9ced7d854ce0e80a0faaa60ad3
Gerrit-Change-Number: 18462
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sat, 30 Apr 2022 01:13:38 +0000
Gerrit-HasComments: Yes
[kudu-CR] [client] fix toggling fault-tolerance for a scanner
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18462 )
Change subject: [client] fix toggling fault-tolerance for a scanner
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18462/1/src/kudu/client/scan_token-test.cc
File src/kudu/client/scan_token-test.cc:
http://gerrit.cloudera.org:8080/#/c/18462/1/src/kudu/client/scan_token-test.cc@1085
PS1, Line 1085: ASSERT_EQ(KuduScanner::ReadMode::READ_LATEST, sc.read_mode());
> Right, I might expect that kind of behavior in some ResetToDefault() call o
Yep, that would be surprising and ResetToDefault() might be a good alternative.
BTW, it's a bit surprising even now if trying something
scanner.SetFaultTolerant(true);
scanner.SetReadMode(READ_LATEST);
It seems to be a bike-shedding avenue, but I guess the fault-tolerant is made some sort of fuzzy compound now, and the API doesn't provide a consistent API to work with it.
Anyways, not preserving the READ_AT_SNAPSHOT mode makes some Java tests fail, so that's a hard indication we shouldn't touch the read mode while calling SetFaultTolerant(false).
--
To view, visit http://gerrit.cloudera.org:8080/18462
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic50f9d62623d8b9ced7d854ce0e80a0faaa60ad3
Gerrit-Change-Number: 18462
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 02 May 2022 17:43:42 +0000
Gerrit-HasComments: Yes
[kudu-CR] [client] fix toggling fault-tolerance for a scanner
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18462 )
Change subject: [client] fix toggling fault-tolerance for a scanner
......................................................................
Patch Set 2: Verified+1
Unrelated test failure in DisableWriteWhenExceedingQuotaTest.TestDisableWritePrivilegeWhenExceedingSizeQuota
--
To view, visit http://gerrit.cloudera.org:8080/18462
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic50f9d62623d8b9ced7d854ce0e80a0faaa60ad3
Gerrit-Change-Number: 18462
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 02 May 2022 17:44:00 +0000
Gerrit-HasComments: No