[kudu-CR] KUDU-2584: Prevent flaky off-by-one errors in backup tests
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/11815 ) Change subject: KUDU-2584: Prevent flaky off-by-one errors in backup tests .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/11815/2/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala File java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala: http://gerrit.cloudera.org:8080/#/c/11815/2/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala@356 PS2, Line 356: error nit: why not use 'info' instead of 'error'? http://gerrit.cloudera.org:8080/#/c/11815/2/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala@368 PS2, Line 368: nowMs + 1 Does this mean the default value for timestampMs in KuduBackupOptions should be 'nowMs + 1'? -- To view, visit http://gerrit.cloudera.org:8080/11815 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia0f1b4a4138cc8c913543a68fad748927cdc439d Gerrit-Change-Number: 11815 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 06 Nov 2018 07:27:41 + Gerrit-HasComments: Yes
[kudu-CR] Limit number of rowsets in compaction selection to 32 in TSAN mode
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11885 ) Change subject: Limit number of rowsets in compaction selection to 32 in TSAN mode .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/11885/1/src/kudu/tablet/tablet.cc File src/kudu/tablet/tablet.cc: http://gerrit.cloudera.org:8080/#/c/11885/1/src/kudu/tablet/tablet.cc@1378 PS1, Line 1378: take its compact_flush_lock > See L1407 below. The lock is moved into the 'picked' collection. Right, it seems I missed that the move() part. http://gerrit.cloudera.org:8080/#/c/11885/1/src/kudu/tablet/tablet.cc@1385 PS1, Line 1385: #if defined(THREAD_SANITIZER) > This is an alternative way to do indentation? I'm not understanding what th Yep; the point was to keep the '#' in the beginning of the line. It might be easier to read, but I don't see that in the google style guide, so feel free to ignore. -- To view, visit http://gerrit.cloudera.org:8080/11885 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I01ad4ba3a13995c194c3308d72c1eb9b611ef766 Gerrit-Change-Number: 11885 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 06 Nov 2018 06:33:09 + Gerrit-HasComments: Yes
[kudu-CR] Limit number of rowsets in compaction selection to 32 in TSAN mode
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11885 ) Change subject: Limit number of rowsets in compaction selection to 32 in TSAN mode .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/11885/1/src/kudu/tablet/tablet.cc File src/kudu/tablet/tablet.cc: http://gerrit.cloudera.org:8080/#/c/11885/1/src/kudu/tablet/tablet.cc@1378 PS1, Line 1378: take its compact_flush_lock > Wait, but those compact_flush_locks are taken and released one by one by th I missed the move() part. Please ignore this comment. -- To view, visit http://gerrit.cloudera.org:8080/11885 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I01ad4ba3a13995c194c3308d72c1eb9b611ef766 Gerrit-Change-Number: 11885 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 06 Nov 2018 06:22:35 + Gerrit-HasComments: Yes
[kudu-CR] Limit number of rowsets in compaction selection to 32 in TSAN mode
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/11885 ) Change subject: Limit number of rowsets in compaction selection to 32 in TSAN mode .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/11885/1/src/kudu/tablet/tablet.cc File src/kudu/tablet/tablet.cc: http://gerrit.cloudera.org:8080/#/c/11885/1/src/kudu/tablet/tablet.cc@1378 PS1, Line 1378: take its compact_flush_lock > Wait, but those compact_flush_locks are taken and released one by one by th See L1407 below. The lock is moved into the 'picked' collection. http://gerrit.cloudera.org:8080/#/c/11885/1/src/kudu/tablet/tablet.cc@1385 PS1, Line 1385: #if defined(THREAD_SANITIZER) > style nit: there is an alternative style for those ifdefs, if you want: This is an alternative way to do indentation? I'm not understanding what the point of the difference is. -- To view, visit http://gerrit.cloudera.org:8080/11885 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I01ad4ba3a13995c194c3308d72c1eb9b611ef766 Gerrit-Change-Number: 11885 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 06 Nov 2018 06:19:29 + Gerrit-HasComments: Yes
[kudu-CR] Limit number of rowsets in compaction selection to 32 in TSAN mode
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11885 ) Change subject: Limit number of rowsets in compaction selection to 32 in TSAN mode .. Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/11885/1/src/kudu/tablet/tablet.cc File src/kudu/tablet/tablet.cc: http://gerrit.cloudera.org:8080/#/c/11885/1/src/kudu/tablet/tablet.cc@1378 PS1, Line 1378: take its compact_flush_lock Wait, but those compact_flush_locks are taken and released one by one by this code, right? How could it happen that one thread takes and holds more than 64 of those? http://gerrit.cloudera.org:8080/#/c/11885/1/src/kudu/tablet/tablet.cc@1385 PS1, Line 1385: #if defined(THREAD_SANITIZER) style nit: there is an alternative style for those ifdefs, if you want: # if defined(...) const auto kMaxPickedUnderTsan = 32; # endif http://gerrit.cloudera.org:8080/#/c/11885/1/src/kudu/tablet/tablet.cc@1386 PS1, Line 1386: const nit: constexpr ? -- To view, visit http://gerrit.cloudera.org:8080/11885 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I01ad4ba3a13995c194c3308d72c1eb9b611ef766 Gerrit-Change-Number: 11885 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 06 Nov 2018 06:13:30 + Gerrit-HasComments: Yes
[kudu-CR] Disable rowset compaction in TestUndoDeltaBlockGc
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11884 ) Change subject: Disable rowset compaction in TestUndoDeltaBlockGc .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11884 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If23f4568d10d83b7f463663cfc0d4052f2602224 Gerrit-Change-Number: 11884 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Comment-Date: Tue, 06 Nov 2018 05:44:35 + Gerrit-HasComments: No
[kudu-CR] Add "network plane" as part of ConnectionId
Hello Kudu Jenkins, Sailesh Mukil, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11681 to look at the new patch set (#4). Change subject: Add "network_plane" as part of ConnectionId .. Add "network_plane" as part of ConnectionId The motivation for doing so is to allow N services on the same host to be multiplexed on M different connections. For instance, a server may host multiple KRPC based services: one for control command and one for data transfer. Separating the connections between the control channel and the data channel prevents unnecessary delays of the control commands due to being stuck behind large data transfers from client to server. By default, the network_plane of a new ConnectionId is set to "default_network_plane". A user can change it to a different value by calling Proxy::set_network_plane() on the ConnectionId. Change-Id: I6767e631fd9530ea54f5ed63ff4c8c179ab216b2 --- M src/kudu/rpc/connection_id.cc M src/kudu/rpc/connection_id.h M src/kudu/rpc/messenger.h M src/kudu/rpc/proxy.cc M src/kudu/rpc/proxy.h M src/kudu/rpc/rpc-test.cc 6 files changed, 106 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/81/11681/4 -- To view, visit http://gerrit.cloudera.org:8080/11681 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6767e631fd9530ea54f5ed63ff4c8c179ab216b2 Gerrit-Change-Number: 11681 Gerrit-PatchSet: 4 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1918 Prevent hijacking of scanner IDs
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/6348 )
Change subject: KUDU-1918 Prevent hijacking of scanner IDs
..
Patch Set 7: Code-Review+2
(5 comments)
LGTM, just a few nits.
http://gerrit.cloudera.org:8080/#/c/6348/7/src/kudu/client/client-test.cc
File src/kudu/client/client-test.cc:
http://gerrit.cloudera.org:8080/#/c/6348/7/src/kudu/client/client-test.cc@5765
PS7, Line 5765: ASSERT_OK(bad_guy_scanner.Open());
nit: does it make sense to test the behavior of the KuduScanner::Open() method
in the case when the scanner tries to pre-fetch some rows (i.e. set the initial
batch size to non-zero and see how it works)?
http://gerrit.cloudera.org:8080/#/c/6348/7/src/kudu/tserver/scanners.cc
File src/kudu/tserver/scanners.cc:
http://gerrit.cloudera.org:8080/#/c/6348/7/src/kudu/tserver/scanners.cc@168
PS7, Line 168: if (username != ret->remote_user().username()) {
: *error_code = TabletServerErrorPB::NOT_AUTHORIZED;
: return Status::NotAuthorized(Substitute("User $0 doesn't own
scanner $1",
: username,
scanner_id));
: }
nit: do we prefer exposing the fact that the scanner exists but the caller is
not authorized to access it versus reporting NotFound error and logging into
the log about non-authorized attempt to access the scanner?
http://gerrit.cloudera.org:8080/#/c/6348/7/src/kudu/tserver/scanners.cc@173
PS7, Line 173: ret
nit: wrap into std::move() ?
http://gerrit.cloudera.org:8080/#/c/6348/7/src/kudu/tserver/tablet_server-test.cc
File src/kudu/tserver/tablet_server-test.cc:
http://gerrit.cloudera.org:8080/#/c/6348/7/src/kudu/tserver/tablet_server-test.cc@3406
PS7, Line 3406: it
nit: them ?
http://gerrit.cloudera.org:8080/#/c/6348/7/src/kudu/tserver/tablet_server-test.cc@3437
PS7, Line 3437: const
nit: might it be constexpr?
--
To view, visit http://gerrit.cloudera.org:8080/6348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic91fa0ca471bd674e35aa2f8de3806b88ad4b3b4
Gerrit-Change-Number: 6348
Gerrit-PatchSet: 7
Gerrit-Owner: Todd Lipcon
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: David Ribeiro Alves
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 06 Nov 2018 02:56:41 +
Gerrit-HasComments: Yes
[kudu-CR] Limit number of rowsets in compaction selection to 32 in TSAN mode
Will Berkeley has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11885 Change subject: Limit number of rowsets in compaction selection to 32 in TSAN mode .. Limit number of rowsets in compaction selection to 32 in TSAN mode TSAN limits the number of simultaneous lock acquisitions in a single thread to 64 when using the deadlock detector[1]. However, compaction can select up to 128 (128MB budget / 1MB min rowset size) rowsets in a single op. kudu-tool-test's TestNonRandomWorkloadLoadgen almost always hits TSAN's limit when the KUDU-1400 changes following this patch are applied. This patch prevents this by limiting the number of rowsets selected for a compaction to 32 when running under TSAN. I ran the test with the KUDU-1400 changes on top and saw 97/100 failures. With the change, I saw 100 successes. [1]: https://github.com/google/sanitizers/issues/950 Change-Id: I01ad4ba3a13995c194c3308d72c1eb9b611ef766 --- M src/kudu/tablet/tablet.cc 1 file changed, 19 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/85/11885/1 -- To view, visit http://gerrit.cloudera.org:8080/11885 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I01ad4ba3a13995c194c3308d72c1eb9b611ef766 Gerrit-Change-Number: 11885 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley
[kudu-CR] Disable rowset compaction in TestUndoDeltaBlockGc
Will Berkeley has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11884 Change subject: Disable rowset compaction in TestUndoDeltaBlockGc .. Disable rowset compaction in TestUndoDeltaBlockGc In a follow-up to this patch, rowset compaction is enhanced to merge small rowsets. This will cause TestUndoDeltaBlockGc to be flaky, since it creates small rowsets with undo deltas and tries to wait until they are GC'd by the UndoDeltaBlockGc maintenance op; however, if rowset compaction merges the rowsets after the undos have passed the AHM but before UndoDeltaBlockGc runs, no undos will be deleted by the UndoDeltaBlockGc task and the test will fail. This fixes the test by simply disabling rowset compactions. I ran the test 100 times with the change and the KUDU-1400 change on top and saw zero failures. Change-Id: If23f4568d10d83b7f463663cfc0d4052f2602224 --- M src/kudu/integration-tests/tablet_history_gc-itest.cc 1 file changed, 6 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/11884/1 -- To view, visit http://gerrit.cloudera.org:8080/11884 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: If23f4568d10d83b7f463663cfc0d4052f2602224 Gerrit-Change-Number: 11884 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley
[kudu-CR] Add "network plane" as part of ConnectionId
Hello Kudu Jenkins, Sailesh Mukil, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11681 to look at the new patch set (#3). Change subject: Add "network_plane" as part of ConnectionId .. Add "network_plane" as part of ConnectionId The motivation for doing so is to allow N services on the same host to be multiplexed on M different connections. For instance, a server may host multiple KRPC based services: one for control command and one for data transfer. Separating the connections between the control channel and the data channel prevents unnecessary delays of the control commands due to being stuck behind large data transfers from client to server. By default, the network_plane of a new ConnectionId is set to "default_network_plane". A user can change it to a non-default value by calling Proxy::set_network_plane() on the ConnectionId. Change-Id: I6767e631fd9530ea54f5ed63ff4c8c179ab216b2 --- M src/kudu/rpc/connection_id.cc M src/kudu/rpc/connection_id.h M src/kudu/rpc/messenger.h M src/kudu/rpc/proxy.cc M src/kudu/rpc/proxy.h M src/kudu/rpc/rpc-test.cc 6 files changed, 102 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/81/11681/3 -- To view, visit http://gerrit.cloudera.org:8080/11681 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6767e631fd9530ea54f5ed63ff4c8c179ab216b2 Gerrit-Change-Number: 11681 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Add "network plane" as part of ConnectionId
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/11681 ) Change subject: Add "network_plane" as part of ConnectionId .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/11681/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11681/1//COMMIT_MSG@7 PS1, Line 7: Add "service_name" as part of ConnectionId > Thanks for the idea. Will look into it. Sorry for the delay. I adopted option 1. Thanks. -- To view, visit http://gerrit.cloudera.org:8080/11681 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6767e631fd9530ea54f5ed63ff4c8c179ab216b2 Gerrit-Change-Number: 11681 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 06 Nov 2018 02:13:36 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2584: Prevent flaky off-by-one errors in backup tests
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11815 ) Change subject: KUDU-2584: Prevent flaky off-by-one errors in backup tests .. Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/11815/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11815/2//COMMIT_MSG@13 PS2, Line 13: The underlying reason for adding 1 ms is that we pass : the timestamp in ms granularity but the snapshot time : consists of microseconds plus a logical clock. This : means if the data is inserted with a fraction of a ms : remaining it could be truncated and unread. I left some comments on this in the code; please also update this paragraph accordingly. http://gerrit.cloudera.org:8080/#/c/11815/2/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackupRDD.scala File java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackupRDD.scala: http://gerrit.cloudera.org:8080/#/c/11815/2/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackupRDD.scala@138 PS2, Line 138: kuduContext.timestampAccumulator.add(kuduContext.syncClient.getLastPropagatedTimestamp) This is so that if you're using the same KuduContext for a backup job and a write, the timestamps "seen" during the backup job's scan will propagate into the clients so that they'll be sent during the write too? http://gerrit.cloudera.org:8080/#/c/11815/2/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala File java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala: http://gerrit.cloudera.org:8080/#/c/11815/2/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala@364 PS2, Line 364: plus a logical clock This detail isn't relevant to the problem/solution discussion, is it? http://gerrit.cloudera.org:8080/#/c/11815/2/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala@364 PS2, Line 364: This means : // if the data is inserted with a fraction of a ms remaining it could be truncated and : // unread. In practice this is always true; what are the odds that a timestamp value of micro granularity is generated with exactly 0 micros? So I think the more interesting bit to document is that the test has to run fast enough such that the currentTimeMillis() call on L353 ends up with the same ms value as the write's timestamp (after truncating the micros). -- To view, visit http://gerrit.cloudera.org:8080/11815 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia0f1b4a4138cc8c913543a68fad748927cdc439d Gerrit-Change-Number: 11815 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 05 Nov 2018 23:48:48 + Gerrit-HasComments: Yes
[kudu-CR] [tools] reference comparison mode for rebalancing algo tests
Hello Will Berkeley, Tidy Bot, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11870 to look at the new patch set (#3). Change subject: [tools] reference comparison mode for rebalancing algo tests .. [tools] reference comparison mode for rebalancing algo tests Introduced comparison mode for rebalancing algorithms' tests. For current rebalancing algorithms, it's natural to re-order contiguous moves of the same weight. That's because of: * Iterating over elements of a hash container keyed by the weight of a move. * Randomly choosing among multiple options of the same weight. This patch adds MovesOrderingComparison::IGNORE option into one test configuration of the RebalanceAlgoUnitTest.LocationBalancingSimpleST scenario. That fixes the breakage of the test on Ubuntu 18. Change-Id: I8363f013b5bf8caa3e3b967c64eccca95c763a91 --- M src/kudu/tools/rebalance_algo-test.cc 1 file changed, 60 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/11870/3 -- To view, visit http://gerrit.cloudera.org:8080/11870 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8363f013b5bf8caa3e3b967c64eccca95c763a91 Gerrit-Change-Number: 11870 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Will Berkeley
[kudu-CR] [compaction] KUDU-1400: Improve rowset compaction policy to consider merging small DRSs
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/11869 ) Change subject: [compaction] KUDU-1400: Improve rowset compaction policy to consider merging small DRSs .. Patch Set 3: > I'm not sure about the kudu-tool-test failure. TSAN reports a > deadlock on a rowset's compact_flush_lock It's not a deadlock. Instead, the test flushes many rowsets and, as expected, the new rowset compaction wants to compact them. There end up being so many to compact at once (60+) that taking all the rowsets compact_flush_locks overflows the fixed (64) size of an array that the TSAN code uses as part of tracking lock acquisitions and their order. -- To view, visit http://gerrit.cloudera.org:8080/11869 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7b421c6ed77d28ebab9b91a4d6fcb1e825997e6c Gerrit-Change-Number: 11869 Gerrit-PatchSet: 3 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 05 Nov 2018 23:02:33 + Gerrit-HasComments: No
[kudu-CR] [tools] reference comparison mode for rebalancing algo tests
Hello Will Berkeley, Tidy Bot, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11870 to look at the new patch set (#2). Change subject: [tools] reference comparison mode for rebalancing algo tests .. [tools] reference comparison mode for rebalancing algo tests Introduced comparison mode for the rebalancing algorithms' tests. There are some implementation details like using the hash (not tree) dictionary collections and additional randomness among the moves of equal cost in the cross-location rebalancing algorithms. In some scenarios, when the order of the resulting moves might be different from one run to another due to the mentioned details, it's necessary to normalize the reference and the actual results before comparison. Change-Id: I8363f013b5bf8caa3e3b967c64eccca95c763a91 --- M src/kudu/tools/rebalance_algo-test.cc 1 file changed, 59 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/11870/2 -- To view, visit http://gerrit.cloudera.org:8080/11870 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8363f013b5bf8caa3e3b967c64eccca95c763a91 Gerrit-Change-Number: 11870 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Will Berkeley
[kudu-CR] Add TS UUID to alter table-randomized-test "restart TS" step
Will Berkeley has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11877 ) Change subject: Add TS UUID to alter_table-randomized-test "restart TS" step .. Add TS UUID to alter_table-randomized-test "restart TS" step While debugging a failure of the randomized alter test, I needed to know which tablet server was restarted. The test's logs for it are I1102 21:39:02.323911 7768 alter_table-randomized-test.cc:120] Restarting TS 2 I1102 21:39:02.430003 7768 alter_table-randomized-test.cc:125] TS 2 Restarted which don't help me so much because I don't know which UUID is which index. It's possible to use the startup messages in the logs to figure out which UUID was restarted, but it'd be easier if it were just logged. So that's what it does now: I1105 10:17:26.197420 2830984064 alter_table-randomized-test.cc:122] Restarting TS fef46d70bd71410c8caa0aeb2a3cbafc (index 2) I1105 10:17:26.375321 2830984064 alter_table-randomized-test.cc:126] TS fef46d70bd71410c8caa0aeb2a3cbafc (index 2) restarted Change-Id: Ifbd2ee5ca3e3d7788812162e6188905adc3a6ec3 Reviewed-on: http://gerrit.cloudera.org:8080/11877 Reviewed-by: Andrew Wong Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin --- M src/kudu/integration-tests/alter_table-randomized-test.cc 1 file changed, 8 insertions(+), 7 deletions(-) Approvals: Andrew Wong: Looks good to me, approved Kudu Jenkins: Verified Alexey Serbin: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/11877 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ifbd2ee5ca3e3d7788812162e6188905adc3a6ec3 Gerrit-Change-Number: 11877 Gerrit-PatchSet: 2 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley
[kudu-CR] [sentry] add AuthzProvider
Hao Hao has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11659 ) Change subject: [sentry] add AuthzProvider .. [sentry] add AuthzProvider This commit adds a high-level abstraction which handles authorizations on Kudu operations, called AuthzProvider. It has a default implementation which always allow any operations for any users, and a SentryAuthzProvider implementation which connects to the Sentry service for authorization metadata. AuthzProvider, along with its implementations, is placed in the master module. The idea is to decouple it from Sentry since in the future, other authorization implementations might be introduced. A follow-up commit will integrate the AuthzProvider into CatalogManager to perform authorization checks on Master RPCs. Change-Id: I254828d640cd905e33dbaf0fe100d660bc9e6772 Reviewed-on: http://gerrit.cloudera.org:8080/11659 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin Reviewed-by: Andrew Wong --- M src/kudu/hms/hms_catalog.cc M src/kudu/master/CMakeLists.txt A src/kudu/master/authz_provider.h A src/kudu/master/default_authz_provider.h A src/kudu/master/sentry_authz_provider-test.cc A src/kudu/master/sentry_authz_provider.cc A src/kudu/master/sentry_authz_provider.h A src/kudu/sentry/sentry-test-base.h M src/kudu/sentry/sentry_action-test.cc M src/kudu/sentry/sentry_action.cc M src/kudu/sentry/sentry_action.h M src/kudu/sentry/sentry_client-test.cc M src/kudu/sentry/sentry_client.cc M src/kudu/sentry/sentry_client.h 14 files changed, 994 insertions(+), 117 deletions(-) Approvals: Kudu Jenkins: Verified Alexey Serbin: Looks good to me, but someone else must approve Andrew Wong: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/11659 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I254828d640cd905e33dbaf0fe100d660bc9e6772 Gerrit-Change-Number: 11659 Gerrit-PatchSet: 15 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [sentry] add AuthzProvider
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11659 ) Change subject: [sentry] add AuthzProvider .. Patch Set 10: (1 comment) http://gerrit.cloudera.org:8080/#/c/11659/10/src/kudu/master/sentry_authz_provider.cc File src/kudu/master/sentry_authz_provider.cc: http://gerrit.cloudera.org:8080/#/c/11659/10/src/kudu/master/sentry_authz_provider.cc@84 PS10, Line 84: 1 > If the retry logic proves to be ineffective or anti-productive, we can add I agree -- it doesn't make sense to change those defaults unless we have some data to base that decision upon. In other words, current default values look good enough to me :) -- To view, visit http://gerrit.cloudera.org:8080/11659 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I254828d640cd905e33dbaf0fe100d660bc9e6772 Gerrit-Change-Number: 11659 Gerrit-PatchSet: 10 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Mon, 05 Nov 2018 19:16:28 + Gerrit-HasComments: Yes
[kudu-CR] Add TS UUID to alter table-randomized-test "restart TS" step
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11877 ) Change subject: Add TS UUID to alter_table-randomized-test "restart TS" step .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11877 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifbd2ee5ca3e3d7788812162e6188905adc3a6ec3 Gerrit-Change-Number: 11877 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 05 Nov 2018 18:50:16 + Gerrit-HasComments: No
[kudu-CR] [sentry] add AuthzProvider
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11659 ) Change subject: [sentry] add AuthzProvider .. Patch Set 14: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/11659/10/src/kudu/master/sentry_authz_provider.cc File src/kudu/master/sentry_authz_provider.cc: http://gerrit.cloudera.org:8080/#/c/11659/10/src/kudu/master/sentry_authz_provider.cc@84 PS10, Line 84: u > Do you have some recommendations on how to test the default values? Also, w If the retry logic proves to be ineffective or anti-productive, we can add backoff in the future. I think Dan added the defaults based on his "gut feel"; we'll have to see how well the defaults work in practice. -- To view, visit http://gerrit.cloudera.org:8080/11659 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I254828d640cd905e33dbaf0fe100d660bc9e6772 Gerrit-Change-Number: 11659 Gerrit-PatchSet: 14 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Mon, 05 Nov 2018 18:32:30 + Gerrit-HasComments: Yes
[kudu-CR] Add TS UUID to alter table-randomized-test "restart TS" step
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11877 ) Change subject: Add TS UUID to alter_table-randomized-test "restart TS" step .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11877 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifbd2ee5ca3e3d7788812162e6188905adc3a6ec3 Gerrit-Change-Number: 11877 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 05 Nov 2018 18:28:10 + Gerrit-HasComments: No
[kudu-CR] [examples] Add basic Spark example (scala)
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11788 ) Change subject: [examples] Add basic Spark example (scala) .. [examples] Add basic Spark example (scala) This patch adds a basic Kudu-Spark example that utilizes the Kudu-Spark integration. It will allow users to pull down the pom.xml and scala source, then build and execute from their local machine. Change-Id: I9ba09f0118c054a07b951e241c31d66245c57d3f Reviewed-on: http://gerrit.cloudera.org:8080/11788 Reviewed-by: Will Berkeley Tested-by: Kudu Jenkins Reviewed-by: Grant Henke --- A examples/scala/spark-example/README.adoc A examples/scala/spark-example/pom.xml A examples/scala/spark-example/src/main/scala/org/apache/kudu/spark/examples/SparkExample.scala 3 files changed, 287 insertions(+), 0 deletions(-) Approvals: Will Berkeley: Looks good to me, but someone else must approve Kudu Jenkins: Verified Grant Henke: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/11788 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I9ba09f0118c054a07b951e241c31d66245c57d3f Gerrit-Change-Number: 11788 Gerrit-PatchSet: 13 Gerrit-Owner: Mitch Barnett Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mitch Barnett Gerrit-Reviewer: Will Berkeley
[kudu-CR] [examples] Add basic Spark example (scala)
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/11788 ) Change subject: [examples] Add basic Spark example (scala) .. Patch Set 12: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11788 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9ba09f0118c054a07b951e241c31d66245c57d3f Gerrit-Change-Number: 11788 Gerrit-PatchSet: 12 Gerrit-Owner: Mitch Barnett Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mitch Barnett Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 05 Nov 2018 18:21:46 + Gerrit-HasComments: No
[kudu-CR] Add TS UUID to alter table-randomized-test "restart TS" step
Will Berkeley has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11877 Change subject: Add TS UUID to alter_table-randomized-test "restart TS" step .. Add TS UUID to alter_table-randomized-test "restart TS" step While debugging a failure of the randomized alter test, I needed to know which tablet server was restarted. The test's logs for it are I1102 21:39:02.323911 7768 alter_table-randomized-test.cc:120] Restarting TS 2 I1102 21:39:02.430003 7768 alter_table-randomized-test.cc:125] TS 2 Restarted which don't help me so much because I don't know which UUID is which index. It's possible to use the startup messages in the logs to figure out which UUID was restarted, but it'd be easier if it were just logged. So that's what it does now: I1105 10:17:26.197420 2830984064 alter_table-randomized-test.cc:122] Restarting TS fef46d70bd71410c8caa0aeb2a3cbafc (index 2) I1105 10:17:26.375321 2830984064 alter_table-randomized-test.cc:126] TS fef46d70bd71410c8caa0aeb2a3cbafc (index 2) restarted Change-Id: Ifbd2ee5ca3e3d7788812162e6188905adc3a6ec3 --- M src/kudu/integration-tests/alter_table-randomized-test.cc 1 file changed, 8 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/11877/1 -- To view, visit http://gerrit.cloudera.org:8080/11877 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ifbd2ee5ca3e3d7788812162e6188905adc3a6ec3 Gerrit-Change-Number: 11877 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley
[kudu-CR] KUDU-1918 Prevent hijacking of scanner IDs
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/6348 ) Change subject: KUDU-1918 Prevent hijacking of scanner IDs .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/6348 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic91fa0ca471bd674e35aa2f8de3806b88ad4b3b4 Gerrit-Change-Number: 6348 Gerrit-PatchSet: 7 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Mon, 05 Nov 2018 17:44:51 + Gerrit-HasComments: No
[kudu-CR] [sentry] add AuthzProvider
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/11659 ) Change subject: [sentry] add AuthzProvider .. Patch Set 14: (1 comment) http://gerrit.cloudera.org:8080/#/c/11659/10/src/kudu/master/sentry_authz_provider.cc File src/kudu/master/sentry_authz_provider.cc: http://gerrit.cloudera.org:8080/#/c/11659/10/src/kudu/master/sentry_authz_provider.cc@84 PS10, Line 84: u > I took a look into the thrift client code. Apparently, the approach used t Do you have some recommendations on how to test the default values? Also, would you mind I address it in a different patch as this is more related to thrift client module. -- To view, visit http://gerrit.cloudera.org:8080/11659 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I254828d640cd905e33dbaf0fe100d660bc9e6772 Gerrit-Change-Number: 11659 Gerrit-PatchSet: 14 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Mon, 05 Nov 2018 17:54:37 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2584: Prevent flaky off-by-one errors in backup tests
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/11815 ) Change subject: KUDU-2584: Prevent flaky off-by-one errors in backup tests .. Patch Set 2: Verified+1 Failure was a know DefaultSourceTest flake. -- To view, visit http://gerrit.cloudera.org:8080/11815 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia0f1b4a4138cc8c913543a68fad748927cdc439d Gerrit-Change-Number: 11815 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 05 Nov 2018 17:44:33 + Gerrit-HasComments: No
[kudu-CR] KUDU-2584: Prevent flaky off-by-one errors in backup tests
Grant Henke has removed a vote on this change. Change subject: KUDU-2584: Prevent flaky off-by-one errors in backup tests .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/11815 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: Ia0f1b4a4138cc8c913543a68fad748927cdc439d Gerrit-Change-Number: 11815 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley
[kudu-CR] [sentry] add AuthzProvider
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11659 ) Change subject: [sentry] add AuthzProvider .. Patch Set 14: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/11659/10/src/kudu/master/sentry_authz_provider.cc File src/kudu/master/sentry_authz_provider.cc: http://gerrit.cloudera.org:8080/#/c/11659/10/src/kudu/master/sentry_authz_provider.cc@84 PS10, Line 84: u > Hmm, I think the scenario is bit different where here is the flag for Kudu I took a look into the thrift client code. Apparently, the approach used there is different than in Kudu client and looks simpler. That's fine, especially if those default values for the parameters proved to be reasonable after some testing. -- To view, visit http://gerrit.cloudera.org:8080/11659 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I254828d640cd905e33dbaf0fe100d660bc9e6772 Gerrit-Change-Number: 11659 Gerrit-PatchSet: 14 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Mon, 05 Nov 2018 17:15:40 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2584: Prevent flaky off-by-one errors in backup tests
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/11815 ) Change subject: KUDU-2584: Prevent flaky off-by-one errors in backup tests .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/11815/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11815/1//COMMIT_MSG@13 PS1, Line 13: : The off-by-one errors are possibly due to very fast : test runs where the writes and backups start : in under 1ms. They could also be due to some : underlying issue that is not well understood. > +1 I am suspicious of fixing a test failure like this with a sleep, and par Done. I added an explanation and +1 ms to the snapshot scan. I also added logging just in case we see this flake in the future. http://gerrit.cloudera.org:8080/#/c/11815/1//COMMIT_MSG@20 PS1, Line 20: > dist-test works with Java now right? Can you run some experiments to demons This failure is very difficult to produce in dist-test. I ran 500 tests before and after changes and couldn't produce it in either. -- To view, visit http://gerrit.cloudera.org:8080/11815 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia0f1b4a4138cc8c913543a68fad748927cdc439d Gerrit-Change-Number: 11815 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 05 Nov 2018 17:09:54 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2584: Prevent flaky off-by-one errors in backup tests
Hello Will Berkeley, Mike Percy, Kudu Jenkins, Adar Dembo, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11815 to look at the new patch set (#2). Change subject: KUDU-2584: Prevent flaky off-by-one errors in backup tests .. KUDU-2584: Prevent flaky off-by-one errors in backup tests This patch adds 1 ms to the target snapshot time when a backup is taken. This ensures that we don’t have flakes due to off-by-one errors where all the values are not read. The underlying reason for adding 1 ms is that we pass the timestamp in ms granularity but the snapshot time consists of microseconds plus a logical clock. This means if the data is inserted with a fraction of a ms remaining it could be truncated and unread. Additionaly this patch copies over the timestamp propagation call from the KuduRDD and ensures the Spark tests use the Kudu client from the KuduContext. This should further prevent future snapshot issues. This patch also includes an auto-formating change in KuduBackupOptions that must have been missed in a previous commit. Change-Id: Ia0f1b4a4138cc8c913543a68fad748927cdc439d --- M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackupOptions.scala M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackupRDD.scala M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduTestSuite.scala 4 files changed, 29 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/11815/2 -- To view, visit http://gerrit.cloudera.org:8080/11815 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia0f1b4a4138cc8c913543a68fad748927cdc439d Gerrit-Change-Number: 11815 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley
