[kudu-CR] [tools] select only tablet leader for RWYW
Alexey Serbin has submitted this change and it was merged. Change subject: [tools] select only tablet leader for RWYW .. [tools] select only tablet leader for RWYW To enable stable read-what-you-write behavior in case of multiple replicas, use LEADER_ONLY selection for scan operations. This is a follow-up for 5385af86dc61e56dbe601f0ad6c2bc8fba30ed7c. Change-Id: Ied2e7a6b7c31850e585c9d61655429d59c83ba89 Reviewed-on: http://gerrit.cloudera.org:8080/4571 Reviewed-by: David Ribeiro Alves Tested-by: Alexey Serbin --- M src/kudu/tools/tool_action_test.cc 1 file changed, 1 insertion(+), 0 deletions(-) Approvals: David Ribeiro Alves: Looks good to me, approved Alexey Serbin: Verified -- To view, visit http://gerrit.cloudera.org:8080/4571 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ied2e7a6b7c31850e585c9d61655429d59c83ba89 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans
[kudu-CR] [tools] select only tablet leader for RWYW
Alexey Serbin has posted comments on this change. Change subject: [tools] select only tablet leader for RWYW .. Patch Set 1: > Patch Set 1: Verified+1 Removed -1 by Jenkins because of unrelated failure of DeleteTableTest.TestAutoTombstoneAfterCrashDuringTabletCopy Built at ve0518.halxg.cloudera.com and verified by running dist_test.py 1K iterations with kudu-tool-test -- To view, visit http://gerrit.cloudera.org:8080/4571 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ied2e7a6b7c31850e585c9d61655429d59c83ba89 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-HasComments: No
[kudu-CR] [tools] select only tablet leader for RWYW
Alexey Serbin has posted comments on this change. Change subject: [tools] select only tablet leader for RWYW .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/4571 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ied2e7a6b7c31850e585c9d61655429d59c83ba89 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-HasComments: No
[kudu-CR] [tools] Implement a manual leader step down for a tablet
Hello David Ribeiro Alves, Adar Dembo, Todd Lipcon, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4533 to look at the new patch set (#5). Change subject: [tools] Implement a manual leader_step_down for a tablet .. [tools] Implement a manual leader_step_down for a tablet This change introduces a leader_step_down functionality under 'kudu tablet'. This tool may be handy to recover from situations when a single tablet server is overloaded and we want to kick off a new election to balance the load across the clusters. Although it is not guaranteed that a different replica will be elected as the leader, this is an optimistic effort to elect a new tablet server as the leader for the given tablet in the cluster. Also snuck in a small change to display host:port details with 'kudu table list --list_tablets' command. Change-Id: Ia046a28a2008f4f5d1e955f57752a32a1ddc5ab8 --- M src/kudu/tools/kudu-admin-test.cc M src/kudu/tools/tool_action_cluster.cc M src/kudu/tools/tool_action_local_replica.cc M src/kudu/tools/tool_action_table.cc M src/kudu/tools/tool_action_tablet.cc 5 files changed, 187 insertions(+), 44 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/33/4533/5 -- To view, visit http://gerrit.cloudera.org:8080/4533 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia046a28a2008f4f5d1e955f57752a32a1ddc5ab8 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1657: read-only FsManager::Open on active tablet can crash
Dinesh Bhat has posted comments on this change. Change subject: KUDU-1657: read-only FsManager::Open on active tablet can crash .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/4551/2/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: PS2, Line 499: en to by another lbm, we must reinspect th > Compaction writes new blocks to the container, and old ones are hole punche Great, thanks. -- To view, visit http://gerrit.cloudera.org:8080/4551 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If68b04f1f1b8cd099120a220b1245ecf8f422770 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-1657: read-only FsManager::Open on active tablet can crash
Dan Burkert has submitted this change and it was merged. Change subject: KUDU-1657: read-only FsManager::Open on active tablet can crash .. KUDU-1657: read-only FsManager::Open on active tablet can crash This fixes a race condition when opening a read-only FsManager on a data directory with lbm containers that are actively being appended to. See the code comment for more details. A best-effort repro test case is included; it typically takes about 35 seconds to repro without the fix. This issue was identified because it was causing alter_table-randomized-test to be significantly flaky. Change-Id: If68b04f1f1b8cd099120a220b1245ecf8f422770 Reviewed-on: http://gerrit.cloudera.org:8080/4551 Reviewed-by: Adar Dembo Tested-by: Dan Burkert --- M src/kudu/fs/log_block_manager.cc M src/kudu/integration-tests/CMakeLists.txt A src/kudu/integration-tests/open-readonly-fs-itest.cc 3 files changed, 158 insertions(+), 1 deletion(-) Approvals: Dan Burkert: Verified Adar Dembo: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/4551 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: If68b04f1f1b8cd099120a220b1245ecf8f422770 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1657: read-only FsManager::Open on active tablet can crash
Dan Burkert has posted comments on this change. Change subject: KUDU-1657: read-only FsManager::Open on active tablet can crash .. Patch Set 7: Verified+1 Jenkins is completely hosed and I'm tired of waiting. -- To view, visit http://gerrit.cloudera.org:8080/4551 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If68b04f1f1b8cd099120a220b1245ecf8f422770 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] [tools] Implement a manual leader step down for a tablet
Dan Burkert has posted comments on this change. Change subject: [tools] Implement a manual leader_step_down for a tablet .. Patch Set 4: Please rebase this patch before pushing another revision. -- To view, visit http://gerrit.cloudera.org:8080/4533 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia046a28a2008f4f5d1e955f57752a32a1ddc5ab8 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] [tools] select only tablet leader for RWYW
David Ribeiro Alves has posted comments on this change. Change subject: [tools] select only tablet leader for RWYW .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4571 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ied2e7a6b7c31850e585c9d61655429d59c83ba89 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] [tools] Implement a manual leader step down for a tablet
Hello David Ribeiro Alves, Adar Dembo, Todd Lipcon, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4533 to look at the new patch set (#4). Change subject: [tools] Implement a manual leader_step_down for a tablet .. [tools] Implement a manual leader_step_down for a tablet This change introduces a leader_step_down functionality under 'kudu tablet'. This tool may be handy to recover from situations when a single tablet server is overloaded and we want to kick off a new election to balance the load across the clusters. Although it is not guaranteed that a different replica will be elected as the leader, this is an optimistic effort to elect a new tablet server as the leader for the given tablet in the cluster. Also snuck in a small change to display host:port details with 'kudu table list --list_tablets' command. Change-Id: Ia046a28a2008f4f5d1e955f57752a32a1ddc5ab8 --- M src/kudu/tools/kudu-admin-test.cc M src/kudu/tools/tool_action_cluster.cc M src/kudu/tools/tool_action_local_replica.cc M src/kudu/tools/tool_action_table.cc M src/kudu/tools/tool_action_tablet.cc 5 files changed, 186 insertions(+), 42 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/33/4533/4 -- To view, visit http://gerrit.cloudera.org:8080/4533 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia046a28a2008f4f5d1e955f57752a32a1ddc5ab8 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [tools] select only tablet leader for RWYW
Alexey Serbin has uploaded a new change for review. http://gerrit.cloudera.org:8080/4571 Change subject: [tools] select only tablet leader for RWYW .. [tools] select only tablet leader for RWYW To enable stable read-what-you-write behavior in case of multiple replicas, use LEADER_ONLY selection for scan operations. This is a follow-up for 5385af86dc61e56dbe601f0ad6c2bc8fba30ed7c. Change-Id: Ied2e7a6b7c31850e585c9d61655429d59c83ba89 --- M src/kudu/tools/tool_action_test.cc 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/4571/1 -- To view, visit http://gerrit.cloudera.org:8080/4571 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ied2e7a6b7c31850e585c9d61655429d59c83ba89 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin
[kudu-CR] KuduPredicate::Clone on IN list predicate causes segfault
Dan Burkert has submitted this change and it was merged. Change subject: KuduPredicate::Clone on IN list predicate causes segfault .. KuduPredicate::Clone on IN list predicate causes segfault Jordan Birdsell uncovered this issue where a cloned IN list predicate causes a segfault when applied to a scanner. During the clone, the values list was being 'preallocated' using the vector(int) constructor, which adds a bunch of default constructed values (in this case, nullptrs since the type is const void*). We previously didn't have any test coverage of the Clone method, so I added a check to every column predicate test to make sure that scans with cloned predicates behave the same as the original predicates. The test segfaults without the fix. Change-Id: I977691390535273c37aa90c3a3663a7c09ea67fc Reviewed-on: http://gerrit.cloudera.org:8080/4568 Reviewed-by: Adar Dembo Tested-by: Kudu Jenkins Reviewed-by: Jordan Birdsell --- M src/kudu/client/predicate-test.cc M src/kudu/client/scan_predicate-internal.h 2 files changed, 21 insertions(+), 3 deletions(-) Approvals: Jordan Birdsell: Looks good to me, but someone else must approve Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4568 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I977691390535273c37aa90c3a3663a7c09ea67fc Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jordan Birdsell Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [tools] Implement a manual leader step down for a tablet
Dinesh Bhat has posted comments on this change. Change subject: [tools] Implement a manual leader_step_down for a tablet .. Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/4533/1/src/kudu/tools/kudu-admin-test.cc File src/kudu/tools/kudu-admin-test.cc: Line 218: tablet_id_ > But why not get the leader info from the consensus info too? Why go to two One reason I was hesitant to use the same routine for leadership info was because GetConsensus collects the details from all the tservers, and I wasn't sure whether one of them could end up returning a stale info depending on when the RPC hits them. In the code below at L234, probability of that RPC hitting when the leader failover is in transition is very likely. If anything, this was mainly due to not knowing the codebase well and staying on safer side(go to master for leader info). PS1, Line 230: "leader_failure_max_missed_heartbeat_periods", : &max_missed_hb_periods_str)); > Now I see what you mean; the number of attempts in AssertEventually is neve Well we don't want to include the under AssertEventually, because it's perfectly possible that even after specified timeout, we never elected a leader which is different than current_leader. ASSERT_NE(new_leader, current_leader) could yield us incorrect results. I will still debug what I observed is just conincidence or there is more to it than just meet the eye. However, the ++attempts kinda ensures that eventually new_leader emerges on the other side. http://gerrit.cloudera.org:8080/#/c/4533/3/src/kudu/tools/kudu-admin-test.cc File src/kudu/tools/kudu-admin-test.cc: PS3, Line 221: // Grab the default settings for heartbeat timeouts. : string hb_timeout_str; : ASSERT_TRUE(google::GetCommandLineOption("raft_heartbeat_interval_ms", : &hb_timeout_str)); : int32 hb_timeout; : ASSERT_TRUE(safe_strto32(hb_timeout_str, &hb_timeout)); : string max_missed_hb_periods_str; : ASSERT_TRUE( : google::GetCommandLineOption( : "leader_failure_max_missed_heartbeat_periods", : &max_missed_hb_periods_str)); : double max_missed_hb_periods; : ASSERT_TRUE(safe_strtod(max_missed_hb_periods_str, : &max_missed_hb_periods)); > You don't need to go through all this trouble (and you certainly don't need Good point, forgot about DECLARE, done. -- To view, visit http://gerrit.cloudera.org:8080/4533 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia046a28a2008f4f5d1e955f57752a32a1ddc5ab8 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [java client] Tight-ish loop in master lookups if a tablet doesn't have a leader
Jean-Daniel Cryans has uploaded a new change for review. http://gerrit.cloudera.org:8080/4570 Change subject: [java client] Tight-ish loop in master lookups if a tablet doesn't have a leader .. [java client] Tight-ish loop in master lookups if a tablet doesn't have a leader There's currently the possiblity of a situation like this: 1. sendRpcToTablet finds the tablet that the RPC is going to doesn't have a leader 2. a master lookup is sent 3. discoverTablet is called back and still doesn't find a leader 4. RetryRpcCallback is invoked right away, going back to step 1 This quickly gets us with this exception: Too many attempts: KuduRpc(method=Write, tablet=redacted, attempt=101, DeadlineTracker(timeout=3, elapsed=4747) Notice how it retried 101 times in less than 5 seconds. This patch changes step 3 so that an exception is thrown so that RetryRpcErrback is invoked instead, which will add delay before retrying the RPC. This bug was found by ITClient. It's not _just_ a flaky test! Change-Id: Ibf2bd53b03551642e4d036d322e1e592b7c2cfec --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java A java/kudu-client/src/main/java/org/apache/kudu/client/NoSuitableReplicaException.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduClient.java 3 files changed, 98 insertions(+), 19 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/4570/1 -- To view, visit http://gerrit.cloudera.org:8080/4570 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ibf2bd53b03551642e4d036d322e1e592b7c2cfec Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans
[kudu-CR] [c++ client] added few deprecation notes
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4569 to look at the new patch set (#2). Change subject: [c++ client] added few deprecation notes .. [c++ client] added few deprecation notes KUDU-1661 Mark kudu::client::KuduClient::GetLatestObservedTimestamp() as experimental Added deprecation notes for methods related to getting and setting hybrid timestamps to perform READ_AT_SNAPSHOT scan: * KuduClient::GetLatestObservedTimestamp() * KuduClient::SetLatestObservedTimestamp() * KuduScanner::SetSnapshotRaw() Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45 --- M src/kudu/client/client.h 1 file changed, 18 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/69/4569/2 -- To view, visit http://gerrit.cloudera.org:8080/4569 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [c++ client] added few deprecation notes
Alexey Serbin has posted comments on this change. Change subject: [c++ client] added few deprecation notes .. Patch Set 1: (1 comment) > (1 comment) > > Shouldn't we wait until the new API lands before deprecating the > old one though? That's a good question. The original intention for this change was to warn users of the API about the anticipated changes (as David mentioned in HipChat channel). We could just add a notice, warning or attention (@notice, @warning and @attention in Doxygen lingo) as comments, but I thought if were about to deprecate the methods pretty soon, I could just add the deprecation note along with corresponding clang/gcc attribute. It would be nice to collect more feedback on this. http://gerrit.cloudera.org:8080/#/c/4569/1/src/kudu/client/client.h File src/kudu/client/client.h: PS1, Line 396: its > Nit: "in signature". Or maybe just "or change in a future release" Done -- To view, visit http://gerrit.cloudera.org:8080/4569 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [tools] added insert-generated-rows into kudu tools
Alexey Serbin has submitted this change and it was merged. Change subject: [tools] added insert-generated-rows into kudu tools .. [tools] added insert-generated-rows into kudu tools The insert-generated-rows tool has been merged into the 'kudu' umbrella toolset. This addresses KUDU-1628. Besides, added ability to run multiple inserter threads and specify additional parameters on batching behavior of the generated write operations. It's possible to run data generating sessions both in MANUAL_FLUSH and AUTO_FLUSH_BACKGROUND modes. Introduced sequential and random modes for the data generator. Overall, these changes allow to use the tool to measure performance of the Kudu C++ client library in simplistic 'push-as-much-as-you-can' scenario: the client generates and sends data as fast as it can. Change-Id: I332927c0b928c9c4fb81a8e26f5c9ed7565299ad Reviewed-on: http://gerrit.cloudera.org:8080/4412 Reviewed-by: Adar Dembo Tested-by: Kudu Jenkins --- M src/kudu/tools/CMakeLists.txt D src/kudu/tools/insert-generated-rows.cc M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action.cc M src/kudu/tools/tool_action.h M src/kudu/tools/tool_action_cluster.cc M src/kudu/tools/tool_action_fs.cc M src/kudu/tools/tool_action_local_replica.cc M src/kudu/tools/tool_action_master.cc M src/kudu/tools/tool_action_pbc.cc M src/kudu/tools/tool_action_remote_replica.cc M src/kudu/tools/tool_action_table.cc M src/kudu/tools/tool_action_tablet.cc A src/kudu/tools/tool_action_test.cc M src/kudu/tools/tool_action_tserver.cc M src/kudu/tools/tool_action_wal.cc M src/kudu/tools/tool_main.cc 17 files changed, 751 insertions(+), 183 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4412 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I332927c0b928c9c4fb81a8e26f5c9ed7565299ad Gerrit-PatchSet: 23 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [tools] added insert-generated-rows into kudu tools
Adar Dembo has posted comments on this change. Change subject: [tools] added insert-generated-rows into kudu tools .. Patch Set 22: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4412 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I332927c0b928c9c4fb81a8e26f5c9ed7565299ad Gerrit-PatchSet: 22 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] [c++ client] added few deprecation notes
Adar Dembo has posted comments on this change. Change subject: [c++ client] added few deprecation notes .. Patch Set 1: (1 comment) Shouldn't we wait until the new API lands before deprecating the old one though? http://gerrit.cloudera.org:8080/#/c/4569/1/src/kudu/client/client.h File src/kudu/client/client.h: PS1, Line 396: its Nit: "in signature". Or maybe just "or change in a future release" Do the same in the other methods, and in the @deprecated tags. -- To view, visit http://gerrit.cloudera.org:8080/4569 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [tools] added insert-generated-rows into kudu tools
Hello Adar Dembo, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4412 to look at the new patch set (#22). Change subject: [tools] added insert-generated-rows into kudu tools .. [tools] added insert-generated-rows into kudu tools The insert-generated-rows tool has been merged into the 'kudu' umbrella toolset. This addresses KUDU-1628. Besides, added ability to run multiple inserter threads and specify additional parameters on batching behavior of the generated write operations. It's possible to run data generating sessions both in MANUAL_FLUSH and AUTO_FLUSH_BACKGROUND modes. Introduced sequential and random modes for the data generator. Overall, these changes allow to use the tool to measure performance of the Kudu C++ client library in simplistic 'push-as-much-as-you-can' scenario: the client generates and sends data as fast as it can. Change-Id: I332927c0b928c9c4fb81a8e26f5c9ed7565299ad --- M src/kudu/tools/CMakeLists.txt D src/kudu/tools/insert-generated-rows.cc M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action.cc M src/kudu/tools/tool_action.h M src/kudu/tools/tool_action_cluster.cc M src/kudu/tools/tool_action_fs.cc M src/kudu/tools/tool_action_local_replica.cc M src/kudu/tools/tool_action_master.cc M src/kudu/tools/tool_action_pbc.cc M src/kudu/tools/tool_action_remote_replica.cc M src/kudu/tools/tool_action_table.cc M src/kudu/tools/tool_action_tablet.cc A src/kudu/tools/tool_action_test.cc M src/kudu/tools/tool_action_tserver.cc M src/kudu/tools/tool_action_wal.cc M src/kudu/tools/tool_main.cc 17 files changed, 751 insertions(+), 183 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/12/4412/22 -- To view, visit http://gerrit.cloudera.org:8080/4412 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I332927c0b928c9c4fb81a8e26f5c9ed7565299ad Gerrit-PatchSet: 22 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [tools] added insert-generated-rows into kudu tools
Alexey Serbin has posted comments on this change. Change subject: [tools] added insert-generated-rows into kudu tools .. Patch Set 21: (1 comment) http://gerrit.cloudera.org:8080/#/c/4412/21/src/kudu/tools/tool_action_test.cc File src/kudu/tools/tool_action_test.cc: PS21, Line 413: = Status::OK(); > Nit: this is the default value, can omit. Done -- To view, visit http://gerrit.cloudera.org:8080/4412 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I332927c0b928c9c4fb81a8e26f5c9ed7565299ad Gerrit-PatchSet: 21 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [c++ client] added few deprecation notes
Alexey Serbin has uploaded a new change for review. http://gerrit.cloudera.org:8080/4569 Change subject: [c++ client] added few deprecation notes .. [c++ client] added few deprecation notes KUDU-1661 Mark kudu::client::KuduClient::GetLatestObservedTimestamp() as experimental Added deprecation notes for methods related to getting and setting hybrid timestamps to perform READ_AT_SNAPSHOT scan: * KuduClient::GetLatestObservedTimestamp() * KuduClient::SetLatestObservedTimestamp() * KuduScanner::SetSnapshotRaw() Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45 --- M src/kudu/client/client.h 1 file changed, 18 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/69/4569/1 -- To view, visit http://gerrit.cloudera.org:8080/4569 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin
[kudu-CR] KuduPredicate::Clone on IN list predicate causes segfault
Jordan Birdsell has posted comments on this change. Change subject: KuduPredicate::Clone on IN list predicate causes segfault .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/4568 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I977691390535273c37aa90c3a3663a7c09ea67fc Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Jordan Birdsell Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] KUDU-1657: read-only FsManager::Open on active tablet can crash
Adar Dembo has posted comments on this change. Change subject: KUDU-1657: read-only FsManager::Open on active tablet can crash .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4551 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If68b04f1f1b8cd099120a220b1245ecf8f422770 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] [tools] added insert-generated-rows into kudu tools
Adar Dembo has posted comments on this change. Change subject: [tools] added insert-generated-rows into kudu tools .. Patch Set 21: (1 comment) http://gerrit.cloudera.org:8080/#/c/4412/21/src/kudu/tools/tool_action_test.cc File src/kudu/tools/tool_action_test.cc: PS21, Line 413: = Status::OK(); Nit: this is the default value, can omit. -- To view, visit http://gerrit.cloudera.org:8080/4412 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I332927c0b928c9c4fb81a8e26f5c9ed7565299ad Gerrit-PatchSet: 21 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-1657: read-only FsManager::Open on active tablet can crash
Hello David Ribeiro Alves, Adar Dembo, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4551 to look at the new patch set (#7). Change subject: KUDU-1657: read-only FsManager::Open on active tablet can crash .. KUDU-1657: read-only FsManager::Open on active tablet can crash This fixes a race condition when opening a read-only FsManager on a data directory with lbm containers that are actively being appended to. See the code comment for more details. A best-effort repro test case is included; it typically takes about 35 seconds to repro without the fix. This issue was identified because it was causing alter_table-randomized-test to be significantly flaky. Change-Id: If68b04f1f1b8cd099120a220b1245ecf8f422770 --- M src/kudu/fs/log_block_manager.cc M src/kudu/integration-tests/CMakeLists.txt A src/kudu/integration-tests/open-readonly-fs-itest.cc 3 files changed, 158 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/51/4551/7 -- To view, visit http://gerrit.cloudera.org:8080/4551 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: If68b04f1f1b8cd099120a220b1245ecf8f422770 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1657: read-only FsManager::Open on active tablet can crash
Dan Burkert has posted comments on this change. Change subject: KUDU-1657: read-only FsManager::Open on active tablet can crash .. Patch Set 5: (2 comments) http://gerrit.cloudera.org:8080/#/c/4551/2/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: PS2, Line 499: en to by another lbm, we must reinspect th > Hmmm, makes sense, thanks. And how does compaction weigh in here ? i.e if c Compaction writes new blocks to the container, and old ones are hole punched out. The effect is that the logical file size never shrinks, it only grows (although the allocated size on disk may shrink when hole punching occurs). http://gerrit.cloudera.org:8080/#/c/4551/5/src/kudu/integration-tests/open-readonly-fs-itest.cc File src/kudu/integration-tests/open-readonly-fs-itest.cc: Line 36: using kudu::client::KuduWriteOperation; > Unused ? Ignore these comments since I don't trust gerrit searches sometime it's used -- To view, visit http://gerrit.cloudera.org:8080/4551 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If68b04f1f1b8cd099120a220b1245ecf8f422770 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KuduPredicate::Clone on IN list predicate causes segfault
Adar Dembo has posted comments on this change. Change subject: KuduPredicate::Clone on IN list predicate causes segfault .. Patch Set 1: Code-Review+2 Would be great to just remove the Clone() API, or deprecate it or something. -- To view, visit http://gerrit.cloudera.org:8080/4568 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I977691390535273c37aa90c3a3663a7c09ea67fc Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Jordan Birdsell Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] [tools] added insert-generated-rows into kudu tools
Alexey Serbin has posted comments on this change. Change subject: [tools] added insert-generated-rows into kudu tools .. Patch Set 20: (2 comments) http://gerrit.cloudera.org:8080/#/c/4412/20/src/kudu/tools/tool_action_test.cc File src/kudu/tools/tool_action_test.cc: PS20, Line 401: supposed > Nit: "assumed", right? Done PS20, Line 405: // It's necessary to have read-what-you-write behavior here. Since : // tablet leader can change and there might be replica propagation delays, : // set the snapshot to the latest observed one to get correct row count. > As JD discovered with ITClient, if you end up scanning from a replica that OK, thanks for the info. I added retry logic for that. It looks uglier now, but it should do the job. -- To view, visit http://gerrit.cloudera.org:8080/4412 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I332927c0b928c9c4fb81a8e26f5c9ed7565299ad Gerrit-PatchSet: 20 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [tools] added insert-generated-rows into kudu tools
Hello Adar Dembo, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4412 to look at the new patch set (#21). Change subject: [tools] added insert-generated-rows into kudu tools .. [tools] added insert-generated-rows into kudu tools The insert-generated-rows tool has been merged into the 'kudu' umbrella toolset. This addresses KUDU-1628. Besides, added ability to run multiple inserter threads and specify additional parameters on batching behavior of the generated write operations. It's possible to run data generating sessions both in MANUAL_FLUSH and AUTO_FLUSH_BACKGROUND modes. Introduced sequential and random modes for the data generator. Overall, these changes allow to use the tool to measure performance of the Kudu C++ client library in simplistic 'push-as-much-as-you-can' scenario: the client generates and sends data as fast as it can. Change-Id: I332927c0b928c9c4fb81a8e26f5c9ed7565299ad --- M src/kudu/tools/CMakeLists.txt D src/kudu/tools/insert-generated-rows.cc M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action.cc M src/kudu/tools/tool_action.h M src/kudu/tools/tool_action_cluster.cc M src/kudu/tools/tool_action_fs.cc M src/kudu/tools/tool_action_local_replica.cc M src/kudu/tools/tool_action_master.cc M src/kudu/tools/tool_action_pbc.cc M src/kudu/tools/tool_action_remote_replica.cc M src/kudu/tools/tool_action_table.cc M src/kudu/tools/tool_action_tablet.cc A src/kudu/tools/tool_action_test.cc M src/kudu/tools/tool_action_tserver.cc M src/kudu/tools/tool_action_wal.cc M src/kudu/tools/tool_main.cc 17 files changed, 751 insertions(+), 183 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/12/4412/21 -- To view, visit http://gerrit.cloudera.org:8080/4412 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I332927c0b928c9c4fb81a8e26f5c9ed7565299ad Gerrit-PatchSet: 21 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KuduPredicate::Clone on IN list predicate causes segfault
Hello Jordan Birdsell, Adar Dembo, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/4568 to review the following change. Change subject: KuduPredicate::Clone on IN list predicate causes segfault .. KuduPredicate::Clone on IN list predicate causes segfault Jordan Birdsell uncovered this issue where a cloned IN list predicate causes a segfault when applied to a scanner. During the clone, the values list was being 'preallocated' using the vector(int) constructor, which adds a bunch of default constructed values (in this case, nullptrs since the type is const void*). We previously didn't have any test coverage of the Clone method, so I added a check to every column predicate test to make sure that scans with cloned predicates behave the same as the original predicates. The test segfaults without the fix. Change-Id: I977691390535273c37aa90c3a3663a7c09ea67fc --- M src/kudu/client/predicate-test.cc M src/kudu/client/scan_predicate-internal.h 2 files changed, 21 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/68/4568/1 -- To view, visit http://gerrit.cloudera.org:8080/4568 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I977691390535273c37aa90c3a3663a7c09ea67fc Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Jordan Birdsell
[kudu-CR] Add a design doc for rpc retry/failover semantics
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/2642 to look at the new patch set (#7). Change subject: Add a design doc for rpc retry/failover semantics .. Add a design doc for rpc retry/failover semantics This adds the final version of the rpc retry/failover doc and includes details of the final implementation. It also includes a guide and hints on how to implement exactly once semantics for other RPCs. Change-Id: Idc2aa40486153b39724e1c9bd09c626b829274c6 --- M docs/design-docs/README.md A docs/design-docs/rpc-retry-and-failover.md 2 files changed, 232 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/42/2642/7 -- To view, visit http://gerrit.cloudera.org:8080/2642 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Idc2aa40486153b39724e1c9bd09c626b829274c6 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Add a design doc for rpc retry/failover semantics
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/2642 to look at the new patch set (#6). Change subject: Add a design doc for rpc retry/failover semantics .. Add a design doc for rpc retry/failover semantics This adds the final version of the rpc retry/failover doc and includes details of the final implementation. It also includes a guide and hints on how to implement exactly once semantics for other RPCs. Change-Id: Idc2aa40486153b39724e1c9bd09c626b829274c6 --- M docs/design-docs/README.md A docs/design-docs/rpc-retry-and-failover.md 2 files changed, 235 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/42/2642/6 -- To view, visit http://gerrit.cloudera.org:8080/2642 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Idc2aa40486153b39724e1c9bd09c626b829274c6 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [util] Fix a minor bug in AssertEventually()
Dinesh Bhat has posted comments on this change. Change subject: [util] Fix a minor bug in AssertEventually() .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/4566/2/src/kudu/common/column_predicate.cc File src/kudu/common/column_predicate.cc: Line 590: default: LOG(FATAL) << "unknown predicate type"; > Out of curiosity, does ToString() emit a warning too? If not, why not? Hah, I had the same Qn when you suggested me to take a look at this routine, my wild guess was that LOG(FATAL) has some sorta exit code which probably ends up with an implicit typecast to string :). In theory, the warning should be caught on all non-void returns: warning: control reaches end of non-void function [-Wreturn-type] -- To view, visit http://gerrit.cloudera.org:8080/4566 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id34b3461164d96261e4a2d1657244332eb33ad0c Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [WIP]KUDU-1640 - [python] Add IN-list predicate support
Dan Burkert has posted comments on this change. Change subject: [WIP]KUDU-1640 - [python] Add IN-list predicate support .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/4548/3/python/kudu/client.pyx File python/kudu/client.pyx: Line 812: del col_name_slice > box_value can throw, right? Should everything between new Slice and del co actually, it seems like col_name_slice doesn't need to be heap allocated. Could you just move it into NewInListPredicate? I am worried about what happens to the values already created if any of the calls to box_value throw; won't they be leaked? -- To view, visit http://gerrit.cloudera.org:8080/4548 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I932dfded62e162cf85e0e12432cf6716311957de Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jordan Birdsell Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] KUDU-1657: read-only FsManager::Open on active tablet can crash
Dinesh Bhat has posted comments on this change. Change subject: KUDU-1657: read-only FsManager::Open on active tablet can crash .. Patch Set 6: (4 comments) http://gerrit.cloudera.org:8080/#/c/4551/2/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: PS2, Line 499: en to by another lbm, we must reinspect th > Sort of; the log block manager is careful to only write to the metadata fil Hmmm, makes sense, thanks. And how does compaction weigh in here ? i.e if compaction kicked in after the check (data_file_sz < record.details) and end up reducing the data_file_size even further ? I am not sure if that's possible or not or perhaps compaction results into writing to a new data file altogether so that situation doesn't arise here ? http://gerrit.cloudera.org:8080/#/c/4551/5/src/kudu/integration-tests/open-readonly-fs-itest.cc File src/kudu/integration-tests/open-readonly-fs-itest.cc: Line 30: using kudu::client::KuduColumnSchema; Unused ? Line 36: using kudu::client::KuduWriteOperation; Unused ? Ignore these comments since I don't trust gerrit searches sometimes Line 92: } 80-char wrap up ? -- To view, visit http://gerrit.cloudera.org:8080/4551 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If68b04f1f1b8cd099120a220b1245ecf8f422770 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-1657: read-only FsManager::Open on active tablet can crash
Adar Dembo has posted comments on this change. Change subject: KUDU-1657: read-only FsManager::Open on active tablet can crash .. Patch Set 6: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/4551/5/src/kudu/integration-tests/open-readonly-fs-itest.cc File src/kudu/integration-tests/open-readonly-fs-itest.cc: PS5, Line 44: const auto kTimeout = MonoDelta::FromSeconds(60); : const char kTableName[] = "test-table"; : const int kNumColumns = 50; > huh? They were at the top of the TEST_F in rev 3. Yeah, but not with the kCamelCase variable name convention we use. -- To view, visit http://gerrit.cloudera.org:8080/4551 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If68b04f1f1b8cd099120a220b1245ecf8f422770 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-1657: read-only FsManager::Open on active tablet can crash
Dan Burkert has posted comments on this change. Change subject: KUDU-1657: read-only FsManager::Open on active tablet can crash .. Patch Set 5: (2 comments) http://gerrit.cloudera.org:8080/#/c/4551/5/src/kudu/integration-tests/open-readonly-fs-itest.cc File src/kudu/integration-tests/open-readonly-fs-itest.cc: PS5, Line 44: const char kTableName[] = "test-table"; : const int kNumColumns = 50; : const auto kTimeout = MonoDelta::FromSeconds(60); > Sorry, I meant at the top of the TEST_F() method itself, but I guess this i huh? They were at the top of the TEST_F in rev 3. PS5, Line 59: flush blocks > Nit: "flush blocks" and "compaction blocks" isn't wrong, but it suggests th Done -- To view, visit http://gerrit.cloudera.org:8080/4551 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If68b04f1f1b8cd099120a220b1245ecf8f422770 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-1657: read-only FsManager::Open on active tablet can crash
Hello David Ribeiro Alves, Adar Dembo, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4551 to look at the new patch set (#6). Change subject: KUDU-1657: read-only FsManager::Open on active tablet can crash .. KUDU-1657: read-only FsManager::Open on active tablet can crash This fixes a race condition when opening a read-only FsManager on a data directory with lbm containers that are actively being appended to. See the code comment for more details. A best-effort repro test case is included; it typically takes about 35 seconds to repro without the fix. This issue was identified because it was causing alter_table-randomized-test to be significantly flaky. Change-Id: If68b04f1f1b8cd099120a220b1245ecf8f422770 --- M src/kudu/fs/log_block_manager.cc M src/kudu/integration-tests/CMakeLists.txt A src/kudu/integration-tests/open-readonly-fs-itest.cc 3 files changed, 156 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/51/4551/6 -- To view, visit http://gerrit.cloudera.org:8080/4551 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: If68b04f1f1b8cd099120a220b1245ecf8f422770 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [WIP]KUDU-1640 - [python] Add IN-list predicate support
Dan Burkert has posted comments on this change. Change subject: [WIP]KUDU-1640 - [python] Add IN-list predicate support .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/4548/3/python/kudu/client.pyx File python/kudu/client.pyx: Line 812: del col_name_slice box_value can throw, right? Should everything between new Slice and del col_name_slice be in a try /finally? -- To view, visit http://gerrit.cloudera.org:8080/4548 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I932dfded62e162cf85e0e12432cf6716311957de Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jordan Birdsell Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] KUDU-1657: read-only FsManager::Open on active tablet can crash
David Ribeiro Alves has posted comments on this change. Change subject: KUDU-1657: read-only FsManager::Open on active tablet can crash .. Patch Set 5: Code-Review+2 +2 from me, once Adar's nits get addressed. Thanks for adding the test. -- To view, visit http://gerrit.cloudera.org:8080/4551 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If68b04f1f1b8cd099120a220b1245ecf8f422770 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] build-and-test.sh: update gcovr location
Adar Dembo has posted comments on this change. Change subject: build-and-test.sh: update gcovr location .. Patch Set 2: Verified+1 No coverage from gerrit here. -- To view, visit http://gerrit.cloudera.org:8080/4567 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8874d298b6176b4deb45749847032e5c2be55955 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] build-and-test.sh: update gcovr location
Adar Dembo has submitted this change and it was merged. Change subject: build-and-test.sh: update gcovr location .. build-and-test.sh: update gcovr location Coverage builds were failing because of this. Change-Id: I8874d298b6176b4deb45749847032e5c2be55955 Reviewed-on: http://gerrit.cloudera.org:8080/4567 Reviewed-by: David Ribeiro Alves Tested-by: Adar Dembo --- M build-support/jenkins/build-and-test.sh 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: David Ribeiro Alves: Looks good to me, approved Adar Dembo: Verified -- To view, visit http://gerrit.cloudera.org:8080/4567 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I8874d298b6176b4deb45749847032e5c2be55955 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] build-and-test.sh: update gcovr location
David Ribeiro Alves has posted comments on this change. Change subject: build-and-test.sh: update gcovr location .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4567 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8874d298b6176b4deb45749847032e5c2be55955 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] [util] Fix a minor bug in AssertEventually()
Adar Dembo has posted comments on this change. Change subject: [util] Fix a minor bug in AssertEventually() .. Patch Set 2: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/4566/2/src/kudu/common/column_predicate.cc File src/kudu/common/column_predicate.cc: Line 590: default: LOG(FATAL) << "unknown predicate type"; Out of curiosity, does ToString() emit a warning too? If not, why not? -- To view, visit http://gerrit.cloudera.org:8080/4566 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id34b3461164d96261e4a2d1657244332eb33ad0c Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] build-and-test.sh: update gcovr location
Hello David Ribeiro Alves, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4567 to look at the new patch set (#2). Change subject: build-and-test.sh: update gcovr location .. build-and-test.sh: update gcovr location Coverage builds were failing because of this. Change-Id: I8874d298b6176b4deb45749847032e5c2be55955 --- M build-support/jenkins/build-and-test.sh 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/67/4567/2 -- To view, visit http://gerrit.cloudera.org:8080/4567 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8874d298b6176b4deb45749847032e5c2be55955 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [util] Fix a minor bug in AssertEventually()
Dinesh Bhat has posted comments on this change. Change subject: [util] Fix a minor bug in AssertEventually() .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/4566/1/src/kudu/common/column_predicate.cc File src/kudu/common/column_predicate.cc: Line 591: return false; > Hmm. If we add a new PredicateType but forget to update this function, coul Sure, added FATAL under default case, I guess can't get around without a return bool value here to avoid this warning. http://gerrit.cloudera.org:8080/#/c/4566/1/src/kudu/util/test_util.cc File src/kudu/util/test_util.cc: Line 189: while (MonoTime::Now() < deadline) { > Might be simpler as a for loop that initializes attempts to 0, checks the d Done -- To view, visit http://gerrit.cloudera.org:8080/4566 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id34b3461164d96261e4a2d1657244332eb33ad0c Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [util] Fix a minor bug in AssertEventually()
Hello Mike Percy, Adar Dembo, Todd Lipcon, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4566 to look at the new patch set (#2). Change subject: [util] Fix a minor bug in AssertEventually() .. [util] Fix a minor bug in AssertEventually() We want to be sleeping with a monotonically increasing timeout value in this routine. After 10 attempts, we sleep for 1000 ms(constant value). But this routine was never increasing the 'attempts' thus falling back to sleep always at 1ms intervals making it inefficient. Also snuck in a change to remove a warning stemming from fresh builds. Change-Id: Id34b3461164d96261e4a2d1657244332eb33ad0c --- M src/kudu/common/column_predicate.cc M src/kudu/util/test_util.cc 2 files changed, 3 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/66/4566/2 -- To view, visit http://gerrit.cloudera.org:8080/4566 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id34b3461164d96261e4a2d1657244332eb33ad0c Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] build-and-test.sh: update gcovr location
David Ribeiro Alves has posted comments on this change. Change subject: build-and-test.sh: update gcovr location .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4567 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8874d298b6176b4deb45749847032e5c2be55955 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] build-and-test.sh: update gcovr location
Adar Dembo has posted comments on this change. Change subject: build-and-test.sh: update gcovr location .. Patch Set 1: Verified+1 gerrit precommit tests don't cover this (perhaps ironically). -- To view, visit http://gerrit.cloudera.org:8080/4567 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8874d298b6176b4deb45749847032e5c2be55955 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] build-and-test.sh: update gcovr location
Adar Dembo has uploaded a new change for review. http://gerrit.cloudera.org:8080/4567 Change subject: build-and-test.sh: update gcovr location .. build-and-test.sh: update gcovr location Coverage builds were failing because of this. Change-Id: I8874d298b6176b4deb45749847032e5c2be55955 --- M build-support/jenkins/build-and-test.sh 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/67/4567/1 -- To view, visit http://gerrit.cloudera.org:8080/4567 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I8874d298b6176b4deb45749847032e5c2be55955 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo
[kudu-CR] KUDU-1657: read-only FsManager::Open on active tablet can crash
Adar Dembo has posted comments on this change. Change subject: KUDU-1657: read-only FsManager::Open on active tablet can crash .. Patch Set 5: (2 comments) http://gerrit.cloudera.org:8080/#/c/4551/5/src/kudu/integration-tests/open-readonly-fs-itest.cc File src/kudu/integration-tests/open-readonly-fs-itest.cc: PS5, Line 44: const char kTableName[] = "test-table"; : const int kNumColumns = 50; : const auto kTimeout = MonoDelta::FromSeconds(60); Sorry, I meant at the top of the TEST_F() method itself, but I guess this is OK too. PS5, Line 59: flush blocks Nit: "flush blocks" and "compaction blocks" isn't wrong, but it suggests that there is a difference in blocks between flushes and compactions, which isn't really the case. How about "we reduce the MRS flush threshold to increase flush frequency and increase the number of MM threads to encourage frequent compactions. The net effect of both of these changes: more blocks are written to disk." -- To view, visit http://gerrit.cloudera.org:8080/4551 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If68b04f1f1b8cd099120a220b1245ecf8f422770 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [docs] Cleanup beta mentions, links
Jean-Daniel Cryans has submitted this change and it was merged. Change subject: [docs] Cleanup beta mentions, links .. [docs] Cleanup beta mentions, links Change-Id: I56fa65be9d027f5b40628aa5db5f9e4a57c1fbc9 Reviewed-on: http://gerrit.cloudera.org:8080/4565 Reviewed-by: David Ribeiro Alves Tested-by: Kudu Jenkins --- M docs/administration.adoc M docs/configuration.adoc M docs/index.adoc 3 files changed, 5 insertions(+), 7 deletions(-) Approvals: David Ribeiro Alves: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4565 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I56fa65be9d027f5b40628aa5db5f9e4a57c1fbc9 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [docs] Release note for OperationResponse.getWriteTimestamp
Jean-Daniel Cryans has submitted this change and it was merged. Change subject: [docs] Release note for OperationResponse.getWriteTimestamp .. [docs] Release note for OperationResponse.getWriteTimestamp See https://gerrit.cloudera.org/#/c/4487/ Change-Id: I6f81b1c70bc8f8be7f5529762ac9a9622dec00f0 Reviewed-on: http://gerrit.cloudera.org:8080/4560 Reviewed-by: Adar Dembo Tested-by: Jean-Daniel Cryans --- M docs/release_notes.adoc 1 file changed, 4 insertions(+), 0 deletions(-) Approvals: Jean-Daniel Cryans: Verified Adar Dembo: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/4560 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I6f81b1c70bc8f8be7f5529762ac9a9622dec00f0 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Jean-Daniel Cryans
[kudu-CR] [docs] Release note for OperationResponse.getWriteTimestamp
Jean-Daniel Cryans has posted comments on this change. Change subject: [docs] Release note for OperationResponse.getWriteTimestamp .. Patch Set 5: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/4560 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6f81b1c70bc8f8be7f5529762ac9a9622dec00f0 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-HasComments: No
[kudu-CR] KUDU-1657: read-only FsManager::Open on active tablet can crash
Dan Burkert has posted comments on this change. Change subject: KUDU-1657: read-only FsManager::Open on active tablet can crash .. Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/4551/3/src/kudu/integration-tests/open-readonly-fs-itest.cc File src/kudu/integration-tests/open-readonly-fs-itest.cc: PS3, Line 54: // To repro KUDU-1657, many flushes need to be happening, so turn knobs in : // order to write and flush as fast as possible. > Nit: that explains flags #2 and #3, but not #1. For #1, I think it's becaus Done PS3, Line 80: int num_columns = 50; : auto timeout = MonoDelta::FromSeconds(60); > Nit: maybe declare these at the top of the test with kFoo syntax, so it's c Done Line 91: CHECK_OK(b.Build(&schema)); > Nit: could be ASSERT_OK since it's in a test. Done -- To view, visit http://gerrit.cloudera.org:8080/4551 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If68b04f1f1b8cd099120a220b1245ecf8f422770 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-1657: read-only FsManager::Open on active tablet can crash
Hello Adar Dembo, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4551 to look at the new patch set (#5). Change subject: KUDU-1657: read-only FsManager::Open on active tablet can crash .. KUDU-1657: read-only FsManager::Open on active tablet can crash This fixes a race condition when opening a read-only FsManager on a data directory with lbm containers that are actively being appended to. See the code comment for more details. A best-effort repro test case is included; it typically takes about 35 seconds to repro without the fix. This issue was identified because it was causing alter_table-randomized-test to be significantly flaky. Change-Id: If68b04f1f1b8cd099120a220b1245ecf8f422770 --- M src/kudu/fs/log_block_manager.cc M src/kudu/integration-tests/CMakeLists.txt A src/kudu/integration-tests/open-readonly-fs-itest.cc 3 files changed, 157 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/51/4551/5 -- To view, visit http://gerrit.cloudera.org:8080/4551 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: If68b04f1f1b8cd099120a220b1245ecf8f422770 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1657: read-only FsManager::Open on active tablet can crash
Hello Adar Dembo, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4551 to look at the new patch set (#4). Change subject: KUDU-1657: read-only FsManager::Open on active tablet can crash .. KUDU-1657: read-only FsManager::Open on active tablet can crash This fixes a race condition when opening a read-only FsManager on a data directory with lbm containers that are actively being appended to. See the code comment for more details. A best-effort repro test case is included; it typically takes about 35 seconds to repro without the fix. This issue was identified because it was causing alter_table-randomized-test to be significantly flaky. Change-Id: If68b04f1f1b8cd099120a220b1245ecf8f422770 --- M src/kudu/fs/log_block_manager.cc M src/kudu/integration-tests/CMakeLists.txt A src/kudu/integration-tests/open-readonly-fs-itest.cc 3 files changed, 157 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/51/4551/4 -- To view, visit http://gerrit.cloudera.org:8080/4551 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: If68b04f1f1b8cd099120a220b1245ecf8f422770 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1657: read-only FsManager::Open on active tablet can crash
Adar Dembo has posted comments on this change. Change subject: KUDU-1657: read-only FsManager::Open on active tablet can crash .. Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/4551/3/src/kudu/integration-tests/open-readonly-fs-itest.cc File src/kudu/integration-tests/open-readonly-fs-itest.cc: PS3, Line 54: // To repro KUDU-1657, many flushes need to be happening, so turn knobs in : // order to write and flush as fast as possible. Nit: that explains flags #2 and #3, but not #1. For #1, I think it's because you don't want the test to eat gobs of disk space right? Or was it because you wanted container file sizes to change as often as possible? PS3, Line 80: int num_columns = 50; : auto timeout = MonoDelta::FromSeconds(60); Nit: maybe declare these at the top of the test with kFoo syntax, so it's clear they're constants that control the performance characteristics of the test? Line 91: CHECK_OK(b.Build(&schema)); Nit: could be ASSERT_OK since it's in a test. -- To view, visit http://gerrit.cloudera.org:8080/4551 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If68b04f1f1b8cd099120a220b1245ecf8f422770 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [util] Fix a minor bug in AssertEventually()
Adar Dembo has posted comments on this change. Change subject: [util] Fix a minor bug in AssertEventually() .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/4566/1/src/kudu/common/column_predicate.cc File src/kudu/common/column_predicate.cc: Line 591: return false; Hmm. If we add a new PredicateType but forget to update this function, couldn't that lead to really strange errors at runtime? Can we stifle the warning in some other way, perhaps by adding a catch-all LOG(FATAL) as was done in ToString()? http://gerrit.cloudera.org:8080/#/c/4566/1/src/kudu/util/test_util.cc File src/kudu/util/test_util.cc: Line 189: while (MonoTime::Now() < deadline) { Might be simpler as a for loop that initializes attempts to 0, checks the deadline, and increments attempts. -- To view, visit http://gerrit.cloudera.org:8080/4566 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id34b3461164d96261e4a2d1657244332eb33ad0c Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [docs] Release note for OperationResponse.getWriteTimestamp
Adar Dembo has posted comments on this change. Change subject: [docs] Release note for OperationResponse.getWriteTimestamp .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4560 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6f81b1c70bc8f8be7f5529762ac9a9622dec00f0 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] KUDU-1657: read-only FsManager::Open on active tablet can crash
Dan Burkert has posted comments on this change. Change subject: KUDU-1657: read-only FsManager::Open on active tablet can crash .. Patch Set 2: (3 comments) Test added in slow mode. http://gerrit.cloudera.org:8080/#/c/4551/2/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: Line 496: if (data_file_size < record.offset() + record.length()) { > PREDICT_FALSE to annotate that this is a rare race Done PS2, Line 499: in order to account for the latest appends > Good find Dan, this is not a review but a curiosity Qn: Looking at the desc Sort of; the log block manager is careful to only write to the metadata file after writing the data file. So if the other thread or process that's reading the metadata file in read-only mode sees an entry for a block in the metadata file, it should be guaranteed that if it then rereads the data file length, the length will be past the end of the block. PS2, Line 504: local_records.back() > Nit: replace with record ? Done -- To view, visit http://gerrit.cloudera.org:8080/4551 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If68b04f1f1b8cd099120a220b1245ecf8f422770 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-1657: read-only FsManager::Open on active tablet can crash
Hello Adar Dembo, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4551 to look at the new patch set (#3). Change subject: KUDU-1657: read-only FsManager::Open on active tablet can crash .. KUDU-1657: read-only FsManager::Open on active tablet can crash This fixes a race condition when opening a read-only FsManager on a data directory with tablets that are actively flushing. See the code comment for more details. A best-effort repro test case is included; it typically takes about 35 seconds to repro without the fix. This issue was identified because it was causing alter_table-randomized-test to be significantly flaky. Change-Id: If68b04f1f1b8cd099120a220b1245ecf8f422770 --- M src/kudu/fs/log_block_manager.cc M src/kudu/integration-tests/CMakeLists.txt A src/kudu/integration-tests/open-readonly-fs-itest.cc 3 files changed, 154 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/51/4551/3 -- To view, visit http://gerrit.cloudera.org:8080/4551 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: If68b04f1f1b8cd099120a220b1245ecf8f422770 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [util] Fix a minor bug in AssertEventually()
Hello Mike Percy, Adar Dembo, Todd Lipcon, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/4566 to review the following change. Change subject: [util] Fix a minor bug in AssertEventually() .. [util] Fix a minor bug in AssertEventually() We want to be sleeping with a monotonically increasing timeout value in this routine. After 10 attempts, we sleep for 1000 ms(constant value). But this routine was never increasing the 'attempts' thus falling back to sleep always at 1ms intervals making it inefficient. Also snuck in a change to remove a warning stemming from fresh builds. Change-Id: Id34b3461164d96261e4a2d1657244332eb33ad0c --- M src/kudu/common/column_predicate.cc M src/kudu/util/test_util.cc 2 files changed, 2 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/66/4566/1 -- To view, visit http://gerrit.cloudera.org:8080/4566 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Id34b3461164d96261e4a2d1657244332eb33ad0c Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [docs] Cleanup beta mentions, links
David Ribeiro Alves has posted comments on this change. Change subject: [docs] Cleanup beta mentions, links .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4565 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I56fa65be9d027f5b40628aa5db5f9e4a57c1fbc9 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] [docs] Cleanup beta mentions, links
Jean-Daniel Cryans has uploaded a new change for review. http://gerrit.cloudera.org:8080/4565 Change subject: [docs] Cleanup beta mentions, links .. [docs] Cleanup beta mentions, links Change-Id: I56fa65be9d027f5b40628aa5db5f9e4a57c1fbc9 --- M docs/administration.adoc M docs/configuration.adoc M docs/index.adoc 3 files changed, 5 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/65/4565/1 -- To view, visit http://gerrit.cloudera.org:8080/4565 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I56fa65be9d027f5b40628aa5db5f9e4a57c1fbc9 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans
[kudu-CR] [docs] Release note for OperationResponse.getWriteTimestamp
Jean-Daniel Cryans has posted comments on this change. Change subject: [docs] Release note for OperationResponse.getWriteTimestamp .. Patch Set 5: Had to rebase due to your changes, Adar. -- To view, visit http://gerrit.cloudera.org:8080/4560 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6f81b1c70bc8f8be7f5529762ac9a9622dec00f0 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] [docs] Release note for OperationResponse.getWriteTimestamp
Hello Adar Dembo, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4560 to look at the new patch set (#5). Change subject: [docs] Release note for OperationResponse.getWriteTimestamp .. [docs] Release note for OperationResponse.getWriteTimestamp See https://gerrit.cloudera.org/#/c/4487/ Change-Id: I6f81b1c70bc8f8be7f5529762ac9a9622dec00f0 --- M docs/release_notes.adoc 1 file changed, 4 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/60/4560/5 -- To view, visit http://gerrit.cloudera.org:8080/4560 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6f81b1c70bc8f8be7f5529762ac9a9622dec00f0 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [tools] added insert-generated-rows into kudu tools
Adar Dembo has posted comments on this change. Change subject: [tools] added insert-generated-rows into kudu tools .. Patch Set 20: (2 comments) http://gerrit.cloudera.org:8080/#/c/4412/20/src/kudu/tools/tool_action_test.cc File src/kudu/tools/tool_action_test.cc: PS20, Line 401: supposed Nit: "assumed", right? PS20, Line 405: // It's necessary to have read-what-you-write behavior here. Since : // tablet leader can change and there might be replica propagation delays, : // set the snapshot to the latest observed one to get correct row count. As JD discovered with ITClient, if you end up scanning from a replica that is behind the desired timestamp, it'll only wait a certain amount of time to catch up before timing out the scan. You may have to work around that here. See KUDU-1656 for more details. -- To view, visit http://gerrit.cloudera.org:8080/4412 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I332927c0b928c9c4fb81a8e26f5c9ed7565299ad Gerrit-PatchSet: 20 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [tools] added insert-generated-rows into kudu tools
Hello Adar Dembo, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4412 to look at the new patch set (#19). Change subject: [tools] added insert-generated-rows into kudu tools .. [tools] added insert-generated-rows into kudu tools The insert-generated-rows tool has been merged into the 'kudu' umbrella toolset. This addresses KUDU-1628. Besides, added ability to run multiple inserter threads and specify additional parameters on batching behavior of the generated write operations. It's possible to run data generating sessions both in MANUAL_FLUSH and AUTO_FLUSH_BACKGROUND modes. Introduced sequential and random modes for the data generator. Overall, these changes allow to use the tool to measure performance of the Kudu C++ client library in simplistic 'push-as-much-as-you-can' scenario: the client generates and sends data as fast as it can. Change-Id: I332927c0b928c9c4fb81a8e26f5c9ed7565299ad --- M src/kudu/tools/CMakeLists.txt D src/kudu/tools/insert-generated-rows.cc M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action.cc M src/kudu/tools/tool_action.h M src/kudu/tools/tool_action_cluster.cc M src/kudu/tools/tool_action_fs.cc M src/kudu/tools/tool_action_local_replica.cc M src/kudu/tools/tool_action_master.cc M src/kudu/tools/tool_action_pbc.cc M src/kudu/tools/tool_action_remote_replica.cc M src/kudu/tools/tool_action_table.cc M src/kudu/tools/tool_action_tablet.cc A src/kudu/tools/tool_action_test.cc M src/kudu/tools/tool_action_tserver.cc M src/kudu/tools/tool_action_wal.cc M src/kudu/tools/tool_main.cc 17 files changed, 722 insertions(+), 183 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/12/4412/19 -- To view, visit http://gerrit.cloudera.org:8080/4412 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I332927c0b928c9c4fb81a8e26f5c9ed7565299ad Gerrit-PatchSet: 19 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] c++ client: stop requiring the old gcc ABI
Dan Burkert has submitted this change and it was merged. Change subject: c++ client: stop requiring the old gcc ABI .. c++ client: stop requiring the old gcc ABI With the upgrade to clang 3.9 and the transition to libc++ for TSAN, we can now safely remove the old gcc ABI guard rail without breaking TSAN builds. The impact on backwards compatibility is immaterial. At least one Kudu vendor has shipped a client package for Ubuntu 16.04 built against the old ABI; that package will be built against the new ABI going forward. Any application built against the old ABI will be incompatible with said package once the ABI changes. But, c++ client consumers are few and far between, and the major consumer of record (Apache Impala) does not yet build on Ubuntu 16.04. Change-Id: Ia06edb8a4699fc842a2072e1edab5cfaa14c4381 Reviewed-on: http://gerrit.cloudera.org:8080/4515 Reviewed-by: Dan Burkert Tested-by: Dan Burkert --- M .ycm_extra_conf.py M CMakeLists.txt M docs/release_notes.adoc M python/setup.py M src/kudu/client/client.h M src/kudu/client/client_samples-test.sh M src/kudu/client/samples/CMakeLists.txt M thirdparty/build-thirdparty.sh 8 files changed, 9 insertions(+), 51 deletions(-) Approvals: Dan Burkert: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/4515 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ia06edb8a4699fc842a2072e1edab5cfaa14c4381 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] thirdparty: stifle unused argument warnings when building with clang
Dan Burkert has submitted this change and it was merged. Change subject: thirdparty: stifle unused argument warnings when building with clang .. thirdparty: stifle unused argument warnings when building with clang Dan tested my various thirdparty patches on macOS and reported that clang emits thousands of unused argument warnings when building llvm, gflags, and gtest. The first is due to the use of EXTRA_LDFLAGS in the llvm build; the others have always been there. We can tackle this in one of two ways: 1. Add -Qunused-arguments to EXTRA_CXXFLAGS. This isn't as easy as it sounds, because we need to restrict this to clang-based builds (gcc doesn't recognize the parameter), and sometimes that happens via CC/CXX environment variables and other times implicitly. 2. Narrow our prolific additions of -L... and -Wl,-rpath,... such that they're only added when needed. I chose approach #2, which also meant remembering all of our interdependencies. As best I can tell, here is the complete list: - glog depends on gflags - glog depends on libunwind - pmemobj depends on pmem - bitshuffle depends on lz4 With such a short list, approach #2 isn't actually that bad. I tested it by building thirdparty with my system's clang. It worked once I disabled -Werror in the nvml build. I tried very hard not to regress commit 2567ed0. My system should be using gold, though I noticed that only the shared objects in installed/tsan had RUNPATH entries; the ones in installed/uninstrumented and installed/common used RPATH. Nevertheless, I ran both debug and tsan tests, which should have exercised all of them. Change-Id: I41db7fa29c6823c2914881e3fe91844b0c315779 Reviewed-on: http://gerrit.cloudera.org:8080/4514 Reviewed-by: Dan Burkert Tested-by: Dan Burkert --- M thirdparty/build-definitions.sh M thirdparty/build-thirdparty.sh 2 files changed, 36 insertions(+), 22 deletions(-) Approvals: Dan Burkert: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/4514 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I41db7fa29c6823c2914881e3fe91844b0c315779 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] c++ client: stop requiring the old gcc ABI
Dan Burkert has posted comments on this change. Change subject: c++ client: stop requiring the old gcc ABI .. Patch Set 4: Code-Review+2 Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/4515 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia06edb8a4699fc842a2072e1edab5cfaa14c4381 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] thirdparty: build C dependencies with TSAN instrumentation too
Dan Burkert has posted comments on this change. Change subject: thirdparty: build C dependencies with TSAN instrumentation too .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/4559 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie8274ff7bc57016733cdd3c26858483f74e68faf Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] thirdparty: build C dependencies with TSAN instrumentation too
Dan Burkert has submitted this change and it was merged. Change subject: thirdparty: build C dependencies with TSAN instrumentation too .. thirdparty: build C dependencies with TSAN instrumentation too Our C library dependencies are pretty small so this isn't a huge hit, and it makes the sanitizer-based dependency stack much more consistent. Now, 'common' is only for tools and header-only dependencies, while uninstrumented/tsan/... include every library. With additional sanitization come new suppressions. I didn't spend a whole lot of time trying to figure out which were legitimate. Change-Id: Ie8274ff7bc57016733cdd3c26858483f74e68faf Reviewed-on: http://gerrit.cloudera.org:8080/4559 Reviewed-by: Dan Burkert Tested-by: Kudu Jenkins Tested-by: Dan Burkert --- M build-support/tsan-suppressions.txt M cmake_modules/FindPmem.cmake M thirdparty/build-definitions.sh M thirdparty/build-thirdparty.sh 4 files changed, 144 insertions(+), 121 deletions(-) Approvals: Dan Burkert: Looks good to me, approved; Verified Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4559 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ie8274ff7bc57016733cdd3c26858483f74e68faf Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] thirdparty: stifle unused argument warnings when building with clang
Dan Burkert has posted comments on this change. Change subject: thirdparty: stifle unused argument warnings when building with clang .. Patch Set 4: Code-Review+2 Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/4514 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I41db7fa29c6823c2914881e3fe91844b0c315779 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] thirdparty: split into dependency groups
Adar Dembo has submitted this change and it was merged. Change subject: thirdparty: split into dependency groups .. thirdparty: split into dependency groups The monolithic thirdparty build is now quite a bit larger than it used to be on account of the extra LLVM build. Let's see if we can't speed it up. The idea is simple: carve it up into disjoint sections so that individual sections can be rebuilt as needed. This patch separates the various portions of the thirdparty build into "dependency groups". Conceptually, a dependency group is a set of dependencies built a certain way, but the implementation is really just a set of non-overlapping code fragments in build-thirdparty.sh. The initial set of groups are: - common: dependencies that are never instrumented. - uninstrumented: dependencies that may be instrumented, but aren't in this build. - tsan: dependencies that may be instrumented, and are indeed in this build (with -fsanitize=thread). These three generally map to the existing "common", "uninstrumented", and "tsan" thirdparty subdirectories. There's an obvious pattern here for future sanitizer builds (e.g. MSAN would provide an "msan" dependency group). The new build-if-necessary.sh can accept an argument that maps to a set of dependency groups representing a particular build. Every dependency group has its own hash/stamp file so that it is only rebuilt when needed. This also fixes a bug in the stamp file approach that prevented it from actually rebuilding anything. Change-Id: I549262858f98b5ce6c78e786e8c8d8134ba2ed36 Reviewed-on: http://gerrit.cloudera.org:8080/4513 Reviewed-by: Dan Burkert Tested-by: Dan Burkert --- M CMakeLists.txt M build-support/jenkins/build-and-test.sh M thirdparty/.gitignore M thirdparty/build-if-necessary.sh M thirdparty/build-thirdparty.sh 5 files changed, 271 insertions(+), 202 deletions(-) Approvals: Dan Burkert: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/4513 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I549262858f98b5ce6c78e786e8c8d8134ba2ed36 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] thirdparty: split into dependency groups
Dan Burkert has posted comments on this change. Change subject: thirdparty: split into dependency groups .. Patch Set 4: Code-Review+2 Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/4513 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I549262858f98b5ce6c78e786e8c8d8134ba2ed36 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] thirdparty: fix up libtool scripts if needed
Adar Dembo has submitted this change and it was merged. Change subject: thirdparty: fix up libtool scripts if needed .. thirdparty: fix up libtool scripts if needed Older versions of libtool (such as the version found on el6) do not pass -stdlib=libc++ found in CXXFLAGS down to the libtool link command line, and as a result, the shared object is linked against the system libstdc++. The net effect is that some thirdparty shared objects sprout @@GLIBCXX dependencies. Mysteriously, Kudu libraries and binaries themselves do not, and the @@GLIBCXX dependencies appear to be satisfied via libc++ somehow. Nevertheless, this is confusing and could prove problematic down the road, so here's a quick hack to "fix up" libtool scripts after they are generated via configure. Change-Id: Id51c10d38984e892496621135634d21f3e2386ce Reviewed-on: http://gerrit.cloudera.org:8080/4512 Reviewed-by: Dan Burkert Tested-by: Dan Burkert --- M thirdparty/build-definitions.sh M thirdparty/build-thirdparty.sh A thirdparty/postflight.py 3 files changed, 136 insertions(+), 0 deletions(-) Approvals: Dan Burkert: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/4512 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Id51c10d38984e892496621135634d21f3e2386ce Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] thirdparty: fix up libtool scripts if needed
Dan Burkert has posted comments on this change. Change subject: thirdparty: fix up libtool scripts if needed .. Patch Set 5: Code-Review+2 Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/4512 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id51c10d38984e892496621135634d21f3e2386ce Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] thirdparty: use libc++ instead libstdc++ for TSAN builds
Adar Dembo has submitted this change and it was merged. Change subject: thirdparty: use libc++ instead libstdc++ for TSAN builds .. thirdparty: use libc++ instead libstdc++ for TSAN builds This all began because I wanted two things: 1. To use the new gcc 5 ABI on platforms that default to it (such as Ubuntu Xenial). Other applications compiled on these platforms will use the new ABI, and the fact that the Kudu client forces them to use the old ABI is quite unfriendly. 2. To have working local TSAN builds again, which broke following the gcc 5 ABI transition in Xenial. There are a number of interconnected issues at play: A. Until 3.9, LLVM did not recognize gcc's new ABI tags, which prevented Kudu's codegen module from building properly against the new ABI. B. For TSAN builds, we rebuild some thirdparty dependencies against the libstdc++ from thirdparty, but the LLVM libraries are not one of them. This may work when the system libstdc++ and the thirdparty libstdc++ are of the same version, but becomes increasingly untenable as the versions differ. Why? Because libstdc++ only guarantees forward compatibility; that is, a binary linked against libstdc++ can only be used with a libstdc++ of the same version or newer. Attempts to run with an older libstdc++ can lead to run time errors about missing GLIBCXX symbols. As a point of reference, on Xenial the two libraries are more than a major version apart. C. Continuing B, libstdc++ from gcc 5 actually breaks backward compatibility for certain C++11-only symbols by moving them to an inline namespace (e.g. std::error_category is now std::_V2::error_category). The LLVM libraries use these symbols, which means LLVM built against a gcc 5 libstdc++ cannot link against the older libstdc++ in thirdparty. D. As the libstdc++ in thirdparty is from gcc 4, it is not multilib and does not provide new ABI symbols (e.g. std::__cxx11::string). Meaning, if the rest of Kudu tried to use the new ABI, TSAN builds would fail because the libstdc++ in use lacks new ABI symbols. After upgrading LLVM, the path of least resistance was to upgrade libstdc++ in thirdparty, but what a saga that turned out to be. After much trial and error, I gave up; I could not build libstdc++ from gcc 5 with clang, and we must use clang to realize the latest -fsanitize=thread support. Are there any alternatives? Well, we can follow Chromium's lead and use libc++ for TSAN instead of libstdc++. I think this makes sense for several reasons: - The LLVM build, such as it is, is much more friendly than gcc's build. Building libstdc++ out of all of gcc was always a little hacky. - There's at least one large open-source project (Chromium) that's successfully gone down this path. That brings us to this patch, which is largely about replacing libstdc++ with libc++. Here are additional interesting details: o We now build entire set of TSAN-duplicated dependencies with -fsanitize=thread, not just protobuf. It doesn't affect correctness much either way, but it's simpler and an easier concept to extend to future sanitizers that DO care (e.g. MSAN). o We now build LLVM twice: once against the system libstdc++ for build tools and the regular LLVM libraries, and a second time against libc++ for instrumented LLVM libraries. The first build is a little hokey: it'd be more "pure" to build LLVM three times: once for build tools, once for LLVM libraries, and once for instrumented LLVM libraries. But these builds are super long so we optimize by combining the first two. The downside is that the first build now places build tools in 'installed-deps' instead of 'installed'. I played around with placing build tools in 'installed' while placing the libraries in 'installed-deps', but found that to be too hacky. o The full thirdparty build is now quite a bit longer on account of the second LLVM library build. I tried to mitigate this by reducing the number of extra cruft built each time. An upcoming patch will address this further by splitting thirdparty into separate modules. o libc++ depends on libc++abi, so we build that first. o The libc++ and libc++abi builds are done standalone rather than with the LLVM library build, because it isn't possible to do them together AND have the LLVM libraries depend on libc++. o build_python may now be invoked more than once, so I've changed it to be idempotent within the same run of build-thirdparty.sh. Change-Id: Id9e68126ae21e04469053009c5b3e4b588415895 Reviewed-on: http://gerrit.cloudera.org:8080/4511 Reviewed-by: Dan Burkert Tested-by: Dan Burkert Tested-by: Kudu Jenkins --- M CMakeLists.txt M build-support/dist_test.py M build-support/run-test.sh M build-support/run_dist_test.py M cmake_modules/FindPmem.cmake M src/kudu/codegen/CMakeLists.txt M thirdparty/build-definitions.sh M thirdparty/build-thirdparty.sh M thirdp
[kudu-CR] thirdparty: use libc++ instead libstdc++ for TSAN builds
Dan Burkert has posted comments on this change. Change subject: thirdparty: use libc++ instead libstdc++ for TSAN builds .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4511 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id9e68126ae21e04469053009c5b3e4b588415895 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] thirdparty: use libc++ instead libstdc++ for TSAN builds
Dan Burkert has posted comments on this change. Change subject: thirdparty: use libc++ instead libstdc++ for TSAN builds .. Patch Set 3: Verified+1 do it -- To view, visit http://gerrit.cloudera.org:8080/4511 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id9e68126ae21e04469053009c5b3e4b588415895 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] [tools] Implement a manual leader step down for a tablet
Adar Dembo has posted comments on this change. Change subject: [tools] Implement a manual leader_step_down for a tablet .. Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/4533/1/src/kudu/tools/kudu-admin-test.cc File src/kudu/tools/kudu-admin-test.cc: Line 218: tablet_id_ > sure, removed the comparison check, relying on master to report leader info But why not get the leader info from the consensus info too? Why go to two different places to get the combined set of information? That just opens the door for discrepancies between the two. PS1, Line 230: "leader_failure_max_missed_heartbeat_periods", : &max_missed_hb_periods_str)); > Sorry for not being clear before. Lemme try to explain although I haven't r Now I see what you mean; the number of attempts in AssertEventually is never actually incremented. That's a pretty serious problem; can you fix that, and rebase this patch on top of the fix? No unit test needed. As for the correct use of AssertEventually, let's get to the bottom of it as it should work. I can imagine something like this working: string current_leader = ... int64_t current_term = ... AssertEventually([&]() { GetInfoFromConsensus(&new_leader, &new_term) ASSERT_NE(new_leader, current_leader) ASSERT_GT(new_term, current_term) }); It's possible that the broken backoff behavior in AssertEventually is responsible, which is why I suggested you fix the bug and rebase this patch on top of it. If not, use gdb or temporary logging to figure out what's going on. http://gerrit.cloudera.org:8080/#/c/4533/3/src/kudu/tools/kudu-admin-test.cc File src/kudu/tools/kudu-admin-test.cc: PS3, Line 221: // Grab the default settings for heartbeat timeouts. : string hb_timeout_str; : ASSERT_TRUE(google::GetCommandLineOption("raft_heartbeat_interval_ms", : &hb_timeout_str)); : int32 hb_timeout; : ASSERT_TRUE(safe_strto32(hb_timeout_str, &hb_timeout)); : string max_missed_hb_periods_str; : ASSERT_TRUE( : google::GetCommandLineOption( : "leader_failure_max_missed_heartbeat_periods", : &max_missed_hb_periods_str)); : double max_missed_hb_periods; : ASSERT_TRUE(safe_strtod(max_missed_hb_periods_str, : &max_missed_hb_periods)); You don't need to go through all this trouble (and you certainly don't need to do it in a loop; it won't change during the loop). Just access FLAGS_raft_heartbeat_interval_ms and FLAGS_leader_failure_max_missed_heartbeat_periods directly, and DECLARE_* them at the top of the file. -- To view, visit http://gerrit.cloudera.org:8080/4533 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia046a28a2008f4f5d1e955f57752a32a1ddc5ab8 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] thirdparty: reorganize tree
Adar Dembo has submitted this change and it was merged. Change subject: thirdparty: reorganize tree .. thirdparty: reorganize tree This patch changes the organization of the thirdparty tree. The new layout looks like this: - installed: _all_ installed dependencies, with 'common', 'uninstrumented', and 'tsan' subdirectories. - build: build directories for all dependencies. - src: source directories for all dependencies. Additionally, the patch changes the build logic for each dependency so that its build output is fully isolated from its source directory, and from other build output (if the dependency is built multiple times). Why do this? 1. Build isolation simplifies building dependencies multiple times (i.e. for different sanitizers) and makes it much safer. 2. It also means cleaning up build output doesn't mean redownloading all of the sources (i.e. no need to 'git clean -xdf thirdparty'). 3. The grouping of all installed locations under the shared 'installed' subdirectory makes it a tad easier to blow it all away. 4. It also eases the transition for the remaining LLVM and thirdparty patches, as the conflicting build output will be copied to a different set of directories. 5. It slims down thirdparty/.gitignore; adding a new dependency no longer requires updating .gitignore. Change-Id: I06aa75ab5e81f2563986244950e9dcda06c2d383 Reviewed-on: http://gerrit.cloudera.org:8080/4550 Tested-by: Kudu Jenkins Reviewed-by: Dan Burkert --- M .ycm_extra_conf.py M CMakeLists.txt M README.adoc M build-support/dist_test.py M build-support/jenkins/build-and-test.sh M build-support/lint.sh M build-support/run_dist_test.py M docs/installation.adoc M docs/support/scripts/make_site.sh M java/kudu-client/pom.xml M src/kudu/client/client_samples-test.sh M src/kudu/scripts/benchmarks.sh M src/kudu/scripts/tpch.sh M thirdparty/.gitignore M thirdparty/build-definitions.sh M thirdparty/build-thirdparty.sh M thirdparty/download-thirdparty.sh M thirdparty/vars.sh 18 files changed, 334 insertions(+), 217 deletions(-) Approvals: Dan Burkert: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4550 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I06aa75ab5e81f2563986244950e9dcda06c2d383 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [tools] added insert-generated-rows into kudu tools
Adar Dembo has posted comments on this change. Change subject: [tools] added insert-generated-rows into kudu tools .. Patch Set 18: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4412 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I332927c0b928c9c4fb81a8e26f5c9ed7565299ad Gerrit-PatchSet: 18 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] [tools] Implement a manual leader step down for a tablet
Dinesh Bhat has posted comments on this change. Change subject: [tools] Implement a manual leader_step_down for a tablet .. Patch Set 3: (9 comments) TFTR Adar, pls see responses inline, also pls see why my jenkins are still failing. I see that they are still using ...llvm-3.8.0.src/ as per the logs. http://gerrit.cloudera.org:8080/#/c/4533/1/src/kudu/tools/kudu-admin-test.cc File src/kudu/tools/kudu-admin-test.cc: PS1, Line 204: string c > The CHECK_* macros come from glog, and they all do the same thing regardles Thank you for that awesome clarity on when to use them. This has gone in my favorite notes :) Line 218: tablet_id_ > Unfortunately a test environment with leader failure detection isn't determ sure, removed the comparison check, relying on master to report leader info now. I have kept the getconsensus intact to grab the new term info. PS1, Line 230: "leader_failure_max_missed_heartbeat_periods", : &max_missed_hb_periods_str)); > Couple things: Sorry for not being clear before. Lemme try to explain although I haven't root caused the issue myself yet. When I used AssertEventually the GetConsensus comes out around 1.5 secs or so with a new_term. But what is not clear to me yet is, if I sleep for ~1.5 secs to allow the consensus to settle down, I see eventually a new_leader emerging which won't happen if I keep using GetConsensus via AssertEventually(despite 20 retries). Logs are confimring that GetConsensus indeed is detecting the new leader correctly, it's just that new_leader happens to be the current_leader everytime. I want to see why I see same leader gets elected again and again unless I allow some more time window before calling GetConsensus in which case I see new_leader coming up after few retries. With the fix of '++attempts' however, I eventually see another leader coming up so thats kinda hiding the problem. I think that ++attempts fix should have been there to keep the sleep time monotonically increasing. PS1, Line 241: ASSERT_OK(GetTermFromConsensus(tservers, tablet_id_, :&new_ter > Again, the leader can change whenever, so this doesn't seem safe. Done Line 264: > The actual check (not in the CHECK sense of the word, but the comparison an yeah, missed out removing here i think. done. http://gerrit.cloudera.org:8080/#/c/4533/2/src/kudu/tools/kudu-admin-test.cc File src/kudu/tools/kudu-admin-test.cc: PS2, Line 251: AGS_num_ > Still got some leftover CHECK_* statements that should be converted to thei Done, thanks for explaining the distinctions earlier. http://gerrit.cloudera.org:8080/#/c/4533/1/src/kudu/tools/tool_action_tablet.cc File src/kudu/tools/tool_action_tablet.cc: PS1, Line 274: : > You missed these comments down here. Ugh... sorry, for some reason I totally missed out scrolling down thinking that only change I had introduced was in the routine above :) PS1, Line 278: : > I admit, this one was my bad, but can you consolidate all of these descript Done PS1, Line 280: > Likewise for this, it's copied extensively. Done -- To view, visit http://gerrit.cloudera.org:8080/4533 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia046a28a2008f4f5d1e955f57752a32a1ddc5ab8 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [tools] Implement a manual leader step down for a tablet
Hello David Ribeiro Alves, Adar Dembo, Todd Lipcon, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4533 to look at the new patch set (#3). Change subject: [tools] Implement a manual leader_step_down for a tablet .. [tools] Implement a manual leader_step_down for a tablet This change introduces a leader_step_down functionality under 'kudu tablet'. This tool may be handy to recover from situations when a single tablet server is overloaded and we want to kick off a new election to balance the load across the clusters. Although it is not guaranteed that a different replica will be elected as the leader, this is an optimistic effort to elect a new tablet server as the leader for the given tablet in the cluster. Also snuck in a small change to display host:port details with 'kudu table list --list_tablets' command. Change-Id: Ia046a28a2008f4f5d1e955f57752a32a1ddc5ab8 --- M src/kudu/tools/kudu-admin-test.cc M src/kudu/tools/tool_action_cluster.cc M src/kudu/tools/tool_action_local_replica.cc M src/kudu/tools/tool_action_table.cc M src/kudu/tools/tool_action_tablet.cc M src/kudu/util/test_util.cc 6 files changed, 198 insertions(+), 42 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/33/4533/3 -- To view, visit http://gerrit.cloudera.org:8080/4533 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia046a28a2008f4f5d1e955f57752a32a1ddc5ab8 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon