[kudu-CR] [consensus] FAILED UNRECOVERABLE replica health status
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9625 Change subject: [consensus] FAILED_UNRECOVERABLE replica health status .. [consensus] FAILED_UNRECOVERABLE replica health status Added HealthStatus::FAILED_UNRECOVERABLE for a tablet replica. This is to mark replicas behind which are not able to catch up with the leader due to GC-collected logs and other unrecoverable cases. With the introduction of the FAILED_UNRECOVERABLE health status, the replica management scheme becomes hybrid: the system evicts replicas with FAILED_UNRECOVERABLE health status before adding a replacement. This patch is part of the fix to address KUDU-2342. Change-Id: I35637c5bda6681b732dbc2bbf94b9d4258b12095 --- M src/kudu/consensus/consensus_queue.cc M src/kudu/consensus/metadata.proto M src/kudu/consensus/quorum_util-test.cc M src/kudu/consensus/quorum_util.cc 4 files changed, 186 insertions(+), 106 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/25/9625/1 -- To view, visit http://gerrit.cloudera.org:8080/9625 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I35637c5bda6681b732dbc2bbf94b9d4258b12095 Gerrit-Change-Number: 9625 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin
[kudu-CR] KUDU-1704: add python client support for READ YOUR WRITES mode
Hao Hao has removed a vote on this change. Change subject: KUDU-1704: add python client support for READ_YOUR_WRITES mode .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/9617 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I281a73ead2d606e698ff7f44ddb2cd1c78ffdd2a Gerrit-Change-Number: 9617 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-1704: add python client support for READ YOUR WRITES mode
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/9617 ) Change subject: KUDU-1704: add python client support for READ_YOUR_WRITES mode .. Patch Set 2: Verified+1 Unrelated flaky test failure. -- To view, visit http://gerrit.cloudera.org:8080/9617 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I281a73ead2d606e698ff7f44ddb2cd1c78ffdd2a Gerrit-Change-Number: 9617 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Wed, 14 Mar 2018 04:06:01 + Gerrit-HasComments: No
[kudu-CR] KUDU-1704: add python client support for READ YOUR WRITES mode
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/9617 ) Change subject: KUDU-1704: add python client support for READ_YOUR_WRITES mode .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/9617/2/python/kudu/tests/test_scanner.py File python/kudu/tests/test_scanner.py: http://gerrit.cloudera.org:8080/#/c/9617/2/python/kudu/tests/test_scanner.py@207 PS2, Line 207: scanner.set_read_mode(kudu.READ_LATEST)\ > Nit: should this use the string style instead of the enum style to match be I think it is ok, I think the idea is either style should work. -- To view, visit http://gerrit.cloudera.org:8080/9617 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I281a73ead2d606e698ff7f44ddb2cd1c78ffdd2a Gerrit-Change-Number: 9617 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Wed, 14 Mar 2018 04:05:06 + Gerrit-HasComments: Yes
[kudu-CR](branch-1.6.x) KUDU-2343. java: properly reconnect to new leader master after failover
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9616 ) Change subject: KUDU-2343. java: properly reconnect to new leader master after failover .. KUDU-2343. java: properly reconnect to new leader master after failover This fixes the "fake" location information returned in response to a ConnectToMaster RPC to include a distinct "fake UUID" for each master. Previously, we were using an empty string for the UUID of the masters. This caused collisions in the ConnectionCache, which is keyed by server UUIDs. The fake UUID added by this patch matches the fake UUID already in use by AsyncKuduClient.newMasterRpcProxy. This should allow us to share the RPC connection between the ConnectToMaster RPCs and the subsequent GetTableLocation RPCs, which is also a benefit for latency after a failover or on a fresh client. Additionally, this will help with various log messages that previously would print an empty UUID string. A prior version of this patch solved the problem by changing the key for the ConnectionCache to be based on IP address, which has other benefits in terms of future support for servers changing their DNS resolution at runtime. However, since this patch is intended for backport into prior releases, this simpler approach is taken for now. A TODO is added for the longer-term idea. An existing test which tested killing a master now runs in a second mode which restarts the master. This reproduced the bug prior to the fix. This patch also cleans up that test somewhat - it was doing some buggy logic to attempt to kill more than one tablet server, but in fact just called "killTabletServer" three times on the same one. Killing three tablet servers never made sense, either, since the table in the test only had three replicas. Neither did it make sense to start six tablet servers for the test. Change-Id: I36f96c6712800e398ed46887d97d4b09fd993b04 Reviewed-on: http://gerrit.cloudera.org:8080/9612 Reviewed-by: Alexey Serbin Tested-by: Todd Lipcon (cherry picked from commit 4c2bb92f14e1346928ef16dacd63812602683ed2) Reviewed-on: http://gerrit.cloudera.org:8080/9616 Reviewed-by: Grant Henke Tested-by: Grant Henke --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToClusterResponse.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestClientFailoverSupport.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectionCache.java 6 files changed, 65 insertions(+), 19 deletions(-) Approvals: Grant Henke: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/9616 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.6.x Gerrit-MessageType: merged Gerrit-Change-Id: I36f96c6712800e398ed46887d97d4b09fd993b04 Gerrit-Change-Number: 9616 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1704: add python client support for READ YOUR WRITES mode
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/9617 ) Change subject: KUDU-1704: add python client support for READ_YOUR_WRITES mode .. Patch Set 2: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/9617/2/python/kudu/tests/test_scanner.py File python/kudu/tests/test_scanner.py: http://gerrit.cloudera.org:8080/#/c/9617/2/python/kudu/tests/test_scanner.py@207 PS2, Line 207: scanner.set_read_mode(kudu.READ_LATEST)\ Nit: should this use the string style instead of the enum style to match below? or vice versa. -- To view, visit http://gerrit.cloudera.org:8080/9617 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I281a73ead2d606e698ff7f44ddb2cd1c78ffdd2a Gerrit-Change-Number: 9617 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Wed, 14 Mar 2018 02:15:05 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-1704: add python client support for READ YOUR WRITES mode
Hello David Ribeiro Alves, Kudu Jenkins, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9617 to look at the new patch set (#2). Change subject: KUDU-1704: add python client support for READ_YOUR_WRITES mode .. KUDU-1704: add python client support for READ_YOUR_WRITES mode This patch allows users to specify READ_YOUR_WRITES as a read mode in python client. It adds correponding tests to ensure the mode is actually working. Change-Id: I281a73ead2d606e698ff7f44ddb2cd1c78ffdd2a --- M python/kudu/__init__.py M python/kudu/client.pyx M python/kudu/libkudu_client.pxd M python/kudu/tests/test_scanner.py M python/kudu/tests/test_scantoken.py 5 files changed, 31 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/9617/2 -- To view, visit http://gerrit.cloudera.org:8080/9617 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I281a73ead2d606e698ff7f44ddb2cd1c78ffdd2a Gerrit-Change-Number: 9617 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-1704: add python client support for READ YOUR WRITES mode
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/9617 ) Change subject: KUDU-1704: add python client support for READ_YOUR_WRITES mode .. Patch Set 2: (5 comments) http://gerrit.cloudera.org:8080/#/c/9617/1/python/kudu/client.pyx File python/kudu/client.pyx: http://gerrit.cloudera.org:8080/#/c/9617/1/python/kudu/client.pyx@57 PS1, Line 57: rea > best to have it expanded for consistency (i.e. s/ryw/read_your_writes) Done http://gerrit.cloudera.org:8080/#/c/9617/1/python/kudu/tests/test_scanner.py File python/kudu/tests/test_scanner.py: http://gerrit.cloudera.org:8080/#/c/9617/1/python/kudu/tests/test_scanner.py@185 PS1, Line 185: Test scanning in latest, snapshot and read_your_writes read modes. : """ > replace with: Test scanning in latest, snapshot and read_your_writes read m Done http://gerrit.cloudera.org:8080/#/c/9617/1/python/kudu/tests/test_scanner.py@199 PS1, Line 199: > if using the lower/simpler case names keep using those, for consistency Done http://gerrit.cloudera.org:8080/#/c/9617/1/python/kudu/tests/test_scanner.py@216 PS1, Line 216: > extra space Done http://gerrit.cloudera.org:8080/#/c/9617/1/python/kudu/tests/test_scantoken.py File python/kudu/tests/test_scantoken.py: http://gerrit.cloudera.org:8080/#/c/9617/1/python/kudu/tests/test_scantoken.py@196 PS1, Line 196: Test scanning in latest, snapshot and read_your_writes read modes. : """ > same Done -- To view, visit http://gerrit.cloudera.org:8080/9617 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I281a73ead2d606e698ff7f44ddb2cd1c78ffdd2a Gerrit-Change-Number: 9617 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Wed, 14 Mar 2018 01:55:52 + Gerrit-HasComments: Yes
[kudu-CR](branch-1.7.x) KUDU-2153. Servers should not delete tmp files until after locking directories
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/9622 ) Change subject: KUDU-2153. Servers should not delete tmp files until after locking directories .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/9622 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.7.x Gerrit-MessageType: comment Gerrit-Change-Id: I3a3471c8ce00e77fa1712ea518f6ab281864a08d Gerrit-Change-Number: 9622 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 14 Mar 2018 01:20:41 + Gerrit-HasComments: No
[kudu-CR](branch-1.7.x) KUDU-2153. Servers should not delete tmp files until after locking directories
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9622 Change subject: KUDU-2153. Servers should not delete tmp files until after locking directories .. KUDU-2153. Servers should not delete tmp files until after locking directories This changes the order of FsManager startup to not try to clean tmp files until after successfully locking the data directories. This prevents potential issues such as: - a tserver is already running on some host, and in the middle of consensus voting. Thus it has created a tmp file. - someone accidentally attempts to start a tserver with the same set of data dirs. Prior to this patch, it would delete the tmp file before realizing that it could not successfully lock its data dirs and aborting. - the original tserver would crash or otherwise get very confused because the tmp file it just wrote would be gone. This patch relies on the locking on the block manager instance files to provide exclusive access to some non-block-manager-related storage such as the consensus meta, etc. That means that it's still possible for someone to hit the above issue if they were to start servers with disjoint sets of data dirs but with the same meta dir. However, the patch is still a net improvement because the most likely scenario is that the two servers are started with identical configurations. This patch also removes the block_manager_lock_dirs flag which was apparently unused. It was always marked as 'unsafe' so it's not a compatibility issue to remove it without a deprecation period. Change-Id: I3a3471c8ce00e77fa1712ea518f6ab281864a08d Reviewed-on: http://gerrit.cloudera.org:8080/9596 Reviewed-by: Adar Dembo Tested-by: Todd Lipcon (cherry picked from commit d88e5772ee323c8a305250c7d0aa0b49f67475dc) --- M src/kudu/fs/block_manager.cc M src/kudu/fs/file_block_manager.cc M src/kudu/fs/fs_manager-test.cc M src/kudu/fs/fs_manager.cc M src/kudu/fs/log_block_manager.cc M src/kudu/util/env_posix.cc 6 files changed, 49 insertions(+), 24 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/22/9622/1 -- To view, visit http://gerrit.cloudera.org:8080/9622 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.7.x Gerrit-MessageType: newchange Gerrit-Change-Id: I3a3471c8ce00e77fa1712ea518f6ab281864a08d Gerrit-Change-Number: 9622 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Todd Lipcon
[kudu-CR](branch-1.6.x) KUDU-2343. java: properly reconnect to new leader master after failover
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/9616 ) Change subject: KUDU-2343. java: properly reconnect to new leader master after failover .. Patch Set 1: Verified+1 Code-Review+2 Looks like unrelated TSAN flakiness. -- To view, visit http://gerrit.cloudera.org:8080/9616 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.6.x Gerrit-MessageType: comment Gerrit-Change-Id: I36f96c6712800e398ed46887d97d4b09fd993b04 Gerrit-Change-Number: 9616 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Wed, 14 Mar 2018 01:17:04 + Gerrit-HasComments: No
[kudu-CR](branch-1.6.x) KUDU-2343. java: properly reconnect to new leader master after failover
Grant Henke has removed a vote on this change. Change subject: KUDU-2343. java: properly reconnect to new leader master after failover .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/9616 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.6.x Gerrit-MessageType: deleteVote Gerrit-Change-Id: I36f96c6712800e398ed46887d97d4b09fd993b04 Gerrit-Change-Number: 9616 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins
[kudu-CR](branch-1.7.x) KUDU-2343. java: properly reconnect to new leader master after failover
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9615 ) Change subject: KUDU-2343. java: properly reconnect to new leader master after failover .. KUDU-2343. java: properly reconnect to new leader master after failover This fixes the "fake" location information returned in response to a ConnectToMaster RPC to include a distinct "fake UUID" for each master. Previously, we were using an empty string for the UUID of the masters. This caused collisions in the ConnectionCache, which is keyed by server UUIDs. The fake UUID added by this patch matches the fake UUID already in use by AsyncKuduClient.newMasterRpcProxy. This should allow us to share the RPC connection between the ConnectToMaster RPCs and the subsequent GetTableLocation RPCs, which is also a benefit for latency after a failover or on a fresh client. Additionally, this will help with various log messages that previously would print an empty UUID string. A prior version of this patch solved the problem by changing the key for the ConnectionCache to be based on IP address, which has other benefits in terms of future support for servers changing their DNS resolution at runtime. However, since this patch is intended for backport into prior releases, this simpler approach is taken for now. A TODO is added for the longer-term idea. An existing test which tested killing a master now runs in a second mode which restarts the master. This reproduced the bug prior to the fix. This patch also cleans up that test somewhat - it was doing some buggy logic to attempt to kill more than one tablet server, but in fact just called "killTabletServer" three times on the same one. Killing three tablet servers never made sense, either, since the table in the test only had three replicas. Neither did it make sense to start six tablet servers for the test. Change-Id: I36f96c6712800e398ed46887d97d4b09fd993b04 Reviewed-on: http://gerrit.cloudera.org:8080/9612 Reviewed-by: Alexey Serbin Tested-by: Todd Lipcon (cherry picked from commit 487a21476ba3551b3a7ec98cf96f772a495f31fb) Reviewed-on: http://gerrit.cloudera.org:8080/9615 Tested-by: Kudu Jenkins Reviewed-by: Grant Henke --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToClusterResponse.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestClientFailoverSupport.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectionCache.java 6 files changed, 65 insertions(+), 19 deletions(-) Approvals: Kudu Jenkins: Verified Grant Henke: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/9615 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.7.x Gerrit-MessageType: merged Gerrit-Change-Id: I36f96c6712800e398ed46887d97d4b09fd993b04 Gerrit-Change-Number: 9615 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins
[kudu-CR](branch-1.7.x) KUDU-2343. java: properly reconnect to new leader master after failover
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/9615 ) Change subject: KUDU-2343. java: properly reconnect to new leader master after failover .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/9615 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.7.x Gerrit-MessageType: comment Gerrit-Change-Id: I36f96c6712800e398ed46887d97d4b09fd993b04 Gerrit-Change-Number: 9615 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Wed, 14 Mar 2018 01:15:45 + Gerrit-HasComments: No
[kudu-CR](branch-1.7.x) [docs] 'xcode-select' and 'xcodebuild -license' steps on OS X
Grant Henke has removed a vote on this change. Change subject: [docs] 'xcode-select' and 'xcodebuild -license' steps on OS X .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/9621 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.7.x Gerrit-MessageType: deleteVote Gerrit-Change-Id: I107001bb73b3c77bcf873bdac8be8c07c807a010 Gerrit-Change-Number: 9621 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins
[kudu-CR](branch-1.7.x) [docs] 'xcode-select' and 'xcodebuild -license' steps on OS X
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9621 ) Change subject: [docs] 'xcode-select' and 'xcodebuild -license' steps on OS X .. [docs] 'xcode-select' and 'xcodebuild -license' steps on OS X Added a note on accepting the Xcode license and setup command-line tools after installing Xcode. It seems we implicitly assumed those steps were run as a part of the 'install Xcode' step, but I think it's better to explicitly mention those. Also, added macOS High Sierra as a supported platform (development only). Change-Id: I107001bb73b3c77bcf873bdac8be8c07c807a010 Reviewed-on: http://gerrit.cloudera.org:8080/9611 Reviewed-by: Will Berkeley Reviewed-by: Dan Burkert Reviewed-by: Grant Henke Tested-by: Alexey Serbin (cherry picked from commit 846a78563ec3417a405763c5a2c2460317b63b92) Reviewed-on: http://gerrit.cloudera.org:8080/9621 Tested-by: Grant Henke --- M docs/installation.adoc 1 file changed, 8 insertions(+), 1 deletion(-) Approvals: Grant Henke: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/9621 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.7.x Gerrit-MessageType: merged Gerrit-Change-Id: I107001bb73b3c77bcf873bdac8be8c07c807a010 Gerrit-Change-Number: 9621 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins
[kudu-CR](branch-1.7.x) [docs] 'xcode-select' and 'xcodebuild -license' steps on OS X
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9621 Change subject: [docs] 'xcode-select' and 'xcodebuild -license' steps on OS X .. [docs] 'xcode-select' and 'xcodebuild -license' steps on OS X Added a note on accepting the Xcode license and setup command-line tools after installing Xcode. It seems we implicitly assumed those steps were run as a part of the 'install Xcode' step, but I think it's better to explicitly mention those. Also, added macOS High Sierra as a supported platform (development only). Change-Id: I107001bb73b3c77bcf873bdac8be8c07c807a010 Reviewed-on: http://gerrit.cloudera.org:8080/9611 Reviewed-by: Will Berkeley Reviewed-by: Dan Burkert Reviewed-by: Grant Henke Tested-by: Alexey Serbin (cherry picked from commit 846a78563ec3417a405763c5a2c2460317b63b92) --- M docs/installation.adoc 1 file changed, 8 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/21/9621/1 -- To view, visit http://gerrit.cloudera.org:8080/9621 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.7.x Gerrit-MessageType: newchange Gerrit-Change-Id: I107001bb73b3c77bcf873bdac8be8c07c807a010 Gerrit-Change-Number: 9621 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin
[kudu-CR](branch-1.7.x) [docs] 'xcode-select' and 'xcodebuild -license' steps on OS X
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/9621 ) Change subject: [docs] 'xcode-select' and 'xcodebuild -license' steps on OS X .. Patch Set 1: Verified+1 Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/9621 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.7.x Gerrit-MessageType: comment Gerrit-Change-Id: I107001bb73b3c77bcf873bdac8be8c07c807a010 Gerrit-Change-Number: 9621 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Wed, 14 Mar 2018 01:14:30 + Gerrit-HasComments: No
[kudu-CR] KUDU-1704: add python client support for READ YOUR WRITES mode
David Ribeiro Alves has posted comments on this change. ( http://gerrit.cloudera.org:8080/9617 ) Change subject: KUDU-1704: add python client support for READ_YOUR_WRITES mode .. Patch Set 1: (6 comments) http://gerrit.cloudera.org:8080/#/c/9617/1/python/kudu/client.pyx File python/kudu/client.pyx: http://gerrit.cloudera.org:8080/#/c/9617/1/python/kudu/client.pyx@57 PS1, Line 57: ryw best to have it expanded for consistency (i.e. s/ryw/read_your_writes) http://gerrit.cloudera.org:8080/#/c/9617/1/python/kudu/tests/test_scanner.py File python/kudu/tests/test_scanner.py: http://gerrit.cloudera.org:8080/#/c/9617/1/python/kudu/tests/test_scanner.py@185 PS1, Line 185: Test setting the read mode and scanning against a : snapshot, latest and ryw replace with: Test scanning in latest, snapshot and read_your_writes read modes. http://gerrit.cloudera.org:8080/#/c/9617/1/python/kudu/tests/test_scanner.py@199 PS1, Line 199: READ_LATEST if using the lower/simpler case names keep using those, for consistency http://gerrit.cloudera.org:8080/#/c/9617/1/python/kudu/tests/test_scanner.py@216 PS1, Line 216: extra space http://gerrit.cloudera.org:8080/#/c/9617/1/python/kudu/tests/test_scantoken.py File python/kudu/tests/test_scantoken.py: http://gerrit.cloudera.org:8080/#/c/9617/1/python/kudu/tests/test_scantoken.py@196 PS1, Line 196: Test setting the read mode and scanning against a : snapshot, latest and ryw same http://gerrit.cloudera.org:8080/#/c/9617/1/python/kudu/tests/test_scantoken.py@215 PS1, Line 215: READ_LATEST same -- To view, visit http://gerrit.cloudera.org:8080/9617 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I281a73ead2d606e698ff7f44ddb2cd1c78ffdd2a Gerrit-Change-Number: 9617 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Wed, 14 Mar 2018 00:44:31 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-1704: add python client support for READ YOUR WRITES mode
Hao Hao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9617 Change subject: KUDU-1704: add python client support for READ_YOUR_WRITES mode .. KUDU-1704: add python client support for READ_YOUR_WRITES mode This patch allows users to specify READ_YOUR_WRITES as a read mode in python client. It adds correponding tests to ensure the mode is actually working. Change-Id: I281a73ead2d606e698ff7f44ddb2cd1c78ffdd2a --- M python/kudu/__init__.py M python/kudu/client.pyx M python/kudu/libkudu_client.pxd M python/kudu/tests/test_scanner.py M python/kudu/tests/test_scantoken.py 5 files changed, 31 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/9617/1 -- To view, visit http://gerrit.cloudera.org:8080/9617 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I281a73ead2d606e698ff7f44ddb2cd1c78ffdd2a Gerrit-Change-Number: 9617 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao
[kudu-CR] KUDU-2290: Tool to re-create a tablet
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/9393 ) Change subject: KUDU-2290: Tool to re-create a tablet .. Patch Set 5: (2 comments) http://gerrit.cloudera.org:8080/#/c/9393/5/src/kudu/client/meta_cache.cc File src/kudu/client/meta_cache.cc: http://gerrit.cloudera.org:8080/#/c/9393/5/src/kudu/client/meta_cache.cc@912 PS5, Line 912: // Partition should not have changed. > What is a partition except a pair of bounds, in other words, what does your I've developed a bad habit of using "partition" and "tablet" interchangeably. When a tablet is replaced, its partition (or partition boundaries) stay the same, but the tablet ID changes. That's what I'm getting at here; that the ID has changed. http://gerrit.cloudera.org:8080/#/c/9393/5/src/kudu/master/master.proto File src/kudu/master/master.proto: http://gerrit.cloudera.org:8080/#/c/9393/5/src/kudu/master/master.proto@715 PS5, Line 715: > Keep it optional, please. We should never introduce new required fields. Just to close this out, Dan (and Todd) reminded me that for scalars, optional provides a reasonable default value, one that would have been a valid value to begin with. For example, the default for "bytes" in C++ is the empty string, which would have been valid to send had the record been required too. Meaning, if the empty string is invalid input, it would need to be sanitized regardless of whether the field was optional or required. The default value for embedded messages is null and thus is an exception to this rule (i.e. you can't dereference 'foo' without first validating it via 'has_foo()'), but those are less prevalent than scalars, so the burden isn't as onerous. -- To view, visit http://gerrit.cloudera.org:8080/9393 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifbefbde68e3ca724f04efe0426a3906e5c33ed3c Gerrit-Change-Number: 9393 Gerrit-PatchSet: 5 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Wed, 14 Mar 2018 00:06:59 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2290: Tool to re-create a tablet
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/9393 ) Change subject: KUDU-2290: Tool to re-create a tablet .. Patch Set 5: (3 comments) I have reservations about how this patch introduces a new 'quarantined' table. In no other circumstances do we pollute the table namespace with auto-created names, and I don't think this is a good reason to start. This will become very painful down the line when begin to integrate with thirdparty metadata systems like Hive and Sentry which have no such concept as a quarantined table. I think it would be better to simply swap in the new tablet to the existing table, and provide the option to have the master not delete the tablet, if the data is still relevant. The data could then still be accessed and operated on via the CLI tools which take the tablet ID directly. Even then there are still authorization issues that will need to be figured out eventually. http://gerrit.cloudera.org:8080/#/c/9393/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/9393/5//COMMIT_MSG@26 PS5, Line 26: eventually this will stop > Maybe be more specific; this behavior ends when the cached entries' TTL exp There may be some interesting fallout from this. In particular, I believe this is the first time in which the meta-cache TTL is significant for _positive_ cache locations . Right now I think the TTL only has observable effects for non-covered ranges. There's nothing actionable, but it's maybe something to think about and document. http://gerrit.cloudera.org:8080/#/c/9393/5/src/kudu/client/meta_cache.cc File src/kudu/client/meta_cache.cc: http://gerrit.cloudera.org:8080/#/c/9393/5/src/kudu/client/meta_cache.cc@912 PS5, Line 912: // Partition should not have changed. > Could you clarify this a bit: the partition's _boundaries_ should have not What is a partition except a pair of bounds, in other words, what does your suggestion clarify? http://gerrit.cloudera.org:8080/#/c/9393/5/src/kudu/master/master.proto File src/kudu/master/master.proto: http://gerrit.cloudera.org:8080/#/c/9393/5/src/kudu/master/master.proto@715 PS5, Line 715: > I consider myself to be a protobuf luddite, but I don't understand why 'opt Keep it optional, please. We should never introduce new required fields. This is a great example, since you could later go back and add a variant of tablet replacement which just deletes the old version, and doesn't move it to a quarantined state. If we lived in a perfect world with perfect foresight we could use required, but we don't and we shouldn't. -- To view, visit http://gerrit.cloudera.org:8080/9393 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifbefbde68e3ca724f04efe0426a3906e5c33ed3c Gerrit-Change-Number: 9393 Gerrit-PatchSet: 5 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 13 Mar 2018 23:45:30 + Gerrit-HasComments: Yes
[kudu-CR](branch-1.6.x) KUDU-2343. java: properly reconnect to new leader master after failover
Todd Lipcon has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9616 Change subject: KUDU-2343. java: properly reconnect to new leader master after failover .. KUDU-2343. java: properly reconnect to new leader master after failover This fixes the "fake" location information returned in response to a ConnectToMaster RPC to include a distinct "fake UUID" for each master. Previously, we were using an empty string for the UUID of the masters. This caused collisions in the ConnectionCache, which is keyed by server UUIDs. The fake UUID added by this patch matches the fake UUID already in use by AsyncKuduClient.newMasterRpcProxy. This should allow us to share the RPC connection between the ConnectToMaster RPCs and the subsequent GetTableLocation RPCs, which is also a benefit for latency after a failover or on a fresh client. Additionally, this will help with various log messages that previously would print an empty UUID string. A prior version of this patch solved the problem by changing the key for the ConnectionCache to be based on IP address, which has other benefits in terms of future support for servers changing their DNS resolution at runtime. However, since this patch is intended for backport into prior releases, this simpler approach is taken for now. A TODO is added for the longer-term idea. An existing test which tested killing a master now runs in a second mode which restarts the master. This reproduced the bug prior to the fix. This patch also cleans up that test somewhat - it was doing some buggy logic to attempt to kill more than one tablet server, but in fact just called "killTabletServer" three times on the same one. Killing three tablet servers never made sense, either, since the table in the test only had three replicas. Neither did it make sense to start six tablet servers for the test. Change-Id: I36f96c6712800e398ed46887d97d4b09fd993b04 Reviewed-on: http://gerrit.cloudera.org:8080/9612 Reviewed-by: Alexey Serbin Tested-by: Todd Lipcon (cherry picked from commit 4c2bb92f14e1346928ef16dacd63812602683ed2) --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToClusterResponse.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestClientFailoverSupport.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectionCache.java 6 files changed, 65 insertions(+), 19 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/9616/1 -- To view, visit http://gerrit.cloudera.org:8080/9616 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.6.x Gerrit-MessageType: newchange Gerrit-Change-Id: I36f96c6712800e398ed46887d97d4b09fd993b04 Gerrit-Change-Number: 9616 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon
[kudu-CR](branch-1.7.x) KUDU-2343. java: properly reconnect to new leader master after failover
Todd Lipcon has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9615 Change subject: KUDU-2343. java: properly reconnect to new leader master after failover .. KUDU-2343. java: properly reconnect to new leader master after failover This fixes the "fake" location information returned in response to a ConnectToMaster RPC to include a distinct "fake UUID" for each master. Previously, we were using an empty string for the UUID of the masters. This caused collisions in the ConnectionCache, which is keyed by server UUIDs. The fake UUID added by this patch matches the fake UUID already in use by AsyncKuduClient.newMasterRpcProxy. This should allow us to share the RPC connection between the ConnectToMaster RPCs and the subsequent GetTableLocation RPCs, which is also a benefit for latency after a failover or on a fresh client. Additionally, this will help with various log messages that previously would print an empty UUID string. A prior version of this patch solved the problem by changing the key for the ConnectionCache to be based on IP address, which has other benefits in terms of future support for servers changing their DNS resolution at runtime. However, since this patch is intended for backport into prior releases, this simpler approach is taken for now. A TODO is added for the longer-term idea. An existing test which tested killing a master now runs in a second mode which restarts the master. This reproduced the bug prior to the fix. This patch also cleans up that test somewhat - it was doing some buggy logic to attempt to kill more than one tablet server, but in fact just called "killTabletServer" three times on the same one. Killing three tablet servers never made sense, either, since the table in the test only had three replicas. Neither did it make sense to start six tablet servers for the test. Change-Id: I36f96c6712800e398ed46887d97d4b09fd993b04 Reviewed-on: http://gerrit.cloudera.org:8080/9612 Reviewed-by: Alexey Serbin Tested-by: Todd Lipcon (cherry picked from commit 487a21476ba3551b3a7ec98cf96f772a495f31fb) --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToClusterResponse.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestClientFailoverSupport.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectionCache.java 6 files changed, 65 insertions(+), 19 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/9615/1 -- To view, visit http://gerrit.cloudera.org:8080/9615 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.7.x Gerrit-MessageType: newchange Gerrit-Change-Id: I36f96c6712800e398ed46887d97d4b09fd993b04 Gerrit-Change-Number: 9615 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon
[kudu-CR] KUDU-2343. java: properly reconnect to new leader master after failover
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/9612 ) Change subject: KUDU-2343. java: properly reconnect to new leader master after failover .. Patch Set 2: Verified+1 Unrelated flake -- To view, visit http://gerrit.cloudera.org:8080/9612 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I36f96c6712800e398ed46887d97d4b09fd993b04 Gerrit-Change-Number: 9612 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Anonymous Coward #314 Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 13 Mar 2018 23:42:44 + Gerrit-HasComments: No
[kudu-CR] KUDU-2343. java: properly reconnect to new leader master after failover
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9612 ) Change subject: KUDU-2343. java: properly reconnect to new leader master after failover .. KUDU-2343. java: properly reconnect to new leader master after failover This fixes the "fake" location information returned in response to a ConnectToMaster RPC to include a distinct "fake UUID" for each master. Previously, we were using an empty string for the UUID of the masters. This caused collisions in the ConnectionCache, which is keyed by server UUIDs. The fake UUID added by this patch matches the fake UUID already in use by AsyncKuduClient.newMasterRpcProxy. This should allow us to share the RPC connection between the ConnectToMaster RPCs and the subsequent GetTableLocation RPCs, which is also a benefit for latency after a failover or on a fresh client. Additionally, this will help with various log messages that previously would print an empty UUID string. A prior version of this patch solved the problem by changing the key for the ConnectionCache to be based on IP address, which has other benefits in terms of future support for servers changing their DNS resolution at runtime. However, since this patch is intended for backport into prior releases, this simpler approach is taken for now. A TODO is added for the longer-term idea. An existing test which tested killing a master now runs in a second mode which restarts the master. This reproduced the bug prior to the fix. This patch also cleans up that test somewhat - it was doing some buggy logic to attempt to kill more than one tablet server, but in fact just called "killTabletServer" three times on the same one. Killing three tablet servers never made sense, either, since the table in the test only had three replicas. Neither did it make sense to start six tablet servers for the test. Change-Id: I36f96c6712800e398ed46887d97d4b09fd993b04 Reviewed-on: http://gerrit.cloudera.org:8080/9612 Reviewed-by: Alexey Serbin Tested-by: Todd Lipcon --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToClusterResponse.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestClientFailoverSupport.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectionCache.java 6 files changed, 65 insertions(+), 19 deletions(-) Approvals: Alexey Serbin: Looks good to me, approved Todd Lipcon: Verified -- To view, visit http://gerrit.cloudera.org:8080/9612 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I36f96c6712800e398ed46887d97d4b09fd993b04 Gerrit-Change-Number: 9612 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Anonymous Coward #314 Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2343. java: properly reconnect to new leader master after failover
Todd Lipcon has removed a vote on this change. Change subject: KUDU-2343. java: properly reconnect to new leader master after failover .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/9612 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I36f96c6712800e398ed46887d97d4b09fd993b04 Gerrit-Change-Number: 9612 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Anonymous Coward #314 Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2343. java: properly reconnect to new leader master after failover
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/9612 ) Change subject: KUDU-2343. java: properly reconnect to new leader master after failover .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/9612/2/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToClusterResponse.java File java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToClusterResponse.java: http://gerrit.cloudera.org:8080/#/c/9612/2/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToClusterResponse.java@a73 PS2, Line 73: > Was this the culprit of the problem? yea, at least this plus the fact that we cached based on UUID. http://gerrit.cloudera.org:8080/#/c/9612/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectionCache.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectionCache.java: http://gerrit.cloudera.org:8080/#/c/9612/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectionCache.java@a65 PS2, Line 65: > Oh, it seems I misunderstood what those entries were, indeed. I should hav yea I didn't much understand what was going on either til I looked with the debugger -- To view, visit http://gerrit.cloudera.org:8080/9612 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I36f96c6712800e398ed46887d97d4b09fd993b04 Gerrit-Change-Number: 9612 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Anonymous Coward #314 Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 13 Mar 2018 23:42:08 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2153. Servers should not delete tmp files until after locking directories
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9596 ) Change subject: KUDU-2153. Servers should not delete tmp files until after locking directories .. KUDU-2153. Servers should not delete tmp files until after locking directories This changes the order of FsManager startup to not try to clean tmp files until after successfully locking the data directories. This prevents potential issues such as: - a tserver is already running on some host, and in the middle of consensus voting. Thus it has created a tmp file. - someone accidentally attempts to start a tserver with the same set of data dirs. Prior to this patch, it would delete the tmp file before realizing that it could not successfully lock its data dirs and aborting. - the original tserver would crash or otherwise get very confused because the tmp file it just wrote would be gone. This patch relies on the locking on the block manager instance files to provide exclusive access to some non-block-manager-related storage such as the consensus meta, etc. That means that it's still possible for someone to hit the above issue if they were to start servers with disjoint sets of data dirs but with the same meta dir. However, the patch is still a net improvement because the most likely scenario is that the two servers are started with identical configurations. This patch also removes the block_manager_lock_dirs flag which was apparently unused. It was always marked as 'unsafe' so it's not a compatibility issue to remove it without a deprecation period. Change-Id: I3a3471c8ce00e77fa1712ea518f6ab281864a08d Reviewed-on: http://gerrit.cloudera.org:8080/9596 Reviewed-by: Adar Dembo Tested-by: Todd Lipcon --- M src/kudu/fs/block_manager.cc M src/kudu/fs/file_block_manager.cc M src/kudu/fs/fs_manager-test.cc M src/kudu/fs/fs_manager.cc M src/kudu/fs/log_block_manager.cc M src/kudu/util/env_posix.cc 6 files changed, 49 insertions(+), 24 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Todd Lipcon: Verified -- To view, visit http://gerrit.cloudera.org:8080/9596 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I3a3471c8ce00e77fa1712ea518f6ab281864a08d Gerrit-Change-Number: 9596 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2153. Servers should not delete tmp files until after locking directories
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/9596 ) Change subject: KUDU-2153. Servers should not delete tmp files until after locking directories .. Patch Set 2: Verified+1 Unrelated failure -- To view, visit http://gerrit.cloudera.org:8080/9596 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3471c8ce00e77fa1712ea518f6ab281864a08d Gerrit-Change-Number: 9596 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 13 Mar 2018 23:40:03 + Gerrit-HasComments: No
[kudu-CR] KUDU-2153. Servers should not delete tmp files until after locking directories
Todd Lipcon has removed a vote on this change. Change subject: KUDU-2153. Servers should not delete tmp files until after locking directories .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/9596 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I3a3471c8ce00e77fa1712ea518f6ab281864a08d Gerrit-Change-Number: 9596 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2343. java: properly reconnect to new leader master after failover
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/9612 ) Change subject: KUDU-2343. java: properly reconnect to new leader master after failover .. Patch Set 2: Code-Review+2 (3 comments) Thank you for fixing this issue. I didn't expect we had no test that would cover the situation described in KUDU-2343 http://gerrit.cloudera.org:8080/#/c/9612/2/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToClusterResponse.java File java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToClusterResponse.java: http://gerrit.cloudera.org:8080/#/c/9612/2/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToClusterResponse.java@a73 PS2, Line 73: Was this the culprit of the problem? http://gerrit.cloudera.org:8080/#/c/9612/2/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java File java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java: http://gerrit.cloudera.org:8080/#/c/9612/2/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java@72 PS2, Line 72: TODO(todd) it would make more sense to key this by IP address rather than by UUID in :* case a server actually changes address and re-registers to the cluster. Yup, that would be closer to the 'nature of the things', but I think for 1.7 the current fix is adequate enough. http://gerrit.cloudera.org:8080/#/c/9612/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectionCache.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectionCache.java: http://gerrit.cloudera.org:8080/#/c/9612/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectionCache.java@a65 PS2, Line 65: > this old comment was just wrong. The extra 1 was from the fake master ID "" Oh, it seems I misunderstood what those entries were, indeed. I should have printed them out. -- To view, visit http://gerrit.cloudera.org:8080/9612 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I36f96c6712800e398ed46887d97d4b09fd993b04 Gerrit-Change-Number: 9612 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Anonymous Coward #314 Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 13 Mar 2018 23:32:42 + Gerrit-HasComments: Yes
[kudu-CR] [docs] 'xcode-select' and 'xcodebuild -license' steps on OS X
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/9611 ) Change subject: [docs] 'xcode-select' and 'xcodebuild -license' steps on OS X .. Patch Set 2: Verified+1 Unrelated flake. -- To view, visit http://gerrit.cloudera.org:8080/9611 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I107001bb73b3c77bcf873bdac8be8c07c807a010 Gerrit-Change-Number: 9611 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 13 Mar 2018 23:04:20 + Gerrit-HasComments: No
[kudu-CR] [docs] 'xcode-select' and 'xcodebuild -license' steps on OS X
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9611 ) Change subject: [docs] 'xcode-select' and 'xcodebuild -license' steps on OS X .. [docs] 'xcode-select' and 'xcodebuild -license' steps on OS X Added a note on accepting the Xcode license and setup command-line tools after installing Xcode. It seems we implicitly assumed those steps were run as a part of the 'install Xcode' step, but I think it's better to explicitly mention those. Also, added macOS High Sierra as a supported platform (development only). Change-Id: I107001bb73b3c77bcf873bdac8be8c07c807a010 Reviewed-on: http://gerrit.cloudera.org:8080/9611 Reviewed-by: Will Berkeley Reviewed-by: Dan Burkert Reviewed-by: Grant Henke Tested-by: Alexey Serbin --- M docs/installation.adoc 1 file changed, 8 insertions(+), 1 deletion(-) Approvals: Will Berkeley: Looks good to me, approved Dan Burkert: Looks good to me, approved Grant Henke: Looks good to me, approved Alexey Serbin: Verified -- To view, visit http://gerrit.cloudera.org:8080/9611 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I107001bb73b3c77bcf873bdac8be8c07c807a010 Gerrit-Change-Number: 9611 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Will Berkeley
[kudu-CR] [docs] 'xcode-select' and 'xcodebuild -license' steps on OS X
Alexey Serbin has removed Kudu Jenkins from this change. ( http://gerrit.cloudera.org:8080/9611 ) Change subject: [docs] 'xcode-select' and 'xcodebuild -license' steps on OS X .. Removed reviewer Kudu Jenkins with the following votes: * Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/9611 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: I107001bb73b3c77bcf873bdac8be8c07c807a010 Gerrit-Change-Number: 9611 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Will Berkeley
[kudu-CR](branch-1.7.x) [release notes]: RYW read mode
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/9613 ) Change subject: [release notes]: RYW read mode .. Patch Set 1: Verified+1 Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/9613 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.7.x Gerrit-MessageType: comment Gerrit-Change-Id: I9906398b0796307c26150f78e0e256e285ded516 Gerrit-Change-Number: 9613 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 13 Mar 2018 22:12:03 + Gerrit-HasComments: No
[kudu-CR](branch-1.7.x) [release notes]: RYW read mode
Hao Hao has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9613 ) Change subject: [release notes]: RYW read mode .. [release notes]: RYW read mode Change-Id: I9906398b0796307c26150f78e0e256e285ded516 Reviewed-on: http://gerrit.cloudera.org:8080/9594 Reviewed-by: Dan Burkert Reviewed-by: Grant Henke Reviewed-by: David Ribeiro Alves Tested-by: Kudu Jenkins (cherry picked from commit 217715aae6eafdebb15fc33a103dea07e3bb37c1) Reviewed-on: http://gerrit.cloudera.org:8080/9613 Reviewed-by: Hao Hao Tested-by: Hao Hao --- M docs/release_notes.adoc 1 file changed, 9 insertions(+), 0 deletions(-) Approvals: Hao Hao: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/9613 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.7.x Gerrit-MessageType: merged Gerrit-Change-Id: I9906398b0796307c26150f78e0e256e285ded516 Gerrit-Change-Number: 9613 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins
[kudu-CR](branch-1.7.x) [release notes]: RYW read mode
Hao Hao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9613 Change subject: [release notes]: RYW read mode .. [release notes]: RYW read mode Change-Id: I9906398b0796307c26150f78e0e256e285ded516 Reviewed-on: http://gerrit.cloudera.org:8080/9594 Reviewed-by: Dan Burkert Reviewed-by: Grant Henke Reviewed-by: David Ribeiro Alves Tested-by: Kudu Jenkins (cherry picked from commit 217715aae6eafdebb15fc33a103dea07e3bb37c1) --- M docs/release_notes.adoc 1 file changed, 9 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/13/9613/1 -- To view, visit http://gerrit.cloudera.org:8080/9613 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.7.x Gerrit-MessageType: newchange Gerrit-Change-Id: I9906398b0796307c26150f78e0e256e285ded516 Gerrit-Change-Number: 9613 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao
[kudu-CR] [release notes]: RYW read mode
Hao Hao has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9594 ) Change subject: [release notes]: RYW read mode .. [release notes]: RYW read mode Change-Id: I9906398b0796307c26150f78e0e256e285ded516 Reviewed-on: http://gerrit.cloudera.org:8080/9594 Reviewed-by: Dan Burkert Reviewed-by: Grant Henke Reviewed-by: David Ribeiro Alves Tested-by: Kudu Jenkins --- M docs/release_notes.adoc 1 file changed, 9 insertions(+), 0 deletions(-) Approvals: Dan Burkert: Looks good to me, but someone else must approve Grant Henke: Looks good to me, approved David Ribeiro Alves: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/9594 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I9906398b0796307c26150f78e0e256e285ded516 Gerrit-Change-Number: 9594 Gerrit-PatchSet: 4 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-2343. java: properly reconnect to new leader master after failover
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/9612 ) Change subject: KUDU-2343. java: properly reconnect to new leader master after failover .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/9612/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectionCache.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectionCache.java: http://gerrit.cloudera.org:8080/#/c/9612/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectionCache.java@a65 PS2, Line 65: this old comment was just wrong. The extra 1 was from the fake master ID "" being distinct from the three masters which used master- UUIDs. -- To view, visit http://gerrit.cloudera.org:8080/9612 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I36f96c6712800e398ed46887d97d4b09fd993b04 Gerrit-Change-Number: 9612 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Anonymous Coward #314 Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 13 Mar 2018 22:08:05 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2343. java: properly reconnect to new leader master after failover
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/9612 ) Change subject: KUDU-2343. java: properly reconnect to new leader master after failover .. Patch Set 2: > Patch Set 1: > > > Example alternate patch which also seems to fix the issue: > > Big fan. OK, I put up the alternate simpler patch for now. Also removed the new assertions and toString changes to make this an easier backport. -- To view, visit http://gerrit.cloudera.org:8080/9612 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I36f96c6712800e398ed46887d97d4b09fd993b04 Gerrit-Change-Number: 9612 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Anonymous Coward #314 Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 13 Mar 2018 22:07:07 + Gerrit-HasComments: No
[kudu-CR] KUDU-2343. java: properly reconnect to new leader master after failover
Hello Alexey Serbin, Jean-Daniel Cryans, Kudu Jenkins, Adar Dembo, Anonymous Coward #314, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9612 to look at the new patch set (#2). Change subject: KUDU-2343. java: properly reconnect to new leader master after failover .. KUDU-2343. java: properly reconnect to new leader master after failover This fixes the "fake" location information returned in response to a ConnectToMaster RPC to include a distinct "fake UUID" for each master. Previously, we were using an empty string for the UUID of the masters. This caused collisions in the ConnectionCache, which is keyed by server UUIDs. The fake UUID added by this patch matches the fake UUID already in use by AsyncKuduClient.newMasterRpcProxy. This should allow us to share the RPC connection between the ConnectToMaster RPCs and the subsequent GetTableLocation RPCs, which is also a benefit for latency after a failover or on a fresh client. Additionally, this will help with various log messages that previously would print an empty UUID string. A prior version of this patch solved the problem by changing the key for the ConnectionCache to be based on IP address, which has other benefits in terms of future support for servers changing their DNS resolution at runtime. However, since this patch is intended for backport into prior releases, this simpler approach is taken for now. A TODO is added for the longer-term idea. An existing test which tested killing a master now runs in a second mode which restarts the master. This reproduced the bug prior to the fix. This patch also cleans up that test somewhat - it was doing some buggy logic to attempt to kill more than one tablet server, but in fact just called "killTabletServer" three times on the same one. Killing three tablet servers never made sense, either, since the table in the test only had three replicas. Neither did it make sense to start six tablet servers for the test. Change-Id: I36f96c6712800e398ed46887d97d4b09fd993b04 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToClusterResponse.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestClientFailoverSupport.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectionCache.java 6 files changed, 65 insertions(+), 19 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/12/9612/2 -- To view, visit http://gerrit.cloudera.org:8080/9612 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I36f96c6712800e398ed46887d97d4b09fd993b04 Gerrit-Change-Number: 9612 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Anonymous Coward #314 Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2153. Servers should not delete tmp files until after locking directories
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/9596 ) Change subject: KUDU-2153. Servers should not delete tmp files until after locking directories .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/9596 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3471c8ce00e77fa1712ea518f6ab281864a08d Gerrit-Change-Number: 9596 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 13 Mar 2018 21:59:46 + Gerrit-HasComments: No
[kudu-CR] KUDU-2153. Servers should not delete tmp files until after locking directories
Todd Lipcon has posted comments on this change. (
http://gerrit.cloudera.org:8080/9596 )
Change subject: KUDU-2153. Servers should not delete tmp files until after
locking directories
..
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/9596/1/src/kudu/fs/fs_manager-test.cc
File src/kudu/fs/fs_manager-test.cc:
http://gerrit.cloudera.org:8080/#/c/9596/1/src/kudu/fs/fs_manager-test.cc@492
PS1, Line 492: }
> Wouldn't a new scope and a flagsaver be more idiomatic?
Done
http://gerrit.cloudera.org:8080/#/c/9596/1/src/kudu/fs/fs_manager.cc
File src/kudu/fs/fs_manager.cc:
http://gerrit.cloudera.org:8080/#/c/9596/1/src/kudu/fs/fs_manager.cc@391
PS1, Line 391: if (!dd_manager_) {
> Do we need the right permissions to go about locking? If not, then +1
Done. We should have fine permissions. The "check permissions" is mostly to
ensure that they aren't too _open_. iirc we added it when we added more
security support, to ensure that people didn't accidentally run with
world-readable data files and think they were secure.
--
To view, visit http://gerrit.cloudera.org:8080/9596
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3a3471c8ce00e77fa1712ea518f6ab281864a08d
Gerrit-Change-Number: 9596
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Tue, 13 Mar 2018 21:45:42 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2153. Servers should not delete tmp files until after locking directories
Hello Andrew Wong, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9596 to look at the new patch set (#2). Change subject: KUDU-2153. Servers should not delete tmp files until after locking directories .. KUDU-2153. Servers should not delete tmp files until after locking directories This changes the order of FsManager startup to not try to clean tmp files until after successfully locking the data directories. This prevents potential issues such as: - a tserver is already running on some host, and in the middle of consensus voting. Thus it has created a tmp file. - someone accidentally attempts to start a tserver with the same set of data dirs. Prior to this patch, it would delete the tmp file before realizing that it could not successfully lock its data dirs and aborting. - the original tserver would crash or otherwise get very confused because the tmp file it just wrote would be gone. This patch relies on the locking on the block manager instance files to provide exclusive access to some non-block-manager-related storage such as the consensus meta, etc. That means that it's still possible for someone to hit the above issue if they were to start servers with disjoint sets of data dirs but with the same meta dir. However, the patch is still a net improvement because the most likely scenario is that the two servers are started with identical configurations. This patch also removes the block_manager_lock_dirs flag which was apparently unused. It was always marked as 'unsafe' so it's not a compatibility issue to remove it without a deprecation period. Change-Id: I3a3471c8ce00e77fa1712ea518f6ab281864a08d --- M src/kudu/fs/block_manager.cc M src/kudu/fs/file_block_manager.cc M src/kudu/fs/fs_manager-test.cc M src/kudu/fs/fs_manager.cc M src/kudu/fs/log_block_manager.cc M src/kudu/util/env_posix.cc 6 files changed, 49 insertions(+), 24 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/96/9596/2 -- To view, visit http://gerrit.cloudera.org:8080/9596 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3a3471c8ce00e77fa1712ea518f6ab281864a08d Gerrit-Change-Number: 9596 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins
[kudu-CR](branch-1.7.x) KUDU-2230: java: sync client exception stack traces should make sense
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9610 ) Change subject: KUDU-2230: java: sync client exception stack traces should make sense .. KUDU-2230: java: sync client exception stack traces should make sense Previously the exceptions thrown by the synchronous client always had a stack trace somewhere deep inside our async callback chain. So, when the user eventually caught this exception, it would be very hard to even know what part of their code failed, since their own code would not even show up in the call chain. This patch simply replaces the exception's stack trace with the stack at the "join" point where the exception is transformed back to a KuduException. An exception which stores the original stack trace is attached as a "suppressed" exception. Change-Id: I874d47b5239bcc8493c15ef763ecfe0fd878d795 Reviewed-on: http://gerrit.cloudera.org:8080/9489 Tested-by: Kudu Jenkins Reviewed-by: Dan Burkert Reviewed-by: Alexey Serbin (cherry picked from commit ce0db915787b58a79109e6faecc6f1daef9f2850) Reviewed-on: http://gerrit.cloudera.org:8080/9610 Reviewed-by: Grant Henke Tested-by: Grant Henke --- M java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectToCluster.java 2 files changed, 30 insertions(+), 6 deletions(-) Approvals: Grant Henke: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/9610 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.7.x Gerrit-MessageType: merged Gerrit-Change-Id: I874d47b5239bcc8493c15ef763ecfe0fd878d795 Gerrit-Change-Number: 9610 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR](branch-1.7.x) KUDU-2230: java: sync client exception stack traces should make sense
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/9610 ) Change subject: KUDU-2230: java: sync client exception stack traces should make sense .. Patch Set 1: Verified+1 Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/9610 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.7.x Gerrit-MessageType: comment Gerrit-Change-Id: I874d47b5239bcc8493c15ef763ecfe0fd878d795 Gerrit-Change-Number: 9610 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 13 Mar 2018 21:20:10 + Gerrit-HasComments: No
[kudu-CR](branch-1.7.x) KUDU-2322: don't spew logs about behind-log-GC follower
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9609 ) Change subject: KUDU-2322: don't spew logs about behind-log-GC follower .. KUDU-2322: don't spew logs about behind-log-GC follower This patch addresses the issue of a leader spewing a lot of INFO log messages about missing logs segments for a follower that falls behind log GC. In the 3-4-3 replica management scheme, such followers linger around until corresponding non-voter replica catches up with the leader. That might take long time. With this patch, the leader throttles messages about missing log segments for a behind-log-GC follower, outputting 1 message per minute. Change-Id: I4438e63aef0be26c149c57f9f58cb54327e41a33 Reviewed-on: http://gerrit.cloudera.org:8080/9573 Reviewed-by: Todd Lipcon Reviewed-by: Alexey Serbin Tested-by: Kudu Jenkins (cherry picked from commit d856107e0b58067f0bbebbbda52f4f67c674897a) Reviewed-on: http://gerrit.cloudera.org:8080/9609 Tested-by: Grant Henke --- M src/kudu/consensus/consensus-test-util.h M src/kudu/consensus/consensus_peers.cc M src/kudu/consensus/consensus_queue.cc M src/kudu/consensus/consensus_queue.h M src/kudu/util/logging.h 5 files changed, 36 insertions(+), 26 deletions(-) Approvals: Alexey Serbin: Looks good to me, approved Grant Henke: Verified -- To view, visit http://gerrit.cloudera.org:8080/9609 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.7.x Gerrit-MessageType: merged Gerrit-Change-Id: I4438e63aef0be26c149c57f9f58cb54327e41a33 Gerrit-Change-Number: 9609 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins
[kudu-CR](branch-1.7.x) KUDU-2230: java: sync client exception stack traces should make sense
Grant Henke has removed a vote on this change. Change subject: KUDU-2230: java: sync client exception stack traces should make sense .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/9610 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.7.x Gerrit-MessageType: deleteVote Gerrit-Change-Id: I874d47b5239bcc8493c15ef763ecfe0fd878d795 Gerrit-Change-Number: 9610 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2343. java: properly reconnect to new leader master after failover
Todd Lipcon has posted comments on this change. (
http://gerrit.cloudera.org:8080/9612 )
Change subject: KUDU-2343. java: properly reconnect to new leader master after
failover
..
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/9612/1/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java
File java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java:
http://gerrit.cloudera.org:8080/#/c/9612/1/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java@143
PS1, Line 143: AsyncKuduClient.LOG.info("getConnection({}) -> {}",
serverInfo.toString(), result == null ? "null" : result.getLogPrefix());
oops, didn't mean to leave this log in
--
To view, visit http://gerrit.cloudera.org:8080/9612
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I36f96c6712800e398ed46887d97d4b09fd993b04
Gerrit-Change-Number: 9612
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Anonymous Coward #314
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Tue, 13 Mar 2018 21:19:48 +
Gerrit-HasComments: Yes
[kudu-CR](branch-1.7.x) KUDU-2322: don't spew logs about behind-log-GC follower
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/9609 ) Change subject: KUDU-2322: don't spew logs about behind-log-GC follower .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/9609 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.7.x Gerrit-MessageType: comment Gerrit-Change-Id: I4438e63aef0be26c149c57f9f58cb54327e41a33 Gerrit-Change-Number: 9609 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 13 Mar 2018 21:19:18 + Gerrit-HasComments: No
[kudu-CR](branch-1.7.x) KUDU-2322: don't spew logs about behind-log-GC follower
Grant Henke has removed a vote on this change. Change subject: KUDU-2322: don't spew logs about behind-log-GC follower .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/9609 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.7.x Gerrit-MessageType: deleteVote Gerrit-Change-Id: I4438e63aef0be26c149c57f9f58cb54327e41a33 Gerrit-Change-Number: 9609 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins
[kudu-CR](branch-1.7.x) Remove tcmalloc spinlock contention metric
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9608 ) Change subject: Remove tcmalloc_spinlock_contention metric .. Remove tcmalloc_spinlock_contention metric Spinlock contention was removed from tcmalloc upstream, so this metric has been set to 0 since we upgraded to tcmalloc 2.6. I looked briefly at reverting its removal upstream, but the revert would be fairly large, so I think it's best to just accept the loss of this instrumentation. Change-Id: I2c29b81a2c692d5d876d2dd5381ba2c295684324 Reviewed-on: http://gerrit.cloudera.org:8080/9595 Tested-by: Todd Lipcon Reviewed-by: Adar Dembo (cherry picked from commit 4889a9156a7e1f51b076f4bc9f44621a1b2f46e4) Reviewed-on: http://gerrit.cloudera.org:8080/9608 Tested-by: Grant Henke Reviewed-by: Grant Henke --- M docs/release_notes.adoc M src/kudu/scripts/parse_metrics_log.py M src/kudu/util/spinlock_profiling-test.cc M src/kudu/util/spinlock_profiling.cc M src/kudu/util/spinlock_profiling.h M src/kudu/util/trace.h M src/kudu/util/trace_metrics.h 7 files changed, 7 insertions(+), 103 deletions(-) Approvals: Grant Henke: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/9608 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.7.x Gerrit-MessageType: merged Gerrit-Change-Id: I2c29b81a2c692d5d876d2dd5381ba2c295684324 Gerrit-Change-Number: 9608 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2343. java: properly reconnect to new leader master after failover
Jean-Daniel Cryans has posted comments on this change. ( http://gerrit.cloudera.org:8080/9612 ) Change subject: KUDU-2343. java: properly reconnect to new leader master after failover .. Patch Set 1: > Example alternate patch which also seems to fix the issue: Big fan. -- To view, visit http://gerrit.cloudera.org:8080/9612 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I36f96c6712800e398ed46887d97d4b09fd993b04 Gerrit-Change-Number: 9612 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Anonymous Coward #314 Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 13 Mar 2018 21:16:41 + Gerrit-HasComments: No
[kudu-CR](branch-1.7.x) Remove tcmalloc spinlock contention metric
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/9608 ) Change subject: Remove tcmalloc_spinlock_contention metric .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/9608 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.7.x Gerrit-MessageType: comment Gerrit-Change-Id: I2c29b81a2c692d5d876d2dd5381ba2c295684324 Gerrit-Change-Number: 9608 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 13 Mar 2018 21:16:40 + Gerrit-HasComments: No
[kudu-CR] KUDU-2343. java: properly reconnect to new leader master after failover
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/9612 ) Change subject: KUDU-2343. java: properly reconnect to new leader master after failover .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/9612/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/9612/1//COMMIT_MSG@10 PS1, Line 10: We don't currently use the : master UUIDs in the Java client, so the connections to all masters were : getting conflated in the cache. > Also I just double checked, and ConnectToMasterResponsePB doesn't include t Example alternate patch which also seems to fix the issue: https://gist.github.com/8f0f8e38b04fb567f014e248e6f5d79b (maybe this is better for a 1.7-targeted and backportable fix?) -- To view, visit http://gerrit.cloudera.org:8080/9612 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I36f96c6712800e398ed46887d97d4b09fd993b04 Gerrit-Change-Number: 9612 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Anonymous Coward #314 Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 13 Mar 2018 21:15:26 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2343. java: properly reconnect to new leader master after failover
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/9612 ) Change subject: KUDU-2343. java: properly reconnect to new leader master after failover .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/9612/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/9612/1//COMMIT_MSG@10 PS1, Line 10: We don't currently use the : master UUIDs in the Java client, so the connections to all masters were : getting conflated in the cache. > In typical java-client form, we have multiple ways where these rpc proxies Also I just double checked, and ConnectToMasterResponsePB doesn't include the master's UUID. So, the client is never aware of master UUIDs even though the masters have them. So, we could have fixed this bug by filling in a pseudo-ID like hostport.toString() there so as to match the UUID used in newMasterRpcProxy. Do you think that's better? -- To view, visit http://gerrit.cloudera.org:8080/9612 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I36f96c6712800e398ed46887d97d4b09fd993b04 Gerrit-Change-Number: 9612 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Anonymous Coward #314 Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 13 Mar 2018 21:10:47 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2343. java: properly reconnect to new leader master after failover
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/9612 ) Change subject: KUDU-2343. java: properly reconnect to new leader master after failover .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/9612/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/9612/1//COMMIT_MSG@10 PS1, Line 10: We don't currently use the : master UUIDs in the Java client, so the connections to all masters were : getting conflated in the cache. > Just passing through, but can you show me where this happens? I dug around In typical java-client form, we have multiple ways where these rpc proxies get created. newMasterRpcProxy is only used in the 'connectToMasters' function, which is only used when we first connect to the cluster. After we locate a master leader, we make a 'fake' GetTableLocationsResponsePB in ConnectToClusterResponse.getAsTableLocations(). We don't fill in the UUID field there. I suppose filling in that with the master's UUID would have been an alternate way to fix this, rather than caching based on IP/port. However I think the ip/port cache makes more sense anyway in the case that a server re-registers on a different IP/port. (currently we have some server-side deficiencies that prevent that, but we shouldn't compound it with client-side issues) -- To view, visit http://gerrit.cloudera.org:8080/9612 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I36f96c6712800e398ed46887d97d4b09fd993b04 Gerrit-Change-Number: 9612 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Anonymous Coward #314 Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 13 Mar 2018 21:07:18 + Gerrit-HasComments: Yes
[kudu-CR](branch-1.7.x) Remove tcmalloc spinlock contention metric
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/9608 ) Change subject: Remove tcmalloc_spinlock_contention metric .. Patch Set 1: Verified+1 Overriding Jenkins verification due to a TSAN failure on clock sync. -- To view, visit http://gerrit.cloudera.org:8080/9608 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.7.x Gerrit-MessageType: comment Gerrit-Change-Id: I2c29b81a2c692d5d876d2dd5381ba2c295684324 Gerrit-Change-Number: 9608 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 13 Mar 2018 21:01:12 + Gerrit-HasComments: No
[kudu-CR](branch-1.7.x) Remove tcmalloc spinlock contention metric
Grant Henke has removed a vote on this change. Change subject: Remove tcmalloc_spinlock_contention metric .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/9608 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.7.x Gerrit-MessageType: deleteVote Gerrit-Change-Id: I2c29b81a2c692d5d876d2dd5381ba2c295684324 Gerrit-Change-Number: 9608 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [release notes]: RYW read mode
David Ribeiro Alves has posted comments on this change. ( http://gerrit.cloudera.org:8080/9594 ) Change subject: [release notes]: RYW read mode .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/9594 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9906398b0796307c26150f78e0e256e285ded516 Gerrit-Change-Number: 9594 Gerrit-PatchSet: 3 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 13 Mar 2018 20:56:39 + Gerrit-HasComments: No
[kudu-CR] KUDU-2343. java: properly reconnect to new leader master after failover
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/9612 ) Change subject: KUDU-2343. java: properly reconnect to new leader master after failover .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/9612/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/9612/1//COMMIT_MSG@10 PS1, Line 10: We don't currently use the : master UUIDs in the Java client, so the connections to all masters were : getting conflated in the cache. Just passing through, but can you show me where this happens? I dug around and found AsyncKuduClient.newMasterRpcProxy, which creates a ServerInfo with UUID "master-" and that ought to uniquely identify every master in a multi-master cluster. -- To view, visit http://gerrit.cloudera.org:8080/9612 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I36f96c6712800e398ed46887d97d4b09fd993b04 Gerrit-Change-Number: 9612 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Anonymous Coward #314 Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 13 Mar 2018 20:49:02 + Gerrit-HasComments: Yes
[kudu-CR](branch-1.7.x) [tools] KUDU-2331: Allow move tool to work when uninvolved tserver is down
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9607 ) Change subject: [tools] KUDU-2331: Allow move tool to work when uninvolved tserver is down .. [tools] KUDU-2331: Allow move tool to work when uninvolved tserver is down The move tool requires a clean ksck for the tablet it's acting on for safety reasons, but the check for this failed if a tablet server was down, even if the tablet server didn't host a replica for the tablet and wasn't the destination server. This fixes the move command so a down tserver uninvolved in the move won't prevent the move. Change-Id: Id1fc60233f4f50f478da7ccb104e37df3067400c Reviewed-on: http://gerrit.cloudera.org:8080/9510 Reviewed-by: Adar Dembo Tested-by: Kudu Jenkins (cherry picked from commit 13155d4a2f79f522b8213fbb99588109cc04b175) Reviewed-on: http://gerrit.cloudera.org:8080/9607 Tested-by: Grant Henke Reviewed-by: Will Berkeley --- M src/kudu/tools/kudu-admin-test.cc M src/kudu/tools/tool_action_tablet.cc 2 files changed, 60 insertions(+), 33 deletions(-) Approvals: Grant Henke: Verified Will Berkeley: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/9607 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.7.x Gerrit-MessageType: merged Gerrit-Change-Id: Id1fc60233f4f50f478da7ccb104e37df3067400c Gerrit-Change-Number: 9607 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley
[kudu-CR](branch-1.7.x) [tools] KUDU-2331: Allow move tool to work when uninvolved tserver is down
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/9607 ) Change subject: [tools] KUDU-2331: Allow move tool to work when uninvolved tserver is down .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/9607 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.7.x Gerrit-MessageType: comment Gerrit-Change-Id: Id1fc60233f4f50f478da7ccb104e37df3067400c Gerrit-Change-Number: 9607 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 13 Mar 2018 20:44:19 + Gerrit-HasComments: No
[kudu-CR](branch-1.7.x) [tools] KUDU-2331: Allow move tool to work when uninvolved tserver is down
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/9607 ) Change subject: [tools] KUDU-2331: Allow move tool to work when uninvolved tserver is down .. Patch Set 1: Verified+1 Overriding Jenkins verification. Failed because of unrelated flaky. -- To view, visit http://gerrit.cloudera.org:8080/9607 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.7.x Gerrit-MessageType: comment Gerrit-Change-Id: Id1fc60233f4f50f478da7ccb104e37df3067400c Gerrit-Change-Number: 9607 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 13 Mar 2018 20:37:30 + Gerrit-HasComments: No
[kudu-CR](branch-1.7.x) [tools] KUDU-2331: Allow move tool to work when uninvolved tserver is down
Grant Henke has removed a vote on this change. Change subject: [tools] KUDU-2331: Allow move tool to work when uninvolved tserver is down .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/9607 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.7.x Gerrit-MessageType: deleteVote Gerrit-Change-Id: Id1fc60233f4f50f478da7ccb104e37df3067400c Gerrit-Change-Number: 9607 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-2343. java: properly reconnect to new leader master after failover
Hello Alexey Serbin, Anonymous Coward #314, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/9612 to review the following change. Change subject: KUDU-2343. java: properly reconnect to new leader master after failover .. KUDU-2343. java: properly reconnect to new leader master after failover This fixes the connection cache in the java client to be based on IP address and port rather than based on UUID. We don't currently use the master UUIDs in the Java client, so the connections to all masters were getting conflated in the cache. This would cause the client to reconnect back to a follower master even if the leader had changed and even if it had properly identified the address of the new leader. An existing test which tested killing a master now runs in a second mode which restarts the master. This reproduced the bug prior to the fix. This patch also cleans up that test somewhat - it was doing some buggy logic to attempt to kill more than one tablet server, but in fact just called "killTabletServer" three times on the same one. Killing three tablet servers never made sense, either, since the table in the test only had three replicas. Neither did it make sense to start six tablet servers for the test. Along the way, this patch also improves toString() output for a number of Java client structures. This makes it easier to debug Connection-level issues. Change-Id: I36f96c6712800e398ed46887d97d4b09fd993b04 --- M java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java M java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java M java/kudu-client/src/main/java/org/apache/kudu/client/TableLocationsCache.java M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestClientFailoverSupport.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestRemoteTablet.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestServerInfo.java 9 files changed, 105 insertions(+), 37 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/12/9612/1 -- To view, visit http://gerrit.cloudera.org:8080/9612 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I36f96c6712800e398ed46887d97d4b09fd993b04 Gerrit-Change-Number: 9612 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Anonymous Coward #314
[kudu-CR] [release notes]: RYW read mode
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/9594 ) Change subject: [release notes]: RYW read mode .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/9594 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9906398b0796307c26150f78e0e256e285ded516 Gerrit-Change-Number: 9594 Gerrit-PatchSet: 3 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 13 Mar 2018 20:18:22 + Gerrit-HasComments: No
[kudu-CR] [release notes]: RYW read mode
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/9594 ) Change subject: [release notes]: RYW read mode .. Patch Set 3: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/9594 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9906398b0796307c26150f78e0e256e285ded516 Gerrit-Change-Number: 9594 Gerrit-PatchSet: 3 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 13 Mar 2018 20:15:29 + Gerrit-HasComments: No
[kudu-CR] [docs] 'xcode-select' and 'xcodebuild -license' steps on OS X
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/9611 ) Change subject: [docs] 'xcode-select' and 'xcodebuild -license' steps on OS X .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/9611 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I107001bb73b3c77bcf873bdac8be8c07c807a010 Gerrit-Change-Number: 9611 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 13 Mar 2018 20:15:18 + Gerrit-HasComments: No
[kudu-CR] [docs] 'xcode-select' and 'xcodebuild -license' steps on OS X
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/9611 ) Change subject: [docs] 'xcode-select' and 'xcodebuild -license' steps on OS X .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/9611 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I107001bb73b3c77bcf873bdac8be8c07c807a010 Gerrit-Change-Number: 9611 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 13 Mar 2018 20:14:46 + Gerrit-HasComments: No
[kudu-CR] KUDU-2305: Limit sidecars to INT MAX and fortify socket code
Hello Michael Ho, Kudu Jenkins, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9601 to look at the new patch set (#2). Change subject: KUDU-2305: Limit sidecars to INT_MAX and fortify socket code .. KUDU-2305: Limit sidecars to INT_MAX and fortify socket code Inspection of the code revealed some other local variables that could overflow with large messages. This patch takes two approaches to eliminate the issues. First, it limits the total size of the messages by limiting the total size of the sidecars to INT_MAX. The total size of the protobuf and header components of the message should be considerably smaller, so limiting the sidecars to INT_MAX eliminates messages that are larger than UINT_MAX. This also means that the sidecar offsets, which are unsigned 32-bit integers, are also safe. Given that the rpc_max_message_size is 32-bit integer, the receiver would reject any message with sidecars this large anyway. This also helps with the networking codepath, as any given sidecar will have a size less than INT_MAX, so all Slices that interact with Writev() are each less than INT_MAX. Second, even with sidecars limited to INT_MAX, the headers and protobuf parts of the messages mean that certain messages could still exceed INT_MAX. This patch changes some of the sockets codepath to tolerate iovec's that reference more than INT_MAX bytes total. Specifically, it changes Writev()'s nwritten bytes to an int64_t for both TlsSocket and Socket. TlsSocket works because it is sending each Slice individually. The first change limited any given Slice to INT_MAX, so each individual Write() should not be impacted. For Socket, Writev() uses sendmsg(). It should do partial network sends to handle this case. Any Write() call specifies its size with a 32-bit integer, and that will not be impacted by this patch. Testing: - Modified TestRpcSidecarLimits() to verify that sidecars are limited to INT_MAX. Change-Id: I2d041e214b15d9c22b810588643798e2b3bc5c24 --- M src/kudu/rpc/inbound_call.cc M src/kudu/rpc/inbound_call.h M src/kudu/rpc/outbound_call.cc M src/kudu/rpc/rpc-test.cc M src/kudu/rpc/rpc_controller.cc M src/kudu/rpc/rpc_controller.h M src/kudu/rpc/serialization.cc M src/kudu/rpc/transfer.cc M src/kudu/rpc/transfer.h M src/kudu/security/tls_socket-test.cc M src/kudu/security/tls_socket.cc M src/kudu/security/tls_socket.h M src/kudu/util/net/socket.cc M src/kudu/util/net/socket.h 14 files changed, 76 insertions(+), 21 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/01/9601/2 -- To view, visit http://gerrit.cloudera.org:8080/9601 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2d041e214b15d9c22b810588643798e2b3bc5c24 Gerrit-Change-Number: 9601 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2305: Limit sidecars to INT MAX and fortify socket code
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/9601 ) Change subject: KUDU-2305: Limit sidecars to INT_MAX and fortify socket code .. Patch Set 1: (6 comments) http://gerrit.cloudera.org:8080/#/c/9601/1/src/kudu/rpc/inbound_call.h File src/kudu/rpc/inbound_call.h: http://gerrit.cloudera.org:8080/#/c/9601/1/src/kudu/rpc/inbound_call.h@253 PS1, Line 253: outbound_sidecars_size_ > I think 'total_size_' or 'size_bytes_' or something is more clear, since 's Changed to "outbound_sidecars_total_bytes_" http://gerrit.cloudera.org:8080/#/c/9601/1/src/kudu/rpc/inbound_call.cc File src/kudu/rpc/inbound_call.cc: http://gerrit.cloudera.org:8080/#/c/9601/1/src/kudu/rpc/inbound_call.cc@186 PS1, Line 186: absolute_sidecar_offset - protobuf_msg_size > these are both uint32_t, right? maybe we should change absolute_sidecar_off Good point, best to be defensive. Changed absolute_sidecar_offset to an int64_t and added a check for it be less than UINT_MAX. http://gerrit.cloudera.org:8080/#/c/9601/1/src/kudu/rpc/inbound_call.cc@221 PS1, Line 221: exceeds limit > would exceed Done http://gerrit.cloudera.org:8080/#/c/9601/1/src/kudu/rpc/rpc_controller.h File src/kudu/rpc/rpc_controller.h: http://gerrit.cloudera.org:8080/#/c/9601/1/src/kudu/rpc/rpc_controller.h@275 PS1, Line 275: int64_t outbound_sidecars_size_ = 0; > same comment as on the InboundCall Done http://gerrit.cloudera.org:8080/#/c/9601/1/src/kudu/rpc/rpc_controller.cc File src/kudu/rpc/rpc_controller.cc: http://gerrit.cloudera.org:8080/#/c/9601/1/src/kudu/rpc/rpc_controller.cc@153 PS1, Line 153: exceeds limit > would exceed Done http://gerrit.cloudera.org:8080/#/c/9601/1/src/kudu/rpc/transfer.h File src/kudu/rpc/transfer.h: http://gerrit.cloudera.org:8080/#/c/9601/1/src/kudu/rpc/transfer.h@51 PS1, Line 51: kMaxTotalSidecarSize = INT_MAX > it seems elsewhere in this patch you are checking against INT_MAX instead o Yes, my comments are confusing. The checks for sidecars use kMaxTotalSidecarBytes. However, several of the checks are to verify that arguments don't overflow. For example, SerializeMessage() takes additional_size as an int32_t, but the caller is using an int64_t local variable, so it's a bit different from the sidecar limits. I do something similar for the sidecar offsets, which are limited to uint32_t. I changed the comments to clarify this a bit. I also changed rpc-test.cc to use kMaxTotalSidecarBytes rather than INT_MAX. -- To view, visit http://gerrit.cloudera.org:8080/9601 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2d041e214b15d9c22b810588643798e2b3bc5c24 Gerrit-Change-Number: 9601 Gerrit-PatchSet: 1 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 13 Mar 2018 20:05:45 + Gerrit-HasComments: Yes
[kudu-CR] [docs] 'xcode-select' and 'xcodebuild -license' steps on OS X
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/9611 ) Change subject: [docs] 'xcode-select' and 'xcodebuild -license' steps on OS X .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/9611 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I107001bb73b3c77bcf873bdac8be8c07c807a010 Gerrit-Change-Number: 9611 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 13 Mar 2018 19:56:26 + Gerrit-HasComments: No
[kudu-CR] [docs] 'xcode-select' and 'xcodebuild -license' steps on OS X
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/9611 ) Change subject: [docs] 'xcode-select' and 'xcodebuild -license' steps on OS X .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/9611/1/docs/installation.adoc File docs/installation.adoc: http://gerrit.cloudera.org:8080/#/c/9611/1/docs/installation.adoc@532 PS1, Line 532: set > should this be 'install'? Done -- To view, visit http://gerrit.cloudera.org:8080/9611 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I107001bb73b3c77bcf873bdac8be8c07c807a010 Gerrit-Change-Number: 9611 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 13 Mar 2018 19:47:10 + Gerrit-HasComments: Yes
[kudu-CR] [docs] 'xcode-select' and 'xcodebuild -license' steps on OS X
Hello Will Berkeley, Dan Burkert, Kudu Jenkins, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9611 to look at the new patch set (#2). Change subject: [docs] 'xcode-select' and 'xcodebuild -license' steps on OS X .. [docs] 'xcode-select' and 'xcodebuild -license' steps on OS X Added a note on accepting the Xcode license and setup command-line tools after installing Xcode. It seems we implicitly assumed those steps were run as a part of the 'install Xcode' step, but I think it's better to explicitly mention those. Also, added macOS High Sierra as a supported platform (development only). Change-Id: I107001bb73b3c77bcf873bdac8be8c07c807a010 --- M docs/installation.adoc 1 file changed, 8 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/11/9611/2 -- To view, visit http://gerrit.cloudera.org:8080/9611 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I107001bb73b3c77bcf873bdac8be8c07c807a010 Gerrit-Change-Number: 9611 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley
[kudu-CR] [docs] 'xcode-select' and 'xcodebuild -license' steps on OS X
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/9611 ) Change subject: [docs] 'xcode-select' and 'xcodebuild -license' steps on OS X .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/9611 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I107001bb73b3c77bcf873bdac8be8c07c807a010 Gerrit-Change-Number: 9611 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 13 Mar 2018 19:44:14 + Gerrit-HasComments: No
[kudu-CR] [release notes]: RYW read mode
Hello Dan Burkert, David Ribeiro Alves, Kudu Jenkins, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9594 to look at the new patch set (#3). Change subject: [release notes]: RYW read mode .. [release notes]: RYW read mode Change-Id: I9906398b0796307c26150f78e0e256e285ded516 --- M docs/release_notes.adoc 1 file changed, 9 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/94/9594/3 -- To view, visit http://gerrit.cloudera.org:8080/9594 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9906398b0796307c26150f78e0e256e285ded516 Gerrit-Change-Number: 9594 Gerrit-PatchSet: 3 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [release notes]: RYW read mode
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/9594 ) Change subject: [release notes]: RYW read mode .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/9594/2/docs/release_notes.adoc File docs/release_notes.adoc: http://gerrit.cloudera.org:8080/#/c/9594/2/docs/release_notes.adoc@78 PS2, Line 78: C++ and Java > Capitalize C++ and Java. Done -- To view, visit http://gerrit.cloudera.org:8080/9594 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9906398b0796307c26150f78e0e256e285ded516 Gerrit-Change-Number: 9594 Gerrit-PatchSet: 3 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 13 Mar 2018 19:37:37 + Gerrit-HasComments: Yes
[kudu-CR] [release notes] notes on 3-4-3 replica management scheme
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/9577 ) Change subject: [release_notes] notes on 3-4-3 replica management scheme .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/9577/1/docs/release_notes.adoc File docs/release_notes.adoc: http://gerrit.cloudera.org:8080/#/c/9577/1/docs/release_notes.adoc@58 PS1, Line 58: * KUDU-1097: 3-4-3 replica management scheme is used by default. The new 3-4-3 > Good point. I'll add some notes on that and squash this update with the up I think this should be in the 'new features' section, considering that it's an option with its own flag. -- To view, visit http://gerrit.cloudera.org:8080/9577 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icfb1c4000abe5e8dce23ff4c840f05108ac56d5a Gerrit-Change-Number: 9577 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Comment-Date: Tue, 13 Mar 2018 19:09:50 + Gerrit-HasComments: Yes
[kudu-CR] [release notes]: RYW read mode
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/9594 ) Change subject: [release notes]: RYW read mode .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/9594/2/docs/release_notes.adoc File docs/release_notes.adoc: http://gerrit.cloudera.org:8080/#/c/9594/2/docs/release_notes.adoc@78 PS2, Line 78: c++ and java Capitalize C++ and Java. -- To view, visit http://gerrit.cloudera.org:8080/9594 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9906398b0796307c26150f78e0e256e285ded516 Gerrit-Change-Number: 9594 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 13 Mar 2018 19:08:41 + Gerrit-HasComments: Yes
[kudu-CR] [docs] 'xcode-select' and 'xcodebuild -license' steps on OS X
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/9611 ) Change subject: [docs] 'xcode-select' and 'xcodebuild -license' steps on OS X .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/9611/1/docs/installation.adoc File docs/installation.adoc: http://gerrit.cloudera.org:8080/#/c/9611/1/docs/installation.adoc@532 PS1, Line 532: set should this be 'install'? -- To view, visit http://gerrit.cloudera.org:8080/9611 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I107001bb73b3c77bcf873bdac8be8c07c807a010 Gerrit-Change-Number: 9611 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 13 Mar 2018 19:07:41 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2259: add real user to AuthenticationCredentialsPB
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/9374 ) Change subject: KUDU-2259: add real user to AuthenticationCredentialsPB .. Patch Set 3: (8 comments) http://gerrit.cloudera.org:8080/#/c/9374/2/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java File java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java: http://gerrit.cloudera.org:8080/#/c/9374/2/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java@156 PS2, Line 156: getKerbero > Not sure I understand: the condition for this 'if' closure is 'pb.hasAuthnT I misunderstood, thanks for pointing that out. http://gerrit.cloudera.org:8080/#/c/9374/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurityContextRealUser.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurityContextRealUser.java: http://gerrit.cloudera.org:8080/#/c/9374/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurityContextRealUser.java@73 PS2, Line 73: try (KuduClient client = > wrap Done http://gerrit.cloudera.org:8080/#/c/9374/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurityContextRealUser.java@77 PS2, Line 77: > yea, probably guess it's worth a JIRA Done http://gerrit.cloudera.org:8080/#/c/9374/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurityContextRealUser.java@82 PS2, Line 82: // Try again with a correct real user. > mind wrapping this? Done http://gerrit.cloudera.org:8080/#/c/9374/2/src/kudu/client/client-internal.h File src/kudu/client/client-internal.h: http://gerrit.cloudera.org:8080/#/c/9374/2/src/kudu/client/client-internal.h@233 PS2, Line 233: // is built. > can you comment that this never changes after construction? ie it's effecti Done http://gerrit.cloudera.org:8080/#/c/9374/2/src/kudu/client/client-test.cc File src/kudu/client/client-test.cc: http://gerrit.cloudera.org:8080/#/c/9374/2/src/kudu/client/client-test.cc@5359 PS2, Line 5359: > ASSERT_OK ? Done http://gerrit.cloudera.org:8080/#/c/9374/2/src/kudu/client/client-test.cc@5362 PS2, Line 5362: table_creator->table_name(kTableName) > yea seems like probably should. maybe put this in the same JIRA as the oth Done http://gerrit.cloudera.org:8080/#/c/9374/2/src/kudu/client/client.proto File src/kudu/client/client.proto: http://gerrit.cloudera.org:8080/#/c/9374/2/src/kudu/client/client.proto@116 PS2, Line 116: root > nit: if I'm not mistaken, some time ago there was an idea to keep whole cer Isn't that kind of redundant? I thought with PKI you typically just trust root certs, and then intermediate / leaf certs are required to ship around any intermediate signing certs with themselves. -- To view, visit http://gerrit.cloudera.org:8080/9374 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5d2d901d42501ecfc0f6372f68cf7335eb188b45 Gerrit-Change-Number: 9374 Gerrit-PatchSet: 3 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 13 Mar 2018 19:04:25 + Gerrit-HasComments: Yes
[kudu-CR] [docs] 'xcode-select' and 'xcodebuild -license' steps on OS X
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9611 Change subject: [docs] 'xcode-select' and 'xcodebuild -license' steps on OS X .. [docs] 'xcode-select' and 'xcodebuild -license' steps on OS X Added a note on accepting the Xcode license and setup command-line tools after installing Xcode. It seems we implicitly assumed those steps were run as a part of the 'install Xcode' step, but I think it's better to explicitly mention those. Also, added macOS High Sierra as a supported platform (development only). Change-Id: I107001bb73b3c77bcf873bdac8be8c07c807a010 --- M docs/installation.adoc 1 file changed, 8 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/11/9611/1 -- To view, visit http://gerrit.cloudera.org:8080/9611 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I107001bb73b3c77bcf873bdac8be8c07c807a010 Gerrit-Change-Number: 9611 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin
[kudu-CR] KUDU-2259: add real user to AuthenticationCredentialsPB
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9374 to look at the new patch set (#3). Change subject: KUDU-2259: add real user to AuthenticationCredentialsPB .. KUDU-2259: add real user to AuthenticationCredentialsPB This commit adds the 'real user' to the authn credentials token, which is used when negotiating connections with SASL PLAIN authentication. This is useful when scan tokens are being sent to remote tasks, it's not possible to authenticate with a signed authn token to the remote server[1], coarse-grained ACLs have been set, and the 'planner' and 'executor' processes are being run with different users. This problematic scenario might also have been solved by allowing tokens to be used in all scenarios, even when encryption is disabled, but the approach taken by this commit allows that invariant to remain. [1]: this most often occurs because the remote server has encryption disabled. Change-Id: I5d2d901d42501ecfc0f6372f68cf7335eb188b45 --- M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java M java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java A java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurityContextRealUser.java M src/kudu/client/client-internal.cc M src/kudu/client/client-internal.h M src/kudu/client/client-test.cc M src/kudu/client/client.cc M src/kudu/client/client.proto M src/kudu/client/master_rpc.cc M src/kudu/client/master_rpc.h M src/kudu/client/meta_cache.cc M src/kudu/mini-cluster/external_mini_cluster.cc M src/kudu/rpc/connection_id.cc M src/kudu/rpc/server_negotiation.cc M src/kudu/rpc/user_credentials.cc M src/kudu/rpc/user_credentials.h M src/kudu/util/user.cc 17 files changed, 243 insertions(+), 40 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/74/9374/3 -- To view, visit http://gerrit.cloudera.org:8080/9374 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5d2d901d42501ecfc0f6372f68cf7335eb188b45 Gerrit-Change-Number: 9374 Gerrit-PatchSet: 3 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR](branch-1.7.x) KUDU-2230: java: sync client exception stack traces should make sense
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9610 Change subject: KUDU-2230: java: sync client exception stack traces should make sense .. KUDU-2230: java: sync client exception stack traces should make sense Previously the exceptions thrown by the synchronous client always had a stack trace somewhere deep inside our async callback chain. So, when the user eventually caught this exception, it would be very hard to even know what part of their code failed, since their own code would not even show up in the call chain. This patch simply replaces the exception's stack trace with the stack at the "join" point where the exception is transformed back to a KuduException. An exception which stores the original stack trace is attached as a "suppressed" exception. Change-Id: I874d47b5239bcc8493c15ef763ecfe0fd878d795 Reviewed-on: http://gerrit.cloudera.org:8080/9489 Tested-by: Kudu Jenkins Reviewed-by: Dan Burkert Reviewed-by: Alexey Serbin (cherry picked from commit ce0db915787b58a79109e6faecc6f1daef9f2850) --- M java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectToCluster.java 2 files changed, 30 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/9610/1 -- To view, visit http://gerrit.cloudera.org:8080/9610 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.7.x Gerrit-MessageType: newchange Gerrit-Change-Id: I874d47b5239bcc8493c15ef763ecfe0fd878d795 Gerrit-Change-Number: 9610 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Todd Lipcon
[kudu-CR](branch-1.7.x) KUDU-2322: don't spew logs about behind-log-GC follower
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/9609 ) Change subject: KUDU-2322: don't spew logs about behind-log-GC follower .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/9609 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.7.x Gerrit-MessageType: comment Gerrit-Change-Id: I4438e63aef0be26c149c57f9f58cb54327e41a33 Gerrit-Change-Number: 9609 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 13 Mar 2018 18:48:14 + Gerrit-HasComments: No
[kudu-CR](branch-1.7.x) KUDU-2322: don't spew logs about behind-log-GC follower
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9609 Change subject: KUDU-2322: don't spew logs about behind-log-GC follower .. KUDU-2322: don't spew logs about behind-log-GC follower This patch addresses the issue of a leader spewing a lot of INFO log messages about missing logs segments for a follower that falls behind log GC. In the 3-4-3 replica management scheme, such followers linger around until corresponding non-voter replica catches up with the leader. That might take long time. With this patch, the leader throttles messages about missing log segments for a behind-log-GC follower, outputting 1 message per minute. Change-Id: I4438e63aef0be26c149c57f9f58cb54327e41a33 Reviewed-on: http://gerrit.cloudera.org:8080/9573 Reviewed-by: Todd Lipcon Reviewed-by: Alexey Serbin Tested-by: Kudu Jenkins (cherry picked from commit d856107e0b58067f0bbebbbda52f4f67c674897a) --- M src/kudu/consensus/consensus-test-util.h M src/kudu/consensus/consensus_peers.cc M src/kudu/consensus/consensus_queue.cc M src/kudu/consensus/consensus_queue.h M src/kudu/util/logging.h 5 files changed, 36 insertions(+), 26 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/09/9609/1 -- To view, visit http://gerrit.cloudera.org:8080/9609 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.7.x Gerrit-MessageType: newchange Gerrit-Change-Id: I4438e63aef0be26c149c57f9f58cb54327e41a33 Gerrit-Change-Number: 9609 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin
[kudu-CR](branch-1.7.x) Remove tcmalloc spinlock contention metric
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9608 Change subject: Remove tcmalloc_spinlock_contention metric .. Remove tcmalloc_spinlock_contention metric Spinlock contention was removed from tcmalloc upstream, so this metric has been set to 0 since we upgraded to tcmalloc 2.6. I looked briefly at reverting its removal upstream, but the revert would be fairly large, so I think it's best to just accept the loss of this instrumentation. Change-Id: I2c29b81a2c692d5d876d2dd5381ba2c295684324 Reviewed-on: http://gerrit.cloudera.org:8080/9595 Tested-by: Todd Lipcon Reviewed-by: Adar Dembo (cherry picked from commit 4889a9156a7e1f51b076f4bc9f44621a1b2f46e4) --- M docs/release_notes.adoc M src/kudu/scripts/parse_metrics_log.py M src/kudu/util/spinlock_profiling-test.cc M src/kudu/util/spinlock_profiling.cc M src/kudu/util/spinlock_profiling.h M src/kudu/util/trace.h M src/kudu/util/trace_metrics.h 7 files changed, 7 insertions(+), 103 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/08/9608/1 -- To view, visit http://gerrit.cloudera.org:8080/9608 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.7.x Gerrit-MessageType: newchange Gerrit-Change-Id: I2c29b81a2c692d5d876d2dd5381ba2c295684324 Gerrit-Change-Number: 9608 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Todd Lipcon
[kudu-CR](branch-1.7.x) [tools] KUDU-2331: Allow move tool to work when uninvolved tserver is down
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9607 Change subject: [tools] KUDU-2331: Allow move tool to work when uninvolved tserver is down .. [tools] KUDU-2331: Allow move tool to work when uninvolved tserver is down The move tool requires a clean ksck for the tablet it's acting on for safety reasons, but the check for this failed if a tablet server was down, even if the tablet server didn't host a replica for the tablet and wasn't the destination server. This fixes the move command so a down tserver uninvolved in the move won't prevent the move. Change-Id: Id1fc60233f4f50f478da7ccb104e37df3067400c Reviewed-on: http://gerrit.cloudera.org:8080/9510 Reviewed-by: Adar Dembo Tested-by: Kudu Jenkins (cherry picked from commit 13155d4a2f79f522b8213fbb99588109cc04b175) --- M src/kudu/tools/kudu-admin-test.cc M src/kudu/tools/tool_action_tablet.cc 2 files changed, 60 insertions(+), 33 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/9607/1 -- To view, visit http://gerrit.cloudera.org:8080/9607 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.7.x Gerrit-MessageType: newchange Gerrit-Change-Id: Id1fc60233f4f50f478da7ccb104e37df3067400c Gerrit-Change-Number: 9607 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke
[kudu-CR] [release notes]: RYW read mode
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/9594 ) Change subject: [release notes]: RYW read mode .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/9594/1/docs/release_notes.adoc File docs/release_notes.adoc: http://gerrit.cloudera.org:8080/#/c/9594/1/docs/release_notes.adoc@67 PS1, Line 67: > nit: "new scan read mode" or a "new read mode for scans" and add "that user Done -- To view, visit http://gerrit.cloudera.org:8080/9594 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9906398b0796307c26150f78e0e256e285ded516 Gerrit-Change-Number: 9594 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 13 Mar 2018 18:42:32 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2290: Tool to re-create a tablet
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/9393 ) Change subject: KUDU-2290: Tool to re-create a tablet .. Patch Set 5: (14 comments) http://gerrit.cloudera.org:8080/#/c/9393/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/9393/5//COMMIT_MSG@11 PS5, Line 11: is Nit: omit this http://gerrit.cloudera.org:8080/#/c/9393/5//COMMIT_MSG@19 PS5, Line 19: Given that tablet_id is globally unique and table_name already identifies the table for humans, what value does the inclusion of table_id add? http://gerrit.cloudera.org:8080/#/c/9393/5//COMMIT_MSG@24 PS5, Line 24: its Nit: it's http://gerrit.cloudera.org:8080/#/c/9393/5//COMMIT_MSG@26 PS5, Line 26: eventually this will stop Maybe be more specific; this behavior ends when the cached entries' TTL expires? http://gerrit.cloudera.org:8080/#/c/9393/5/src/kudu/client/meta_cache.cc File src/kudu/client/meta_cache.cc: http://gerrit.cloudera.org:8080/#/c/9393/5/src/kudu/client/meta_cache.cc@912 PS5, Line 912: // Partition should not have changed. Could you clarify this a bit: the partition's _boundaries_ should have not changed, http://gerrit.cloudera.org:8080/#/c/9393/5/src/kudu/client/meta_cache.cc@919 PS5, Line 919: // In this case, 'tablets_by_key' will be empty but a RemoteTablet : // will exist in the cache. I don't understand this case. AFAICT tablet replacement is "atomic", which means the client sees one of two states for a replaced tablet: 1. Tablet with ID foo from key x to y. 2. Tablet with ID bar from key x to y. Since the boundaries of the tablet haven't changed, doesn't this mean tablet_lower_bound is a valid way to look up the cached entry regardless of which state the client is exposed to? Looking at the code some more, it seems that following tablet replacement, 'remote' is going to be null so we'll skip all of this, erase the tablet's partition from tablets_by_key (L937), set up a new RemoteTablet for the replacement, and insert the tablet's partition again (L950). At no point is the new tablet exposed in tablets_by_id_ without any corresponding partitions in tablets_by_key. I must be missing something here... http://gerrit.cloudera.org:8080/#/c/9393/5/src/kudu/master/catalog_manager.h File src/kudu/master/catalog_manager.h: http://gerrit.cloudera.org:8080/#/c/9393/5/src/kudu/master/catalog_manager.h@581 PS5, Line 581: Status ReplaceTablet(const std::string& tablet_id, master::ReplaceTabletResponsePB* resp); It'd be nice to have testing of this new method outside of the CLI tests. Both in isolation as well as in a "stress" environment (i.e. ongoing concurrent data operations to the table and/or replaced tablet). It'd also be nice to test concurrent metadata operations: what happens if I delete a table and replace one of its tablets at the same time? What happens if I alter it concurrently? Etc. http://gerrit.cloudera.org:8080/#/c/9393/5/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/9393/5/src/kudu/master/catalog_manager.cc@4302 PS5, Line 4302: // To be safe, we'll take the global catalog manager lock for the rest of this method. While I appreciate the motivation to block out everything else, I don't think this is a good idea. First, the global catalog manager lock is a spinlock, which means anyone waiting on this lock will be spinning, and they'll be waiting for a long time because in ReplaceTablet the lock is held during the catalog write, which involves network and disk IO. Second, the catalog manager lock is never held while acquiring other catalog manager locks; breaking that invariant raises the possibility of deadlocks elsewhere. The only exceptions I'm aware of to this rule are during catalog manager initialization, at which point RPCs are largely rejected anyway. Can we treat this like any other DDL method and acquire the right locks at the right time? The background task does tablet replacement in CatalogManager::ProcessPendingAssignments; can that be used as a template for how to do it safely here? http://gerrit.cloudera.org:8080/#/c/9393/5/src/kudu/master/catalog_manager.cc@4303 PS5, Line 4303: lock Style nit: Use l (or l_foo) for lock guards. It especially helps in this case, since lock and lock_ are so similar. http://gerrit.cloudera.org:8080/#/c/9393/5/src/kudu/master/master.proto File src/kudu/master/master.proto: http://gerrit.cloudera.org:8080/#/c/9393/5/src/kudu/master/master.proto@707 PS5, Line 707: recovered Nit: "manually recovered", to make it clear that any actual recovery is out of scope of Kudu's normal operations? http://gerrit.cloudera.org:8080/#/c/9393/5/src/kudu/master/master.proto@715 PS5, Line 715: I consider myself to be a protobuf luddite, but I don't understand why 'optional' is appropriate for the below fields. There's the phi
[kudu-CR] [release notes]: RYW read mode
Hello Dan Burkert, David Ribeiro Alves, Kudu Jenkins, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9594 to look at the new patch set (#2). Change subject: [release notes]: RYW read mode .. [release notes]: RYW read mode Change-Id: I9906398b0796307c26150f78e0e256e285ded516 --- M docs/release_notes.adoc 1 file changed, 9 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/94/9594/2 -- To view, visit http://gerrit.cloudera.org:8080/9594 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9906398b0796307c26150f78e0e256e285ded516 Gerrit-Change-Number: 9594 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-2230: java: sync client exception stack traces should make sense
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9489 ) Change subject: KUDU-2230: java: sync client exception stack traces should make sense .. KUDU-2230: java: sync client exception stack traces should make sense Previously the exceptions thrown by the synchronous client always had a stack trace somewhere deep inside our async callback chain. So, when the user eventually caught this exception, it would be very hard to even know what part of their code failed, since their own code would not even show up in the call chain. This patch simply replaces the exception's stack trace with the stack at the "join" point where the exception is transformed back to a KuduException. An exception which stores the original stack trace is attached as a "suppressed" exception. Change-Id: I874d47b5239bcc8493c15ef763ecfe0fd878d795 Reviewed-on: http://gerrit.cloudera.org:8080/9489 Tested-by: Kudu Jenkins Reviewed-by: Dan Burkert Reviewed-by: Alexey Serbin --- M java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectToCluster.java 2 files changed, 30 insertions(+), 6 deletions(-) Approvals: Kudu Jenkins: Verified Dan Burkert: Looks good to me, but someone else must approve Alexey Serbin: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/9489 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I874d47b5239bcc8493c15ef763ecfe0fd878d795 Gerrit-Change-Number: 9489 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2322: don't spew logs about behind-log-GC follower
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9573 ) Change subject: KUDU-2322: don't spew logs about behind-log-GC follower .. KUDU-2322: don't spew logs about behind-log-GC follower This patch addresses the issue of a leader spewing a lot of INFO log messages about missing logs segments for a follower that falls behind log GC. In the 3-4-3 replica management scheme, such followers linger around until corresponding non-voter replica catches up with the leader. That might take long time. With this patch, the leader throttles messages about missing log segments for a behind-log-GC follower, outputting 1 message per minute. Change-Id: I4438e63aef0be26c149c57f9f58cb54327e41a33 Reviewed-on: http://gerrit.cloudera.org:8080/9573 Reviewed-by: Todd Lipcon Reviewed-by: Alexey Serbin Tested-by: Kudu Jenkins --- M src/kudu/consensus/consensus-test-util.h M src/kudu/consensus/consensus_peers.cc M src/kudu/consensus/consensus_queue.cc M src/kudu/consensus/consensus_queue.h M src/kudu/util/logging.h 5 files changed, 36 insertions(+), 26 deletions(-) Approvals: Todd Lipcon: Looks good to me, but someone else must approve Alexey Serbin: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/9573 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I4438e63aef0be26c149c57f9f58cb54327e41a33 Gerrit-Change-Number: 9573 Gerrit-PatchSet: 4 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [release notes]: RYW read mode
David Ribeiro Alves has posted comments on this change. ( http://gerrit.cloudera.org:8080/9594 ) Change subject: [release notes]: RYW read mode .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/9594/1/docs/release_notes.adoc File docs/release_notes.adoc: http://gerrit.cloudera.org:8080/#/c/9594/1/docs/release_notes.adoc@67 PS1, Line 67: new read nit: "new scan read mode" or a "new read mode for scans" and add "that users can specify when creating a new scanner in both c++ and java clients" -- To view, visit http://gerrit.cloudera.org:8080/9594 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9906398b0796307c26150f78e0e256e285ded516 Gerrit-Change-Number: 9594 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 13 Mar 2018 18:29:20 + Gerrit-HasComments: Yes
[kudu-CR] Remove tcmalloc spinlock contention metric
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9595 ) Change subject: Remove tcmalloc_spinlock_contention metric .. Remove tcmalloc_spinlock_contention metric Spinlock contention was removed from tcmalloc upstream, so this metric has been set to 0 since we upgraded to tcmalloc 2.6. I looked briefly at reverting its removal upstream, but the revert would be fairly large, so I think it's best to just accept the loss of this instrumentation. Change-Id: I2c29b81a2c692d5d876d2dd5381ba2c295684324 Reviewed-on: http://gerrit.cloudera.org:8080/9595 Tested-by: Todd Lipcon Reviewed-by: Adar Dembo --- M docs/release_notes.adoc M src/kudu/scripts/parse_metrics_log.py M src/kudu/util/spinlock_profiling-test.cc M src/kudu/util/spinlock_profiling.cc M src/kudu/util/spinlock_profiling.h M src/kudu/util/trace.h M src/kudu/util/trace_metrics.h 7 files changed, 7 insertions(+), 103 deletions(-) Approvals: Todd Lipcon: Verified Adar Dembo: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/9595 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I2c29b81a2c692d5d876d2dd5381ba2c295684324 Gerrit-Change-Number: 9595 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2153. Servers should not delete tmp files until after locking directories
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9596 ) Change subject: KUDU-2153. Servers should not delete tmp files until after locking directories .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/9596/1/src/kudu/fs/fs_manager.cc File src/kudu/fs/fs_manager.cc: http://gerrit.cloudera.org:8080/#/c/9596/1/src/kudu/fs/fs_manager.cc@391 PS1, Line 391: CheckAndFixPermissions(); > We ought to gate this behind locking too. It shouldn't affect an existing t Do we need the right permissions to go about locking? If not, then +1 -- To view, visit http://gerrit.cloudera.org:8080/9596 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3471c8ce00e77fa1712ea518f6ab281864a08d Gerrit-Change-Number: 9596 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 13 Mar 2018 18:08:06 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2322: don't spew logs about behind-log-GC follower
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/9573 ) Change subject: KUDU-2322: don't spew logs about behind-log-GC follower .. Patch Set 3: -Verified Code-Review+2 Carrying over Mike's +2 from PS2 -- To view, visit http://gerrit.cloudera.org:8080/9573 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4438e63aef0be26c149c57f9f58cb54327e41a33 Gerrit-Change-Number: 9573 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 13 Mar 2018 18:02:19 + Gerrit-HasComments: No
[kudu-CR] KUDU-2153. Servers should not delete tmp files until after locking directories
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/9596 ) Change subject: KUDU-2153. Servers should not delete tmp files until after locking directories .. Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/9596/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/9596/1//COMMIT_MSG@31 PS1, Line 31: unused Looks like it became unused in https://github.com/apache/kudu/commit/61a227735032974ea0c5c3045be7f092a41ac350. I agree that it's OK to remove; it's exceedingly unlikely that anyone was relying on it pre-1.6, and 'unsafe' should give us that kind of leeway, http://gerrit.cloudera.org:8080/#/c/9596/1/src/kudu/fs/fs_manager-test.cc File src/kudu/fs/fs_manager-test.cc: http://gerrit.cloudera.org:8080/#/c/9596/1/src/kudu/fs/fs_manager-test.cc@492 PS1, Line 492: FLAGS_env_inject_lock_failure_globs = ""; Wouldn't a new scope and a flagsaver be more idiomatic? http://gerrit.cloudera.org:8080/#/c/9596/1/src/kudu/fs/fs_manager.cc File src/kudu/fs/fs_manager.cc: http://gerrit.cloudera.org:8080/#/c/9596/1/src/kudu/fs/fs_manager.cc@391 PS1, Line 391: CheckAndFixPermissions(); We ought to gate this behind locking too. It shouldn't affect an existing tserver (although what if one of the tservers is running with a different umask? Maybe?), but it's a destructive operation and so we should get the locks first. -- To view, visit http://gerrit.cloudera.org:8080/9596 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a3471c8ce00e77fa1712ea518f6ab281864a08d Gerrit-Change-Number: 9596 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 13 Mar 2018 17:30:28 + Gerrit-HasComments: Yes
