[kudu-CR] doxygen for C++ client API
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3619 to look at the new patch set (#8). Change subject: doxygen for C++ client API .. doxygen for C++ client API If doxygen is available, build 'doxygen' taget to generate Doxygen docs from client.h and other Kudu C++ client API files, After the target is built, open ${CMAKE_CURRENT_BINARY_DIR}/docs/doxygen/client_api/html/index.html in your favorite browser to see the generated documentation. If doxygen is available, the 'docs' target generates doxygen documentaion as well since it depends on the 'doxygen' target. Change-Id: Ie7d42fb1c90b83074e357dcecf42489ed9fc4f02 --- M CMakeLists.txt A docs/support/doxygen/client_api.doxy.in M src/kudu/client/client.h M thirdparty/download-thirdparty.sh 4 files changed, 1,294 insertions(+), 716 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/19/3619/8 -- To view, visit http://gerrit.cloudera.org:8080/3619 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie7d42fb1c90b83074e357dcecf42489ed9fc4f02 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR] doxygen for C++ client API
Kudu Jenkins has posted comments on this change. Change subject: doxygen for C++ client API .. Patch Set 8: Build Started http://104.196.14.100/job/kudu-gerrit/2504/ -- To view, visit http://gerrit.cloudera.org:8080/3619 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7d42fb1c90b83074e357dcecf42489ed9fc4f02 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] [java client] Integrate with the replay cache
David Ribeiro Alves has posted comments on this change. Change subject: [java client] Integrate with the replay cache .. Patch Set 4: (4 comments) looks mostly good, does this still have the side effect of making us not ignore duplicated rows where we did before? http://gerrit.cloudera.org:8080/#/c/3631/4/java/kudu-client/src/main/java/org/kududb/client/KuduRpc.java File java/kudu-client/src/main/java/org/kududb/client/KuduRpc.java: PS4, Line 107: long docs http://gerrit.cloudera.org:8080/#/c/3631/4/java/kudu-client/src/main/java/org/kududb/client/Operation.java File java/kudu-client/src/main/java/org/kududb/client/Operation.java: Line 163: boolean isRetryableRpc() { docs http://gerrit.cloudera.org:8080/#/c/3631/4/java/kudu-client/src/main/java/org/kududb/client/RequestTracker.java File java/kudu-client/src/main/java/org/kududb/client/RequestTracker.java: Line 41: public RequestTracker(String clientId) { does the request tracker need to have the client id? http://gerrit.cloudera.org:8080/#/c/3631/4/java/kudu-client/src/main/java/org/kududb/client/TabletClient.java File java/kudu-client/src/main/java/org/kududb/client/TabletClient.java: PS4, Line 254: requestIdBuilder.setFirstIncompleteSeqNo(requestTracker.firstIncomplete()); > In the previous patch I wasn't sending this if the value would have been NO thanks -- To view, visit http://gerrit.cloudera.org:8080/3631 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I108cd30acbc308bfb4577d072c2a8f26d1553c68 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] Add integration tests for replay cache with writes
David Ribeiro Alves has submitted this change and it was merged. Change subject: Add integration tests for replay cache with writes .. Add integration tests for replay cache with writes This adds a couple of new integration tests for replay cache with writes. Both tests start multiple threads writing, independently, to tablet servers simulaneously. The tests leverage the fact that followers are also able to answer requests, once they are cached, and stores all responses, which are compared at the end of the test. Some of the requests (1/3) are "empty" writes, so that we stress the serialization point in transaction_driver.cc without relying on row lock serialization. This adds two different tests, one that stresses a lot of elections and one that crashes nodes. This is inline with other tests we already had in raft_consensus-itest. This also adds a new fault injection point right after the leader sends a request. We currently have one right _before_ the leader sends a request, but having one for after the request is sent encourages stressing the path where a newly elected leader as both incoming client request and ongoing replica transactions, which can possibly race with each other if they correspond to the same write. Finally this changes attempt_no in RequestIdPB to be an int64 instead of just an int. While an int is more than enough in normal operation, the new test generates many more attempts and we need a bigger number to make sure all attempt numbers are unique. I looped this about 1000 times, without related failures. Change-Id: I35722eb1c83f97e886cfe9d6b03ed95bcd62429f Reviewed-on: http://gerrit.cloudera.org:8080/3519 Reviewed-by: Mike Percy Tested-by: Kudu Jenkins --- M src/kudu/consensus/consensus_peers.cc M src/kudu/integration-tests/CMakeLists.txt A src/kudu/integration-tests/exactly_once_writes-itest.cc M src/kudu/integration-tests/raft_consensus-itest.cc M src/kudu/integration-tests/ts_itest-base.h M src/kudu/rpc/rpc_header.proto 6 files changed, 326 insertions(+), 33 deletions(-) Approvals: Mike Percy: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/3519 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I35722eb1c83f97e886cfe9d6b03ed95bcd62429f Gerrit-PatchSet: 35 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] doxygen for C++ client API
Kudu Jenkins has posted comments on this change. Change subject: doxygen for C++ client API .. Patch Set 7: Build Started http://104.196.14.100/job/kudu-gerrit/2503/ -- To view, visit http://gerrit.cloudera.org:8080/3619 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7d42fb1c90b83074e357dcecf42489ed9fc4f02 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] doxygen for C++ client API
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3619 to look at the new patch set (#7). Change subject: doxygen for C++ client API .. doxygen for C++ client API To generate the Doxygen docs from client.h and other client-side files, run 'make doxygen'. After the target is built, open ${CMAKE_CURRENT_BINARY_DIR}/docs/doxygen/client_api/html/index.html in your favorite browser to see the generated documentation. The 'docs' target generates doxygen documentaion as well since it depends on the 'doxygen' target. Change-Id: Ie7d42fb1c90b83074e357dcecf42489ed9fc4f02 --- M CMakeLists.txt A docs/support/doxygen/client_api.doxy.in M src/kudu/client/client.h M thirdparty/build-definitions.sh M thirdparty/build-thirdparty.sh M thirdparty/download-thirdparty.sh M thirdparty/vars.sh 7 files changed, 1,312 insertions(+), 716 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/19/3619/7 -- To view, visit http://gerrit.cloudera.org:8080/3619 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie7d42fb1c90b83074e357dcecf42489ed9fc4f02 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR] Add integration tests for replay cache with writes
Mike Percy has posted comments on this change. Change subject: Add integration tests for replay cache with writes .. Patch Set 34: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3519 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I35722eb1c83f97e886cfe9d6b03ed95bcd62429f Gerrit-PatchSet: 34 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Add integration tests for replay cache with writes
Kudu Jenkins has posted comments on this change. Change subject: Add integration tests for replay cache with writes .. Patch Set 34: Build Started http://104.196.14.100/job/kudu-gerrit/2502/ -- To view, visit http://gerrit.cloudera.org:8080/3519 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I35722eb1c83f97e886cfe9d6b03ed95bcd62429f Gerrit-PatchSet: 34 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Add integration tests for replay cache with writes
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3519 to look at the new patch set (#34). Change subject: Add integration tests for replay cache with writes .. Add integration tests for replay cache with writes This adds a couple of new integration tests for replay cache with writes. Both tests start multiple threads writing, independently, to tablet servers simulaneously. The tests leverage the fact that followers are also able to answer requests, once they are cached, and stores all responses, which are compared at the end of the test. Some of the requests (1/3) are "empty" writes, so that we stress the serialization point in transaction_driver.cc without relying on row lock serialization. This adds two different tests, one that stresses a lot of elections and one that crashes nodes. This is inline with other tests we already had in raft_consensus-itest. This also adds a new fault injection point right after the leader sends a request. We currently have one right _before_ the leader sends a request, but having one for after the request is sent encourages stressing the path where a newly elected leader as both incoming client request and ongoing replica transactions, which can possibly race with each other if they correspond to the same write. Finally this changes attempt_no in RequestIdPB to be an int64 instead of just an int. While an int is more than enough in normal operation, the new test generates many more attempts and we need a bigger number to make sure all attempt numbers are unique. I looped this about 1000 times, without related failures. Change-Id: I35722eb1c83f97e886cfe9d6b03ed95bcd62429f --- M src/kudu/consensus/consensus_peers.cc M src/kudu/integration-tests/CMakeLists.txt A src/kudu/integration-tests/exactly_once_writes-itest.cc M src/kudu/integration-tests/raft_consensus-itest.cc M src/kudu/integration-tests/ts_itest-base.h M src/kudu/rpc/rpc_header.proto 6 files changed, 326 insertions(+), 33 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/19/3519/34 -- To view, visit http://gerrit.cloudera.org:8080/3519 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I35722eb1c83f97e886cfe9d6b03ed95bcd62429f Gerrit-PatchSet: 34 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] client.h: doxygen comments for C++ API
Alexey Serbin has posted comments on this change. Change subject: client.h: doxygen comments for C++ API .. Patch Set 6: (5 comments) Thanks for review! Will post update soon. http://gerrit.cloudera.org:8080/#/c/3619/6/CMakeLists.txt File CMakeLists.txt: Line 1000: # "make doxydocs" target > Sorry to be a buzz kill, but can we just make this target "doxygen" instead Sure, why not. Will do. Line 1015: COMMAND make all install DESTDIR=${DOXY_CLIENT_DESTDIR} > Can this be made to work with Ninja was well? I think you just need to repl That's what I wondering as well, but I could not find it. Thanks -- will replace with CMAKE_MAKE_PROGRAM. http://gerrit.cloudera.org:8080/#/c/3619/6/docs/.gitignore File docs/.gitignore: Line 19 > nit: spurious change Done http://gerrit.cloudera.org:8080/#/c/3619/6/src/kudu/client/client.h File src/kudu/client/client.h: Line 28: #include > This change isn't required for this patch, is it? You are right -- it's not required. Will rollback. http://gerrit.cloudera.org:8080/#/c/3619/6/thirdparty/download-thirdparty.sh File thirdparty/download-thirdparty.sh: Line 262: if ! `which -s doxygen` && [ ! -d $DOXYGEN_DIR ]; then > The -s command-line flag is not available on GNU systems. You can do: This is going away, actually. I'll use the same approach as for the rest of the tools because of uniformity reasons. -- To view, visit http://gerrit.cloudera.org:8080/3619 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7d42fb1c90b83074e357dcecf42489ed9fc4f02 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] [c++-client]: cache non-covering ranges in meta cache
Dan Burkert has posted comments on this change. Change subject: [c++-client]: cache non-covering ranges in meta cache .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/3581/5/src/kudu/client/client-test.cc File src/kudu/client/client-test.cc: Line 1238: ASSERT_FALSE(entry.stale()); > still working on this... Was able to get a clean run of 1000 iterations after removing the insert (which removed an RPC in the middle), and switching to FINE time clock in meta_cache.cc. -- To view, visit http://gerrit.cloudera.org:8080/3581 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I05bcb3fe05d51d7c455e1d68bd2baa6f3c2b9d21 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [c++-client]: cache non-covering ranges in meta cache
Dan Burkert has posted comments on this change. Change subject: [c++-client]: cache non-covering ranges in meta cache .. Patch Set 8: (2 comments) http://gerrit.cloudera.org:8080/#/c/3581/7/src/kudu/client/meta_cache.cc File src/kudu/client/meta_cache.cc: Line 816: // An RPC response containing B, D and E could occur if the lookup partition > Nit: Could you reword this so that the A in "A response..." doesn't seem li Done http://gerrit.cloudera.org:8080/#/c/3581/7/src/kudu/gutil/map-util.h File src/kudu/gutil/map-util.h: Line 207: // Returns a const-reference to the value associated with the greatest key > Not quite; 'above' is not a OrDie method. Done -- To view, visit http://gerrit.cloudera.org:8080/3581 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I05bcb3fe05d51d7c455e1d68bd2baa6f3c2b9d21 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [c++-client]: cache non-covering ranges in meta cache
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3581 to look at the new patch set (#8). Change subject: [c++-client]: cache non-covering ranges in meta cache .. [c++-client]: cache non-covering ranges in meta cache This commit introduces a few features to the meta cache, all with the aim of making it compatible with the upcoming add/drop range partitions feature. 1) Non-covered range partitions are now cached in the meta cache. This is achieved by storing MetaCacheEntry objects in the meta cache's partition-key index instead of RemoteTablets. The MetaCacheEntry holds either a RemoteTablet, in which case it represents a covered partition range, or it represents a non-covered partition range. 2) Entries are now removed from the meta cache's partition-key index when it can be determined that the entries are no longer valid from the results of a GetTableLocations RPC. 3) A basic TTL has been added to the GetTableLocationsResponsePB so that the client can properly refresh the meta cache when necessary. The TTL is configurable by the master, and defaults to one hour. Change-Id: I05bcb3fe05d51d7c455e1d68bd2baa6f3c2b9d21 --- M src/kudu/client/client-test.cc M src/kudu/client/client.h M src/kudu/client/meta_cache.cc M src/kudu/client/meta_cache.h M src/kudu/client/schema.h M src/kudu/client/table-internal.cc M src/kudu/gutil/map-util.h M src/kudu/master/catalog_manager.cc M src/kudu/master/master.proto M src/kudu/tools/ksck_remote.cc 10 files changed, 438 insertions(+), 139 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/81/3581/8 -- To view, visit http://gerrit.cloudera.org:8080/3581 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I05bcb3fe05d51d7c455e1d68bd2baa6f3c2b9d21 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [c++-client]: cache non-covering ranges in meta cache
Kudu Jenkins has posted comments on this change. Change subject: [c++-client]: cache non-covering ranges in meta cache .. Patch Set 8: Build Started http://104.196.14.100/job/kudu-gerrit/2501/ -- To view, visit http://gerrit.cloudera.org:8080/3581 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I05bcb3fe05d51d7c455e1d68bd2baa6f3c2b9d21 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1530: Update docs about OS X build dependency on Xcode package
Mike Percy has submitted this change and it was merged. Change subject: KUDU-1530: Update docs about OS X build dependency on Xcode package .. KUDU-1530: Update docs about OS X build dependency on Xcode package Change-Id: I8962d5539f437dba8b120c70c90c1e384ed550c9 Reviewed-on: http://gerrit.cloudera.org:8080/3653 Reviewed-by: Adar Dembo Tested-by: Mike Percy Reviewed-by: Mike Percy --- M docs/installation.adoc 1 file changed, 3 insertions(+), 4 deletions(-) Approvals: Mike Percy: Looks good to me, approved; Verified Adar Dembo: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/3653 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I8962d5539f437dba8b120c70c90c1e384ed550c9 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Mike Percy
[kudu-CR] KUDU-1530: Update docs about OS X build dependency on Xcode package
Mike Percy has posted comments on this change. Change subject: KUDU-1530: Update docs about OS X build dependency on Xcode package .. Patch Set 3: Code-Review+2 Overriding flakiness due to isolate race condition -- To view, visit http://gerrit.cloudera.org:8080/3653 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8962d5539f437dba8b120c70c90c1e384ed550c9 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] KUDU-1530: Update docs about OS X build dependency on Xcode package
Mike Percy has posted comments on this change. Change subject: KUDU-1530: Update docs about OS X build dependency on Xcode package .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/3653 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8962d5539f437dba8b120c70c90c1e384ed550c9 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] client.h: doxygen comments for C++ API
Mike Percy has posted comments on this change. Change subject: client.h: doxygen comments for C++ API .. Patch Set 6: (5 comments) http://gerrit.cloudera.org:8080/#/c/3619/6/CMakeLists.txt File CMakeLists.txt: Line 1000: # "make doxydocs" target Sorry to be a buzz kill, but can we just make this target "doxygen" instead of "doxydocs"? Line 1015: COMMAND make all install DESTDIR=${DOXY_CLIENT_DESTDIR} Can this be made to work with Ninja was well? I think you just need to replace make with ${CMAKE_MAKE_PROGRAM} http://gerrit.cloudera.org:8080/#/c/3619/6/docs/.gitignore File docs/.gitignore: Line 19 nit: spurious change http://gerrit.cloudera.org:8080/#/c/3619/6/src/kudu/client/client.h File src/kudu/client/client.h: Line 28: #include This change isn't required for this patch, is it? http://gerrit.cloudera.org:8080/#/c/3619/6/thirdparty/download-thirdparty.sh File thirdparty/download-thirdparty.sh: Line 262: if ! `which -s doxygen` && [ ! -d $DOXYGEN_DIR ]; then The -s command-line flag is not available on GNU systems. You can do: if ! which doxygen >/dev/null && [ ! -d "$DOXYGEN_DIR" ]; then -- To view, visit http://gerrit.cloudera.org:8080/3619 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7d42fb1c90b83074e357dcecf42489ed9fc4f02 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] Memory tracking for result tracker
Adar Dembo has posted comments on this change. Change subject: Memory tracking for result tracker .. Patch Set 9: (5 comments) http://gerrit.cloudera.org:8080/#/c/3627/9/src/kudu/rpc/result_tracker.cc File src/kudu/rpc/result_tracker.cc: Line 75: mem_tracker_->Consume(sizeof(ClientState)); If we want to make ClientState memory tracking accurate, we need to account for two additional things: 1. Malloc "slop". sizeof(ClientState) may not account for the actual size of the memory allocation used in "new ClientState()". For example, if the size of the struct is 40 bytes, the allocation may be the next power of 2 (i.e. 64 bytes). We deal with this elsewhere by using kudu_malloc_usable_size(ptr) as the "size of object"; sometimes this is internalized inside a memory_footprint() method. 2. The size of any nested heap pointers. ClientState contains a map, and that map contains nested pointers whose memory consumption isn't being accounted for. Look at how Schema::name_to_index_ is managed to see how you might do that. Line 85: mem_tracker_->Consume(sizeof(CompletionRecord)); Same issues with CompletionRecord and its nested vector of OngoingRpcs (you can use kudu_malloc_usable_size(vector.data()), though you need to check that vector.capacity() >0 first). I think you're already taking care of the nested protobuf elsewhere. Line 234: mem_tracker_->Consume(response->ByteSize()); Should use SpaceUsed(), I think. Line 255: mem_tracker_->Release(sizeof(OnGoingRpcInfo)); We've found that Release() in a loop is generally less efficient than adding up all the memory consumed in the loop and making one call to Release() at the end. That change reduced block manager CPU consumption pretty dramatically on startup. http://gerrit.cloudera.org:8080/#/c/3627/9/src/kudu/server/server_base.cc File src/kudu/server/server_base.cc: PS9, Line 89: if (id != 0) { : StrAppend(&id_str, " ", id); : } Don't need this; each of these memtrackers is already "disambiguated" by virtue of having a unique parent. That said, why bother creating a memtracker for the ResultTracker instead of just reusing the server memtracker? IIRC we only create separate memtrackers for objects that come and go (e.g. tablet, memrowset, deltamemstore). -- To view, visit http://gerrit.cloudera.org:8080/3627 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3b81dda41c8bc7f70380ce426142c34afe6f1625 Gerrit-PatchSet: 9 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] client.h: doxygen comments for C++ API
Kudu Jenkins has posted comments on this change. Change subject: client.h: doxygen comments for C++ API .. Patch Set 6: Build Started http://104.196.14.100/job/kudu-gerrit/2500/ -- To view, visit http://gerrit.cloudera.org:8080/3619 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7d42fb1c90b83074e357dcecf42489ed9fc4f02 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] client.h: doxygen comments for C++ API
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3619 to look at the new patch set (#6). Change subject: client.h: doxygen comments for C++ API .. client.h: doxygen comments for C++ API To generate the Doxygen docs from client.h, run 'make doxydocs'. Open $REPO_ROOT/docs/doxygen/html/index.html in your favorite browser to see the generated documentation. The 'docs' target generates doxygen documentaion as well since it depends on the 'doxydocs' target. Change-Id: Ie7d42fb1c90b83074e357dcecf42489ed9fc4f02 --- M CMakeLists.txt M docs/.gitignore A docs/support/doxygen/client_api.doxy.in M src/kudu/client/client.h M thirdparty/build-definitions.sh M thirdparty/build-thirdparty.sh M thirdparty/download-thirdparty.sh M thirdparty/vars.sh 8 files changed, 1,314 insertions(+), 718 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/19/3619/6 -- To view, visit http://gerrit.cloudera.org:8080/3619 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie7d42fb1c90b83074e357dcecf42489ed9fc4f02 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR] [c++-client]: cache non-covering ranges in meta cache
Adar Dembo has posted comments on this change. Change subject: [c++-client]: cache non-covering ranges in meta cache .. Patch Set 7: (2 comments) http://gerrit.cloudera.org:8080/#/c/3581/7/src/kudu/client/meta_cache.cc File src/kudu/client/meta_cache.cc: Line 816: // A response containing B, D and E could occur if the lookup partition key Nit: Could you reword this so that the A in "A response..." doesn't seem like a reference to the non-covered range A? http://gerrit.cloudera.org:8080/#/c/3581/7/src/kudu/gutil/map-util.h File src/kudu/gutil/map-util.h: Line 207: // Same as above, but returns a const reference. Not quite; 'above' is not a OrDie method. -- To view, visit http://gerrit.cloudera.org:8080/3581 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I05bcb3fe05d51d7c455e1d68bd2baa6f3c2b9d21 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Add integration tests for replay cache with writes
Mike Percy has posted comments on this change. Change subject: Add integration tests for replay cache with writes .. Patch Set 33: (2 comments) http://gerrit.cloudera.org:8080/#/c/3519/33/src/kudu/integration-tests/exactly_once_writes-itest.cc File src/kudu/integration-tests/exactly_once_writes-itest.cc: Line 52: protected: style nit: leave a blank line above Line 53: int seed_; style nit: indentation. Also you could make this const -- To view, visit http://gerrit.cloudera.org:8080/3519 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I35722eb1c83f97e886cfe9d6b03ed95bcd62429f Gerrit-PatchSet: 33 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Add integration tests for replay cache with writes
Kudu Jenkins has posted comments on this change. Change subject: Add integration tests for replay cache with writes .. Patch Set 33: Build Started http://104.196.14.100/job/kudu-gerrit/2499/ -- To view, visit http://gerrit.cloudera.org:8080/3519 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I35722eb1c83f97e886cfe9d6b03ed95bcd62429f Gerrit-PatchSet: 33 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] WIP: Add garbage collection to ResultTracker
Kudu Jenkins has posted comments on this change. Change subject: WIP: Add garbage collection to ResultTracker .. Patch Set 6: Build Started http://104.196.14.100/job/kudu-gerrit/2497/ -- To view, visit http://gerrit.cloudera.org:8080/3628 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2c8e7b7191ca14842a31b64813ed498bdf626fa8 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] Memory tracking for result tracker
Kudu Jenkins has posted comments on this change. Change subject: Memory tracking for result tracker .. Patch Set 9: Build Started http://104.196.14.100/job/kudu-gerrit/2498/ -- To view, visit http://gerrit.cloudera.org:8080/3627 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3b81dda41c8bc7f70380ce426142c34afe6f1625 Gerrit-PatchSet: 9 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Add integration tests for replay cache with writes
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3519 to look at the new patch set (#33). Change subject: Add integration tests for replay cache with writes .. Add integration tests for replay cache with writes This adds a couple of new integration tests for replay cache with writes. Both tests start multiple threads writing, independently, to tablet servers simulaneously. The tests leverage the fact that followers are also able to answer requests, once they are cached, and stores all responses, which are compared at the end of the test. Some of the requests (1/3) are "empty" writes, so that we stress the serialization point in transaction_driver.cc without relying on row lock serialization. This adds two different tests, one that stresses a lot of elections and one that crashes nodes. This is inline with other tests we already had in raft_consensus-itest. This also adds a new fault injection point right after the leader sends a request. We currently have one right _before_ the leader sends a request, but having one for after the request is sent encourages stressing the path where a newly elected leader as both incoming client request and ongoing replica transactions, which can possibly race with each other if they correspond to the same write. Finally this changes attempt_no in RequestIdPB to be an int64 instead of just an int. While an int is more than enough in normal operation, the new test generates many more attempts and we need a bigger number to make sure all attempt numbers are unique. I looped this about 1000 times, without related failures. Change-Id: I35722eb1c83f97e886cfe9d6b03ed95bcd62429f --- M src/kudu/consensus/consensus_peers.cc M src/kudu/integration-tests/CMakeLists.txt A src/kudu/integration-tests/exactly_once_writes-itest.cc M src/kudu/integration-tests/raft_consensus-itest.cc M src/kudu/integration-tests/ts_itest-base.h M src/kudu/rpc/rpc_header.proto 6 files changed, 325 insertions(+), 33 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/19/3519/33 -- To view, visit http://gerrit.cloudera.org:8080/3519 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I35722eb1c83f97e886cfe9d6b03ed95bcd62429f Gerrit-PatchSet: 33 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] WIP: Add garbage collection to ResultTracker
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3628 to look at the new patch set (#6). Change subject: WIP: Add garbage collection to ResultTracker .. WIP: Add garbage collection to ResultTracker This still needs testing and hooking up to the mm. Change-Id: I2c8e7b7191ca14842a31b64813ed498bdf626fa8 --- M src/kudu/rpc/result_tracker.cc M src/kudu/rpc/result_tracker.h M src/kudu/rpc/rpc-stress-test.cc M src/kudu/rpc/rpc_header.proto M src/kudu/rpc/service_if.cc M src/kudu/tablet/tablet_bootstrap.cc M src/kudu/tablet/transactions/transaction_driver.cc 7 files changed, 163 insertions(+), 13 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/3628/6 -- To view, visit http://gerrit.cloudera.org:8080/3628 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2c8e7b7191ca14842a31b64813ed498bdf626fa8 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Integrate the result tracker with writes
David Ribeiro Alves has submitted this change and it was merged. Change subject: Integrate the result tracker with writes .. Integrate the result tracker with writes This patch integrates the result tracker with write transactions, including: - Support for caching responses on tablet bootstrap. - Support for caching responses for follower transactions. - Handling of races between client originated and (previous?) leader originated transactions. - An explanation of the interaction between the result tracker and the transaction driver in transaction_driver.h. For integration tests, this patch removes the check that allowed Status::AlreadyPresent() that we added as we didn't have support for exactly once semantics. Without this check, in slow mode, some tests like raft_consensus-itest would fail 100% of the time, due to rows being inserted multiple times. Because we'd have 100% failure rate without replay cache for writes and because testing it specifically is involved, this patch doesn't include additional tests, these will be pushed as a follow up. Change-Id: I1fa2f8db33653960f4749237b8993baba0929893 Reviewed-on: http://gerrit.cloudera.org:8080/3449 Reviewed-by: David Ribeiro Alves Reviewed-by: Jean-Daniel Cryans Tested-by: Kudu Jenkins --- M src/kudu/client/batcher.cc M src/kudu/consensus/consensus.proto M src/kudu/integration-tests/test_workload.cc M src/kudu/integration-tests/test_workload.h M src/kudu/master/sys_catalog.cc M src/kudu/rpc/rpc_context.cc M src/kudu/rpc/rpc_context.h M src/kudu/tablet/tablet_bootstrap-test.cc M src/kudu/tablet/tablet_bootstrap.cc M src/kudu/tablet/tablet_bootstrap.h M src/kudu/tablet/tablet_peer-test.cc M src/kudu/tablet/tablet_peer.cc M src/kudu/tablet/tablet_peer.h M src/kudu/tablet/transactions/transaction.h M src/kudu/tablet/transactions/transaction_driver.cc M src/kudu/tablet/transactions/transaction_driver.h M src/kudu/tablet/transactions/write_transaction.cc M src/kudu/tablet/transactions/write_transaction.h M src/kudu/tools/insert-generated-rows.cc M src/kudu/tserver/CMakeLists.txt M src/kudu/tserver/remote_bootstrap_session-test.cc M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/ts_tablet_manager.cc M src/kudu/tserver/ts_tablet_manager.h M src/kudu/tserver/tserver_service.proto 25 files changed, 399 insertions(+), 41 deletions(-) Approvals: David Ribeiro Alves: Looks good to me, approved Jean-Daniel Cryans: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/3449 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I1fa2f8db33653960f4749237b8993baba0929893 Gerrit-PatchSet: 29 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Add a way to include request ids in log-dump
David Ribeiro Alves has submitted this change and it was merged. Change subject: Add a way to include request ids in log-dump .. Add a way to include request ids in log-dump This adds a way to output the request ids, if they exist, in log-dump.cc. This is helpful when debugging as it allows to see which writes the request ids get associated with. Change-Id: I88d7c65887a98544ee83b5b4bc0817bea7222131 Reviewed-on: http://gerrit.cloudera.org:8080/3612 Reviewed-by: David Ribeiro Alves Tested-by: David Ribeiro Alves --- M src/kudu/consensus/log-dump.cc 1 file changed, 10 insertions(+), 2 deletions(-) Approvals: David Ribeiro Alves: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/3612 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I88d7c65887a98544ee83b5b4bc0817bea7222131 Gerrit-PatchSet: 9 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Mike Percy
[kudu-CR] Add a way to include request ids in log-dump
David Ribeiro Alves has posted comments on this change. Change subject: Add a way to include request ids in log-dump .. Patch Set 8: Verified+1 unrelated java flakiness. -- To view, visit http://gerrit.cloudera.org:8080/3612 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I88d7c65887a98544ee83b5b4bc0817bea7222131 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] Add integration tests for replay cache with writes
Mike Percy has posted comments on this change. Change subject: Add integration tests for replay cache with writes .. Patch Set 32: (2 comments) http://gerrit.cloudera.org:8080/#/c/3519/32/src/kudu/integration-tests/ts_itest-base.h File src/kudu/integration-tests/ts_itest-base.h: Line 101: // TODO remove this once we have ResultTracker GC > There's no JIRA, the GC patch is up for review. Will remove these comments ok Line 493: void AssertNoTabletServersCrashed() { > This is just a move from the child class. the raft itest uses this. Happy t ok -- To view, visit http://gerrit.cloudera.org:8080/3519 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I35722eb1c83f97e886cfe9d6b03ed95bcd62429f Gerrit-PatchSet: 32 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Add integration tests for replay cache with writes
David Ribeiro Alves has posted comments on this change. Change subject: Add integration tests for replay cache with writes .. Patch Set 32: (4 comments) http://gerrit.cloudera.org:8080/#/c/3519/30/src/kudu/integration-tests/exactly_once_writes-itest.cc File src/kudu/integration-tests/exactly_once_writes-itest.cc: Line 62: Random random(1234); > How about initialize an int seed_ = SeedRandom() in the SetUp() function an Done http://gerrit.cloudera.org:8080/#/c/3519/32/src/kudu/integration-tests/exactly_once_writes-itest.cc File src/kudu/integration-tests/exactly_once_writes-itest.cc: Line 37: void DoTestWritesWithExactlyOnceSemantics(const vector& ts_flags, > nit: Put method declarations in same order as definitions (just switch them Done http://gerrit.cloudera.org:8080/#/c/3519/32/src/kudu/integration-tests/ts_itest-base.h File src/kudu/integration-tests/ts_itest-base.h: Line 101: // TODO remove this once we have ResultTracker GC > Mind adding a JIRA# to this TODO, if there is one? There's no JIRA, the GC patch is up for review. Will remove these comments as I enable this by default when it lands. Line 493: void AssertNoTabletServersCrashed() { > This is already implemented as ExternalMiniCluster::AssertNoCrashes() This is just a move from the child class. the raft itest uses this. Happy to remove this and swap to the other method in a follow up. -- To view, visit http://gerrit.cloudera.org:8080/3519 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I35722eb1c83f97e886cfe9d6b03ed95bcd62429f Gerrit-PatchSet: 32 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Add integration tests for replay cache with writes
Mike Percy has posted comments on this change. Change subject: Add integration tests for replay cache with writes .. Patch Set 30: (4 comments) http://gerrit.cloudera.org:8080/#/c/3519/30/src/kudu/integration-tests/exactly_once_writes-itest.cc File src/kudu/integration-tests/exactly_once_writes-itest.cc: Line 62: Random random(1234); How about initialize an int seed_ = SeedRandom() in the SetUp() function and then initialize from seed_ here? That way we get more randomness in the test but we can still reproduce the sequence if needed by passing --test_random_seed to the test at runtime. http://gerrit.cloudera.org:8080/#/c/3519/32/src/kudu/integration-tests/exactly_once_writes-itest.cc File src/kudu/integration-tests/exactly_once_writes-itest.cc: Line 37: void DoTestWritesWithExactlyOnceSemantics(const vector& ts_flags, nit: Put method declarations in same order as definitions (just switch them) http://gerrit.cloudera.org:8080/#/c/3519/32/src/kudu/integration-tests/ts_itest-base.h File src/kudu/integration-tests/ts_itest-base.h: Line 101: // TODO remove this once we have ResultTracker GC Mind adding a JIRA# to this TODO, if there is one? Line 493: void AssertNoTabletServersCrashed() { This is already implemented as ExternalMiniCluster::AssertNoCrashes() -- To view, visit http://gerrit.cloudera.org:8080/3519 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I35722eb1c83f97e886cfe9d6b03ed95bcd62429f Gerrit-PatchSet: 30 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Move the maintenance manager to util
David Ribeiro Alves has submitted this change and it was merged. Change subject: Move the maintenance manager to util .. Move the maintenance manager to util This moves the maintenance manager to util, along with its required protobufs. This move is required as we'll need a server-level MaintenanceMananager to run garbage collection on the ResultTracker. Change-Id: Ifcf072d443ac3d069bda15b9dc0f8c442b9ac5c0 Reviewed-on: http://gerrit.cloudera.org:8080/3656 Reviewed-by: Jean-Daniel Cryans Tested-by: Kudu Jenkins --- M src/kudu/integration-tests/full_stack-insert-scan-test.cc M src/kudu/master/master.cc M src/kudu/tablet/CMakeLists.txt M src/kudu/tablet/tablet.cc M src/kudu/tablet/tablet.proto M src/kudu/tablet/tablet_mm_ops.h M src/kudu/tablet/tablet_peer-test.cc M src/kudu/tablet/tablet_peer_mm_ops.cc M src/kudu/tablet/tablet_peer_mm_ops.h M src/kudu/tserver/mini_tablet_server.cc M src/kudu/tserver/tablet_server-test-base.h M src/kudu/tserver/tablet_server.cc M src/kudu/tserver/tserver-path-handlers.cc M src/kudu/util/CMakeLists.txt R src/kudu/util/maintenance_manager-test.cc R src/kudu/util/maintenance_manager.cc R src/kudu/util/maintenance_manager.h A src/kudu/util/maintenance_manager.proto 18 files changed, 85 insertions(+), 59 deletions(-) Approvals: Jean-Daniel Cryans: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/3656 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ifcf072d443ac3d069bda15b9dc0f8c442b9ac5c0 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Move the maintenance manager to util
David Ribeiro Alves has posted comments on this change. Change subject: Move the maintenance manager to util .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/3656/3/src/kudu/util/maintenance_manager.h File src/kudu/util/maintenance_manager.h: Line 243: void GetMaintenanceManagerStatusDump(MaintenanceManagerStatusPB* out_pb); > Since this is a pointer, you don't even need to include maintenance_manager But then I'll have to fwd declare it in a bunch of places. Since this is only a code move mind if I drop this? -- To view, visit http://gerrit.cloudera.org:8080/3656 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifcf072d443ac3d069bda15b9dc0f8c442b9ac5c0 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Memory tracking for result tracker
David Ribeiro Alves has posted comments on this change. Change subject: Memory tracking for result tracker .. Patch Set 8: Verified+1 unrelated failure (mm test) -- To view, visit http://gerrit.cloudera.org:8080/3627 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3b81dda41c8bc7f70380ce426142c34afe6f1625 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] rw mutex: prevent recursive use
Hello Dan Burkert, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3641 to look at the new patch set (#6). Change subject: rw_mutex: prevent recursive use .. rw_mutex: prevent recursive use Todd provided an example[1] of deadlocked rwlocks due to a fairness policy. In the example, T1 (holding the lock for reading) join()ed on T2 (trying to acquire the lock for reading) all while T3 was trying to acquire the lock for writing. The lock's fairness policy prevented T2 from acquiring the read lock thus deadlocking all three threads. The takeaway is to be careful when calling join() while holding locks. Beyond that, deadlocks can arise if rwlocks are taken recursively. That's not a feature we need in our rwlocks, so I tried to disable it at the pthread level. Unfortunately, the best I can do is disable recursive write lock acquisition; read locks are apparently always recursive (see "man pthread_rwlockattr_setkind_np"). So instead, I built recursive checking into the RWMutex itself. It's quite slow so it's only present in debug builds. Note that pthread rwlocks do have some built-in deadlock detection (i.e. lock calls may return EDEADLK), but it doesn't appear to be comprehensive. 1. https://issues.apache.org/jira/browse/HDFS-2223?focusedCommentId=13097647&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13097647 Change-Id: I7ae30ec123a16c39ef0c15ee2d2176f807df03db --- M src/kudu/util/CMakeLists.txt A src/kudu/util/rw_mutex-test.cc M src/kudu/util/rw_mutex.cc M src/kudu/util/rw_mutex.h 4 files changed, 314 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/41/3641/6 -- To view, visit http://gerrit.cloudera.org:8080/3641 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7ae30ec123a16c39ef0c15ee2d2176f807df03db Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Todd Lipcon
[kudu-CR] rw mutex: prevent recursive use
Kudu Jenkins has posted comments on this change. Change subject: rw_mutex: prevent recursive use .. Patch Set 6: Build Started http://104.196.14.100/job/kudu-gerrit/2496/ -- To view, visit http://gerrit.cloudera.org:8080/3641 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7ae30ec123a16c39ef0c15ee2d2176f807df03db Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1530: Update docs about OS X build dependency on Xcode package
Adar Dembo has posted comments on this change. Change subject: KUDU-1530: Update docs about OS X build dependency on Xcode package .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3653 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8962d5539f437dba8b120c70c90c1e384ed550c9 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] KUDU-1530: Update docs about OS X build dependency on Xcode package
Kudu Jenkins has posted comments on this change. Change subject: KUDU-1530: Update docs about OS X build dependency on Xcode package .. Patch Set 3: Build Started http://104.196.14.100/job/kudu-gerrit/2495/ -- To view, visit http://gerrit.cloudera.org:8080/3653 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8962d5539f437dba8b120c70c90c1e384ed550c9 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] KUDU-1530: Update docs about OS X build dependency on Xcode package
Hello Adar Dembo, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3653 to look at the new patch set (#3). Change subject: KUDU-1530: Update docs about OS X build dependency on Xcode package .. KUDU-1530: Update docs about OS X build dependency on Xcode package Change-Id: I8962d5539f437dba8b120c70c90c1e384ed550c9 --- M docs/installation.adoc 1 file changed, 3 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/3653/3 -- To view, visit http://gerrit.cloudera.org:8080/3653 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8962d5539f437dba8b120c70c90c1e384ed550c9 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] WIP: Add garbage collection to ResultTracker
David Ribeiro Alves has posted comments on this change. Change subject: WIP: Add garbage collection to ResultTracker .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/3628/5/src/kudu/rpc/result_tracker.cc File src/kudu/rpc/result_tracker.cc: PS5, Line 91: // If the arriving request is older than our per-client GC watermark, report its : // staleness to the client. > Yeah that's what I meant. So is this coming back as a row error or an RPC e coming back as an RPC error (we don't cache results for individual rows, only for whole batches) -- To view, visit http://gerrit.cloudera.org:8080/3628 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2c8e7b7191ca14842a31b64813ed498bdf626fa8 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] Move the maintenance manager to util
Adar Dembo has posted comments on this change. Change subject: Move the maintenance manager to util .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/3656/3/src/kudu/util/maintenance_manager.h File src/kudu/util/maintenance_manager.h: Line 243: void GetMaintenanceManagerStatusDump(MaintenanceManagerStatusPB* out_pb); Since this is a pointer, you don't even need to include maintenance_manager.pb.h; you could forward declare this instead. -- To view, visit http://gerrit.cloudera.org:8080/3656 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifcf072d443ac3d069bda15b9dc0f8c442b9ac5c0 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [c++-client]: cache non-covering ranges in meta cache
Dan Burkert has posted comments on this change. Change subject: [c++-client]: cache non-covering ranges in meta cache .. Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/3581/5/src/kudu/client/client-test.cc File src/kudu/client/client-test.cc: Line 1238: // Force a lookup and ensure the entry is refreshed. > The TTL is 25ms, so it's conceivable that on a bogged down test environment still working on this... -- To view, visit http://gerrit.cloudera.org:8080/3581 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I05bcb3fe05d51d7c455e1d68bd2baa6f3c2b9d21 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [c++-client]: cache non-covering ranges in meta cache
Dan Burkert has posted comments on this change. Change subject: [c++-client]: cache non-covering ranges in meta cache .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/3581/5/src/kudu/client/meta_cache.cc File src/kudu/client/meta_cache.cc: Line 885: *cache_entry = *DCHECK_NOTNULL(FindFloorOrNull(tablets_by_key, rpc.partition_key())); > There is no FindFloorOrDie. I've updated to be a normal CHECK. I original Woops, I did actually implement FindFloorOrDie and switch this over to it. -- To view, visit http://gerrit.cloudera.org:8080/3581 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I05bcb3fe05d51d7c455e1d68bd2baa6f3c2b9d21 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [c++-client]: cache non-covering ranges in meta cache
Dan Burkert has posted comments on this change. Change subject: [c++-client]: cache non-covering ranges in meta cache .. Patch Set 7: (13 comments) http://gerrit.cloudera.org:8080/#/c/3581/5/src/kudu/client/client-test.cc File src/kudu/client/client-test.cc: PS5, Line 1223: // Clear the cache. : meta_cache->ClearCacheForTesting(); : internal::MetaCacheEntry entry; : ASSERT_FALSE(meta_cache->LookupTabletByKeyFastPath(client_table_.get(), "", &entry)); : > You can instantiate a FlagSaver instead. Done http://gerrit.cloudera.org:8080/#/c/3581/5/src/kudu/client/client.h File src/kudu/client/client.h: Line 300: FRIEND_TEST(ClientTest, TestScanTimeout); > Nit: sort alphabetically. Done http://gerrit.cloudera.org:8080/#/c/3581/5/src/kudu/client/meta_cache.cc File src/kudu/client/meta_cache.cc: PS5, Line 753: if (new_status.IsServiceUnavailable()) { : // One or more of the tablets is not running; retry after a backoff period. : mutable_retrier()->DelayedRetry(this, new_status); : ignore_result(delete_me.release()); : return; : } > To be clear, this has nothing to do with non-covering range partition suppo Yes, that's correct. This manifested in some of the more aggressive add/drop partitions tests that come in the next patch, but I thought it was more appropriate to fix in this patch. Line 790: const auto& tablet_locations = rpc.resp().tablet_locations(); > rpc.resp().tablet_locations() is called four times, maybe pull it out into Done PS5, Line 817: lthough the existence of A as an initial > Can you pull this (and end) into local variables? Done PS5, Line 848: entry(expiration_time, last_upper_bound, tablet_lower_bound); > This assumes that a tablet can't change in partition size (i.e. that should We discussed this off-line and decided that the CHECKs are fine for now. A malformed master response shouldn't result in memory corruption, and having a CHECK is good from a debugging standpoint. PS5, Line 865: scoped_refptr remote = FindPtrOrNull(tablets_by_id_, tablet_id); : if (remote.get() != nullptr) { : // Partition should not have changed. : DCHECK_EQ(tablet_lower_bound, remote->partition().partition_key_start()); : DCHECK_EQ(tablet_upper_bound, remote->partition().partition_key_end()); > Maybe encapsulate this (and the non-covered range variant) in a helper meth I'm a little hesitant to do that because the cases are so short (just a few lines), every one is subtly different, and there is locking going on. PS5, Line 874: // Update the entry TTL. : auto& entry = FindOrDie(tablets_by_k > What if the total number of tablets is a multiple of MAX_RETURNED_TABLE_LOC correct, that's exactly how it should work (we can't infer the presence of a non-covered range in that case). It will be discovered if a lookup is requested for that range. Line 885: tablets_by_key.lower_bound(tablet_upper_bound)); > Why FindFloorOrNull if you DCHECK_NOTNULL() right after? Seems unsafe. There is no FindFloorOrDie. I've updated to be a normal CHECK. I originally had it as a DCHECK since it should only fire if there are bugs in the code, but I doubt in makes much of a performance difference. http://gerrit.cloudera.org:8080/#/c/3581/5/src/kudu/client/table-internal.cc File src/kudu/client/table-internal.cc: PS5, Line 133: if (s.ok()) { : break; : } else { : LOG(WARNING) << "Error getting table locations: " << s.ToString() << ", retrying."; : } > Likewise, this isn't logically part of this change, right? Right, same scenario. http://gerrit.cloudera.org:8080/#/c/3581/5/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: Line 150: DEFINE_int32(table_locations_ttl_ms, 60 * 60 * 1000, // 1 hour > How about expressing this as: Done http://gerrit.cloudera.org:8080/#/c/3581/5/src/kudu/tools/ksck_remote.cc File src/kudu/tools/ksck_remote.cc: Line 283: SleepFor(MonoDelta::FromMilliseconds(100 * retries)); > That seems like a rather high sleep value. Does it cause the ksck tests to Done Line 320: new KsckTabletReplica(replica.ts_info().permanent_uuid(), is_leader, is_follower))); > Nit: wasn't the old indentation correct? Done -- To view, visit http://gerrit.cloudera.org:8080/3581 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I05bcb3fe05d51d7c455e1d68bd2baa6f3c2b9d21 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerri
[kudu-CR] KUDU-1311 [master] support adding and dropping range partitions
Kudu Jenkins has posted comments on this change. Change subject: KUDU-1311 [master] support adding and dropping range partitions .. Patch Set 5: Build Started http://104.196.14.100/job/kudu-gerrit/2493/ -- To view, visit http://gerrit.cloudera.org:8080/3648 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I42437f365397baf9d4b39b5b17a1587fae70c4be Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] KUDU-1311 [master] support adding and dropping range partitions
Adar Dembo has posted comments on this change. Change subject: KUDU-1311 [master] support adding and dropping range partitions .. Patch Set 4: (37 comments) http://gerrit.cloudera.org:8080/#/c/3648/4/src/kudu/client/client.cc File src/kudu/client/client.cc: PS4, Line 892: RETURN_NOT_OK(data_->client_->data_->WaitForAlterTableToFinish( : data_->client_, alter_name, deadline)); If this fails, don't we want to clear the meta cache anyway? Maybe ClearCache() should be set up as a ScopedCleanup thing right after L888-889. Line 908: // It is sufficient to clear that cache even if the table alteration is not Nit: "the cache". Also, not sure about "sufficient"; it implies that there was something more we could do but have chosen not to. Maybe you meant "necessary"? http://gerrit.cloudera.org:8080/#/c/3648/4/src/kudu/client/client.h File src/kudu/client/client.h: PS4, Line 537: unless the existing range partitions are dropped first What happens if a single KuduTableAlterer has DropRange and AddRange on the same range? Is that legal, provided they're in that order? Line 541: // this method returns, however other existing clients may have to wait for a Don't you mean "when Alter() returns success"? http://gerrit.cloudera.org:8080/#/c/3648/4/src/kudu/client/meta_cache.cc File src/kudu/client/meta_cache.cc: Line 19: What were the include and using changes for? http://gerrit.cloudera.org:8080/#/c/3648/4/src/kudu/client/table_alterer-internal.h File src/kudu/client/table_alterer-internal.h: Line 57: struct PartitioningStep { I think the code would be net less complex if PartitionStep and Step were combined. The overhead of two unique_ptrs per Step for non-partitioning steps is minimal, and it'll simplify ToRequest() somewhat. http://gerrit.cloudera.org:8080/#/c/3648/4/src/kudu/integration-tests/alter_table-randomized-test.cc File src/kudu/integration-tests/alter_table-randomized-test.cc: Line 363: LOG(INFO) << "Adding column " << name; Were these LOG statements just for debugging or do you want to keep them? Line 459: scanner.SetTimeoutMillis(6); What's the significance of this value? Line 510: vector master_addrs_; Unused? PS4, Line 547: } else if (r < 850 && t.num_range_partitions() < kMaxRangePartitions) { : t.AddRangePartition(); : } else if (r < 900) { : t.DropRangePartition(); It would be cool if this also tested "compound" operations, i.e. an AlterTable() that adds a partition, drops another, and adds a column too. http://gerrit.cloudera.org:8080/#/c/3648/4/src/kudu/integration-tests/alter_table-test.cc File src/kudu/integration-tests/alter_table-test.cc: Line 27: #include "kudu/client/client.h" Nit: the previous sort order was more correct. Line 36: #include "kudu/master/master-test-util.h" Nit: likewise, this was more correct before. Line 463: session->SetTimeoutMillis(15 * 1000); Why this value? Line 468: gscoped_ptr insert(table->NewInsert()); Use unique_ptr here? PS4, Line 470: if (table->schema().num_columns() > 1) { : RETURN_NOT_OK(insert->mutable_row()->SetInt32(1, i)); : } The assumption being that every column is going to be an Int32? Line 550: int AlterTableTest::CountRows(const string& table_name) { Can you reuse CountTableRows() from client-test-util.h? There may be some other useful goodies there. Line 1064: TEST_F(AlterTableTest, TestAlterRangePartitioning) { How about a test case for one AlterTable() adding and dropping the same partition? Also, what about negative test cases? Like dropping partitions that don't exist, adding partitions that overlap with existing ones, adding the same partition twice, etc. Line 1065: gscoped_ptr table_alterer; unique_ptr? PS4, Line 1091: table_alterer->wait(true); : ASSERT_OK(table_alterer->AddRangePartition(add_lower.release(), add_upper.release())->Alter()); Combine? PS4, Line 1097: ASSERT_OK(InsertRowsSequential(table_name, 0, 100)); : ASSERT_EQ(100, CountRows(table_name)); This is to test that IsAlterTableInProgress()->false actually means we can insert/scan? http://gerrit.cloudera.org:8080/#/c/3648/4/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: PS4, Line 1215: Schema schema; : RETURN_NOT_OK(SchemaFromPB(table->metadata().state().pb.schema(), &schema)); : PartitionSchema partition_schema; : RETURN_NOT_OK(PartitionSchema::FromPB(table->metadata().state().pb.partition_schema(), : schema, &partition_schema)); This isn't safe; you should be getting the schema() through the COWLocked object. PS4, Line 1223: std::lock_guard l(table->lock_); : tablets = table->tablet_map_; This is w
[kudu-CR] [c++-client]: cache non-covering ranges in meta cache
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3581 to look at the new patch set (#7). Change subject: [c++-client]: cache non-covering ranges in meta cache .. [c++-client]: cache non-covering ranges in meta cache This commit introduces a few features to the meta cache, all with the aim of making it compatible with the upcoming add/drop range partitions feature. 1) Non-covered range partitions are now cached in the meta cache. This is achieved by storing MetaCacheEntry objects in the meta cache's partition-key index instead of RemoteTablets. The MetaCacheEntry holds either a RemoteTablet, in which case it represents a covered partition range, or it represents a non-covered partition range. 2) Entries are now removed from the meta cache's partition-key index when it can be determined that the entries are no longer valid from the results of a GetTableLocations RPC. 3) A basic TTL has been added to the GetTableLocationsResponsePB so that the client can properly refresh the meta cache when necessary. The TTL is configurable by the master, and defaults to one hour. Change-Id: I05bcb3fe05d51d7c455e1d68bd2baa6f3c2b9d21 --- M src/kudu/client/client-test.cc M src/kudu/client/client.h M src/kudu/client/meta_cache.cc M src/kudu/client/meta_cache.h M src/kudu/client/schema.h M src/kudu/client/table-internal.cc M src/kudu/gutil/map-util.h M src/kudu/master/catalog_manager.cc M src/kudu/master/master.proto M src/kudu/tools/ksck_remote.cc 10 files changed, 421 insertions(+), 122 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/81/3581/7 -- To view, visit http://gerrit.cloudera.org:8080/3581 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I05bcb3fe05d51d7c455e1d68bd2baa6f3c2b9d21 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1311 [master] support adding and dropping range partitions
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3648 to look at the new patch set (#5). Change subject: KUDU-1311 [master] support adding and dropping range partitions .. KUDU-1311 [master] support adding and dropping range partitions Change-Id: I42437f365397baf9d4b39b5b17a1587fae70c4be --- M src/kudu/client/client-internal.cc M src/kudu/client/client-internal.h M src/kudu/client/client-test.cc M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/meta_cache.cc M src/kudu/client/meta_cache.h M src/kudu/client/table_alterer-internal.cc M src/kudu/client/table_alterer-internal.h M src/kudu/common/partition.cc M src/kudu/integration-tests/alter_table-randomized-test.cc M src/kudu/integration-tests/alter_table-test.cc M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/master.proto M src/kudu/master/master_service.cc M src/kudu/tools/ksck.cc 17 files changed, 713 insertions(+), 119 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/48/3648/5 -- To view, visit http://gerrit.cloudera.org:8080/3648 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I42437f365397baf9d4b39b5b17a1587fae70c4be Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [c++-client]: cache non-covering ranges in meta cache
Kudu Jenkins has posted comments on this change. Change subject: [c++-client]: cache non-covering ranges in meta cache .. Patch Set 7: Build Started http://104.196.14.100/job/kudu-gerrit/2494/ -- To view, visit http://gerrit.cloudera.org:8080/3581 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I05bcb3fe05d51d7c455e1d68bd2baa6f3c2b9d21 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Move the maintenance manager to util
Jean-Daniel Cryans has posted comments on this change. Change subject: Move the maintenance manager to util .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3656 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifcf072d443ac3d069bda15b9dc0f8c442b9ac5c0 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Integrate the result tracker with writes
Jean-Daniel Cryans has posted comments on this change. Change subject: Integrate the result tracker with writes .. Patch Set 28: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3449 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1fa2f8db33653960f4749237b8993baba0929893 Gerrit-PatchSet: 28 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] WIP: Add garbage collection to ResultTracker
Jean-Daniel Cryans has posted comments on this change. Change subject: WIP: Add garbage collection to ResultTracker .. Patch Set 5: (2 comments) http://gerrit.cloudera.org:8080/#/c/3628/5/src/kudu/rpc/result_tracker.cc File src/kudu/rpc/result_tracker.cc: PS5, Line 45: > not sure that's a rule. but Done Looking almost anywhere else it's done with trailing whitespaces. PS5, Line 91: // If the arriving request is older than our per-client GC watermark, report its : // staleness to the client. > you mean fatal in the sense that the client can't recover? yeah (not in the Yeah that's what I meant. So is this coming back as a row error or an RPC error? -- To view, visit http://gerrit.cloudera.org:8080/3628 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2c8e7b7191ca14842a31b64813ed498bdf626fa8 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] WIP: Add garbage collection to ResultTracker
David Ribeiro Alves has posted comments on this change. Change subject: WIP: Add garbage collection to ResultTracker .. Patch Set 5: (3 comments) http://gerrit.cloudera.org:8080/#/c/3628/5/src/kudu/rpc/result_tracker.cc File src/kudu/rpc/result_tracker.cc: PS5, Line 45: > nit: shouldn't whitespaces trail for multi-line strings? not sure that's a rule. but Done PS5, Line 91: // If the arriving request is older than our per-client GC watermark, report its : // staleness to the client. > This is fatal, right? There's nothing the client can do? you mean fatal in the sense that the client can't recover? yeah (not in the "crash the server" sense though) http://gerrit.cloudera.org:8080/#/c/3628/5/src/kudu/rpc/result_tracker.h File src/kudu/rpc/result_tracker.h: PS5, Line 227: ClientSttate > nit Done -- To view, visit http://gerrit.cloudera.org:8080/3628 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2c8e7b7191ca14842a31b64813ed498bdf626fa8 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] Integrate the result tracker with writes
David Ribeiro Alves has posted comments on this change. Change subject: Integrate the result tracker with writes .. Patch Set 28: Code-Review+2 rebased this due to a problem with the mm patch. keeping the +2 -- To view, visit http://gerrit.cloudera.org:8080/3449 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1fa2f8db33653960f4749237b8993baba0929893 Gerrit-PatchSet: 28 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Add a way to include request ids in log-dump
David Ribeiro Alves has posted comments on this change. Change subject: Add a way to include request ids in log-dump .. Patch Set 8: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3612 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I88d7c65887a98544ee83b5b4bc0817bea7222131 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] Add a way to include request ids in log-dump
David Ribeiro Alves has posted comments on this change. Change subject: Add a way to include request ids in log-dump .. Patch Set 8: tests failed due to a problem with the mm patch. just a rebase, keeping the +2 -- To view, visit http://gerrit.cloudera.org:8080/3612 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I88d7c65887a98544ee83b5b4bc0817bea7222131 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] Add integration tests for replay cache with writes
Kudu Jenkins has posted comments on this change. Change subject: Add integration tests for replay cache with writes .. Patch Set 32: Build Started http://104.196.14.100/job/kudu-gerrit/2492/ -- To view, visit http://gerrit.cloudera.org:8080/3519 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I35722eb1c83f97e886cfe9d6b03ed95bcd62429f Gerrit-PatchSet: 32 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Add a way to include request ids in log-dump
Kudu Jenkins has posted comments on this change. Change subject: Add a way to include request ids in log-dump .. Patch Set 8: Build Started http://104.196.14.100/job/kudu-gerrit/2490/ -- To view, visit http://gerrit.cloudera.org:8080/3612 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I88d7c65887a98544ee83b5b4bc0817bea7222131 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] Integrate the result tracker with writes
Kudu Jenkins has posted comments on this change. Change subject: Integrate the result tracker with writes .. Patch Set 28: Build Started http://104.196.14.100/job/kudu-gerrit/2491/ -- To view, visit http://gerrit.cloudera.org:8080/3449 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1fa2f8db33653960f4749237b8993baba0929893 Gerrit-PatchSet: 28 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1530: Update docs about OS X build dependency on Xcode package
Dinesh Bhat has posted comments on this change. Change subject: KUDU-1530: Update docs about OS X build dependency on Xcode package .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/3653/2/docs/installation.adoc File docs/installation.adoc: Line 474: The Xcode package is necessary for compiling Kudu. Some of the instructions > Along the same lines, will it be obvious to a macOS user how to get the Xco Yeah I like that idea, perhaps we can simply point 'Xcode' to point to https://developer.apple.com/xcode/ ? -- To view, visit http://gerrit.cloudera.org:8080/3653 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8962d5539f437dba8b120c70c90c1e384ed550c9 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] Move the maintenance manager to util
Kudu Jenkins has posted comments on this change. Change subject: Move the maintenance manager to util .. Patch Set 3: Build Started http://104.196.14.100/job/kudu-gerrit/2489/ -- To view, visit http://gerrit.cloudera.org:8080/3656 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifcf072d443ac3d069bda15b9dc0f8c442b9ac5c0 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Move the maintenance manager to util
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3656 to look at the new patch set (#3). Change subject: Move the maintenance manager to util .. Move the maintenance manager to util This moves the maintenance manager to util, along with its required protobufs. This move is required as we'll need a server-level MaintenanceMananager to run garbage collection on the ResultTracker. Change-Id: Ifcf072d443ac3d069bda15b9dc0f8c442b9ac5c0 --- M src/kudu/integration-tests/full_stack-insert-scan-test.cc M src/kudu/master/master.cc M src/kudu/tablet/CMakeLists.txt M src/kudu/tablet/tablet.cc M src/kudu/tablet/tablet.proto M src/kudu/tablet/tablet_mm_ops.h M src/kudu/tablet/tablet_peer-test.cc M src/kudu/tablet/tablet_peer_mm_ops.cc M src/kudu/tablet/tablet_peer_mm_ops.h M src/kudu/tserver/mini_tablet_server.cc M src/kudu/tserver/tablet_server-test-base.h M src/kudu/tserver/tablet_server.cc M src/kudu/tserver/tserver-path-handlers.cc M src/kudu/util/CMakeLists.txt R src/kudu/util/maintenance_manager-test.cc R src/kudu/util/maintenance_manager.cc R src/kudu/util/maintenance_manager.h A src/kudu/util/maintenance_manager.proto 18 files changed, 85 insertions(+), 59 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/3656/3 -- To view, visit http://gerrit.cloudera.org:8080/3656 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifcf072d443ac3d069bda15b9dc0f8c442b9ac5c0 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Move the maintenance manager to util
David Ribeiro Alves has posted comments on this change. Change subject: Move the maintenance manager to util .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/3656/2/src/kudu/tablet/CMakeLists.txt File src/kudu/tablet/CMakeLists.txt: PS2, Line 100: ADD_KUDU_TEST(maintenance_manager-test) > Forgot to remove this line? too... many... patches... Done -- To view, visit http://gerrit.cloudera.org:8080/3656 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifcf072d443ac3d069bda15b9dc0f8c442b9ac5c0 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Move the maintenance manager to util
Jean-Daniel Cryans has posted comments on this change. Change subject: Move the maintenance manager to util .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/3656/2/src/kudu/tablet/CMakeLists.txt File src/kudu/tablet/CMakeLists.txt: PS2, Line 100: ADD_KUDU_TEST(maintenance_manager-test) Forgot to remove this line? -- To view, visit http://gerrit.cloudera.org:8080/3656 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifcf072d443ac3d069bda15b9dc0f8c442b9ac5c0 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Add a way to include request ids in log-dump
David Ribeiro Alves has posted comments on this change. Change subject: Add a way to include request ids in log-dump .. Patch Set 7: Code-Review+2 just rebases, keeping earlier +2s -- To view, visit http://gerrit.cloudera.org:8080/3612 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I88d7c65887a98544ee83b5b4bc0817bea7222131 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] Integrate the result tracker with writes
Jean-Daniel Cryans has posted comments on this change. Change subject: Integrate the result tracker with writes .. Patch Set 27: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3449 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1fa2f8db33653960f4749237b8993baba0929893 Gerrit-PatchSet: 27 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Integrate the result tracker with writes
David Ribeiro Alves has posted comments on this change. Change subject: Integrate the result tracker with writes .. Patch Set 26: (2 comments) http://gerrit.cloudera.org:8080/#/c/3449/26/src/kudu/tablet/transactions/write_transaction.h File src/kudu/tablet/transactions/write_transaction.h: PS26, Line 192: there > nit: these? Done PS26, Line 193: ; > nit: ' Done -- To view, visit http://gerrit.cloudera.org:8080/3449 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1fa2f8db33653960f4749237b8993baba0929893 Gerrit-PatchSet: 26 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Integrate the result tracker with writes
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3449 to look at the new patch set (#26). Change subject: Integrate the result tracker with writes .. Integrate the result tracker with writes This patch integrates the result tracker with write transactions, including: - Support for caching responses on tablet bootstrap. - Support for caching responses for follower transactions. - Handling of races between client originated and (previous?) leader originated transactions. - An explanation of the interaction between the result tracker and the transaction driver in transaction_driver.h. For integration tests, this patch removes the check that allowed Status::AlreadyPresent() that we added as we didn't have support for exactly once semantics. Without this check, in slow mode, some tests like raft_consensus-itest would fail 100% of the time, due to rows being inserted multiple times. Because we'd have 100% failure rate without replay cache for writes and because testing it specifically is involved, this patch doesn't include additional tests, these will be pushed as a follow up. Change-Id: I1fa2f8db33653960f4749237b8993baba0929893 --- M src/kudu/client/batcher.cc M src/kudu/consensus/consensus.proto M src/kudu/integration-tests/test_workload.cc M src/kudu/integration-tests/test_workload.h M src/kudu/master/sys_catalog.cc M src/kudu/rpc/rpc_context.cc M src/kudu/rpc/rpc_context.h M src/kudu/tablet/tablet_bootstrap-test.cc M src/kudu/tablet/tablet_bootstrap.cc M src/kudu/tablet/tablet_bootstrap.h M src/kudu/tablet/tablet_peer-test.cc M src/kudu/tablet/tablet_peer.cc M src/kudu/tablet/tablet_peer.h M src/kudu/tablet/transactions/transaction.h M src/kudu/tablet/transactions/transaction_driver.cc M src/kudu/tablet/transactions/transaction_driver.h M src/kudu/tablet/transactions/write_transaction.cc M src/kudu/tablet/transactions/write_transaction.h M src/kudu/tools/insert-generated-rows.cc M src/kudu/tserver/CMakeLists.txt M src/kudu/tserver/remote_bootstrap_session-test.cc M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/ts_tablet_manager.cc M src/kudu/tserver/ts_tablet_manager.h M src/kudu/tserver/tserver_service.proto 25 files changed, 399 insertions(+), 41 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/49/3449/26 -- To view, visit http://gerrit.cloudera.org:8080/3449 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1fa2f8db33653960f4749237b8993baba0929893 Gerrit-PatchSet: 26 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Integrate the result tracker with writes
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3449 to look at the new patch set (#27). Change subject: Integrate the result tracker with writes .. Integrate the result tracker with writes This patch integrates the result tracker with write transactions, including: - Support for caching responses on tablet bootstrap. - Support for caching responses for follower transactions. - Handling of races between client originated and (previous?) leader originated transactions. - An explanation of the interaction between the result tracker and the transaction driver in transaction_driver.h. For integration tests, this patch removes the check that allowed Status::AlreadyPresent() that we added as we didn't have support for exactly once semantics. Without this check, in slow mode, some tests like raft_consensus-itest would fail 100% of the time, due to rows being inserted multiple times. Because we'd have 100% failure rate without replay cache for writes and because testing it specifically is involved, this patch doesn't include additional tests, these will be pushed as a follow up. Change-Id: I1fa2f8db33653960f4749237b8993baba0929893 --- M src/kudu/client/batcher.cc M src/kudu/consensus/consensus.proto M src/kudu/integration-tests/test_workload.cc M src/kudu/integration-tests/test_workload.h M src/kudu/master/sys_catalog.cc M src/kudu/rpc/rpc_context.cc M src/kudu/rpc/rpc_context.h M src/kudu/tablet/tablet_bootstrap-test.cc M src/kudu/tablet/tablet_bootstrap.cc M src/kudu/tablet/tablet_bootstrap.h M src/kudu/tablet/tablet_peer-test.cc M src/kudu/tablet/tablet_peer.cc M src/kudu/tablet/tablet_peer.h M src/kudu/tablet/transactions/transaction.h M src/kudu/tablet/transactions/transaction_driver.cc M src/kudu/tablet/transactions/transaction_driver.h M src/kudu/tablet/transactions/write_transaction.cc M src/kudu/tablet/transactions/write_transaction.h M src/kudu/tools/insert-generated-rows.cc M src/kudu/tserver/CMakeLists.txt M src/kudu/tserver/remote_bootstrap_session-test.cc M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/ts_tablet_manager.cc M src/kudu/tserver/ts_tablet_manager.h M src/kudu/tserver/tserver_service.proto 25 files changed, 399 insertions(+), 41 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/49/3449/27 -- To view, visit http://gerrit.cloudera.org:8080/3449 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1fa2f8db33653960f4749237b8993baba0929893 Gerrit-PatchSet: 27 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Integrate the result tracker with writes
Kudu Jenkins has posted comments on this change. Change subject: Integrate the result tracker with writes .. Patch Set 27: Build Started http://104.196.14.100/job/kudu-gerrit/2488/ -- To view, visit http://gerrit.cloudera.org:8080/3449 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1fa2f8db33653960f4749237b8993baba0929893 Gerrit-PatchSet: 27 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Move the maintenance manager to util
David Ribeiro Alves has uploaded a new patch set (#2). Change subject: Move the maintenance manager to util .. Move the maintenance manager to util This moves the maintenance manager to util, along with its required protobufs. This move is required as we'll need a server-level MaintenanceMananager to run garbage collection on the ResultTracker. Change-Id: Ifcf072d443ac3d069bda15b9dc0f8c442b9ac5c0 --- M src/kudu/integration-tests/full_stack-insert-scan-test.cc M src/kudu/master/master.cc M src/kudu/tablet/CMakeLists.txt M src/kudu/tablet/tablet.cc M src/kudu/tablet/tablet.proto M src/kudu/tablet/tablet_mm_ops.h M src/kudu/tablet/tablet_peer-test.cc M src/kudu/tablet/tablet_peer_mm_ops.cc M src/kudu/tablet/tablet_peer_mm_ops.h M src/kudu/tserver/mini_tablet_server.cc M src/kudu/tserver/tablet_server-test-base.h M src/kudu/tserver/tablet_server.cc M src/kudu/tserver/tserver-path-handlers.cc M src/kudu/util/CMakeLists.txt R src/kudu/util/maintenance_manager-test.cc R src/kudu/util/maintenance_manager.cc R src/kudu/util/maintenance_manager.h A src/kudu/util/maintenance_manager.proto 18 files changed, 85 insertions(+), 58 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/3656/2 -- To view, visit http://gerrit.cloudera.org:8080/3656 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifcf072d443ac3d069bda15b9dc0f8c442b9ac5c0 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Integrate the result tracker with writes
Jean-Daniel Cryans has posted comments on this change. Change subject: Integrate the result tracker with writes .. Patch Set 26: (2 comments) http://gerrit.cloudera.org:8080/#/c/3449/26/src/kudu/tablet/transactions/write_transaction.h File src/kudu/tablet/transactions/write_transaction.h: PS26, Line 192: there nit: these? PS26, Line 193: ; nit: ' -- To view, visit http://gerrit.cloudera.org:8080/3449 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1fa2f8db33653960f4749237b8993baba0929893 Gerrit-PatchSet: 26 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Add integration tests for replay cache with writes
David Ribeiro Alves has posted comments on this change. Change subject: Add integration tests for replay cache with writes .. Patch Set 24: (1 comment) http://gerrit.cloudera.org:8080/#/c/3519/24/src/kudu/integration-tests/raft_consensus-itest.cc File src/kudu/integration-tests/raft_consensus-itest.cc: PS24, Line 1184: defined(__SANITIZE_ADDRESS__) || defined(ADDRESS_SANITIZER) > We should only need -DADDRESS_SANITIZER because it's something we put in. S Done -- To view, visit http://gerrit.cloudera.org:8080/3519 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I35722eb1c83f97e886cfe9d6b03ed95bcd62429f Gerrit-PatchSet: 24 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Memory tracking for result tracker
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3627 to look at the new patch set (#8). Change subject: Memory tracking for result tracker .. Memory tracking for result tracker This adds memory tracking to ResultTracker, making sure we account for the memory as we cache responses for clients' requests. Testing wise this adds memory consumption checks to rpc-stress-test.cc. Change-Id: I3b81dda41c8bc7f70380ce426142c34afe6f1625 --- M src/kudu/rpc/result_tracker.cc M src/kudu/rpc/result_tracker.h M src/kudu/rpc/rpc-stress-test.cc M src/kudu/rpc/rpc-test-base.h M src/kudu/server/server_base.cc 5 files changed, 106 insertions(+), 15 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/27/3627/8 -- To view, visit http://gerrit.cloudera.org:8080/3627 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3b81dda41c8bc7f70380ce426142c34afe6f1625 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Memory tracking for result tracker
David Ribeiro Alves has posted comments on this change. Change subject: Memory tracking for result tracker .. Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/3627/6//COMMIT_MSG Commit Message: PS6, Line 10: client's > nit: clients' Done -- To view, visit http://gerrit.cloudera.org:8080/3627 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3b81dda41c8bc7f70380ce426142c34afe6f1625 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Memory tracking for result tracker
Kudu Jenkins has posted comments on this change. Change subject: Memory tracking for result tracker .. Patch Set 8: Build Started http://104.196.14.100/job/kudu-gerrit/2487/ -- To view, visit http://gerrit.cloudera.org:8080/3627 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3b81dda41c8bc7f70380ce426142c34afe6f1625 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Add a way to include request ids in log-dump
Kudu Jenkins has posted comments on this change. Change subject: Add a way to include request ids in log-dump .. Patch Set 7: Build Started http://104.196.14.100/job/kudu-gerrit/2483/ -- To view, visit http://gerrit.cloudera.org:8080/3612 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I88d7c65887a98544ee83b5b4bc0817bea7222131 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] Memory tracking for result tracker
Kudu Jenkins has posted comments on this change. Change subject: Memory tracking for result tracker .. Patch Set 7: Build Started http://104.196.14.100/job/kudu-gerrit/2485/ -- To view, visit http://gerrit.cloudera.org:8080/3627 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3b81dda41c8bc7f70380ce426142c34afe6f1625 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Move the maintenance manager to util
Kudu Jenkins has posted comments on this change. Change subject: Move the maintenance manager to util .. Patch Set 2: Build Started http://104.196.14.100/job/kudu-gerrit/2482/ -- To view, visit http://gerrit.cloudera.org:8080/3656 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifcf072d443ac3d069bda15b9dc0f8c442b9ac5c0 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Add integration tests for replay cache with writes
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3519 to look at the new patch set (#31). Change subject: Add integration tests for replay cache with writes .. Add integration tests for replay cache with writes This adds a couple of new integration tests for replay cache with writes. Both tests start multiple threads writing, independently, to tablet servers simulaneously. The tests leverage the fact that followers are also able to answer requests, once they are cached, and stores all responses, which are compared at the end of the test. Some of the requests (1/3) are "empty" writes, so that we stress the serialization point in transaction_driver.cc without relying on row lock serialization. This adds two different tests, one that stresses a lot of elections and one that crashes nodes. This is inline with other tests we already had in raft_consensus-itest. This also adds a new fault injection point right after the leader sends a request. We currently have one right _before_ the leader sends a request, but having one for after the request is sent encourages stressing the path where a newly elected leader as both incoming client request and ongoing replica transactions, which can possibly race with each other if they correspond to the same write. Finally this changes attempt_no in RequestIdPB to be an int64 instead of just an int. While an int is more than enough in normal operation, the new test generates many more attempts and we need a bigger number to make sure all attempt numbers are unique. I looped this about 1000 times, without related failures. Change-Id: I35722eb1c83f97e886cfe9d6b03ed95bcd62429f --- M src/kudu/consensus/consensus_peers.cc M src/kudu/integration-tests/CMakeLists.txt A src/kudu/integration-tests/exactly_once_writes-itest.cc M src/kudu/integration-tests/raft_consensus-itest.cc M src/kudu/integration-tests/ts_itest-base.h M src/kudu/rpc/rpc_header.proto 6 files changed, 322 insertions(+), 33 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/19/3519/31 -- To view, visit http://gerrit.cloudera.org:8080/3519 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I35722eb1c83f97e886cfe9d6b03ed95bcd62429f Gerrit-PatchSet: 31 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Add integration tests for replay cache with writes
Kudu Jenkins has posted comments on this change. Change subject: Add integration tests for replay cache with writes .. Patch Set 31: Build Started http://104.196.14.100/job/kudu-gerrit/2486/ -- To view, visit http://gerrit.cloudera.org:8080/3519 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I35722eb1c83f97e886cfe9d6b03ed95bcd62429f Gerrit-PatchSet: 31 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Integrate the result tracker with writes
Kudu Jenkins has posted comments on this change. Change subject: Integrate the result tracker with writes .. Patch Set 26: Build Started http://104.196.14.100/job/kudu-gerrit/2484/ -- To view, visit http://gerrit.cloudera.org:8080/3449 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1fa2f8db33653960f4749237b8993baba0929893 Gerrit-PatchSet: 26 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Disable exactly once semantics by default and add a flag to enable it for tests
David Ribeiro Alves has submitted this change and it was merged. Change subject: Disable exactly once semantics by default and add a flag to enable it for tests .. Disable exactly once semantics by default and add a flag to enable it for tests Since exactly once semantics is still missing some pieces, like garbage collection this disables it by default on the server, but adds a flag to allow enabling it and enables it in all tablet server tests, by default. Change-Id: I77096be608afb31194f62f04a946bd3f42537a35 Reviewed-on: http://gerrit.cloudera.org:8080/3506 Reviewed-by: Jean-Daniel Cryans Tested-by: Kudu Jenkins --- M src/kudu/integration-tests/external_mini_cluster-itest-base.h M src/kudu/integration-tests/remote_bootstrap-itest.cc M src/kudu/integration-tests/ts_itest-base.h M src/kudu/rpc/rpc-test-base.h M src/kudu/rpc/service_if.cc M src/kudu/tserver/tablet_server-test-base.h 6 files changed, 26 insertions(+), 1 deletion(-) Approvals: Jean-Daniel Cryans: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/3506 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I77096be608afb31194f62f04a946bd3f42537a35 Gerrit-PatchSet: 17 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Move the maintenance manager to util
David Ribeiro Alves has posted comments on this change. Change subject: Move the maintenance manager to util .. Patch Set 1: (16 comments) This change is required as we'll need a server-wide MM for the result tracker. Added comment in that regard to the commit message. http://gerrit.cloudera.org:8080/#/c/3656/1//COMMIT_MSG Commit Message: PS1, Line 9: it's > nit: its Done Line 10: It doesn't change the namespace (kudu::) since that would be more involved. > But isn't kudu:: the right namespace for something in util? Done http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/integration-tests/full_stack-insert-scan-test.cc File src/kudu/integration-tests/full_stack-insert-scan-test.cc: Line 40: #include "kudu/util/maintenance_manager.h" > Nit: resort. Done http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/master/master.cc File src/kudu/master/master.cc: Line 39: #include "kudu/util/maintenance_manager.h" > Nit: resort. Done http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/tablet/CMakeLists.txt File src/kudu/tablet/CMakeLists.txt: Line 36: ../util/maintenance_manager.cc > What's this doing here? The maintenance manager is now part of libkudu_util clion refactor. Done Line 101: ADD_KUDU_TEST(maintenance_manager-test) > The test should be moved too. Done http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/tablet/tablet.cc File src/kudu/tablet/tablet.cc: Line 48: #include "kudu/util/maintenance_manager.h" > Nit: resort. Done http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/tablet/tablet_peer-test.cc File src/kudu/tablet/tablet_peer-test.cc: Line 35: #include "kudu/util/maintenance_manager.h" > Nit: resort. Done http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/tablet/tablet_peer_mm_ops.cc File src/kudu/tablet/tablet_peer_mm_ops.cc: Line 27: #include "kudu/util/maintenance_manager.h" > Nit: resort. Done http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/tserver/mini_tablet_server.cc File src/kudu/tserver/mini_tablet_server.cc: Line 34: #include "kudu/util/maintenance_manager.h" > Nit: resort. Done http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/tserver/tablet_server-test-base.h File src/kudu/tserver/tablet_server-test-base.h: Line 43: #include "kudu/util/maintenance_manager.h" > Nit: resort. Done http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/tserver/tablet_server.cc File src/kudu/tserver/tablet_server.cc: Line 30: #include "kudu/util/maintenance_manager.h" > Nit: resort. Done http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/tserver/tserver-path-handlers.cc File src/kudu/tserver/tserver-path-handlers.cc: Line 35: #include "kudu/util/maintenance_manager.h" > Nit: resort. Done http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/util/maintenance_manager-test.cc File src/kudu/util/maintenance_manager-test.cc: Line 25: #include "maintenance_manager.h" > Should be kudu/util, and resort. Done http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/util/maintenance_manager.cc File src/kudu/util/maintenance_manager.cc: Line 18: #include "maintenance_manager.h" > Nit: kudu/util. Done http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/util/maintenance_manager.h File src/kudu/util/maintenance_manager.h: Line 17: #pragma once > Nit: separate from the license with an empty line. Done -- To view, visit http://gerrit.cloudera.org:8080/3656 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifcf072d443ac3d069bda15b9dc0f8c442b9ac5c0 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [java client] Integrate with the replay cache
Jean-Daniel Cryans has posted comments on this change. Change subject: [java client] Integrate with the replay cache .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/3631/4/java/kudu-client/src/main/java/org/kududb/client/TabletClient.java File java/kudu-client/src/main/java/org/kududb/client/TabletClient.java: PS4, Line 254: requestIdBuilder.setFirstIncompleteSeqNo(requestTracker.firstIncomplete()); In the previous patch I wasn't sending this if the value would have been NO_SEQ_NO, but the C++ client does it and David thinks that field should be required. -- To view, visit http://gerrit.cloudera.org:8080/3631 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I108cd30acbc308bfb4577d072c2a8f26d1553c68 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] [java client] Integrate with the replay cache
Kudu Jenkins has posted comments on this change. Change subject: [java client] Integrate with the replay cache .. Patch Set 4: Build Started http://104.196.14.100/job/kudu-gerrit/2481/ -- To view, visit http://gerrit.cloudera.org:8080/3631 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I108cd30acbc308bfb4577d072c2a8f26d1553c68 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] [java client] Integrate with the replay cache
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3631 to look at the new patch set (#4). Change subject: [java client] Integrate with the replay cache .. [java client] Integrate with the replay cache This patch adds the required functionality to have the Java client use the server-side replay cache. Change-Id: I108cd30acbc308bfb4577d072c2a8f26d1553c68 --- M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduScanner.java M java/kudu-client/src/main/java/org/kududb/client/Batch.java M java/kudu-client/src/main/java/org/kududb/client/KuduRpc.java M java/kudu-client/src/main/java/org/kududb/client/Operation.java A java/kudu-client/src/main/java/org/kududb/client/RequestTracker.java M java/kudu-client/src/main/java/org/kududb/client/TabletClient.java M java/kudu-client/src/test/java/org/kududb/client/ITClient.java M java/kudu-client/src/test/java/org/kududb/client/MiniKuduCluster.java M java/kudu-client/src/test/java/org/kududb/client/TestLeaderFailover.java A java/kudu-client/src/test/java/org/kududb/client/TestRequestTracker.java 11 files changed, 209 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/31/3631/4 -- To view, visit http://gerrit.cloudera.org:8080/3631 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I108cd30acbc308bfb4577d072c2a8f26d1553c68 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Change version from 1.0.0-SNAPSHOT to 0.10.0-SNAPSHOT
Jean-Daniel Cryans has submitted this change and it was merged. Change subject: Change version from 1.0.0-SNAPSHOT to 0.10.0-SNAPSHOT .. Change version from 1.0.0-SNAPSHOT to 0.10.0-SNAPSHOT Change-Id: If8aa4a73ef33c413a52c5cb69d81f0edb192135e Reviewed-on: http://gerrit.cloudera.org:8080/3651 Reviewed-by: Mike Percy Tested-by: Kudu Jenkins --- 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-flume-sink/pom.xml M java/kudu-mapreduce/pom.xml M java/kudu-spark/pom.xml M java/pom.xml M version.txt 9 files changed, 10 insertions(+), 10 deletions(-) Approvals: Mike Percy: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/3651 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: If8aa4a73ef33c413a52c5cb69d81f0edb192135e Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR] Memory tracking for result tracker
Jean-Daniel Cryans has posted comments on this change. Change subject: Memory tracking for result tracker .. Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/3627/6//COMMIT_MSG Commit Message: PS6, Line 10: client's nit: clients' (regular noun, plural, to show their possession) -- To view, visit http://gerrit.cloudera.org:8080/3627 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3b81dda41c8bc7f70380ce426142c34afe6f1625 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Move the maintenance manager to util
Jean-Daniel Cryans has posted comments on this change. Change subject: Move the maintenance manager to util .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/3656/1//COMMIT_MSG Commit Message: PS1, Line 9: it's nit: its (it's my pet peeve even though I frequently make the same mistake) -- To view, visit http://gerrit.cloudera.org:8080/3656 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifcf072d443ac3d069bda15b9dc0f8c442b9ac5c0 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Move the maintenance manager to util
Adar Dembo has posted comments on this change. Change subject: Move the maintenance manager to util .. Patch Set 1: (15 comments) What's the motivation for this move? http://gerrit.cloudera.org:8080/#/c/3656/1//COMMIT_MSG Commit Message: Line 10: It doesn't change the namespace (kudu::) since that would be more involved. But isn't kudu:: the right namespace for something in util? http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/integration-tests/full_stack-insert-scan-test.cc File src/kudu/integration-tests/full_stack-insert-scan-test.cc: Line 40: #include "kudu/util/maintenance_manager.h" Nit: resort. http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/master/master.cc File src/kudu/master/master.cc: Line 39: #include "kudu/util/maintenance_manager.h" Nit: resort. http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/tablet/CMakeLists.txt File src/kudu/tablet/CMakeLists.txt: Line 36: ../util/maintenance_manager.cc What's this doing here? The maintenance manager is now part of libkudu_util, so shouldn't this just be removed? Line 101: ADD_KUDU_TEST(maintenance_manager-test) The test should be moved too. http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/tablet/tablet.cc File src/kudu/tablet/tablet.cc: Line 48: #include "kudu/util/maintenance_manager.h" Nit: resort. http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/tablet/tablet_peer-test.cc File src/kudu/tablet/tablet_peer-test.cc: Line 35: #include "kudu/util/maintenance_manager.h" Nit: resort. http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/tablet/tablet_peer_mm_ops.cc File src/kudu/tablet/tablet_peer_mm_ops.cc: Line 27: #include "kudu/util/maintenance_manager.h" Nit: resort. http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/tserver/mini_tablet_server.cc File src/kudu/tserver/mini_tablet_server.cc: Line 34: #include "kudu/util/maintenance_manager.h" Nit: resort. http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/tserver/tablet_server-test-base.h File src/kudu/tserver/tablet_server-test-base.h: Line 43: #include "kudu/util/maintenance_manager.h" Nit: resort. http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/tserver/tablet_server.cc File src/kudu/tserver/tablet_server.cc: Line 30: #include "kudu/util/maintenance_manager.h" Nit: resort. http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/tserver/tserver-path-handlers.cc File src/kudu/tserver/tserver-path-handlers.cc: Line 35: #include "kudu/util/maintenance_manager.h" Nit: resort. http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/util/maintenance_manager-test.cc File src/kudu/util/maintenance_manager-test.cc: Line 25: #include "maintenance_manager.h" Should be kudu/util, and resort. http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/util/maintenance_manager.cc File src/kudu/util/maintenance_manager.cc: Line 18: #include "maintenance_manager.h" Nit: kudu/util. http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/util/maintenance_manager.h File src/kudu/util/maintenance_manager.h: Line 17: #pragma once Nit: separate from the license with an empty line. -- To view, visit http://gerrit.cloudera.org:8080/3656 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifcf072d443ac3d069bda15b9dc0f8c442b9ac5c0 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Integrate the result tracker with writes
David Ribeiro Alves has posted comments on this change. Change subject: Integrate the result tracker with writes .. Patch Set 25: (1 comment) http://gerrit.cloudera.org:8080/#/c/3449/25/src/kudu/tablet/transactions/transaction_driver.h File src/kudu/tablet/transactions/transaction_driver.h: PS25, Line 176: request were > nit: either request should be plural or were should be was. Done -- To view, visit http://gerrit.cloudera.org:8080/3449 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1fa2f8db33653960f4749237b8993baba0929893 Gerrit-PatchSet: 25 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Integrate the result tracker with writes
David Ribeiro Alves has posted comments on this change. Change subject: Integrate the result tracker with writes .. Patch Set 25: (13 comments) http://gerrit.cloudera.org:8080/#/c/3449/25/src/kudu/tablet/transactions/transaction_driver.cc File src/kudu/tablet/transactions/transaction_driver.cc: PS25, Line 210: it's > nit: its Done PS25, Line 268: er."); > Any more info that we can add here? Added the request id. Note that the user won't see this as the request is transparently retried. http://gerrit.cloudera.org:8080/#/c/3449/25/src/kudu/tablet/transactions/transaction_driver.h File src/kudu/tablet/transactions/transaction_driver.h: PS25, Line 124: client originated > nit: client-originated Done PS25, Line 124: a > nit: remove Done PS25, Line 149: to to > nit Done PS25, Line 155: it's > nit: its Done PS25, Line 156: requests > nit: request? same for the other "requests" on that line. Done PS25, Line 157: > nit Done PS25, Line 168: and > nit: remove that 'and' Done PS25, Line 169: received > nit: receive Done PS25, Line 302: drivers > nit: driver Done http://gerrit.cloudera.org:8080/#/c/3449/25/src/kudu/tablet/transactions/write_transaction.h File src/kudu/tablet/transactions/write_transaction.h: PS25, Line 192: pointers to the rpc context, request and response, lifecyle : // is managed by the rpc subsystem. > Since you're here, mind rewriting this sentence? Start with upper case, the Done PS25, Line 194: Request > nit: 'request_' Done -- To view, visit http://gerrit.cloudera.org:8080/3449 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1fa2f8db33653960f4749237b8993baba0929893 Gerrit-PatchSet: 25 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-1530: Update docs about OS X build dependency on Xcode package
Adar Dembo has posted comments on this change. Change subject: KUDU-1530: Update docs about OS X build dependency on Xcode package .. Patch Set 2: Code-Review+2 (1 comment) Looks good, just one question. http://gerrit.cloudera.org:8080/#/c/3653/2/docs/installation.adoc File docs/installation.adoc: Line 474: The Xcode package is necessary for compiling Kudu. Some of the instructions Along the same lines, will it be obvious to a macOS user how to get the Xcode package? Or should we include that in the documentation here? -- To view, visit http://gerrit.cloudera.org:8080/3653 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8962d5539f437dba8b120c70c90c1e384ed550c9 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] KUDU-1358 (part 3): new multi-master stress test
Adar Dembo has posted comments on this change. Change subject: KUDU-1358 (part 3): new multi-master stress test .. Patch Set 9: Verified+1 chrpath() failures. -- To view, visit http://gerrit.cloudera.org:8080/3611 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I40b5b78c100a7b427b2f4aac3a54665e82a9618c Gerrit-PatchSet: 9 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] rw mutex: prevent recursive use
Adar Dembo has posted comments on this change. Change subject: rw_mutex: prevent recursive use .. Patch Set 5: Verified+1 KUDU-1527 and isolate failures. -- To view, visit http://gerrit.cloudera.org:8080/3641 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7ae30ec123a16c39ef0c15ee2d2176f807df03db Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1358 (part 2): heartbeat to every master
Adar Dembo has posted comments on this change. Change subject: KUDU-1358 (part 2): heartbeat to every master .. Patch Set 9: Verified+1 Isolate failed. -- To view, visit http://gerrit.cloudera.org:8080/3610 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic85ac4193462d21c989dbd7874b451e8eaab8e3e Gerrit-PatchSet: 9 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No