[kudu-CR](branch-1.0.x) [client] avoid circular deps in time-based flusher
Kudu Jenkins has posted comments on this change. Change subject: [client] avoid circular deps in time-based flusher .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/3403/ -- To view, visit http://gerrit.cloudera.org:8080/4403 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I59825981a600f5882ee476479c2ddf16b495c1f9 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: branch-1.0.x Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] [client] avoid circular deps in time-based flusher
Todd Lipcon has submitted this change and it was merged. Change subject: [client] avoid circular deps in time-based flusher .. [client] avoid circular deps in time-based flusher The boost::bind() makes cast of parameters during the call, not during creation of the functor object: http://stackoverflow.com/questions/11255144/why-does-boostbind-store-arguments-of-the-type-passed-in-rather-than-of-the-ty So, it's necessary to pass weak pointers to the background auto-flush task to avoid circular dependencies between client::KuduSession::Data and rpc::Messenger. Besides, it does not make much sense to store shared reference to messenger in KuduSession::Data since it's always passed as a weak reference and then promoting to a shared one during the call in all usage scenarios. Thanks to Adar and Todd spotting the usual suspect there. This is a follow-up for 93be1310d227cf05025864654ca3f6713c2ddc2c. Change-Id: I59825981a600f5882ee476479c2ddf16b495c1f9 Reviewed-on: http://gerrit.cloudera.org:8080/4395 Reviewed-by: Adar Dembo Tested-by: Kudu Jenkins Tested-by: Todd Lipcon --- M src/kudu/client/session-internal.cc M src/kudu/client/session-internal.h 2 files changed, 15 insertions(+), 15 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Todd Lipcon: Verified Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4395 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I59825981a600f5882ee476479c2ddf16b495c1f9 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR](branch-1.0.x) [client] avoid circular deps in time-based flusher
Todd Lipcon has uploaded a new change for review. http://gerrit.cloudera.org:8080/4403 Change subject: [client] avoid circular deps in time-based flusher .. [client] avoid circular deps in time-based flusher The boost::bind() makes cast of parameters during the call, not during creation of the functor object: http://stackoverflow.com/questions/11255144/why-does-boostbind-store-arguments-of-the-type-passed-in-rather-than-of-the-ty So, it's necessary to pass weak pointers to the background auto-flush task to avoid circular dependencies between client::KuduSession::Data and rpc::Messenger. Besides, it does not make much sense to store shared reference to messenger in KuduSession::Data since it's always passed as a weak reference and then promoting to a shared one during the call in all usage scenarios. Thanks to Adar and Todd spotting the usual suspect there. This is a follow-up for 93be1310d227cf05025864654ca3f6713c2ddc2c. Change-Id: I59825981a600f5882ee476479c2ddf16b495c1f9 Reviewed-on: http://gerrit.cloudera.org:8080/4395 Reviewed-by: Adar Dembo Tested-by: Kudu Jenkins Tested-by: Todd Lipcon (cherry picked from commit 1a062253e3fdc900a4b0b418520d2870b6de8846) --- M src/kudu/client/session-internal.cc M src/kudu/client/session-internal.h 2 files changed, 15 insertions(+), 15 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/03/4403/1 -- To view, visit http://gerrit.cloudera.org:8080/4403 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I59825981a600f5882ee476479c2ddf16b495c1f9 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: branch-1.0.x Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin
[kudu-CR] [client] avoid circular deps in time-based flusher
Todd Lipcon has posted comments on this change. Change subject: [client] avoid circular deps in time-based flusher .. Patch Set 3: Verified+1 I managed to write a test which reliably leaks without this patch: https://gist.github.com/be6e56af8f40803315aa2c43fbd6c9c6 So, I think this is at least an improvement. Let's pull it into 1.0.0rc0 and do some focused client stress testing during the voting period. -- To view, visit http://gerrit.cloudera.org:8080/4395 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I59825981a600f5882ee476479c2ddf16b495c1f9 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] build-and-test: only run cmake once
Todd Lipcon has posted comments on this change. Change subject: build-and-test: only run cmake once .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4401 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8f9eb27d69ac4d120a0bdad1d1695b4f59b12641 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1090: relax MemTracker uniqueness constraint
Todd Lipcon has posted comments on this change. Change subject: KUDU-1090: relax MemTracker uniqueness constraint .. Patch Set 2: (5 comments) http://gerrit.cloudera.org:8080/#/c/4394/2/src/kudu/integration-tests/external_mini_cluster-test.cc File src/kudu/integration-tests/external_mini_cluster-test.cc: Line 128: // any crashes. might make more sense to put this in a different test case... this test in theory is supposed to be just testing the test infra, not actual functionality/bugs Line 142: MonoDelta::FromMicroseconds(100)); maybe you'd hit the race more with a super low value like 1us? maybe even start multiple threads if you really wanna make a solid stress test of this? Line 154: work.StopAndJoin(); might be more fruitful if you set the log rolling threshold to be really low, and inserted much more data, since the particular MRS-related test is a per-segment thing. Line 156: // Restart the cluster. Nothing should crash. how about an AssertNoCrashes before the restart, too? http://gerrit.cloudera.org:8080/#/c/4394/2/src/kudu/util/mem_tracker.cc File src/kudu/util/mem_tracker.cc: Line 237: LOG(FATAL) << should we consider DFATAL here? would it actually cause incorrect behavior to hit this in a real cluster? -- To view, visit http://gerrit.cloudera.org:8080/4394 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iea8e3d383878e829188eaca65d639bb44d8b8146 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [client] performance optimizations
Alexey Serbin has posted comments on this change. Change subject: [client] performance optimizations .. Patch Set 2: (1 comment) Thank you for the review! Will address the rest of the comments after addressing the tests flakiness issue. http://gerrit.cloudera.org:8080/#/c/4385/2//COMMIT_MSG Commit Message: Line 10: which gave about 50% boost while running in scenario when > Is there a test that you can point to that demonstrates this speed up? Any Yes, there is. I implemented a test which resembles InserLoadgen from java-examples, and this is what I'm using for performance comparison in this scenario. Will post it for review soon. As for the real numbers, I got the following when running that test on ve0518.halxg.cloudera.com, 1 thread, 8M rows per thread: Before the changes: -> buffer space: 32MiB -> buffers num : 2 -> flush_every_n_rows: 2000 total : 22470.3 ms per row: 0.00280879 ms After the changes: total : 11913.5 ms per row: 0.00148918 ms -- To view, visit http://gerrit.cloudera.org:8080/4385 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4b57fc7355f9f673f30861ec30cb6b48cdf656d2 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [client] avoid circular deps in time-based flusher
Adar Dembo has posted comments on this change. Change subject: [client] avoid circular deps in time-based flusher .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4395 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I59825981a600f5882ee476479c2ddf16b495c1f9 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] [client] avoid circular deps in time-based flusher
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4395 to look at the new patch set (#3). Change subject: [client] avoid circular deps in time-based flusher .. [client] avoid circular deps in time-based flusher The boost::bind() makes cast of parameters during the call, not during creation of the functor object: http://stackoverflow.com/questions/11255144/why-does-boostbind-store-arguments-of-the-type-passed-in-rather-than-of-the-ty So, it's necessary to pass weak pointers to the background auto-flush task to avoid circular dependencies between client::KuduSession::Data and rpc::Messenger. Besides, it does not make much sense to store shared reference to messenger in KuduSession::Data since it's always passed as a weak reference and then promoting to a shared one during the call in all usage scenarios. Thanks to Adar and Todd spotting the usual suspect there. This is a follow-up for 93be1310d227cf05025864654ca3f6713c2ddc2c. Change-Id: I59825981a600f5882ee476479c2ddf16b495c1f9 --- M src/kudu/client/session-internal.cc M src/kudu/client/session-internal.h 2 files changed, 15 insertions(+), 15 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/95/4395/3 -- To view, visit http://gerrit.cloudera.org:8080/4395 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I59825981a600f5882ee476479c2ddf16b495c1f9 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [client] avoid circular deps in time-based flusher
Kudu Jenkins has posted comments on this change. Change subject: [client] avoid circular deps in time-based flusher .. Patch Set 3: Build Started http://104.196.14.100/job/kudu-gerrit/3402/ -- To view, visit http://gerrit.cloudera.org:8080/4395 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I59825981a600f5882ee476479c2ddf16b495c1f9 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] [client] avoid circular deps in time-based flusher
Alexey Serbin has posted comments on this change. Change subject: [client] avoid circular deps in time-based flusher .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/4395/2/src/kudu/client/session-internal.cc File src/kudu/client/session-internal.cc: PS2, Line 43: std:: > Nit: don't need std prefix here (if you add using). Good observation. However, there is also sp::weak_ptr here for KuduSession, so I would like to keep this prefix. -- To view, visit http://gerrit.cloudera.org:8080/4395 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I59825981a600f5882ee476479c2ddf16b495c1f9 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [client] avoid circular deps in time-based flusher
Alexey Serbin has posted comments on this change. Change subject: [client] avoid circular deps in time-based flusher .. Patch Set 1: > (1 comment) > > I left you some comments just as you revved PS2. Not sure if you > saw them. Thank for the reminder -- I addressed them as well. -- To view, visit http://gerrit.cloudera.org:8080/4395 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I59825981a600f5882ee476479c2ddf16b495c1f9 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] [c++ client] AUTO FLUSH BACKGROUND optimizations
Alexey Serbin has posted comments on this change. Change subject: [c++ client] AUTO_FLUSH_BACKGROUND optimizations .. Patch Set 2: (2 comments) Thank you for the review! I posted the updated version. http://gerrit.cloudera.org:8080/#/c/4308/2//COMMIT_MSG Commit Message: Line 13: for the mutation buffer. Changing it from 80% to 50% gave good > s/good/some actual numbers (e.g. XX%-YY% throughput increase or something) Done Line 53: The tests were run at ve0518.halxg.cloudera.com against binaries > dont point to an internal cloudera machine on the commit message, if needed Good idea -- I'll add info CPU cores and available memory. -- To view, visit http://gerrit.cloudera.org:8080/4308 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1f0aa6d02c51bb063498709e8570e8c7214a31a0 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] [c++ client] AUTO FLUSH BACKGROUND optimizations
Kudu Jenkins has posted comments on this change. Change subject: [c++ client] AUTO_FLUSH_BACKGROUND optimizations .. Patch Set 3: Build Started http://104.196.14.100/job/kudu-gerrit/3401/ -- To view, visit http://gerrit.cloudera.org:8080/4308 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1f0aa6d02c51bb063498709e8570e8c7214a31a0 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] [c++ client] AUTO FLUSH BACKGROUND optimizations
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4308 to look at the new patch set (#3). Change subject: [c++ client] AUTO_FLUSH_BACKGROUND optimizations .. [c++ client] AUTO_FLUSH_BACKGROUND optimizations Optimizations after initial performance testing of the Kudu C++ client library with AUTO_FLUSH_BACKGROUND flush mode support. The most important tuning is the default flush watermark for the mutation buffer. Changing it from 80% to 50% gave near 30% performance boost in throughput for scenarios when a client pushes data to the server as fast as it can, using workloads of 8M rows like (int64, int32, string, string, int) where strings are about 32 bytes long in average. Each thread ran its own single-session KuduClient, where each session was running in AUTO_FLUSH_BACKGROUND flush mode. 1-thread insertion (8M rows per thread) 80% watermark: total : 35229.7 ms per row: 0.00440372 ms 50% watermark: total : 22562.8 ms per row: 0.00282035 ms 2-thread insertion (4M rows per thread) 80% watermark: total : 19683.6 ms per row: 0.00246046 ms 50% watermark: total : 12931.8 ms per row: 0.00161647 ms 4-thread insertion (2M rows per thread) 80% watermark: total : 11941.9 ms per row: 0.00149274 ms 50% watermark: total : 7724.68 ms per row: 0.000965585 ms Other related session parameters: mutation buffer size: 7M (default) maximum number of batchers: 2(default) time-based flush interval: 1 second (default) The tests were run dual Intel(R) Xeon(R) CPU E5-2680 v3 @ 2.50GHz (12 cores per CPU) with 98GiB of memory. This is a follow-up for 93be1310d227cf05025864654ca3f6713c2ddc2c. Change-Id: I1f0aa6d02c51bb063498709e8570e8c7214a31a0 --- M src/kudu/client/batcher.cc M src/kudu/client/batcher.h M src/kudu/client/client.h M src/kudu/client/session-internal.cc M src/kudu/client/write_op.cc 5 files changed, 10 insertions(+), 13 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/08/4308/3 -- To view, visit http://gerrit.cloudera.org:8080/4308 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1f0aa6d02c51bb063498709e8570e8c7214a31a0 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-1301 - [python] Tests leak tmp directory
Todd Lipcon has posted comments on this change. Change subject: KUDU-1301 - [python] Tests leak tmp directory .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4372 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I95d03ab1c23c7712ab647e37a646d5e22a758a9b Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jordan Birdsell Gerrit-Reviewer: Jordan Birdsell Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1301 - [python] Tests leak tmp directory
Todd Lipcon has submitted this change and it was merged. Change subject: KUDU-1301 - [python] Tests leak tmp directory .. KUDU-1301 - [python] Tests leak tmp directory In the past the python tests have been leaking tmp directories and processes for the master and tablet servers. Previously, with commit 89380b53d48b085dfe394b9966360d1ef1a4b038, the check for temp leak was disabled for python tests to avoid errors. This patch fixes both the temp directory and the process leak and re-enables the leak check for python. Change-Id: I95d03ab1c23c7712ab647e37a646d5e22a758a9b Reviewed-on: http://gerrit.cloudera.org:8080/4372 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon --- M build-support/jenkins/build-and-test.sh M python/kudu/tests/test_scanner.py 2 files changed, 10 insertions(+), 18 deletions(-) Approvals: Todd Lipcon: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4372 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I95d03ab1c23c7712ab647e37a646d5e22a758a9b Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jordan Birdsell Gerrit-Reviewer: Jordan Birdsell Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Bump version to 1.1.0-SNAPSHOT
Todd Lipcon has posted comments on this change. Change subject: Bump version to 1.1.0-SNAPSHOT .. Patch Set 1: Code-Review+2 Verified+1 Flaky java tests, self-reviewing since it's just build-related -- To view, visit http://gerrit.cloudera.org:8080/4400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifd9db10a73fecbb73cebe66c16d16e9d11a98750 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR](branch-1.0.x) Change version to non-SNAPSHOT in branch
Todd Lipcon has posted comments on this change. Change subject: Change version to non-SNAPSHOT in branch .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4399 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibc73006692673591a78c1bf3a101058ad62fc014 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: branch-1.0.x Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Bump version to 1.1.0-SNAPSHOT
Todd Lipcon has submitted this change and it was merged. Change subject: Bump version to 1.1.0-SNAPSHOT .. Bump version to 1.1.0-SNAPSHOT Change-Id: Ifd9db10a73fecbb73cebe66c16d16e9d11a98750 Reviewed-on: http://gerrit.cloudera.org:8080/4400 Reviewed-by: Todd Lipcon Tested-by: Todd Lipcon --- M java/interface-annotations/pom.xml M java/kudu-client-tools/pom.xml M java/kudu-client/pom.xml M java/kudu-csd/pom.xml M java/kudu-csd/src/descriptor/service.sdl M java/kudu-flume-sink/pom.xml M java/kudu-mapreduce/pom.xml M java/kudu-spark/pom.xml M java/pom.xml M version.txt 10 files changed, 11 insertions(+), 11 deletions(-) Approvals: Todd Lipcon: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/4400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ifd9db10a73fecbb73cebe66c16d16e9d11a98750 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Todd Lipcon
[kudu-CR](branch-1.0.x) Revert "java: fix leak of TabletClient objects in client2tablets map"
Todd Lipcon has submitted this change and it was merged. Change subject: Revert "java: fix leak of TabletClient objects in client2tablets map" .. Revert "java: fix leak of TabletClient objects in client2tablets map" This reverts commit d5082d8ec1218e3f3bd2143d117ddd64772a6162. It was fonud to be unstable on master, so reverting for branch-1.0. Change-Id: I2e9e227af0acfb677572a8baad9692ce08c46be0 Reviewed-on: http://gerrit.cloudera.org:8080/4398 Tested-by: Kudu Jenkins Reviewed-by: David Ribeiro Alves --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.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/TestAsyncKuduSession.java 4 files changed, 5 insertions(+), 72 deletions(-) Approvals: David Ribeiro Alves: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4398 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I2e9e227af0acfb677572a8baad9692ce08c46be0 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: branch-1.0.x Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR](branch-1.0.x) Change version to non-SNAPSHOT in branch
Todd Lipcon has submitted this change and it was merged. Change subject: Change version to non-SNAPSHOT in branch .. Change version to non-SNAPSHOT in branch Change-Id: Ibc73006692673591a78c1bf3a101058ad62fc014 Reviewed-on: http://gerrit.cloudera.org:8080/4399 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon --- M java/interface-annotations/pom.xml M java/kudu-client-tools/pom.xml M java/kudu-client/pom.xml M java/kudu-csd/pom.xml M java/kudu-csd/src/descriptor/service.sdl M java/kudu-flume-sink/pom.xml M java/kudu-mapreduce/pom.xml M java/kudu-spark/pom.xml M java/pom.xml M version.txt 10 files changed, 11 insertions(+), 11 deletions(-) Approvals: Todd Lipcon: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4399 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ibc73006692673591a78c1bf3a101058ad62fc014 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: branch-1.0.x Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR](branch-1.0.x) Revert "java: fix leak of TabletClient objects in client2tablets map"
David Ribeiro Alves has posted comments on this change. Change subject: Revert "java: fix leak of TabletClient objects in client2tablets map" .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4398 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2e9e227af0acfb677572a8baad9692ce08c46be0 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: branch-1.0.x Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] build-and-test: only run cmake once
Hello Todd Lipcon, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/4401 to review the following change. Change subject: build-and-test: only run cmake once .. build-and-test: only run cmake once Running it twice as we were before was confusing, and the way in which we'd override BUILD_TYPE was error-prone (e.g. the ASAN-specific check for disabling core dumps was broken). Change-Id: I8f9eb27d69ac4d120a0bdad1d1695b4f59b12641 --- M build-support/jenkins/build-and-test.sh 1 file changed, 38 insertions(+), 21 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/01/4401/1 -- To view, visit http://gerrit.cloudera.org:8080/4401 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I8f9eb27d69ac4d120a0bdad1d1695b4f59b12641 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Todd Lipcon
[kudu-CR] build-and-test: only run cmake once
Kudu Jenkins has posted comments on this change. Change subject: build-and-test: only run cmake once .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/3400/ -- To view, visit http://gerrit.cloudera.org:8080/4401 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8f9eb27d69ac4d120a0bdad1d1695b4f59b12641 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] debug-util-test: address flakiness
Adar Dembo has submitted this change and it was merged. Change subject: debug-util-test: address flakiness .. debug-util-test: address flakiness In TSAN builds, this test seems to fail sometimes due to not starting the 'SleeperThread' fast enough. It used to wait only 1 second for the thread to start. This now uses 'AssertEventually' which has a much longer (30-second) timeout. Change-Id: I02b1273352ac8521dee01b7e0b4d4df21e90e517 Reviewed-on: http://gerrit.cloudera.org:8080/4346 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo --- M src/kudu/util/debug-util-test.cc 1 file changed, 3 insertions(+), 7 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4346 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I02b1273352ac8521dee01b7e0b4d4df21e90e517 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] debug-util-test: address flakiness
Adar Dembo has posted comments on this change. Change subject: debug-util-test: address flakiness .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4346 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I02b1273352ac8521dee01b7e0b4d4df21e90e517 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] Bump version to 1.1.0-SNAPSHOT
Kudu Jenkins has posted comments on this change. Change subject: Bump version to 1.1.0-SNAPSHOT .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/3399/ -- To view, visit http://gerrit.cloudera.org:8080/4400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifd9db10a73fecbb73cebe66c16d16e9d11a98750 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR](branch-1.0.x) Revert "java: fix leak of TabletClient objects in client2tablets map"
Kudu Jenkins has posted comments on this change. Change subject: Revert "java: fix leak of TabletClient objects in client2tablets map" .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/3397/ -- To view, visit http://gerrit.cloudera.org:8080/4398 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2e9e227af0acfb677572a8baad9692ce08c46be0 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: branch-1.0.x Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR](branch-1.0.x) Change version to non-SNAPSHOT in branch
Todd Lipcon has uploaded a new change for review. http://gerrit.cloudera.org:8080/4399 Change subject: Change version to non-SNAPSHOT in branch .. Change version to non-SNAPSHOT in branch Change-Id: Ibc73006692673591a78c1bf3a101058ad62fc014 --- M java/interface-annotations/pom.xml M java/kudu-client-tools/pom.xml M java/kudu-client/pom.xml M java/kudu-csd/pom.xml M java/kudu-csd/src/descriptor/service.sdl M java/kudu-flume-sink/pom.xml M java/kudu-mapreduce/pom.xml M java/kudu-spark/pom.xml M java/pom.xml M version.txt 10 files changed, 11 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/99/4399/1 -- To view, visit http://gerrit.cloudera.org:8080/4399 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ibc73006692673591a78c1bf3a101058ad62fc014 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: branch-1.0.x Gerrit-Owner: Todd Lipcon
[kudu-CR](branch-1.0.x) Change version to non-SNAPSHOT in branch
Kudu Jenkins has posted comments on this change. Change subject: Change version to non-SNAPSHOT in branch .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/3398/ -- To view, visit http://gerrit.cloudera.org:8080/4399 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibc73006692673591a78c1bf3a101058ad62fc014 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: branch-1.0.x Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] Bump version to 1.1.0-SNAPSHOT
Todd Lipcon has uploaded a new change for review. http://gerrit.cloudera.org:8080/4400 Change subject: Bump version to 1.1.0-SNAPSHOT .. Bump version to 1.1.0-SNAPSHOT Change-Id: Ifd9db10a73fecbb73cebe66c16d16e9d11a98750 --- M java/interface-annotations/pom.xml M java/kudu-client-tools/pom.xml M java/kudu-client/pom.xml M java/kudu-csd/pom.xml M java/kudu-csd/src/descriptor/service.sdl M java/kudu-flume-sink/pom.xml M java/kudu-mapreduce/pom.xml M java/kudu-spark/pom.xml M java/pom.xml M version.txt 10 files changed, 11 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/00/4400/1 -- To view, visit http://gerrit.cloudera.org:8080/4400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ifd9db10a73fecbb73cebe66c16d16e9d11a98750 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon
[kudu-CR](branch-1.0.x) Revert "java: fix leak of TabletClient objects in client2tablets map"
Todd Lipcon has uploaded a new change for review. http://gerrit.cloudera.org:8080/4398 Change subject: Revert "java: fix leak of TabletClient objects in client2tablets map" .. Revert "java: fix leak of TabletClient objects in client2tablets map" This reverts commit d5082d8ec1218e3f3bd2143d117ddd64772a6162. It was fonud to be unstable on master, so reverting for branch-1.0. Change-Id: I2e9e227af0acfb677572a8baad9692ce08c46be0 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.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/TestAsyncKuduSession.java 4 files changed, 5 insertions(+), 72 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/4398/1 -- To view, visit http://gerrit.cloudera.org:8080/4398 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I2e9e227af0acfb677572a8baad9692ce08c46be0 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: branch-1.0.x Gerrit-Owner: Todd Lipcon
[kudu-CR] Add two RAT excludes
Todd Lipcon has submitted this change and it was merged. Change subject: Add two RAT excludes .. Add two RAT excludes - .avsc files can't have a comment header - The HTML template used for the doxygen footer seems small enough that it's not a big deal to add the license header, vs the extra cost of putting all the license text multiple times into all of the generated doc HTML. Change-Id: Ia7970326680af1643e81c248346de3714bdfe72f Reviewed-on: http://gerrit.cloudera.org:8080/4396 Reviewed-by: Mike Percy Tested-by: Kudu Jenkins --- M build-support/release/rat_exclude_files.txt 1 file changed, 2 insertions(+), 0 deletions(-) Approvals: Mike Percy: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4396 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ia7970326680af1643e81c248346de3714bdfe72f Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1605. Blocks can be incorrectly deleted if TS crashes mid-copy
Todd Lipcon has submitted this change and it was merged. Change subject: KUDU-1605. Blocks can be incorrectly deleted if TS crashes mid-copy .. KUDU-1605. Blocks can be incorrectly deleted if TS crashes mid-copy This fixes a bug in the way we handle tablet copies while replacing existing tombstoned tablets: - a tablet exists in TABLET_DATA_TOMBSTONED state - we begin copying a new replica on top of this one -- this calls TabletMetadata::ReplaceSuperBlock() using the remote superblock (importantly, this remote superblock contains remote block IDs) - we crash mid-copy - on restart, we see the "TABLET_DATA_COPYING" state and "roll forward" the deletion of this tablet. However the block IDs here are the IDs from the remote machine, and we incorrectly delete a bunch of blocks. This has always been an issue, but was made worse in 0.10 by the fix for KUDU-1538. After fixing KUDU-1538, the likelihood of a remote block ID matching a local one is quite high, whereas before we'd usually not see this bug. The fix here is relatively simple: rather than writing the remote superblock to disk when starting the copy, we just change the state of the existing superblock to indicate 'copying'. Change-Id: Ica25c5e4e5894ea80e416d9a4ad44dd25e0c6d53 Reviewed-on: http://gerrit.cloudera.org:8080/4392 Reviewed-by: Mike Percy Tested-by: Todd Lipcon --- M src/kudu/integration-tests/delete_table-test.cc M src/kudu/integration-tests/test_workload.cc M src/kudu/integration-tests/test_workload.h M src/kudu/tablet/tablet_metadata.cc M src/kudu/tablet/tablet_metadata.h M src/kudu/tserver/tablet_copy_client.cc M src/kudu/tserver/ts_tablet_manager.cc 7 files changed, 165 insertions(+), 65 deletions(-) Approvals: Mike Percy: Looks good to me, approved Todd Lipcon: Verified -- To view, visit http://gerrit.cloudera.org:8080/4392 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ica25c5e4e5894ea80e416d9a4ad44dd25e0c6d53 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [docs/doxygen] removed auto-generated comments
Todd Lipcon has submitted this change and it was merged. Change subject: [docs/doxygen] removed auto-generated comments .. [docs/doxygen] removed auto-generated comments The template auto-generated comments for doxygen directives have been removed to stay away from licensing issues. Change-Id: I9100754130af782fbe4e8e58c9fcf1376e5c293a Reviewed-on: http://gerrit.cloudera.org:8080/4397 Reviewed-by: Todd Lipcon Reviewed-by: Mike Percy Tested-by: Kudu Jenkins --- M docs/support/doxygen/client_api.doxy.in 1 file changed, 13 insertions(+), 40 deletions(-) Approvals: Mike Percy: Looks good to me, approved Todd Lipcon: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4397 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I9100754130af782fbe4e8e58c9fcf1376e5c293a Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1605. Blocks can be incorrectly deleted if TS crashes mid-copy
Todd Lipcon has posted comments on this change. Change subject: KUDU-1605. Blocks can be incorrectly deleted if TS crashes mid-copy .. Patch Set 3: Verified+1 Known-flaky java test -- To view, visit http://gerrit.cloudera.org:8080/4392 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ica25c5e4e5894ea80e416d9a4ad44dd25e0c6d53 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] [client] avoid circular deps in time-based flusher
Adar Dembo has posted comments on this change. Change subject: [client] avoid circular deps in time-based flusher .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/4395/1/src/kudu/client/session-internal.h File src/kudu/client/session-internal.h: PS1, Line 161: // The reference to the client's messenger (keeping the reference instead of : // declaring friendship to KuduClient and accessing it via the client_). > Will add some info into the comment, OK. Yeah, that sounds reasonable. -- To view, visit http://gerrit.cloudera.org:8080/4395 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I59825981a600f5882ee476479c2ddf16b495c1f9 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [client] avoid circular deps in time-based flusher
Alexey Serbin has posted comments on this change. Change subject: [client] avoid circular deps in time-based flusher .. Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/4395/1/src/kudu/client/session-internal.cc File src/kudu/client/session-internal.cc: Line 514: if (PREDICT_TRUE(messenger)) { > Can't it be false if it's called out of ScheduleOnReactor()? Ah, right observation. And it will get at this point only in case of AUTO_FLUSH_BACKGROUND mode, btw. Line 517: _1, weak_messenger, weak_session, false), > Yeah, it doesn't affect correctness. ok, thanks! http://gerrit.cloudera.org:8080/#/c/4395/1/src/kudu/client/session-internal.h File src/kudu/client/session-internal.h: Line 58: const sp::weak_ptr& messenger); > I agree with Dan that it'd be cleanest to pass the messenger and the client Done PS1, Line 161: // The reference to the client's messenger (keeping the reference instead of : // declaring friendship to KuduClient and accessing it via the client_). > Should update this comment to explain why it's a weak pointer. Will add some info into the comment, OK. I have the following reasoning: we pass it around as a weak reference anyway, and then convert into a shared one. Why would we store it as shared? -- To view, visit http://gerrit.cloudera.org:8080/4395 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I59825981a600f5882ee476479c2ddf16b495c1f9 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [client] avoid circular deps in time-based flusher
Alexey Serbin has posted comments on this change. Change subject: [client] avoid circular deps in time-based flusher .. Patch Set 2: > How sure are we that this won't negatively affect the > non-auto-flush code path? i.e is this risky to pull into 1.0.0 so > late in the game? The auto-flush background task should not be run in any other mode except for AUTO_FLUSH_BACKGROUND, so that code should not be executed in other modes. That's why I think it's safe to pull into 1.0.0. -- To view, visit http://gerrit.cloudera.org:8080/4395 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I59825981a600f5882ee476479c2ddf16b495c1f9 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] [client] avoid circular deps in time-based flusher
Adar Dembo has posted comments on this change. Change subject: [client] avoid circular deps in time-based flusher .. Patch Set 2: (1 comment) I left you some comments just as you revved PS2. Not sure if you saw them. http://gerrit.cloudera.org:8080/#/c/4395/2/src/kudu/client/session-internal.cc File src/kudu/client/session-internal.cc: PS2, Line 43: std:: Nit: don't need std prefix here (if you add using). -- To view, visit http://gerrit.cloudera.org:8080/4395 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I59825981a600f5882ee476479c2ddf16b495c1f9 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [client] avoid circular deps in time-based flusher
Kudu Jenkins has posted comments on this change. Change subject: [client] avoid circular deps in time-based flusher .. Patch Set 2: Build Started http://104.196.14.100/job/kudu-gerrit/3396/ -- To view, visit http://gerrit.cloudera.org:8080/4395 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I59825981a600f5882ee476479c2ddf16b495c1f9 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] [client] avoid circular deps in time-based flusher
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4395 to look at the new patch set (#2). Change subject: [client] avoid circular deps in time-based flusher .. [client] avoid circular deps in time-based flusher The boost::bind() makes cast of parameters during the call, not during creation of the functor object: http://stackoverflow.com/questions/11255144/why-does-boostbind-store-arguments-of-the-type-passed-in-rather-than-of-the-ty So, it's necessary to pass weak pointers to the background auto-flush task to avoid circular dependencies between client::KuduSession::Data and rpc::Messenger. Besides, it does not make much sense to store shared reference to messenger in KuduSession::Data since it's always passed as a weak reference and then promoting to a shared one during the call in all usage scenarios. Thanks to Adar and Todd spotting the usual suspect there. This is a follow-up for 93be1310d227cf05025864654ca3f6713c2ddc2c. Change-Id: I59825981a600f5882ee476479c2ddf16b495c1f9 --- M src/kudu/client/session-internal.cc M src/kudu/client/session-internal.h 2 files changed, 5 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/95/4395/2 -- To view, visit http://gerrit.cloudera.org:8080/4395 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I59825981a600f5882ee476479c2ddf16b495c1f9 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [client] avoid circular deps in time-based flusher
Adar Dembo has posted comments on this change. Change subject: [client] avoid circular deps in time-based flusher .. Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/4395/1/src/kudu/client/session-internal.cc File src/kudu/client/session-internal.cc: Line 514: if (PREDICT_TRUE(messenger)) { > I changed the session to keep a weak reference only. But it should be true Can't it be false if it's called out of ScheduleOnReactor()? Line 517: _1, weak_messenger, weak_session, false), > Good suggestion -- will do. But just to make sure I'm not missing anything Yeah, it doesn't affect correctness. http://gerrit.cloudera.org:8080/#/c/4395/1/src/kudu/client/session-internal.h File src/kudu/client/session-internal.h: Line 58: const sp::weak_ptr& messenger); > Using the reference is not crucial, just to be consistent with other places I agree with Dan that it'd be cleanest to pass the messenger and the client in the same way. PS1, Line 161: // The reference to the client's messenger (keeping the reference instead of : // declaring friendship to KuduClient and accessing it via the client_). Should update this comment to explain why it's a weak pointer. Why does it need to be weak, actually? Now we know we'll have weak references from the timer task to the session and manager; isn't that enough? -- To view, visit http://gerrit.cloudera.org:8080/4395 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I59825981a600f5882ee476479c2ddf16b495c1f9 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [client] avoid circular deps in time-based flusher
Alexey Serbin has posted comments on this change. Change subject: [client] avoid circular deps in time-based flusher .. Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/4395/1/src/kudu/client/session-internal.cc File src/kudu/client/session-internal.cc: Line 514: if (PREDICT_TRUE(messenger)) { > This can't ever be false, right? I think the callstack of this method incl I changed the session to keep a weak reference only. But it should be true, otherwise this code is not supposed to be called, right. However, for sanity it would like to keep this, if there aren't objections. Line 517: _1, weak_messenger, weak_session, false), > std::move both messenger and session. Good suggestion -- will do. But just to make sure I'm not missing anything here: adding std::move is not crucial here, right? http://gerrit.cloudera.org:8080/#/c/4395/1/src/kudu/client/session-internal.h File src/kudu/client/session-internal.h: Line 58: const sp::weak_ptr& messenger); > I think this should be std::, and why use a reference? Using the reference is not crucial, just to be consistent with other places where we don't want to update reference counter. But I agree updating the counter while passing the parameter would be safer. Right, thanks -- and that's why there was an error when building it at Linux, not OS X. Will change. -- To view, visit http://gerrit.cloudera.org:8080/4395 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I59825981a600f5882ee476479c2ddf16b495c1f9 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [docs/doxygen] removed auto-generated comments
Mike Percy has posted comments on this change. Change subject: [docs/doxygen] removed auto-generated comments .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4397 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9100754130af782fbe4e8e58c9fcf1376e5c293a Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] tool: port kudu-fs dump, remove kudu-fs list, fs tool
Todd Lipcon has submitted this change and it was merged. Change subject: tool: port kudu-fs_dump, remove kudu-fs_list, fs_tool .. tool: port kudu-fs_dump, remove kudu-fs_list, fs_tool This change ports fs_dump actions under 'kudu local_replica '. Additionally this has following re-organizations: - moved dump_cfile action under 'kudu fs dump cfile'. - kudu-fs_list tool has been removed altogether, but some of the functionalities are retained under 'local_replica' and 'fs dump' sub-actions. - fs_tool library is stripped off, and all those routines are in respective action files. Also added tests under kudu-tool-test to exercise each of these fs tools. Change-Id: I1ec628b65613011d8c48b6239c13762276425966 Reviewed-on: http://gerrit.cloudera.org:8080/4305 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon --- M docs/release_notes.adoc M src/kudu/integration-tests/master_migration-itest.cc M src/kudu/tablet/rowset_metadata.h M src/kudu/tools/CMakeLists.txt D src/kudu/tools/fs_tool.cc D src/kudu/tools/fs_tool.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action.h M src/kudu/tools/tool_action_fs.cc M src/kudu/tools/tool_action_local_replica.cc M src/kudu/tools/tool_action_tablet.cc 11 files changed, 766 insertions(+), 823 deletions(-) Approvals: Todd Lipcon: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4305 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I1ec628b65613011d8c48b6239c13762276425966 Gerrit-PatchSet: 15 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] tool: port ts-cli
Todd Lipcon has submitted this change and it was merged. Change subject: tool: port ts-cli .. tool: port ts-cli I chose to expose common server functionality in new 'master' and 'tserver' modes rather than consolidating them into a shared 'server' mode; I found this to be more more intuitive. I also snuck in a change to ksck to use FLAGS_timeout_ms for RPC timeouts in client-based operations. Change-Id: Ifb5a59fd690c2dd09e4e76858469d81f9d501371 Reviewed-on: http://gerrit.cloudera.org:8080/4373 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon --- M build-support/dist_test.py M docs/release_notes.adoc M src/kudu/client/scan_batch.h M src/kudu/client/schema.h M src/kudu/tablet/tablet_peer.h M src/kudu/tools/CMakeLists.txt M src/kudu/tools/ksck_remote.cc M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/kudu-ts-cli-test.cc M src/kudu/tools/tool_action.h M src/kudu/tools/tool_action_common.cc M src/kudu/tools/tool_action_common.h A src/kudu/tools/tool_action_master.cc A src/kudu/tools/tool_action_remote_replica.cc M src/kudu/tools/tool_action_tablet.cc A src/kudu/tools/tool_action_tserver.cc M src/kudu/tools/tool_main.cc D src/kudu/tools/ts-cli.cc 18 files changed, 758 insertions(+), 522 deletions(-) Approvals: Todd Lipcon: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4373 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ifb5a59fd690c2dd09e4e76858469d81f9d501371 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] tool: port kudu-fs dump, remove kudu-fs list, fs tool
Todd Lipcon has posted comments on this change. Change subject: tool: port kudu-fs_dump, remove kudu-fs_list, fs_tool .. Patch Set 14: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4305 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1ec628b65613011d8c48b6239c13762276425966 Gerrit-PatchSet: 14 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] tool: port ts-cli
Todd Lipcon has posted comments on this change. Change subject: tool: port ts-cli .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4373 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifb5a59fd690c2dd09e4e76858469d81f9d501371 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] [docs/doxygen] removed auto-generated comments
Todd Lipcon has posted comments on this change. Change subject: [docs/doxygen] removed auto-generated comments .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4397 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9100754130af782fbe4e8e58c9fcf1376e5c293a Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] [docs/doxygen] removed auto-generated comments
Kudu Jenkins has posted comments on this change. Change subject: [docs/doxygen] removed auto-generated comments .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/3395/ -- To view, visit http://gerrit.cloudera.org:8080/4397 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9100754130af782fbe4e8e58c9fcf1376e5c293a Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] [docs/doxygen] removed auto-generated comments
Alexey Serbin has uploaded a new change for review. http://gerrit.cloudera.org:8080/4397 Change subject: [docs/doxygen] removed auto-generated comments .. [docs/doxygen] removed auto-generated comments The template auto-generated comments for doxygen directives have been removed to stay away from licensing issues. Change-Id: I9100754130af782fbe4e8e58c9fcf1376e5c293a --- M docs/support/doxygen/client_api.doxy.in 1 file changed, 13 insertions(+), 40 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/97/4397/1 -- To view, visit http://gerrit.cloudera.org:8080/4397 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I9100754130af782fbe4e8e58c9fcf1376e5c293a Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin
[kudu-CR] Add two RAT excludes
Mike Percy has posted comments on this change. Change subject: Add two RAT excludes .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4396 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia7970326680af1643e81c248346de3714bdfe72f Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] Add two RAT excludes
Kudu Jenkins has posted comments on this change. Change subject: Add two RAT excludes .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/3394/ -- To view, visit http://gerrit.cloudera.org:8080/4396 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia7970326680af1643e81c248346de3714bdfe72f Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] Add two RAT excludes
Hello Mike Percy, Alexey Serbin, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/4396 to review the following change. Change subject: Add two RAT excludes .. Add two RAT excludes - .avsc files can't have a comment header - The HTML template used for the doxygen footer seems small enough that it's not a big deal to add the license header, vs the extra cost of putting all the license text multiple times into all of the generated doc HTML. Change-Id: Ia7970326680af1643e81c248346de3714bdfe72f --- M build-support/release/rat_exclude_files.txt 1 file changed, 2 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/96/4396/1 -- To view, visit http://gerrit.cloudera.org:8080/4396 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ia7970326680af1643e81c248346de3714bdfe72f Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Mike Percy
[kudu-CR] [client] avoid circular deps in time-based flusher
Dan Burkert has posted comments on this change. Change subject: [client] avoid circular deps in time-based flusher .. Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/4395/1/src/kudu/client/session-internal.cc File src/kudu/client/session-internal.cc: Line 514: if (PREDICT_TRUE(messenger)) { This can't ever be false, right? I think the callstack of this method includes the KuduSession which has a strong reference. Line 517: _1, weak_messenger, weak_session, false), std::move both messenger and session. http://gerrit.cloudera.org:8080/#/c/4395/1/src/kudu/client/session-internal.h File src/kudu/client/session-internal.h: Line 58: const sp::weak_ptr& messenger); I think this should be std::, and why use a reference? -- To view, visit http://gerrit.cloudera.org:8080/4395 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I59825981a600f5882ee476479c2ddf16b495c1f9 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-1605. Blocks can be incorrectly deleted if TS crashes mid-copy
Mike Percy has posted comments on this change. Change subject: KUDU-1605. Blocks can be incorrectly deleted if TS crashes mid-copy .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4392 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ica25c5e4e5894ea80e416d9a4ad44dd25e0c6d53 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1605. Blocks can be incorrectly deleted if TS crashes mid-copy
Todd Lipcon has posted comments on this change. Change subject: KUDU-1605. Blocks can be incorrectly deleted if TS crashes mid-copy .. Patch Set 3: took care of all the nits -- To view, visit http://gerrit.cloudera.org:8080/4392 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ica25c5e4e5894ea80e416d9a4ad44dd25e0c6d53 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1605. Blocks can be incorrectly deleted if TS crashes mid-copy
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4392 to look at the new patch set (#3). Change subject: KUDU-1605. Blocks can be incorrectly deleted if TS crashes mid-copy .. KUDU-1605. Blocks can be incorrectly deleted if TS crashes mid-copy This fixes a bug in the way we handle tablet copies while replacing existing tombstoned tablets: - a tablet exists in TABLET_DATA_TOMBSTONED state - we begin copying a new replica on top of this one -- this calls TabletMetadata::ReplaceSuperBlock() using the remote superblock (importantly, this remote superblock contains remote block IDs) - we crash mid-copy - on restart, we see the "TABLET_DATA_COPYING" state and "roll forward" the deletion of this tablet. However the block IDs here are the IDs from the remote machine, and we incorrectly delete a bunch of blocks. This has always been an issue, but was made worse in 0.10 by the fix for KUDU-1538. After fixing KUDU-1538, the likelihood of a remote block ID matching a local one is quite high, whereas before we'd usually not see this bug. The fix here is relatively simple: rather than writing the remote superblock to disk when starting the copy, we just change the state of the existing superblock to indicate 'copying'. Change-Id: Ica25c5e4e5894ea80e416d9a4ad44dd25e0c6d53 --- M src/kudu/integration-tests/delete_table-test.cc M src/kudu/integration-tests/test_workload.cc M src/kudu/integration-tests/test_workload.h M src/kudu/tablet/tablet_metadata.cc M src/kudu/tablet/tablet_metadata.h M src/kudu/tserver/tablet_copy_client.cc M src/kudu/tserver/ts_tablet_manager.cc 7 files changed, 165 insertions(+), 65 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/4392/3 -- To view, visit http://gerrit.cloudera.org:8080/4392 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ica25c5e4e5894ea80e416d9a4ad44dd25e0c6d53 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR] KUDU-1605. Blocks can be incorrectly deleted if TS crashes mid-copy
Kudu Jenkins has posted comments on this change. Change subject: KUDU-1605. Blocks can be incorrectly deleted if TS crashes mid-copy .. Patch Set 3: Build Started http://104.196.14.100/job/kudu-gerrit/3393/ -- To view, visit http://gerrit.cloudera.org:8080/4392 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ica25c5e4e5894ea80e416d9a4ad44dd25e0c6d53 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] KUDU-1605. Blocks can be incorrectly deleted if TS crashes mid-copy
Mike Percy has posted comments on this change. Change subject: KUDU-1605. Blocks can be incorrectly deleted if TS crashes mid-copy .. Patch Set 2: (5 comments) looks good, just a few nits http://gerrit.cloudera.org:8080/#/c/4392/2/src/kudu/integration-tests/delete_table-test.cc File src/kudu/integration-tests/delete_table-test.cc: PS2, Line 451: 0 kTsIndex PS2, Line 503: 0 kTsIndex PS2, Line 528: 0 kTsIndex PS2, Line 546: 0 kTsIndex http://gerrit.cloudera.org:8080/#/c/4392/1/src/kudu/tserver/ts_tablet_manager.cc File src/kudu/tserver/ts_tablet_manager.cc: Line 892: << "of type " << TabletDataState_Name(data_state); nit: indentation -- To view, visit http://gerrit.cloudera.org:8080/4392 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ica25c5e4e5894ea80e416d9a4ad44dd25e0c6d53 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] KUDU-1605. Blocks can be incorrectly deleted if TS crashes mid-copy
Kudu Jenkins has posted comments on this change. Change subject: KUDU-1605. Blocks can be incorrectly deleted if TS crashes mid-copy .. Patch Set 2: Build Started http://104.196.14.100/job/kudu-gerrit/3392/ -- To view, visit http://gerrit.cloudera.org:8080/4392 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ica25c5e4e5894ea80e416d9a4ad44dd25e0c6d53 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] Remove a spurious warning left in raft consensus state.cc
Todd Lipcon has submitted this change and it was merged. Change subject: Remove a spurious warning left in raft_consensus_state.cc .. Remove a spurious warning left in raft_consensus_state.cc Change-Id: I0bcada22bc31e1640e5e047c2326f61dc617705d Reviewed-on: http://gerrit.cloudera.org:8080/4391 Reviewed-by: Mike Percy Tested-by: Kudu Jenkins --- M src/kudu/consensus/raft_consensus_state.cc 1 file changed, 0 insertions(+), 1 deletion(-) Approvals: Mike Percy: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4391 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I0bcada22bc31e1640e5e047c2326f61dc617705d Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1605. Blocks can be incorrectly deleted if TS crashes mid-copy
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4392 to look at the new patch set (#2). Change subject: KUDU-1605. Blocks can be incorrectly deleted if TS crashes mid-copy .. KUDU-1605. Blocks can be incorrectly deleted if TS crashes mid-copy This fixes a bug in the way we handle tablet copies while replacing existing tombstoned tablets: - a tablet exists in TABLET_DATA_TOMBSTONED state - we begin copying a new replica on top of this one -- this calls TabletMetadata::ReplaceSuperBlock() using the remote superblock (importantly, this remote superblock contains remote block IDs) - we crash mid-copy - on restart, we see the "TABLET_DATA_COPYING" state and "roll forward" the deletion of this tablet. However the block IDs here are the IDs from the remote machine, and we incorrectly delete a bunch of blocks. This has always been an issue, but was made worse in 0.10 by the fix for KUDU-1538. After fixing KUDU-1538, the likelihood of a remote block ID matching a local one is quite high, whereas before we'd usually not see this bug. The fix here is relatively simple: rather than writing the remote superblock to disk when starting the copy, we just change the state of the existing superblock to indicate 'copying'. Change-Id: Ica25c5e4e5894ea80e416d9a4ad44dd25e0c6d53 --- M src/kudu/integration-tests/delete_table-test.cc M src/kudu/integration-tests/test_workload.cc M src/kudu/integration-tests/test_workload.h M src/kudu/tablet/tablet_metadata.cc M src/kudu/tablet/tablet_metadata.h M src/kudu/tserver/tablet_copy_client.cc M src/kudu/tserver/ts_tablet_manager.cc 7 files changed, 165 insertions(+), 65 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/4392/2 -- To view, visit http://gerrit.cloudera.org:8080/4392 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ica25c5e4e5894ea80e416d9a4ad44dd25e0c6d53 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR] [client] avoid circular deps in time-based flusher
Todd Lipcon has posted comments on this change. Change subject: [client] avoid circular deps in time-based flusher .. Patch Set 1: How sure are we that this won't negatively affect the non-auto-flush code path? i.e is this risky to pull into 1.0.0 so late in the game? -- To view, visit http://gerrit.cloudera.org:8080/4395 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I59825981a600f5882ee476479c2ddf16b495c1f9 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] tool: port ts-cli
Todd Lipcon has posted comments on this change. Change subject: tool: port ts-cli .. Patch Set 5: Just rebased -- To view, visit http://gerrit.cloudera.org:8080/4373 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifb5a59fd690c2dd09e4e76858469d81f9d501371 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] tool: port kudu-fs dump, remove kudu-fs list, fs tool
Todd Lipcon has posted comments on this change. Change subject: tool: port kudu-fs_dump, remove kudu-fs_list, fs_tool .. Patch Set 8: Just rebased and put this in a series with Dinesh's patch. Will commit if it passes -- To view, visit http://gerrit.cloudera.org:8080/4305 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1ec628b65613011d8c48b6239c13762276425966 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] tool: port kudu-fs dump, remove kudu-fs list, fs tool
Hello Adar Dembo, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4305 to look at the new patch set (#14). Change subject: tool: port kudu-fs_dump, remove kudu-fs_list, fs_tool .. tool: port kudu-fs_dump, remove kudu-fs_list, fs_tool This change ports fs_dump actions under 'kudu local_replica '. Additionally this has following re-organizations: - moved dump_cfile action under 'kudu fs dump cfile'. - kudu-fs_list tool has been removed altogether, but some of the functionalities are retained under 'local_replica' and 'fs dump' sub-actions. - fs_tool library is stripped off, and all those routines are in respective action files. Also added tests under kudu-tool-test to exercise each of these fs tools. Change-Id: I1ec628b65613011d8c48b6239c13762276425966 --- M docs/release_notes.adoc M src/kudu/integration-tests/master_migration-itest.cc M src/kudu/tablet/rowset_metadata.h M src/kudu/tools/CMakeLists.txt D src/kudu/tools/fs_tool.cc D src/kudu/tools/fs_tool.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action.h M src/kudu/tools/tool_action_fs.cc M src/kudu/tools/tool_action_local_replica.cc M src/kudu/tools/tool_action_tablet.cc 11 files changed, 766 insertions(+), 823 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/05/4305/14 -- To view, visit http://gerrit.cloudera.org:8080/4305 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1ec628b65613011d8c48b6239c13762276425966 Gerrit-PatchSet: 14 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] tool: port kudu-fs dump, remove kudu-fs list, fs tool
Kudu Jenkins has posted comments on this change. Change subject: tool: port kudu-fs_dump, remove kudu-fs_list, fs_tool .. Patch Set 14: Build Started http://104.196.14.100/job/kudu-gerrit/3391/ -- To view, visit http://gerrit.cloudera.org:8080/4305 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1ec628b65613011d8c48b6239c13762276425966 Gerrit-PatchSet: 14 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] tool: port ts-cli
Kudu Jenkins has posted comments on this change. Change subject: tool: port ts-cli .. Patch Set 5: Build Started http://104.196.14.100/job/kudu-gerrit/3390/ -- To view, visit http://gerrit.cloudera.org:8080/4373 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifb5a59fd690c2dd09e4e76858469d81f9d501371 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] tool: port ts-cli
Hello Dinesh Bhat, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4373 to look at the new patch set (#5). Change subject: tool: port ts-cli .. tool: port ts-cli I chose to expose common server functionality in new 'master' and 'tserver' modes rather than consolidating them into a shared 'server' mode; I found this to be more more intuitive. I also snuck in a change to ksck to use FLAGS_timeout_ms for RPC timeouts in client-based operations. Change-Id: Ifb5a59fd690c2dd09e4e76858469d81f9d501371 --- M build-support/dist_test.py M docs/release_notes.adoc M src/kudu/client/scan_batch.h M src/kudu/client/schema.h M src/kudu/tablet/tablet_peer.h M src/kudu/tools/CMakeLists.txt M src/kudu/tools/ksck_remote.cc M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/kudu-ts-cli-test.cc M src/kudu/tools/tool_action.h M src/kudu/tools/tool_action_common.cc M src/kudu/tools/tool_action_common.h A src/kudu/tools/tool_action_master.cc A src/kudu/tools/tool_action_remote_replica.cc M src/kudu/tools/tool_action_tablet.cc A src/kudu/tools/tool_action_tserver.cc M src/kudu/tools/tool_main.cc D src/kudu/tools/ts-cli.cc 18 files changed, 758 insertions(+), 522 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/73/4373/5 -- To view, visit http://gerrit.cloudera.org:8080/4373 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifb5a59fd690c2dd09e4e76858469d81f9d501371 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [client] avoid circular deps in time-based flusher
Kudu Jenkins has posted comments on this change. Change subject: [client] avoid circular deps in time-based flusher .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/3389/ -- To view, visit http://gerrit.cloudera.org:8080/4395 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I59825981a600f5882ee476479c2ddf16b495c1f9 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] [client] avoid circular deps in time-based flusher
Alexey Serbin has uploaded a new change for review. http://gerrit.cloudera.org:8080/4395 Change subject: [client] avoid circular deps in time-based flusher .. [client] avoid circular deps in time-based flusher The boost::bind() makes cast of parameters during the call, not during creation of the functor object: http://stackoverflow.com/questions/11255144/why-does-boostbind-store-arguments-of-the-type-passed-in-rather-than-of-the-ty So, it's necessary to pass weak pointers to the background auto-flush task to avoid circular dependencies between client::KuduSession::Data and rpc::Messenger. Besides, it does not make much sense to store shared reference to messenger in KuduSession::Data since it's always passed as a weak reference and then promoting to a shared one during the call in all usage scenarios. Thanks to Adar and Todd spotting the usual suspect there. This is a follow-up for 93be1310d227cf05025864654ca3f6713c2ddc2c. Change-Id: I59825981a600f5882ee476479c2ddf16b495c1f9 --- M src/kudu/client/session-internal.cc M src/kudu/client/session-internal.h 2 files changed, 5 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/95/4395/1 -- To view, visit http://gerrit.cloudera.org:8080/4395 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I59825981a600f5882ee476479c2ddf16b495c1f9 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin
[kudu-CR] KUDU-1135 (part 1): avoid flushing cmeta to disk twice when voting
David Ribeiro Alves has posted comments on this change. Change subject: KUDU-1135 (part 1): avoid flushing cmeta to disk twice when voting .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4333 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iecc55bc9e96dcdc918ede1190b7cbac719f95506 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] tool: port ts-cli
Todd Lipcon has posted comments on this change. Change subject: tool: port ts-cli .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4373 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifb5a59fd690c2dd09e4e76858469d81f9d501371 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Remove a spurious warning left in raft consensus state.cc
Mike Percy has posted comments on this change. Change subject: Remove a spurious warning left in raft_consensus_state.cc .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4391 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0bcada22bc31e1640e5e047c2326f61dc617705d Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] KUDU-1090: relax MemTracker uniqueness constraint
Hello Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4394 to look at the new patch set (#2). Change subject: KUDU-1090: relax MemTracker uniqueness constraint .. KUDU-1090: relax MemTracker uniqueness constraint This patch relaxes the MemTracker uniqueness constraint to avoid certain rare crashes (e.g. web UI takes a reference to a MemRowSet tracker during Tablet::RewindSchemaForBootstrap). Details on these crashes can be found in the associated bug report. More specifically, the constraint is preserved, but it is only enforced during FindTracker() or FindOrCreateTracker(). This way it is there for MemTracker users that really should avoid duplicates (e.g. enforcing that every tablet has just one MemTracker for all of its DeltaMemStores), but out of the way for everyone else. With this change, we can remove the various hacks and workarounds that were sprinkled in various tests. Without the patch, the new test failed once out of 1000 dist-test runs. I could not get it to fail locally at all; maybe I'm doing something wrong? Change-Id: Iea8e3d383878e829188eaca65d639bb44d8b8146 --- M src/kudu/consensus/consensus_queue-test.cc M src/kudu/consensus/log_cache-test.cc M src/kudu/consensus/log_cache.cc M src/kudu/fs/block_manager-stress-test.cc M src/kudu/fs/block_manager-test.cc M src/kudu/fs/file_block_manager.cc M src/kudu/fs/fs_manager-test.cc M src/kudu/fs/log_block_manager.cc M src/kudu/integration-tests/CMakeLists.txt M src/kudu/integration-tests/external_mini_cluster-test.cc M src/kudu/integration-tests/linked_list-test-util.h M src/kudu/integration-tests/master_failover-itest.cc M src/kudu/server/server_base.cc M src/kudu/tablet/memrowset.cc M src/kudu/tablet/tablet.cc M src/kudu/tablet/transactions/transaction_tracker.cc M src/kudu/util/mem_tracker-test.cc M src/kudu/util/mem_tracker.cc M src/kudu/util/mem_tracker.h 19 files changed, 128 insertions(+), 135 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/94/4394/2 -- To view, visit http://gerrit.cloudera.org:8080/4394 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iea8e3d383878e829188eaca65d639bb44d8b8146 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1090: relax MemTracker uniqueness constraint
Kudu Jenkins has posted comments on this change. Change subject: KUDU-1090: relax MemTracker uniqueness constraint .. Patch Set 2: Build Started http://104.196.14.100/job/kudu-gerrit/3388/ -- To view, visit http://gerrit.cloudera.org:8080/4394 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iea8e3d383878e829188eaca65d639bb44d8b8146 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] WIP: KUDU-1090 test
Adar Dembo has abandoned this change. Change subject: WIP: KUDU-1090 test .. Abandoned Oops, meant to squash this in with the other patch. -- To view, visit http://gerrit.cloudera.org:8080/4393 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: abandon Gerrit-Change-Id: Icf5d9d0569ff304a4c32bbd7edc860df7554d71b Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] WIP: KUDU-1090 test
Kudu Jenkins has posted comments on this change. Change subject: WIP: KUDU-1090 test .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/3386/ -- To view, visit http://gerrit.cloudera.org:8080/4393 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icf5d9d0569ff304a4c32bbd7edc860df7554d71b Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1090: relax MemTracker uniqueness constraint
Kudu Jenkins has posted comments on this change. Change subject: KUDU-1090: relax MemTracker uniqueness constraint .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/3387/ -- To view, visit http://gerrit.cloudera.org:8080/4394 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iea8e3d383878e829188eaca65d639bb44d8b8146 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1090: relax MemTracker uniqueness constraint
Hello Todd Lipcon, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/4394 to review the following change. Change subject: KUDU-1090: relax MemTracker uniqueness constraint .. KUDU-1090: relax MemTracker uniqueness constraint This patch relaxes the MemTracker uniqueness constraint to avoid certain rare crashes (e.g. web UI takes a reference to a MemRowSet tracker during Tablet::RewindSchemaForBootstrap). Details on these crashes can be found in the associated bug report. More specifically, the constraint is preserved, but it is only enforced during FindTracker() or FindOrCreateTracker(). This way it is there for MemTracker users that really should avoid duplicates (e.g. enforcing that every tablet has just one MemTracker for all of its DeltaMemStores), but out of the way for everyone else. With this change, we can remove the various hacks and workarounds that were sprinkled in various tests. Without the patch, the new test failed once out of 1000 dist-test runs. I could not get it to fail locally at all; maybe I'm doing something wrong? Change-Id: Iea8e3d383878e829188eaca65d639bb44d8b8146 --- M src/kudu/consensus/consensus_queue-test.cc M src/kudu/consensus/log_cache-test.cc M src/kudu/consensus/log_cache.cc M src/kudu/fs/block_manager-stress-test.cc M src/kudu/fs/block_manager-test.cc M src/kudu/fs/file_block_manager.cc M src/kudu/fs/fs_manager-test.cc M src/kudu/fs/log_block_manager.cc M src/kudu/integration-tests/CMakeLists.txt M src/kudu/integration-tests/external_mini_cluster-test.cc M src/kudu/integration-tests/master_failover-itest.cc M src/kudu/server/server_base.cc M src/kudu/tablet/memrowset.cc M src/kudu/tablet/tablet.cc M src/kudu/tablet/transactions/transaction_tracker.cc M src/kudu/util/mem_tracker-test.cc M src/kudu/util/mem_tracker.cc M src/kudu/util/mem_tracker.h 18 files changed, 66 insertions(+), 117 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/94/4394/1 -- To view, visit http://gerrit.cloudera.org:8080/4394 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Iea8e3d383878e829188eaca65d639bb44d8b8146 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Todd Lipcon
[kudu-CR] WIP: KUDU-1090 test
Hello Todd Lipcon, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/4393 to review the following change. Change subject: WIP: KUDU-1090 test .. WIP: KUDU-1090 test Change-Id: Icf5d9d0569ff304a4c32bbd7edc860df7554d71b --- M src/kudu/integration-tests/external_mini_cluster-test.cc M src/kudu/integration-tests/linked_list-test-util.h 2 files changed, 63 insertions(+), 19 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/93/4393/1 -- To view, visit http://gerrit.cloudera.org:8080/4393 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Icf5d9d0569ff304a4c32bbd7edc860df7554d71b Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Remove a spurious warning left in raft consensus state.cc
Kudu Jenkins has posted comments on this change. Change subject: Remove a spurious warning left in raft_consensus_state.cc .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/3384/ -- To view, visit http://gerrit.cloudera.org:8080/4391 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0bcada22bc31e1640e5e047c2326f61dc617705d Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] KUDU-1605. Blocks can be incorrectly deleted if TS crashes mid-copy
Hello Mike Percy, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/4392 to review the following change. Change subject: KUDU-1605. Blocks can be incorrectly deleted if TS crashes mid-copy .. KUDU-1605. Blocks can be incorrectly deleted if TS crashes mid-copy This fixes a bug in the way we handle tablet copies while replacing existing tombstoned tablets: - a tablet exists in TABLET_DATA_TOMBSTONED state - we begin copying a new replica on top of this one -- this calls TabletMetadata::ReplaceSuperBlock() using the remote superblock (importantly, this remote superblock contains remote block IDs) - we crash mid-copy - on restart, we see the "TABLET_DATA_COPYING" state and "roll forward" - the deletion of this tablet. However the block IDs here are the IDs from - the remote machine, and we incorrectly delete a bunch of blocks. This has always been an issue, but was made worse in 0.10 by the fix for KUDU-1538. After fixing KUDU-1538, the likelihood of a remote block ID matching a local one is quite high, whereas before we'd usually not see this bug. The fix here is relatively simple: rather than writing the remote superblock to disk when starting the copy, we just change the state of the existing superblock to indicate 'copying'. Change-Id: Ica25c5e4e5894ea80e416d9a4ad44dd25e0c6d53 --- M src/kudu/integration-tests/delete_table-test.cc M src/kudu/integration-tests/test_workload.cc M src/kudu/integration-tests/test_workload.h M src/kudu/tablet/tablet_metadata.cc M src/kudu/tablet/tablet_metadata.h M src/kudu/tserver/tablet_copy_client.cc M src/kudu/tserver/ts_tablet_manager.cc 7 files changed, 165 insertions(+), 65 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/4392/1 -- To view, visit http://gerrit.cloudera.org:8080/4392 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ica25c5e4e5894ea80e416d9a4ad44dd25e0c6d53 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Mike Percy
[kudu-CR] Remove a spurious warning left in raft consensus state.cc
Hello Mike Percy, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/4391 to review the following change. Change subject: Remove a spurious warning left in raft_consensus_state.cc .. Remove a spurious warning left in raft_consensus_state.cc Change-Id: I0bcada22bc31e1640e5e047c2326f61dc617705d --- M src/kudu/consensus/raft_consensus_state.cc 1 file changed, 0 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/91/4391/1 -- To view, visit http://gerrit.cloudera.org:8080/4391 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I0bcada22bc31e1640e5e047c2326f61dc617705d Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Mike Percy
[kudu-CR] KUDU-1605. Blocks can be incorrectly deleted if TS crashes mid-copy
Kudu Jenkins has posted comments on this change. Change subject: KUDU-1605. Blocks can be incorrectly deleted if TS crashes mid-copy .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/3385/ -- To view, visit http://gerrit.cloudera.org:8080/4392 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ica25c5e4e5894ea80e416d9a4ad44dd25e0c6d53 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] Take 2: fix flakiness in tablet history gc-itest
Mike Percy has submitted this change and it was merged. Change subject: Take 2: fix flakiness in tablet_history_gc-itest .. Take 2: fix flakiness in tablet_history_gc-itest 3a9ea63b9f210b64124b2d9d2b5f14ed1214f272 attempted to fix this bug, but I added the flag setting to the wrong test case. Take two, this time on the test case that was actually flaky! Change-Id: I669cac8a696c61e683485fec175d1241ee34410a Reviewed-on: http://gerrit.cloudera.org:8080/4388 Tested-by: Kudu Jenkins Reviewed-by: Mike Percy --- M src/kudu/integration-tests/tablet_history_gc-itest.cc 1 file changed, 5 insertions(+), 3 deletions(-) Approvals: Mike Percy: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4388 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I669cac8a696c61e683485fec175d1241ee34410a Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR](gh-pages) Blogpost describing predicate evaluation pushdown
Jean-Daniel Cryans has posted comments on this change. Change subject: Blogpost describing predicate evaluation pushdown .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/4384/2/_posts/predicate-pushdown.md File _posts/predicate-pushdown.md: PS2, Line 10: General Availability > Done. Is just 1.0 fine? Or should it be version 1.0/v1.0? Is it widely unde GA wouldn't be since it's not a concept we have in Kudu. We've been talking about 1.0 for a while now though and we're about to release it so I would just say "1.0". Line 14: > Will do. Wasn't sure how to bridge the header to here since I thought it mi I don't consider the first part above as an intro, it's more like context. This blog post is really about your work, and not your internship. You could also make that first part smaller and move the rest at the end. But I really think this blog post needs its own proper introduction about its content. -- To view, visit http://gerrit.cloudera.org:8080/4384 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia3b593959194a6ce9190f562339dc04a1d8fceba Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Take 2: fix flakiness in tablet history gc-itest
Mike Percy has posted comments on this change. Change subject: Take 2: fix flakiness in tablet_history_gc-itest .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4388 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I669cac8a696c61e683485fec175d1241ee34410a Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] [c++ client] AUTO FLUSH BACKGROUND optimizations
David Ribeiro Alves has posted comments on this change. Change subject: [c++ client] AUTO_FLUSH_BACKGROUND optimizations .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/4308/2//COMMIT_MSG Commit Message: Line 13: for the mutation buffer. Changing it from 80% to 50% gave good s/good/some actual numbers (e.g. XX%-YY% throughput increase or something) Line 53: The tests were run at ve0518.halxg.cloudera.com against binaries dont point to an internal cloudera machine on the commit message, if needed (though I doubt it) you can mention the machine specs. -- To view, visit http://gerrit.cloudera.org:8080/4308 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1f0aa6d02c51bb063498709e8570e8c7214a31a0 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] Add more release notes for new features in 1.0
Todd Lipcon has posted comments on this change. Change subject: Add more release notes for new features in 1.0 .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4370 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ideac208aa377b52cc9910c05a9fa4d25a333d49a Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: No
[kudu-CR] Add more release notes for new features in 1.0
Todd Lipcon has submitted this change and it was merged. Change subject: Add more release notes for new features in 1.0 .. Add more release notes for new features in 1.0 Change-Id: Ideac208aa377b52cc9910c05a9fa4d25a333d49a Reviewed-on: http://gerrit.cloudera.org:8080/4370 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon --- M docs/release_notes.adoc 1 file changed, 34 insertions(+), 0 deletions(-) Approvals: Todd Lipcon: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4370 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ideac208aa377b52cc9910c05a9fa4d25a333d49a Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] Take 2: fix flakiness in tablet history gc-itest
Kudu Jenkins has posted comments on this change. Change subject: Take 2: fix flakiness in tablet_history_gc-itest .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/3383/ -- To view, visit http://gerrit.cloudera.org:8080/4388 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I669cac8a696c61e683485fec175d1241ee34410a Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] Take 2: fix flakiness in tablet history gc-itest
Hello Mike Percy, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/4388 to review the following change. Change subject: Take 2: fix flakiness in tablet_history_gc-itest .. Take 2: fix flakiness in tablet_history_gc-itest 3a9ea63b9f210b64124b2d9d2b5f14ed1214f272 attempted to fix this bug, but I added the flag setting to the wrong test case. Take two, this time on the test case that was actually flaky! Change-Id: I669cac8a696c61e683485fec175d1241ee34410a --- M src/kudu/integration-tests/tablet_history_gc-itest.cc 1 file changed, 5 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/88/4388/1 -- To view, visit http://gerrit.cloudera.org:8080/4388 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I669cac8a696c61e683485fec175d1241ee34410a Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Mike Percy
[kudu-CR] [client] performance optimizations
David Ribeiro Alves has posted comments on this change. Change subject: [client] performance optimizations .. Patch Set 2: (7 comments) http://gerrit.cloudera.org:8080/#/c/4385/2//COMMIT_MSG Commit Message: Line 10: which gave about 50% boost while running in scenario when Is there a test that you can point to that demonstrates this speed up? Any real numbers you can post here? http://gerrit.cloudera.org:8080/#/c/4385/2/src/kudu/client/batcher.cc File src/kudu/client/batcher.cc: Line 481 can you move this warning to the ctor? (or to the caller of the ctor) http://gerrit.cloudera.org:8080/#/c/4385/2/src/kudu/client/client.cc File src/kudu/client/client.cc: Line 761: : data_(new KuduSession::Data(client, client->data_->messenger_)) { spurious change http://gerrit.cloudera.org:8080/#/c/4385/2/src/kudu/client/session-internal.cc File src/kudu/client/session-internal.cc: Line 446: // Thread-safety note: the buffer_bytes_limit_ and nit: rewrap to use whole lines Line 448: // from any other thread of control since no thread-safety is advertised what's a "thread of control"? Line 450: // So, no protection while accessing those members. Actually can you move this (and the comment on loc 338) to where the field is declared? http://gerrit.cloudera.org:8080/#/c/4385/2/src/kudu/client/session-internal.h File src/kudu/client/session-internal.h: Line 160: sp::weak_ptr session_; doc -- To view, visit http://gerrit.cloudera.org:8080/4385 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4b57fc7355f9f673f30861ec30cb6b48cdf656d2 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] release notes: improve explanation of repartitioning limitation
David Ribeiro Alves has posted comments on this change. Change subject: release_notes: improve explanation of repartitioning limitation .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4387 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4dfd040ae18d439eedfce44e26b763723f9068e4 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: No
[kudu-CR] release notes: improve explanation of repartitioning limitation
David Ribeiro Alves has submitted this change and it was merged. Change subject: release_notes: improve explanation of repartitioning limitation .. release_notes: improve explanation of repartitioning limitation Change-Id: I4dfd040ae18d439eedfce44e26b763723f9068e4 Reviewed-on: http://gerrit.cloudera.org:8080/4387 Reviewed-by: Will Berkeley Tested-by: Kudu Jenkins Reviewed-by: David Ribeiro Alves --- M docs/release_notes.adoc 1 file changed, 3 insertions(+), 2 deletions(-) Approvals: David Ribeiro Alves: Looks good to me, approved Will Berkeley: Looks good to me, but someone else must approve Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4387 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I4dfd040ae18d439eedfce44e26b763723f9068e4 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley
[kudu-CR] Create base class for MiniCluster and ExternalMiniCluster
Mike Percy has posted comments on this change. Change subject: Create base class for MiniCluster and ExternalMiniCluster .. Patch Set 10: Filed a cpplint issue @ https://github.com/google/styleguide/issues/176 -- To view, visit http://gerrit.cloudera.org:8080/3974 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I62256168a9245c845e99f7253f07e69a225954bf Gerrit-PatchSet: 10 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No