[kudu-CR] Remove a stray WARNING left in deltafile.cc
Todd Lipcon has posted comments on this change. Change subject: Remove a stray WARNING left in deltafile.cc .. Patch Set 1: Code-Review+2 Going to self-+2 this one since it's trivial -- To view, visit http://gerrit.cloudera.org:8080/3211 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5492ddeafa54f0fe5f02a190fecab5e61c3c856b Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Remove a stray WARNING left in deltafile.cc
Todd Lipcon has uploaded a new change for review. http://gerrit.cloudera.org:8080/3211 Change subject: Remove a stray WARNING left in deltafile.cc .. Remove a stray WARNING left in deltafile.cc This was a debugging warning that I accidentally left in when fixing 8d377ac0fe2be0f9176526e9db745ea6dd70ce86. Change-Id: I5492ddeafa54f0fe5f02a190fecab5e61c3c856b --- M src/kudu/tablet/deltafile.cc 1 file changed, 0 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/11/3211/1 -- To view, visit http://gerrit.cloudera.org:8080/3211 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I5492ddeafa54f0fe5f02a190fecab5e61c3c856b Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon
[kudu-CR] KUDU-1444. Get resource metrics of a scan.
Todd Lipcon has posted comments on this change. Change subject: KUDU-1444. Get resource metrics of a scan. .. Patch Set 5: (7 comments) http://gerrit.cloudera.org:8080/#/c/3013/5/src/kudu/client/client.h File src/kudu/client/client.h: Line 29: #include "kudu/client/resource_metrics.h" nit: please sort imports http://gerrit.cloudera.org:8080/#/c/3013/5/src/kudu/client/resource_metrics-internal.h File src/kudu/client/resource_metrics-internal.h: Line 39: void Increment(std::string name, int64_t amount); this can be a const std::string&, or perhaps a StringPiece Line 42: int64_t GetMetric(const std::string name) const; same http://gerrit.cloudera.org:8080/#/c/3013/5/src/kudu/client/resource_metrics.h File src/kudu/client/resource_metrics.h: Line 40: void Increment(std::string name, int64_t amount); const std::string& Line 43: int64_t GetMetric(const std::string name) const; same http://gerrit.cloudera.org:8080/#/c/3013/5/src/kudu/client/scanner-internal.cc File src/kudu/client/scanner-internal.cc: Line 156: resource_metrics_.Increment(field->name(), reflection->GetInt64(resource_metrics, field)); this should probably check whether the field is set (reflection->HasField(resource_metrics, field)), before doing the Increment http://gerrit.cloudera.org:8080/#/c/3013/5/src/kudu/tserver/tablet_service.cc File src/kudu/tserver/tablet_service.cc: Line 88: extern const char* CFILE_CACHE_MISS_BYTES_METRIC_NAME; nit: dont indent inside namespaces -- To view, visit http://gerrit.cloudera.org:8080/3013 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iedaf570a7601651c93275ae0a8565f1e33da842d Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: zhen.zhang Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: zhen.zhang Gerrit-HasComments: Yes
[kudu-CR](branch-0.9.x) java: use truncated randomized exponential backoff for retries
Todd Lipcon has submitted this change and it was merged. Change subject: java: use truncated randomized exponential backoff for retries .. java: use truncated randomized exponential backoff for retries This changes the Java client to use a randomized exponential backoff, starting at 1ms, and going up to 4096ms, rather than the previous linear backoff of (500ms * retry_count). The old backoff was far too aggressive, and resulted in the Java client regularly having high percentile latencies of 500 or 1000ms even when there was spare capacity in the system. I tested this on a 70 node cluster using a read-only YCSB workload, and the 99.99th percentile latency dropped from >1s to 390ms. Combined with further in-flight changes, the 99.99th percentile dropped further to ~56ms. Change-Id: I79256e00cf35c072c60cd2b9034e6c1b10c18b38 Reviewed-on: http://gerrit.cloudera.org:8080/3184 Tested-by: Kudu Jenkins Reviewed-by: Jean-Daniel Cryans Reviewed-by: Adar Dembo (cherry picked from commit 8094b73147e0238cdfd96a141554b5344c801cdb) Reviewed-on: http://gerrit.cloudera.org:8080/3198 --- M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java 1 file changed, 3 insertions(+), 2 deletions(-) Approvals: Jean-Daniel Cryans: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/3198 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I79256e00cf35c072c60cd2b9034e6c1b10c18b38 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: branch-0.9.x Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-745 / KUDU-1463 tablet's table id attribute is empty string
Todd Lipcon has posted comments on this change. Change subject: KUDU-745 / KUDU-1463 tablet's table id attribute is empty string .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3183 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1795a176312f6c2a55c34b7b684ee408f1cf8732 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: No
[kudu-CR] KUDU-745 / KUDU-1463 tablet's table id attribute is empty string
Todd Lipcon has submitted this change and it was merged. Change subject: KUDU-745 / KUDU-1463 tablet's table id attribute is empty string .. KUDU-745 / KUDU-1463 tablet's table id attribute is empty string This fixes the issue where the table_id attribute of a tablet in /metrics was an empty string. As part of this patch, I also ended up picking a couple of magic table_id's. The significant one is that the table_id of sys.catalog is "sys.catalog.id". Change-Id: I1795a176312f6c2a55c34b7b684ee408f1cf8732 Reviewed-on: http://gerrit.cloudera.org:8080/3183 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon --- M src/kudu/consensus/log-test-base.h M src/kudu/master/sys_catalog.cc M src/kudu/master/sys_catalog.h M src/kudu/tablet/tablet-harness.h M src/kudu/tablet/tablet_bootstrap-test.cc M src/kudu/tablet/tablet_metadata.cc M src/kudu/tablet/tablet_metadata.h M src/kudu/tserver/remote_bootstrap_client.cc M src/kudu/tserver/tablet_server-test.cc M src/kudu/tserver/ts_tablet_manager.cc 10 files changed, 20 insertions(+), 5 deletions(-) Approvals: Todd Lipcon: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/3183 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I1795a176312f6c2a55c34b7b684ee408f1cf8732 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] Change flush defaults to encourage parallel IO and larger flushes
Todd Lipcon has posted comments on this change. Change subject: Change flush defaults to encourage parallel IO and larger flushes .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/3186/2/src/kudu/cfile/cfile_writer.cc File src/kudu/cfile/cfile_writer.cc: Line 48: // The default value is optimized for throughput in the case that > There's a similar comment in block_manager.cc; could you update that too? I OK. I agree that the coalesce_close thing is probably not super useful anymore. I'll just remove the comment there and assume that, if we decide to change it at some point, we can put back some useful explanation http://gerrit.cloudera.org:8080/#/c/3186/2/src/kudu/tablet/tablet_peer_mm_ops.cc File src/kudu/tablet/tablet_peer_mm_ops.cc: Line 32: DEFINE_int32(flush_threshold_mb, 1000, > Nit: 1024 (power of 2)? Done -- To view, visit http://gerrit.cloudera.org:8080/3186 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1c70d9c76ed33bbfca5480e1d1f343c6dab36d3b Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Change flush defaults to encourage parallel IO and larger flushes
Hello Jean-Daniel Cryans, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3186 to look at the new patch set (#3). Change subject: Change flush defaults to encourage parallel IO and larger flushes .. Change flush defaults to encourage parallel IO and larger flushes Based on some recent experiments with high throughput writes using YCSB[1], these defaults make more sense for the typical throughput-oriented applications that Kudu is currently targeting. [1] http://getkudu.io/2016/04/26/ycsb.html Change-Id: I1c70d9c76ed33bbfca5480e1d1f343c6dab36d3b --- M docs/release_notes.adoc M src/kudu/cfile/cfile_writer.cc M src/kudu/fs/block_manager.cc M src/kudu/tablet/tablet_peer-test.cc M src/kudu/tablet/tablet_peer_mm_ops.cc 5 files changed, 25 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/86/3186/3 -- To view, visit http://gerrit.cloudera.org:8080/3186 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1c70d9c76ed33bbfca5480e1d1f343c6dab36d3b Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Todd Lipcon
[kudu-CR](branch-0.9.x) service_pool: only log queue overflows once per second
Todd Lipcon has submitted this change and it was merged. Change subject: service_pool: only log queue overflows once per second .. service_pool: only log queue overflows once per second These queue overflow messages are extremely frequent on stress workloads, and I found that the log spew itself caused the server to block on glog locks, compounding the problem. Spewing these messages is rarely helpful, so this patch throttles it to once a second. Change-Id: I636ef39798f9b2ab7d8aec237285c949ec119468 Reviewed-on: http://gerrit.cloudera.org:8080/3185 Tested-by: Kudu Jenkins Reviewed-by: Jean-Daniel Cryans (cherry picked from commit 298806fa32c44e986c2401fa5ac416bcbc491523) Reviewed-on: http://gerrit.cloudera.org:8080/3197 --- M src/kudu/rpc/service_pool.cc 1 file changed, 2 insertions(+), 1 deletion(-) Approvals: Jean-Daniel Cryans: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/3197 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I636ef39798f9b2ab7d8aec237285c949ec119468 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: branch-0.9.x Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1467. Fix incorrect bootstrap replay issue with UPSERT
Hello David Ribeiro Alves, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/3208 to review the following change. Change subject: KUDU-1467. Fix incorrect bootstrap replay issue with UPSERT .. KUDU-1467. Fix incorrect bootstrap replay issue with UPSERT This fixes an issue with UPSERT that was causing fuzz-itest to be ~5% flaky. The issue turned out to be essentially the same as KUDU-1341 (fixed in 1ff209e85b4c2cc4beda7560d328b7ed05e008d2) except pertaining to UPSERT operations which acted as mutations, whereas the original bug was for direct UPDATE operations. In addition to the existing fuzz test which exposes this bug, this patch includes a specific minimal fuzz sequence which triggered the bug deterministically. To verify that the bug is fixed, I looped fuzz-itest 500 times: http://dist-test.cloudera.org/job?job_id=todd.1464152165.32007 Change-Id: Id443e171bc80833737486a96a3fd6fcce7a2363b --- M src/kudu/integration-tests/fuzz-itest.cc M src/kudu/tablet/tablet.cc M src/kudu/tablet/tablet.h 3 files changed, 26 insertions(+), 10 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/08/3208/1 -- To view, visit http://gerrit.cloudera.org:8080/3208 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Id443e171bc80833737486a96a3fd6fcce7a2363b Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: David Ribeiro Alves
[kudu-CR] Integrate the ResultTracker into the rpc subsystem and add a test
David Ribeiro Alves has posted comments on this change. Change subject: Integrate the ResultTracker into the rpc subsystem and add a test .. Patch Set 6: (1 comment) This is still missing a parallel test for the failure case. http://gerrit.cloudera.org:8080/#/c/3192/6/src/kudu/rpc/service_if.cc File src/kudu/rpc/service_if.cc: Line 103: ResultTracker::RpcState state = ctx->GetResultTracker()->CheckDuplicate( > It's interesting that 'req' isn't considered at all in CheckDuplicate(). We that's on purpose. storing the requests is bound to consume much more memory that storing the responses so we need to "trust" that the client sends the same request with the same seq no. In any case the worst that can happen is that the client gets the "wrong" response, if he sent a requests with a previously used seq no. -- To view, visit http://gerrit.cloudera.org:8080/3192 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1d624810350feceefe244e0319e22cac241bf0d6 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Add a RpcContext::RespondFailure() method
David Ribeiro Alves has posted comments on this change. Change subject: Add a RpcContext::RespondFailure() method .. Patch Set 6: That's fair. I separated this mostly so that I can explain why I needed it in the commit message and to see what folks think. I'm ok with merging it if that's the consensus. -- To view, visit http://gerrit.cloudera.org:8080/3191 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9f1387ba0f837046a8056e77f73a3982f06c73a2 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Move the PbTracer in rpc_context.cc to pb_util
David Ribeiro Alves has submitted this change and it was merged. Change subject: Move the PbTracer in rpc_context.cc to pb_util .. Move the PbTracer in rpc_context.cc to pb_util We'll need to use this outside of RpcContext so this moves it to a place where it makes more sense and can be used elsewhere. Change-Id: Ib601deea9fe0dcc6e2428416a6993c7567ff7335 Reviewed-on: http://gerrit.cloudera.org:8080/3194 Tested-by: David Ribeiro Alves Reviewed-by: Adar Dembo --- M src/kudu/rpc/rpc_context.cc M src/kudu/util/pb_util.cc M src/kudu/util/pb_util.h 3 files changed, 54 insertions(+), 36 deletions(-) Approvals: David Ribeiro Alves: Verified Adar Dembo: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/3194 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ib601deea9fe0dcc6e2428416a6993c7567ff7335 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: Todd Lipcon
[kudu-CR] Add a request tracker to track client rpc sequence numbers
David Ribeiro Alves has submitted this change and it was merged. Change subject: Add a request tracker to track client rpc sequence numbers .. Add a request tracker to track client rpc sequence numbers This adds a new component to the rpc subsystem: the RequestTracker. This is inspired by the RequestTracker in "Implementing Linearizability at Large Scale and Low Latency" by Colin Lee et al. and is responsible for generating new sequence numbers for rpcs and to keep track of the incomplete ones so that we're able to implement exactly-once semantics for certain rpcs. Change-Id: I23201625ca02f244dc94205d88dabc01608de471 Reviewed-on: http://gerrit.cloudera.org:8080/3078 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo --- M src/kudu/rpc/CMakeLists.txt A src/kudu/rpc/request_tracker-test.cc A src/kudu/rpc/request_tracker.cc A src/kudu/rpc/request_tracker.h 4 files changed, 224 insertions(+), 0 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/3078 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I23201625ca02f244dc94205d88dabc01608de471 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: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Add request id and sequence number to the rpc request header
David Ribeiro Alves has submitted this change and it was merged. Change subject: Add request id and sequence number to the rpc request header .. Add request id and sequence number to the rpc request header This adds a new (optional) field to RpcHeader: RequestIdPB. This new field contains the information necessary to implement the "replay cache" component, including the unique id of the client and the rpc's sequence number. RequestIdPB also includes two additional "helper" fields, one to keep track of number of attempts of a particular rpc, so that we can skip the replay cache altogether in certain cases and one to let the server know that it can garbage collect completion records. Change-Id: I30b07f070f8e80aaad982ccfe71ffb7e22a6a703 Reviewed-on: http://gerrit.cloudera.org:8080/3079 Tested-by: Kudu Jenkins Reviewed-by: David Ribeiro Alves --- M src/kudu/rpc/rpc_header.proto 1 file changed, 22 insertions(+), 0 deletions(-) Approvals: David Ribeiro Alves: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/3079 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I30b07f070f8e80aaad982ccfe71ffb7e22a6a703 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Add request id and sequence number to the rpc request header
David Ribeiro Alves has posted comments on this change. Change subject: Add request id and sequence number to the rpc request header .. Patch Set 6: submitting this. I can take care of any further comments todd has post-commit -- To view, visit http://gerrit.cloudera.org:8080/3079 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I30b07f070f8e80aaad982ccfe71ffb7e22a6a703 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Add a ResultTracker class that will track server side results
David Ribeiro Alves has posted comments on this change. Change subject: Add a ResultTracker class that will track server side results .. Patch Set 6: (4 comments) http://gerrit.cloudera.org:8080/#/c/3190/6/src/kudu/rpc/result_tracker.h File src/kudu/rpc/result_tracker.h: Line 45: // of the RPC at that the corresponding server function should be executed. If any other value is > Nit: "at that the" --> "and that the" Done Line 49: // client appropriately, > Nit: trailing comma here? Done Line 55: // If, on the other hand, if execution of the server function is not successful then one of > Nit: too many 'ifs' here. Done Line 96: RpcState CheckDuplicate(const std::string& client_id, > I'd rename the function slightly to imply that it's not just a stateless "c was kinda following the paper here. suggestions? CheckDuplicateOrRegister? not terribly accurate (since its also doing something if it's a duplicate) -- To view, visit http://gerrit.cloudera.org:8080/3190 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6718951a9998a6c9b0db35e8f09ff8304591e8b1 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Add a ResultTracker class that will track server side results
David Ribeiro Alves has posted comments on this change. Change subject: Add a ResultTracker class that will track server side results .. Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/3190/3/src/kudu/rpc/result_tracker.cc File src/kudu/rpc/result_tracker.cc: Line 40: lock_guard l(&lock_); > I suspect it will be, as nearly every RPC will call CheckDuplicate(), no? yeah, definitely something we can optimize for later, if this comes up in the hotspots list. we usually follow the policy of implementing a simple solution first and optimizing it later if needed. Added a TODO in this regard though. Line 78: LOG(FATAL) << "Wrong state"; > OK, could we at least log what the wrong state was? true, we can. Done. Line 119: ClientState* client_state = FindOrDie(clients_, client_id); : CompletionRecord* completion_record = FindOrDie(client_state->completion_records, : sequence_number); > Well, this goes back to the lack of documentation thing. :) Well this is not a "new" module per se, IMO this is an change in the semantics of the rpc module. A couple of things: 1 - This is not meant to be used by itself in the majority of cases (single server)as this is hidden behind RpcContext. In the few cases where it's meant to be used stand alone (Write() for one) yes, care would have to be taken but if this fails what would it even reply to the client "Request lost?". Which takes us to point 2. 2 - This doesn't always reply to the client. For instance in for writes If a follower is calling this on some op that came from the leader there might not (and likely won't) be a client to reply to, we're just storing the response in the case it will eventually appears. Meaning this would fail silently if we didn't crash, meaning we'd have a bug we don't know exists as a new client rpc would be treated as new (an instance of the data corruption we have now). I've improved the docs to reflect that this requires that CheckDuplicate() was called before, please take a look, but I don't want to go into too much detail on every possible usage of this. -- To view, visit http://gerrit.cloudera.org:8080/3190 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6718951a9998a6c9b0db35e8f09ff8304591e8b1 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: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Add a ResultTracker class that will track server side results
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3190 to look at the new patch set (#7). Change subject: Add a ResultTracker class that will track server side results .. Add a ResultTracker class that will track server side results This adds the initial version of the ResultTracker class that will be responsible for tracking responses when we want exactly once call semantics. While this is minimally working, i.e it's able to track responses and can be used for exactly once semantics, it's not complete. Future patches will address the missing functionality. Still missing are: - Time based client state cleaning. - Watermark based per client state cleaning. I initially had a unit test for this class, but it relied on templating out the RpcContext (since it's not easy to build one for a unit test and it's not clear what it would do) which became problematic as the class grew. So I decided to add a new rpc-stress-tess that will test this class within the rpc framework. I feel it's easier to do it that way and that, since the client is not being used, the test can still be very straight forward. This test is in a followup patch as it can only run after the integration with the rest of the rpc subsystem. Change-Id: I6718951a9998a6c9b0db35e8f09ff8304591e8b1 --- M src/kudu/rpc/CMakeLists.txt A src/kudu/rpc/result_tracker.cc A src/kudu/rpc/result_tracker.h M src/kudu/rpc/rpc_context.h 4 files changed, 364 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/3190/7 -- To view, visit http://gerrit.cloudera.org:8080/3190 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6718951a9998a6c9b0db35e8f09ff8304591e8b1 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [c++-client]: minimal changes to support tables with non-covering range partitions
Adar Dembo has posted comments on this change.
Change subject: [c++-client]: minimal changes to support tables with
non-covering range partitions
..
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/3177/1/src/kudu/client/meta_cache.cc
File src/kudu/client/meta_cache.cc:
Line 612: const TabletLocationsPB& tablet0 = rpc.resp().tablet_locations(0);
I always thought it was somewhat dangerous to expect any kind of order out of
the tablets in response. Since we're changing this behavior, maybe we can avoid
that now?
Line 618: if (rpc.is_exact_lookup() || rpc.resp().tablet_locations().size()
<= 1) {
<= 1? Given that we're here, why bother checking for 0?
http://gerrit.cloudera.org:8080/#/c/3177/1/src/kudu/common/partition_pruner.h
File src/kudu/common/partition_pruner.h:
Line 62: // Used for testing.
Not true anymore. Update? And what was unsafe/weird about this function to
begin with that it was restricted to tests?
--
To view, visit http://gerrit.cloudera.org:8080/3177
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib25b7a57b14b3d1e4e6dca75b88dad7c19ba7565
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] Remove default table partitioning
Adar Dembo has posted comments on this change. Change subject: Remove default table partitioning .. Patch Set 9: (2 comments) http://gerrit.cloudera.org:8080/#/c/3131/9/java/kudu-client/src/test/java/org/kududb/client/TestKuduSession.java File java/kudu-client/src/test/java/org/kududb/client/TestKuduSession.java: Line 24 Nit: don't unroll. http://gerrit.cloudera.org:8080/#/c/3131/9/java/kudu-spark/src/main/scala/org/kududb/spark/kudu/KuduContext.scala File java/kudu-spark/src/main/scala/org/kududb/spark/kudu/KuduContext.scala: Line 20: import java.util If you're not changing file contents, could you avoid changing the import order? -- To view, visit http://gerrit.cloudera.org:8080/3131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7021d7950f8dbb4918503ea6fab2e6ee35076064 Gerrit-PatchSet: 9 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Misty Stanley-Jones Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Integrate the ResultTracker into the rpc subsystem and add a test
Adar Dembo has posted comments on this change.
Change subject: Integrate the ResultTracker into the rpc subsystem and add a
test
..
Patch Set 6:
(9 comments)
It's becoming increasingly clear to me that Todd needs to take a deep look at
this series; there's too much here that I don't know.
http://gerrit.cloudera.org:8080/#/c/3192/6//COMMIT_MSG
Commit Message:
Line 10: to specify a new mehtod option when defining rpc service methods.
Nit: method
http://gerrit.cloudera.org:8080/#/c/3192/6/src/kudu/common/common.proto
File src/kudu/common/common.proto:
Line 313: // An option for RPC methods that allows to set whether that method's
Why define this here and not in rpc_header.proto? Wouldn't doing that mean
proto-gen-krpc won't depend on kudu_common?
http://gerrit.cloudera.org:8080/#/c/3192/6/src/kudu/rpc/protoc-gen-krpc.cc
File src/kudu/rpc/protoc-gen-krpc.cc:
Line 46: #include "kudu/common/common.pb.h"
Nit: should come before gutil.
Line 172:
Nit: extra line?
http://gerrit.cloudera.org:8080/#/c/3192/6/src/kudu/rpc/rpc_context.h
File src/kudu/rpc/rpc_context.h:
Line 178: bool AreResultsTracked() const { return result_tracker_.get() !=
nullptr; }
I don't think this is necessary given the below method, which is very easy to
use to test for the presence of a tracker.
Line 180: const scoped_refptr GetResultTracker() const {
Shouldn't this return a cref and return the result_tracker_ as-is, instead of
calling get()?
Also, call it result_tracker()?
http://gerrit.cloudera.org:8080/#/c/3192/6/src/kudu/rpc/service_if.cc
File src/kudu/rpc/service_if.cc:
Line 96: if (!call->header().has_request_id()) {
: ctx = new RpcContext(call, req.get(), resp, nullptr);
: } else {
: ctx = new RpcContext(call, req.get(), resp,
method_info->result_tracker);
: }
Convert into a ternary:
RpcContext* ctx = new RpcContext(call, req.get(), resp,
call->header().has_request_id() ? method_info->result_tracker : nullptr);
Line 103: ResultTracker::RpcState state =
ctx->GetResultTracker()->CheckDuplicate(
It's interesting that 'req' isn't considered at all in CheckDuplicate(). We're
relying entirely on seq_no() instead, I believe.
Line 116: LOG(FATAL) << "Unknown state.";
Log the state too.
--
To view, visit http://gerrit.cloudera.org:8080/3192
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1d624810350feceefe244e0319e22cac241bf0d6
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] Allow to set RequestId in the RPC RequestHeader
Adar Dembo has posted comments on this change. Change subject: Allow to set RequestId in the RPC RequestHeader .. Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/3179/2/src/kudu/rpc/rpc_controller.h File src/kudu/rpc/rpc_controller.h: Line 29: #include "kudu/rpc/rpc_header.pb.h" If you're storing unique_ptrs to it, you can do a forward declaration of RequestIdPB and not need this inclusion. Line 108: void SetRequestIdPB(std::unique_ptr request_id); Documentation? Why would I set this? Line 179: // The id of this request. How long does this last? Looks like when a proxy call is made using this controller, it is moved out? -- To view, visit http://gerrit.cloudera.org:8080/3179 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I92566250683ee69fad57a9ae694842d4a0b17ab4 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Add unique id generation to the client
Adar Dembo has posted comments on this change.
Change subject: Add unique id generation to the client
..
Patch Set 11:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/3077/11/src/kudu/integration-tests/client-stress-test.cc
File src/kudu/integration-tests/client-stress-test.cc:
Line 42: namespace client {
> because I needed to make this test a friend class of client. I tried to mak
in client.h:
namespace kudu {
class ClientStressTest_TestUniqueClientIds_Test;
...
}
class KuduClient {
private:
friend class ClientStressTest_TestUniqueClientIds_Test;
}
The closest analogy I see to this is in how meta_cache.h handles
TestMasterLookupPermits.
--
To view, visit http://gerrit.cloudera.org:8080/3077
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie752ab8337cc76c3be1536958c24a8b6c5794650
Gerrit-PatchSet: 11
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: David Ribeiro Alves
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] Add unique id generation to the client
Adar Dembo has posted comments on this change. Change subject: Add unique id generation to the client .. Patch Set 10: (1 comment) http://gerrit.cloudera.org:8080/#/c/3077/10//COMMIT_MSG Commit Message: Line 10: an uuid. The uuid is generated when the client is builgt, by a static Nit: built -- To view, visit http://gerrit.cloudera.org:8080/3077 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie752ab8337cc76c3be1536958c24a8b6c5794650 Gerrit-PatchSet: 10 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Add a RpcContext::RespondFailure() method
Adar Dembo has posted comments on this change. Change subject: Add a RpcContext::RespondFailure() method .. Patch Set 6: This patch is incredibly confusing stand alone. I think it's better served being part of the integration patch, though you may want to collect a second opinion before making the change. -- To view, visit http://gerrit.cloudera.org:8080/3191 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9f1387ba0f837046a8056e77f73a3982f06c73a2 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Add a ResultTracker class that will track server side results
Adar Dembo has posted comments on this change. Change subject: Add a ResultTracker class that will track server side results .. Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/3190/3/src/kudu/rpc/result_tracker.cc File src/kudu/rpc/result_tracker.cc: Line 40: lock_guard l(&lock_); > no, the lock protects access to clients_ and to the ClientState that belong I suspect it will be, as nearly every RPC will call CheckDuplicate(), no? Line 78: LOG(FATAL) << "Wrong state"; > the whole point is that we only call this once per RPC, if the caller got N OK, could we at least log what the wrong state was? Line 119: ClientState* client_state = FindOrDie(clients_, client_id); : CompletionRecord* completion_record = FindOrDie(client_state->completion_records, : sequence_number); > why? if we are responding to an RPC that is not being tracked then it's an Well, this goes back to the lack of documentation thing. :) You're designing a module with "hard" boundaries. That is, if one of us misuses the ResultTracker, it crashes. Without context and in isolation, I tend to assume new modules should have "soft" boundaries, where misuse leads to returned errors. And to be fair, there are advantages to soft boundaries: it's easier to test such a module in isolation. Anyway, there's nothing inherently wrong with hard boundaries, you just need to be very clear in the interface regarding what happens in the event of misuse. -- To view, visit http://gerrit.cloudera.org:8080/3190 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6718951a9998a6c9b0db35e8f09ff8304591e8b1 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: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Add a ResultTracker class that will track server side results
Adar Dembo has posted comments on this change. Change subject: Add a ResultTracker class that will track server side results .. Patch Set 6: (4 comments) http://gerrit.cloudera.org:8080/#/c/3190/6/src/kudu/rpc/result_tracker.h File src/kudu/rpc/result_tracker.h: Line 45: // of the RPC at that the corresponding server function should be executed. If any other value is Nit: "at that the" --> "and that the" Line 49: // client appropriately, Nit: trailing comma here? Line 55: // If, on the other hand, if execution of the server function is not successful then one of Nit: too many 'ifs' here. Line 96: RpcState CheckDuplicate(const std::string& client_id, I'd rename the function slightly to imply that it's not just a stateless "check". That is, it'll actually register the caller's input internally such that RecordCompletionAndRespond() does something with it. -- To view, visit http://gerrit.cloudera.org:8080/3190 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6718951a9998a6c9b0db35e8f09ff8304591e8b1 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Move the PbTracer in rpc_context.cc to pb_util
Adar Dembo has posted comments on this change. Change subject: Move the PbTracer in rpc_context.cc to pb_util .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3194 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib601deea9fe0dcc6e2428416a6993c7567ff7335 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: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Add a request tracker to track client rpc sequence numbers
Adar Dembo has posted comments on this change. Change subject: Add a request tracker to track client rpc sequence numbers .. Patch Set 8: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3078 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I23201625ca02f244dc94205d88dabc01608de471 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Add a ResultTracker class that will track server side results
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3190 to look at the new patch set (#6). Change subject: Add a ResultTracker class that will track server side results .. Add a ResultTracker class that will track server side results This adds the initial version of the ResultTracker class that will be responsible for tracking responses when we want exactly once call semantics. While this is minimally working, i.e it's able to track responses and can be used for exactly once semantics, it's not complete. Future patches will address the missing functionality. Still missing are: - Time based client state cleaning. - Watermark based per client state cleaning. I initially had a unit test for this class, but it relied on templating out the RpcContext (since it's not easy to build one for a unit test and it's not clear what it would do) which became problematic as the class grew. So I decided to add a new rpc-stress-tess that will test this class within the rpc framework. I feel it's easier to do it that way and that, since the client is not being used, the test can still be very straight forward. This test is in a followup patch as it can only run after the integration with the rest of the rpc subsystem. Change-Id: I6718951a9998a6c9b0db35e8f09ff8304591e8b1 --- M src/kudu/rpc/CMakeLists.txt A src/kudu/rpc/result_tracker.cc A src/kudu/rpc/result_tracker.h M src/kudu/rpc/rpc_context.h 4 files changed, 355 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/3190/6 -- To view, visit http://gerrit.cloudera.org:8080/3190 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6718951a9998a6c9b0db35e8f09ff8304591e8b1 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1267: Calling getString() on RowResult for wrong type gave weird exception
Adar Dembo has posted comments on this change.
Change subject: KUDU-1267: Calling getString() on RowResult for wrong type gave
weird exception
..
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/3102/3/java/kudu-client/src/test/java/org/kududb/client/TestKuduClient.java
File java/kudu-client/src/test/java/org/kududb/client/TestKuduClient.java:
Line 154: assertTrue("Scanner sould had returned row",
scanner.hasMoreRows())
Nit: "Scanner should have returned row"
--
To view, visit http://gerrit.cloudera.org:8080/3102
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Idc8c6e5e382ca9f6280072f59acf4257fd13fc6b
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Ted Malaska
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] Add a ResultTracker class that will track server side results
David Ribeiro Alves has posted comments on this change.
Change subject: Add a ResultTracker class that will track server side results
..
Patch Set 3:
(25 comments)
http://gerrit.cloudera.org:8080/#/c/3190/3//COMMIT_MSG
Commit Message:
Line 14: will addressing the missing functionality.
> Nit: will address
Done
Line 21: and it's not clear waht it would do) which became problematic as the
> Nit: what
Done
http://gerrit.cloudera.org:8080/#/c/3190/3/src/kudu/rpc/result_tracker.cc
File src/kudu/rpc/result_tracker.cc:
Line 40: lock_guard l(&lock_);
> AFAICT lock_ only protects clients_, so don't we only need to hold it betwe
no, the lock protects access to clients_ and to the ClientState that belongs to
each client. If we deem this a hotspot we can improve by keeping a per
ClientState lock, but I'd rather do that later, if needed.
Line 46: client_state->last_heard_from = MonoTime::Now(MonoTime::COARSE);
> This suggests that ClientState's constructor should not initialize last_hea
right, Done
Line 55: completion_record->ongoing_rpcs.push_back({response, context});
> DCHECK that context isn't null?
Done
Line 78: LOG(FATAL) << "Wrong state";
> Seems pretty easy to end up here if I call CheckDuplicate() again after get
the whole point is that we only call this once per RPC, if the caller got NEW
before it should have just executed the RPC, there is no reason to call this
again.
Line 119: ClientState* client_state = FindOrDie(clients_, client_id);
: CompletionRecord* completion_record =
FindOrDie(client_state->completion_records,
: sequence_number);
> Seems heavy-handed to die instead of returning a bad status if client_id or
why? if we are responding to an RPC that is not being tracked then it's an
error. I'd rather catch those through a crash since it's incorrect server (not
client) behavior
Line 139: {
> Nit: don't need an extra scope here.
Done
Line 163:
> Nit: got an extra empty line here. Check below too.
Done
Line 188: void ResultTracker::ProcessAck(
> So why include it in the patch at all?
true, removed
http://gerrit.cloudera.org:8080/#/c/3190/3/src/kudu/rpc/result_tracker.h
File src/kudu/rpc/result_tracker.h:
Line 34: typedef rpc::RequestTracker::SequenceNumber SequenceNumber;
> Hmm, doing this here means it'll leak into any includer of this header, and
moved it inside the ResultTracker.
Line 36: // A ResultTracker for RPC results.
> No real class-level comments? What about some design considerations? Or usa
true, my bad. In my defense this was kind of wip-py and wanted to get the first
round of comments. Note that this is not a class for general use. For the big
majority of RPCs this sits behind an RpcContext and server code never has to
use it directly.
This being said I did expand this comment to reflect normal usage and lifecycle.
Line 38: RpcContext is a template argument to allow this class to be tested
without having to setup the
: // whole RPC infrastructure.
> Not the case anymore, right?
Done
Line 45: enum RpcState {
> Nit: would be nice to explain in comments what the different states mean.
Done
Line 51: // Checks and returns the status of the RPC.
> Nit: empty line before.
Done
Line 53: // If the RpcState is anything else it will taked care of internally.
> Nit: "it will taked care of internally" --> should be "taken care of", but
Done
Line 54: // If 'response' and 'context' are non null, they are stored so that
we can later reply
> How can they be null? Isn't that invalid?
expanded the class comment to cover this.
Line 68: // Responds to all RPCs identified 'client_id' and 'sequence_number'
with the same response,
> Nit: identified by
Done
Line 73: void FailAndRespond(const std::string& client_id,
> Nit: overloads are typically frowned on, could you find a naming scheme for
these just correspond to the signatures in RpcContext, which also overloads, so
I think we should keep them the same.
the exception is RespondApplicationError() which I guess we could change, but I
don't think that includes any more information.
Line 86: // Trims the state for a given client.
> This is vague, can you add more detail?
Done
Line 104: ClientState() : last_heard_from(MonoTime::Now(MonoTime::COARSE))
{}
> Nit: we generally default to FINE, saving COARSE for only those rare situat
Done, though AFAICT not sure that's totally true. For instance scanner TLL is
measured with COARSE and it's not performance sensitive
Line 106: std::map completion_records;
> Does this map own the completion records? If so, should use unique_ptr. If
it does, but honestly I think that the ownership semantics are pretty clear and
this will sit in the critical path, so I'd prefer to avoid the extra pointer
indirection. I don't feel super strong about it though, I just don't think it
buys
[kudu-CR] Dependency on Hadoop test classes causes Javadoc warnings and build failure
Will Berkeley has posted comments on this change. Change subject: Dependency on Hadoop test classes causes Javadoc warnings and build failure .. Patch Set 2: What unit test do you mean? Neither JarFinder or the function that calls it has a unit test. Do you want to add a unit test for it? -- To view, visit http://gerrit.cloudera.org:8080/3199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I297f46d930f274666db9d503cf8897fe509470da Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: No
[kudu-CR] KUDU-1267: Calling getString() on RowResult for wrong type gave weird exception
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3102 to look at the new patch set (#3). Change subject: KUDU-1267: Calling getString() on RowResult for wrong type gave weird exception .. KUDU-1267: Calling getString() on RowResult for wrong type gave weird exception Change-Id: Idc8c6e5e382ca9f6280072f59acf4257fd13fc6b --- M java/kudu-client/src/main/java/org/kududb/client/RowResult.java M java/kudu-client/src/test/java/org/kududb/client/TestKuduClient.java 2 files changed, 54 insertions(+), 23 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/02/3102/3 -- To view, visit http://gerrit.cloudera.org:8080/3102 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Idc8c6e5e382ca9f6280072f59acf4257fd13fc6b Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Ted Malaska Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-1267: Calling getString() on RowResult for wrong type gave weird exception
Adar Dembo has posted comments on this change.
Change subject: KUDU-1267: Calling getString() on RowResult for wrong type gave
weird exception
..
Patch Set 1:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/3102/1/java/kudu-client/src/main/java/org/kududb/client/RowResult.java
File java/kudu-client/src/main/java/org/kududb/client/RowResult.java:
Line 568: }
> What happened here?
I think you missed this as well.
http://gerrit.cloudera.org:8080/#/c/3102/1/java/kudu-client/src/test/java/org/kududb/client/TestKuduClient.java
File java/kudu-client/src/test/java/org/kududb/client/TestKuduClient.java:
Line 152: //Do negative testing on string type
> Nit: // Do negative...
I think you missed this one: please add an empty space between "//" and "Do".
Line 154: if (scanner.hasMoreRows()) {
> If this is false, we won't actually run the remainder of the test. Shouldn'
You didn't address this one either.
Line 157: boolean exceptionThrown = false;
: try {
: next.getInt("c2");
: } catch (ClassCastException e) {
: exceptionThrown = true;
: }
: assertTrue("ClassCastException was not thrown when accessing a
string column with getInt", exceptionThrown);
: }
> Here's a simpler way to do it:
Nor this one.
--
To view, visit http://gerrit.cloudera.org:8080/3102
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Idc8c6e5e382ca9f6280072f59acf4257fd13fc6b
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Ted Malaska
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1267: Calling getString() on RowResult for wrong type gave weird exception
Adar Dembo has posted comments on this change. Change subject: KUDU-1267: Calling getString() on RowResult for wrong type gave weird exception .. Patch Set 2: Thanks for making some changes. There were several suggestions you missed; I flagged them for you. Also, you missed the larger suggestion I made the first time around: "your editor appears to have made many indentation changes to RowResult.java. AFAICT they don't make the file any more or less compliant with a coding style, so could you please revert them? They're just unnecessary noise.". -- To view, visit http://gerrit.cloudera.org:8080/3102 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idc8c6e5e382ca9f6280072f59acf4257fd13fc6b Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Ted Malaska Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] Reduce verbosity of Java logs
Jean-Daniel Cryans has posted comments on this change. Change subject: Reduce verbosity of Java logs .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/3203/1/java/kudu-client/src/test/resources/log4j.properties File java/kudu-client/src/test/resources/log4j.properties: Line 18: INFO > What messages will we be missing by requiring INFO or higher? Which thread sent which RPC to which server, and what the response looked like. Also, you're missing how retries are done. -- To view, visit http://gerrit.cloudera.org:8080/3203 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibf26651908f533859f16cb4293a06157f9577c7d Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] Reduce verbosity of Java logs
Adar Dembo has posted comments on this change.
Change subject: Reduce verbosity of Java logs
..
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/3203/1/java/kudu-client/src/test/java/org/kududb/client/MiniKuduCluster.java
File java/kudu-client/src/test/java/org/kududb/client/MiniKuduCluster.java:
Line 398: public ProcessInputStreamLogPrinterRunnable(int port, InputStream
is) {
If you're going to proceed with this change (and I think you should), could you
unplumb the 'port' argument from these?
http://gerrit.cloudera.org:8080/#/c/3203/1/java/kudu-client/src/test/resources/log4j.properties
File java/kudu-client/src/test/resources/log4j.properties:
Line 18: INFO
> This I'm less sure about. It's hard to debug without DEBUG.
What messages will we be missing by requiring INFO or higher?
--
To view, visit http://gerrit.cloudera.org:8080/3203
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibf26651908f533859f16cb4293a06157f9577c7d
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] Integrate the ResultTracker into the rpc subsystem and add a test
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3192 to look at the new patch set (#6). Change subject: Integrate the ResultTracker into the rpc subsystem and add a test .. Integrate the ResultTracker into the rpc subsystem and add a test This integrates the ResultTracker into the rpc subsystem by allowing to specify a new mehtod option when defining rpc service methods. When this method option is specificed _and_ the call's rpc header has a RequestIdPB the results for the call will be tracked and the call may have exactly once semantics. This allows to have the simplest form of exactly once semantics for single server calls. Of course limitations apply, like response persistence across restarts is not supported, neither is propagating/rebuilding responses to/on replicas for replicated calls. If support for this is needed it will have to be done ad-hoc, case by case. Change-Id: I1d624810350feceefe244e0319e22cac241bf0d6 --- M src/kudu/common/common.proto M src/kudu/rpc/CMakeLists.txt M src/kudu/rpc/inbound_call.cc M src/kudu/rpc/protoc-gen-krpc.cc A src/kudu/rpc/rpc-stress-test.cc M src/kudu/rpc/rpc-test-base.h M src/kudu/rpc/rpc_context.cc M src/kudu/rpc/rpc_context.h M src/kudu/rpc/rtest.proto M src/kudu/rpc/service_if.cc M src/kudu/rpc/service_if.h 11 files changed, 324 insertions(+), 37 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/3192/6 -- To view, visit http://gerrit.cloudera.org:8080/3192 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1d624810350feceefe244e0319e22cac241bf0d6 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Add a ResultTracker class that will track server side results
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3190 to look at the new patch set (#5). Change subject: Add a ResultTracker class that will track server side results .. Add a ResultTracker class that will track server side results This adds the initial version of the ResultTracker class that will be responsible for tracking responses when we want exactly once call semantics. While this is minimally working, i.e it's able to track responses and can be used for exactly once semantics, it's not complete. Future patches will addressing the missing functionality. Still missing are: - Time based client state cleaning. - Watermark based per client state cleaning. I initially had a unit test for this class, but it relied on templating out the RpcContext (since it's not easy to build one for a unit test and it's not clear waht it would do) which became problematic as the class grew. So I decided to add a new rpc-stress-tess that will test this class within the rpc framework. I feel it's easier to do it that way and that, since the client is not being used, the test can still be very straight forward. This test is in a followup patch as it can only run after the integration with the rest of the rpc subsystem. Change-Id: I6718951a9998a6c9b0db35e8f09ff8304591e8b1 --- M src/kudu/rpc/CMakeLists.txt A src/kudu/rpc/result_tracker.cc A src/kudu/rpc/result_tracker.h M src/kudu/rpc/rpc_context.h 4 files changed, 340 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/3190/5 -- To view, visit http://gerrit.cloudera.org:8080/3190 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6718951a9998a6c9b0db35e8f09ff8304591e8b1 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Add a RpcContext::RespondFailure() method
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3191 to look at the new patch set (#6). Change subject: Add a RpcContext::RespondFailure() method .. Add a RpcContext::RespondFailure() method In the case of certain errors we use RpcContext::RespondSuccess() because setting the error is call dependent and it was done on the call's request. Since we'll be relying on the RpcContext to call the appropriate method on the ResultTracker, on the common case, we'd still be marking these error cases as successful RPCs whose results we keep. This is problematic for various reasons, the main one being that if a client gets a transient error back (e.g. the call was throttled) all subsequent attemtpts at the same call will get the same result. To avoid this we need to be able to flag that a call failed even if we don't need anything else done to the response. To this goal this patch adds RpcContext::RespondFailure(), which currently does nothing else besides call RespondSuccess(), but will do so when the result tracker is integrated in. Change-Id: I9f1387ba0f837046a8056e77f73a3982f06c73a2 --- M src/kudu/rpc/rpc_context.cc M src/kudu/rpc/rpc_context.h 2 files changed, 16 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/91/3191/6 -- To view, visit http://gerrit.cloudera.org:8080/3191 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9f1387ba0f837046a8056e77f73a3982f06c73a2 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Dependency on Hadoop test classes causes Javadoc warnings and build failure
Jean-Daniel Cryans has posted comments on this change. Change subject: Dependency on Hadoop test classes causes Javadoc warnings and build failure .. Patch Set 2: Can you also pull its unit test? -- To view, visit http://gerrit.cloudera.org:8080/3199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I297f46d930f274666db9d503cf8897fe509470da Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] Reduce verbosity of Java logs
Jean-Daniel Cryans has posted comments on this change. Change subject: Reduce verbosity of Java logs .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/3203/1/java/kudu-client/src/test/resources/log4j.properties File java/kudu-client/src/test/resources/log4j.properties: Line 18: INFO This I'm less sure about. It's hard to debug without DEBUG. -- To view, visit http://gerrit.cloudera.org:8080/3203 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibf26651908f533859f16cb4293a06157f9577c7d Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] Dependency on Hadoop test classes causes Javadoc warnings and build failure
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3199 to look at the new patch set (#2). Change subject: Dependency on Hadoop test classes causes Javadoc warnings and build failure .. Dependency on Hadoop test classes causes Javadoc warnings and build failure This patches fixes a problem introduced by KUDU-733, where Javadoc warnings, caused by a mis-ordering of dependencies due to dependence on JarFinder in hadoop-common for tests, were failing builds. The dependency on hadoop-common was eliminated by forking our own JarFinder class from HBase's JarFinder, which was forked from hadoop-common's JarFinder as part of HBASE-9003. Change-Id: I297f46d930f274666db9d503cf8897fe509470da --- M java/kudu-mapreduce/pom.xml A java/kudu-mapreduce/src/main/java/org/kududb/mapreduce/JarFinder.java M java/kudu-mapreduce/src/main/java/org/kududb/mapreduce/KuduTableMapReduceUtil.java 3 files changed, 181 insertions(+), 10 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/99/3199/2 -- To view, visit http://gerrit.cloudera.org:8080/3199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I297f46d930f274666db9d503cf8897fe509470da Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Reduce verbosity of Java logs
Hello Jean-Daniel Cryans, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/3203 to review the following change. Change subject: Reduce verbosity of Java logs .. Reduce verbosity of Java logs This commit changes the logging level of the Java tests from DEBUG to INFO, and changes the logging format in order to make it more compact. Date and originating class have been removed from log lines, however the time, originating filename, and line remain. The minicluster has been changed to use the relative binary name plus port as the thread name instead of the full path to the binary (e.g. kudu-master:7051 instead of /home/dan/kudu/build/debug/bin/kudu-master). The result is that a typical log line originating from the mini cluster has been reduced from 349 columns to 214. Change-Id: Ibf26651908f533859f16cb4293a06157f9577c7d --- M java/kudu-client/src/test/java/org/kududb/client/MiniKuduCluster.java M java/kudu-client/src/test/resources/log4j.properties M java/kudu-spark/src/test/resources/log4j.properties 3 files changed, 7 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/03/3203/1 -- To view, visit http://gerrit.cloudera.org:8080/3203 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ibf26651908f533859f16cb4293a06157f9577c7d Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans
[kudu-CR] Add a ResultTracker class that will track server side results
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3190 to look at the new patch set (#4). Change subject: Add a ResultTracker class that will track server side results .. Add a ResultTracker class that will track server side results This adds the initial version of the ResultTracker class that will be responsible for tracking responses when we want exactly once call semantics. While this is minimally working, i.e it's able to track responses and can be used for exactly once semantics, it's not complete. Future patches will addressing the missing functionality. Still missing are: - Time based client state cleaning. - Watermark based per client state cleaning. I initially had a unit test for this class, but it relied on templating out the RpcContext (since it's not easy to build one for a unit test and it's not clear waht it would do) which became problematic as the class grew. So I decided to add a new rpc-stress-tess that will test this class within the rpc framework. I feel it's easier to do it that way and that, since the client is not being used, the test can still be very straight forward. This test is in a followup patch as it can only run after the integration with the rest of the rpc subsystem. Change-Id: I6718951a9998a6c9b0db35e8f09ff8304591e8b1 --- M src/kudu/rpc/CMakeLists.txt A src/kudu/rpc/result_tracker.cc A src/kudu/rpc/result_tracker.h M src/kudu/rpc/rpc_context.h 4 files changed, 341 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/3190/4 -- To view, visit http://gerrit.cloudera.org:8080/3190 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6718951a9998a6c9b0db35e8f09ff8304591e8b1 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Integrate the ResultTracker into the rpc subsystem and add a test
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3192 to look at the new patch set (#5). Change subject: Integrate the ResultTracker into the rpc subsystem and add a test .. Integrate the ResultTracker into the rpc subsystem and add a test This integrates the ResultTracker into the rpc subsystem by allowing to specify a new mehtod option when defining rpc service methods. When this method option is specificed _and_ the call's rpc header has a RequestIdPB the results for the call will be tracked and the call may have exactly once semantics. This allows to have the simplest form of exactly once semantics for single server calls. Of course limitations apply, like response persistence across restarts is not supported, neither is propagating/rebuilding responses to/on replicas for replicated calls. If support for this is needed it will have to be done ad-hoc, case by case. Change-Id: I1d624810350feceefe244e0319e22cac241bf0d6 --- M src/kudu/common/common.proto M src/kudu/rpc/CMakeLists.txt M src/kudu/rpc/inbound_call.cc M src/kudu/rpc/protoc-gen-krpc.cc A src/kudu/rpc/rpc-stress-test.cc M src/kudu/rpc/rpc-test-base.h M src/kudu/rpc/rpc_context.cc M src/kudu/rpc/rpc_context.h M src/kudu/rpc/rtest.proto M src/kudu/rpc/service_if.cc M src/kudu/rpc/service_if.h 11 files changed, 326 insertions(+), 37 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/3192/5 -- To view, visit http://gerrit.cloudera.org:8080/3192 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1d624810350feceefe244e0319e22cac241bf0d6 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Add a RpcContext::RespondFailure() method
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3191 to look at the new patch set (#5). Change subject: Add a RpcContext::RespondFailure() method .. Add a RpcContext::RespondFailure() method In the case of certain errors we use RpcContext::RespondSuccess() because setting the error is call dependent and it was done on the call's request. Since we'll be relying on the RpcContext to call the appropriate method on the ResultTracker, on the common case, we'd still be marking these error cases as successful RPCs whose results we keep. This is problematic for various reasons, the main one being that if a client gets a transient error back (e.g. the call was throttled) all subsequent attemtpts at the same call will get the same result. To avoid this we need to be able to flag that a call failed even if we don't need anything else done to the response. To this goal this patch adds RpcContext::RespondFailure(), which currently does nothing else besides call RespondSuccess(), but will do so when the result tracker is integrated in. Change-Id: I9f1387ba0f837046a8056e77f73a3982f06c73a2 --- M src/kudu/rpc/rpc_context.cc M src/kudu/rpc/rpc_context.h 2 files changed, 16 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/91/3191/5 -- To view, visit http://gerrit.cloudera.org:8080/3191 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9f1387ba0f837046a8056e77f73a3982f06c73a2 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Move the PbTracer in rpc_context.cc to pb_util
David Ribeiro Alves has posted comments on this change. Change subject: Move the PbTracer in rpc_context.cc to pb_util .. Patch Set 2: Verified+1 unrelated flake org.kududb.client.TestKuduTable.testGetLocations -- To view, visit http://gerrit.cloudera.org:8080/3194 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib601deea9fe0dcc6e2428416a6993c7567ff7335 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: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] python: add support for specifying partitioning
Wes McKinney has posted comments on this change. Change subject: python: add support for specifying partitioning .. Patch Set 1: This seems reasonable to me. If you anticipate partitioning options growing in complexity, then packaging this up in a configurable object makes sense (versus adding more and more parameters to the create_table method). -- To view, visit http://gerrit.cloudera.org:8080/3196 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie92c897c559fb3070240c51ceb03fe7c2ccd17ba Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Wes McKinney Gerrit-HasComments: No
[kudu-CR] Add request id and sequence number to the rpc request header
David Ribeiro Alves has posted comments on this change. Change subject: Add request id and sequence number to the rpc request header .. Patch Set 6: Keeping the +2 since this was just a rebase. -- To view, visit http://gerrit.cloudera.org:8080/3079 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I30b07f070f8e80aaad982ccfe71ffb7e22a6a703 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Add request id and sequence number to the rpc request header
David Ribeiro Alves has posted comments on this change. Change subject: Add request id and sequence number to the rpc request header .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3079 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I30b07f070f8e80aaad982ccfe71ffb7e22a6a703 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1267: Calling getString() on RowResult for wrong type gave weird exception
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3102 to look at the new patch set (#2). Change subject: KUDU-1267: Calling getString() on RowResult for wrong type gave weird exception .. KUDU-1267: Calling getString() on RowResult for wrong type gave weird exception Change-Id: Idc8c6e5e382ca9f6280072f59acf4257fd13fc6b --- M java/kudu-client/src/main/java/org/kududb/client/RowResult.java M java/kudu-client/src/test/java/org/kududb/client/TestKuduClient.java 2 files changed, 56 insertions(+), 23 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/02/3102/2 -- To view, visit http://gerrit.cloudera.org:8080/3102 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Idc8c6e5e382ca9f6280072f59acf4257fd13fc6b Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Ted Malaska Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-1267: Calling getString() on RowResult for wrong type gave weird exception
Ted Malaska has restored this change. Change subject: KUDU-1267: Calling getString() on RowResult for wrong type gave weird exception .. Restored I am restoring -- To view, visit http://gerrit.cloudera.org:8080/3102 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: restore Gerrit-Change-Id: Idc8c6e5e382ca9f6280072f59acf4257fd13fc6b Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Ted Malaska Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-1267: Calling getString() on RowResult for wrong type gave weird exception
Ted Malaska has abandoned this change. Change subject: KUDU-1267: Calling getString() on RowResult for wrong type gave weird exception .. Abandoned Killing this one -- To view, visit http://gerrit.cloudera.org:8080/3143 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: abandon Gerrit-Change-Id: I6147391fb00e58c47ff07ae0d5f0cb5e3a708726 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Ted Malaska Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Remove default table partitioning
Dan Burkert has posted comments on this change. Change subject: Remove default table partitioning .. Patch Set 9: Looks like only the Python tests are still failing now. -- To view, visit http://gerrit.cloudera.org:8080/3131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7021d7950f8dbb4918503ea6fab2e6ee35076064 Gerrit-PatchSet: 9 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Misty Stanley-Jones Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Fix typo in RPC design doc
Dan Burkert has submitted this change and it was merged. Change subject: Fix typo in RPC design doc .. Fix typo in RPC design doc Change-Id: I43ed70a07b5b54bffb72997e23ec02e3d1ccaecc Reviewed-on: http://gerrit.cloudera.org:8080/3153 Tested-by: Kudu Jenkins Reviewed-by: Dan Burkert --- M docs/design-docs/rpc.md 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Dan Burkert: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/3153 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I43ed70a07b5b54bffb72997e23ec02e3d1ccaecc Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Fix typo in RPC design doc
Dan Burkert has posted comments on this change. Change subject: Fix typo in RPC design doc .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3153 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I43ed70a07b5b54bffb72997e23ec02e3d1ccaecc Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] Move the PbTracer in rpc_context.cc to pb_util
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3194 to look at the new patch set (#2). Change subject: Move the PbTracer in rpc_context.cc to pb_util .. Move the PbTracer in rpc_context.cc to pb_util We'll need to use this outside of RpcContext so this moves it to a place where it makes more sense and can be used elsewhere. Change-Id: Ib601deea9fe0dcc6e2428416a6993c7567ff7335 --- M src/kudu/rpc/rpc_context.cc M src/kudu/util/pb_util.cc M src/kudu/util/pb_util.h 3 files changed, 54 insertions(+), 36 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/94/3194/2 -- To view, visit http://gerrit.cloudera.org:8080/3194 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib601deea9fe0dcc6e2428416a6993c7567ff7335 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR](branch-0.9.x) java: use truncated randomized exponential backoff for retries
Jean-Daniel Cryans has posted comments on this change. Change subject: java: use truncated randomized exponential backoff for retries .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3198 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I79256e00cf35c072c60cd2b9034e6c1b10c18b38 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: branch-0.9.x Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR](branch-0.9.x) service_pool: only log queue overflows once per second
Jean-Daniel Cryans has posted comments on this change. Change subject: service_pool: only log queue overflows once per second .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3197 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I636ef39798f9b2ab7d8aec237285c949ec119468 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: branch-0.9.x Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] Dependency on Hadoop test classes causes impertinent Javadoc warnings
Will Berkeley has uploaded a new change for review. http://gerrit.cloudera.org:8080/3199 Change subject: Dependency on Hadoop test classes causes impertinent Javadoc warnings .. Dependency on Hadoop test classes causes impertinent Javadoc warnings Initial patch to see what Jenkins says since the tests don't run well locally for me...will update and clean up again with another patch set later. Change-Id: I297f46d930f274666db9d503cf8897fe509470da --- M java/kudu-mapreduce/pom.xml A java/kudu-mapreduce/src/main/java/org/kududb/mapreduce/JarFinder.java M java/kudu-mapreduce/src/main/java/org/kududb/mapreduce/KuduTableMapReduceUtil.java 3 files changed, 185 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/99/3199/1 -- To view, visit http://gerrit.cloudera.org:8080/3199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I297f46d930f274666db9d503cf8897fe509470da Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Will Berkeley
[kudu-CR](branch-0.9.x) service_pool: only log queue overflows once per second
Todd Lipcon has uploaded a new change for review. http://gerrit.cloudera.org:8080/3197 Change subject: service_pool: only log queue overflows once per second .. service_pool: only log queue overflows once per second These queue overflow messages are extremely frequent on stress workloads, and I found that the log spew itself caused the server to block on glog locks, compounding the problem. Spewing these messages is rarely helpful, so this patch throttles it to once a second. Change-Id: I636ef39798f9b2ab7d8aec237285c949ec119468 Reviewed-on: http://gerrit.cloudera.org:8080/3185 Tested-by: Kudu Jenkins Reviewed-by: Jean-Daniel Cryans (cherry picked from commit 298806fa32c44e986c2401fa5ac416bcbc491523) --- M src/kudu/rpc/service_pool.cc 1 file changed, 2 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/97/3197/1 -- To view, visit http://gerrit.cloudera.org:8080/3197 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I636ef39798f9b2ab7d8aec237285c949ec119468 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: branch-0.9.x Gerrit-Owner: Todd Lipcon
[kudu-CR](branch-0.9.x) java: use truncated randomized exponential backoff for retries
Todd Lipcon has uploaded a new change for review. http://gerrit.cloudera.org:8080/3198 Change subject: java: use truncated randomized exponential backoff for retries .. java: use truncated randomized exponential backoff for retries This changes the Java client to use a randomized exponential backoff, starting at 1ms, and going up to 4096ms, rather than the previous linear backoff of (500ms * retry_count). The old backoff was far too aggressive, and resulted in the Java client regularly having high percentile latencies of 500 or 1000ms even when there was spare capacity in the system. I tested this on a 70 node cluster using a read-only YCSB workload, and the 99.99th percentile latency dropped from >1s to 390ms. Combined with further in-flight changes, the 99.99th percentile dropped further to ~56ms. Change-Id: I79256e00cf35c072c60cd2b9034e6c1b10c18b38 Reviewed-on: http://gerrit.cloudera.org:8080/3184 Tested-by: Kudu Jenkins Reviewed-by: Jean-Daniel Cryans Reviewed-by: Adar Dembo (cherry picked from commit 8094b73147e0238cdfd96a141554b5344c801cdb) --- M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java 1 file changed, 3 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/3198/1 -- To view, visit http://gerrit.cloudera.org:8080/3198 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I79256e00cf35c072c60cd2b9034e6c1b10c18b38 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: branch-0.9.x Gerrit-Owner: Todd Lipcon
[kudu-CR] service_pool: only log queue overflows once per second
Todd Lipcon has submitted this change and it was merged. Change subject: service_pool: only log queue overflows once per second .. service_pool: only log queue overflows once per second These queue overflow messages are extremely frequent on stress workloads, and I found that the log spew itself caused the server to block on glog locks, compounding the problem. Spewing these messages is rarely helpful, so this patch throttles it to once a second. Change-Id: I636ef39798f9b2ab7d8aec237285c949ec119468 Reviewed-on: http://gerrit.cloudera.org:8080/3185 Tested-by: Kudu Jenkins Reviewed-by: Jean-Daniel Cryans --- M src/kudu/rpc/service_pool.cc 1 file changed, 2 insertions(+), 1 deletion(-) Approvals: Jean-Daniel Cryans: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/3185 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I636ef39798f9b2ab7d8aec237285c949ec119468 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] java: use truncated randomized exponential backoff for retries
Todd Lipcon has submitted this change and it was merged. Change subject: java: use truncated randomized exponential backoff for retries .. java: use truncated randomized exponential backoff for retries This changes the Java client to use a randomized exponential backoff, starting at 1ms, and going up to 4096ms, rather than the previous linear backoff of (500ms * retry_count). The old backoff was far too aggressive, and resulted in the Java client regularly having high percentile latencies of 500 or 1000ms even when there was spare capacity in the system. I tested this on a 70 node cluster using a read-only YCSB workload, and the 99.99th percentile latency dropped from >1s to 390ms. Combined with further in-flight changes, the 99.99th percentile dropped further to ~56ms. Change-Id: I79256e00cf35c072c60cd2b9034e6c1b10c18b38 Reviewed-on: http://gerrit.cloudera.org:8080/3184 Tested-by: Kudu Jenkins Reviewed-by: Jean-Daniel Cryans Reviewed-by: Adar Dembo --- M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java 1 file changed, 3 insertions(+), 2 deletions(-) Approvals: Jean-Daniel Cryans: Looks good to me, approved Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/3184 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I79256e00cf35c072c60cd2b9034e6c1b10c18b38 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] python: add support for specifying partitioning
Todd Lipcon has posted comments on this change. Change subject: python: add support for specifying partitioning .. Patch Set 1: Wes -- would appreciate thoughts on whether this API is reasonably "pythonic" or if there's a better way to go about it. -- To view, visit http://gerrit.cloudera.org:8080/3196 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie92c897c559fb3070240c51ceb03fe7c2ccd17ba Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Wes McKinney Gerrit-HasComments: No
[kudu-CR] python: add support for specifying partitioning
Hello Dan Burkert, Wes McKinney, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/3196 to review the following change. Change subject: python: add support for specifying partitioning .. python: add support for specifying partitioning This adds support for range and hash partitioning when creating a table from the Python client. Unfortunately, range partitioning is less-than-useful right now, since we don't yet have a way to specify the split rows themselves. I elected to defer that work since it's a bit tricky -- the PartialRow constructor currently requires a Table object, and the Table object isn't available until the table has been created. Change-Id: Ie92c897c559fb3070240c51ceb03fe7c2ccd17ba --- M python/kudu/client.pyx M python/kudu/libkudu_client.pxd M python/kudu/tests/test_client.py 3 files changed, 105 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/96/3196/1 -- To view, visit http://gerrit.cloudera.org:8080/3196 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ie92c897c559fb3070240c51ceb03fe7c2ccd17ba Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Wes McKinney
[kudu-CR] Add unique id generation to the client
David Ribeiro Alves has posted comments on this change.
Change subject: Add unique id generation to the client
..
Patch Set 11:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/3077/11/src/kudu/client/client_builder-internal.cc
File src/kudu/client/client_builder-internal.cc:
Line 26: default_rpc_timeout_(MonoDelta::FromSeconds(10)) {}
> Can you revert this change? It's just unnecessary noise.
Done
http://gerrit.cloudera.org:8080/#/c/3077/11/src/kudu/integration-tests/client-stress-test.cc
File src/kudu/integration-tests/client-stress-test.cc:
Line 42: namespace client {
> Hmm, while the test is called client-stress-test, it's part of the integrat
because I needed to make this test a friend class of client. I tried to make
the friend class in client.h refer to this test with the kudu:: prefix, but to
no avail. This is the only way I got it to work without exposing the client_id_
with an accessor.
Maybe I missed something. Ideas?
Line 279: << total_num_rejections << " memory rejections";
> The old indentation was nicer, I think.
Done
--
To view, visit http://gerrit.cloudera.org:8080/3077
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie752ab8337cc76c3be1536958c24a8b6c5794650
Gerrit-PatchSet: 11
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: David Ribeiro Alves
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] Add unique id generation to the client
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3077 to look at the new patch set (#12). Change subject: Add unique id generation to the client .. Add unique id generation to the client This adds unique id generation to the client, which takes the form of an uuid. The uuid is generated when the client is builgt, by a static OidGenerator. This also adds a test that generates 1000 clients and makes sure that the generated ids are unique. Since this test will now run all the time we should get notified if stuff becomes flaky dues to repeated ids. Change-Id: Ie752ab8337cc76c3be1536958c24a8b6c5794650 --- M src/kudu/client/client-internal.h M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/integration-tests/client-stress-test.cc 4 files changed, 27 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/3077/12 -- To view, visit http://gerrit.cloudera.org:8080/3077 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie752ab8337cc76c3be1536958c24a8b6c5794650 Gerrit-PatchSet: 12 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Allow to set RequestId in the RPC RequestHeader
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3179 to look at the new patch set (#2). Change subject: Allow to set RequestId in the RPC RequestHeader .. Allow to set RequestId in the RPC RequestHeader This allows to set a RequestId in the RpcController that will in turn be set in the RequestHeader. This doesn't have specific tests, as other patches test this functionality, but having this be stand-alone allows to work on the client and the server simultaneously. Change-Id: I92566250683ee69fad57a9ae694842d4a0b17ab4 --- M src/kudu/rpc/outbound_call.h M src/kudu/rpc/proxy.cc M src/kudu/rpc/rpc_controller.cc M src/kudu/rpc/rpc_controller.h 4 files changed, 19 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/79/3179/2 -- To view, visit http://gerrit.cloudera.org:8080/3179 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I92566250683ee69fad57a9ae694842d4a0b17ab4 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Fix typo in RPC design doc
Hello Adar Dembo, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3153 to look at the new patch set (#3). Change subject: Fix typo in RPC design doc .. Fix typo in RPC design doc Change-Id: I43ed70a07b5b54bffb72997e23ec02e3d1ccaecc --- M docs/design-docs/rpc.md 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/3153/3 -- To view, visit http://gerrit.cloudera.org:8080/3153 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I43ed70a07b5b54bffb72997e23ec02e3d1ccaecc Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Remove default table partitioning
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3131 to look at the new patch set (#9). Change subject: Remove default table partitioning .. Remove default table partitioning This commit removes the current default of creating tables with range partitioning over the primary key columns with no splits. This default is problematic because it results in a single tablet, which is a known anti-pattern. Kudu can't predict appropriate split rows without knowledge of the dataset, so creating default splits is not technically feasible. A better default to range partitioning would be to hash partition on the primary key columns with a number of buckets based on the number of tablet servers. Unfortunately, it's similarly difficult to predict an appopriate number of hash buckets with knowledge of the data set. Since changing the default would be a breaking change, and we don't currently have a bullet-proof default option, this commit changes the table creator in the C++ and Java clients to force users to explicitly specify at least range or hash partitioning. Users who really do want a table with no partitioning (a single tablet), can still explicitly set the range partition columns to an empty list and provide no split rows. Change-Id: I7021d7950f8dbb4918503ea6fab2e6ee35076064 --- M docs/release_notes.adoc M docs/schema_design.adoc M java/kudu-client-tools/src/main/java/org/kududb/mapreduce/tools/IntegrationTestBigLinkedList.java M java/kudu-client-tools/src/test/java/org/kududb/mapreduce/tools/ITImportCsv.java M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/kududb/client/CreateTableOptions.java M java/kudu-client/src/main/java/org/kududb/client/KuduClient.java M java/kudu-client/src/test/java/org/kududb/client/BaseKuduTest.java M java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduClient.java M java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduSession.java M java/kudu-client/src/test/java/org/kududb/client/TestFlexiblePartitioning.java M java/kudu-client/src/test/java/org/kududb/client/TestHybridTime.java M java/kudu-client/src/test/java/org/kududb/client/TestKuduClient.java M java/kudu-client/src/test/java/org/kududb/client/TestKuduSession.java M java/kudu-client/src/test/java/org/kududb/client/TestKuduTable.java M java/kudu-client/src/test/java/org/kududb/client/TestLeaderFailover.java M java/kudu-client/src/test/java/org/kududb/client/TestMasterFailover.java M java/kudu-client/src/test/java/org/kududb/client/TestRowErrors.java M java/kudu-client/src/test/java/org/kududb/client/TestRowResult.java M java/kudu-client/src/test/java/org/kududb/client/TestScanPredicate.java M java/kudu-client/src/test/java/org/kududb/client/TestScannerMultiTablet.java M java/kudu-client/src/test/java/org/kududb/client/TestStatistics.java M java/kudu-client/src/test/java/org/kududb/client/TestTimeouts.java M java/kudu-flume-sink/src/test/java/org/kududb/flume/sink/KuduSinkTest.java M java/kudu-mapreduce/src/test/java/org/kududb/mapreduce/ITKuduTableInputFormat.java M java/kudu-mapreduce/src/test/java/org/kududb/mapreduce/ITKuduTableOutputFormat.java M java/kudu-mapreduce/src/test/java/org/kududb/mapreduce/ITOutputFormatJob.java M java/kudu-spark/src/main/scala/org/kududb/spark/kudu/KuduContext.scala M java/kudu-spark/src/test/scala/org/kududb/spark/kudu/DefaultSourceTest.scala M java/kudu-spark/src/test/scala/org/kududb/spark/kudu/TestContext.scala M src/kudu/benchmarks/tpch/rpc_line_item_dao.cc M src/kudu/client/client-test.cc M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/predicate-test.cc M src/kudu/client/samples/sample.cc M src/kudu/integration-tests/all_types-itest.cc M src/kudu/integration-tests/alter_table-randomized-test.cc M src/kudu/integration-tests/alter_table-test.cc M src/kudu/integration-tests/create-table-itest.cc M src/kudu/integration-tests/create-table-stress-test.cc M src/kudu/integration-tests/delete_table-test.cc M src/kudu/integration-tests/full_stack-insert-scan-test.cc M src/kudu/integration-tests/fuzz-itest.cc M src/kudu/integration-tests/linked_list-test-util.h M src/kudu/integration-tests/master_failover-itest.cc M src/kudu/integration-tests/master_replication-itest.cc M src/kudu/integration-tests/remote_bootstrap-itest.cc M src/kudu/integration-tests/test_workload.cc M src/kudu/integration-tests/ts_itest-base.h M src/kudu/integration-tests/ts_tablet_manager-itest.cc M src/kudu/integration-tests/update_scan_delta_compact-test.cc M src/kudu/integration-tests/write_throttling-itest.cc M src/kudu/tools/ksck_remote-test.cc 54 files changed, 216 insertions(+), 139 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/31/3131/9 -- To view, visit http://gerrit.cloudera.org:8080/3131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Ger
[kudu-CR] Add a request tracker to track client rpc sequence numbers
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3078 to look at the new patch set (#8). Change subject: Add a request tracker to track client rpc sequence numbers .. Add a request tracker to track client rpc sequence numbers This adds a new component to the rpc subsystem: the RequestTracker. This is inspired by the RequestTracker in "Implementing Linearizability at Large Scale and Low Latency" by Colin Lee et al. and is responsible for generating new sequence numbers for rpcs and to keep track of the incomplete ones so that we're able to implement exactly-once semantics for certain rpcs. Change-Id: I23201625ca02f244dc94205d88dabc01608de471 --- M src/kudu/rpc/CMakeLists.txt A src/kudu/rpc/request_tracker-test.cc A src/kudu/rpc/request_tracker.cc A src/kudu/rpc/request_tracker.h 4 files changed, 224 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/78/3078/8 -- To view, visit http://gerrit.cloudera.org:8080/3078 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I23201625ca02f244dc94205d88dabc01608de471 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Add request id and sequence number to the rpc request header
Hello Adar Dembo, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3079 to look at the new patch set (#6). Change subject: Add request id and sequence number to the rpc request header .. Add request id and sequence number to the rpc request header This adds a new (optional) field to RpcHeader: RequestIdPB. This new field contains the information necessary to implement the "replay cache" component, including the unique id of the client and the rpc's sequence number. RequestIdPB also includes two additional "helper" fields, one to keep track of number of attempts of a particular rpc, so that we can skip the replay cache altogether in certain cases and one to let the server know that it can garbage collect completion records. Change-Id: I30b07f070f8e80aaad982ccfe71ffb7e22a6a703 --- M src/kudu/rpc/rpc_header.proto 1 file changed, 22 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/79/3079/6 -- To view, visit http://gerrit.cloudera.org:8080/3079 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I30b07f070f8e80aaad982ccfe71ffb7e22a6a703 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Add a request tracker to track client rpc sequence numbers
David Ribeiro Alves has posted comments on this change. Change subject: Add a request tracker to track client rpc sequence numbers .. Patch Set 7: (2 comments) http://gerrit.cloudera.org:8080/#/c/3078/7/src/kudu/rpc/request_tracker.h File src/kudu/rpc/request_tracker.h: Line 43: // this allows to get the lowest incomplete RPC. When this information is sent > Nit: instead of "this allows..." say "we can determine the first incomplete Done Line 44: // to the server it can use it to garbage collect rpc results that it might be > Nit: RPC, not rpc Done -- To view, visit http://gerrit.cloudera.org:8080/3078 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I23201625ca02f244dc94205d88dabc01608de471 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR](branch-0.9.x) rpcz: break out trace metrics into separate PB fields
Jean-Daniel Cryans has posted comments on this change. Change subject: rpcz: break out trace metrics into separate PB fields .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3195 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8bd1a249b2f09f8eb63bc64de55147b3d7738a9c Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: branch-0.9.x Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR](branch-0.9.x) rpcz: break out trace metrics into separate PB fields
Jean-Daniel Cryans has submitted this change and it was merged.
Change subject: rpcz: break out trace metrics into separate PB fields
..
rpcz: break out trace metrics into separate PB fields
This removes the trace metrics from the textual trace dump and instead adds
some structured metrics information to the /rpcz page. This should be easier
to read in the /rpcz output, and also easier to parse from any future
analysis tools we might build.
I updated the test and also manually verified the new output on a
CreateTableRequestPB on a local instance. Here's the metrics from that call:
"metrics": [
{
"child_path": "txn",
"key": "apply.queue_time_us",
"value": 168
},
{
"child_path": "txn",
"key": "apply.run_wall_time_us",
"value": 584
},
{
"child_path": "txn",
"key": "apply.run_cpu_time_us",
"value": 584
},
{
"child_path": "txn",
"key": "prepare.run_cpu_time_us",
"value": 684
},
{
"child_path": "txn",
"key": "prepare.queue_time_us",
"value": 168
},
{
"child_path": "txn",
"key": "prepare.run_wall_time_us",
"value": 683
},
{
"child_path": "txn",
"key": "threads_started",
"value": 2
},
{
"child_path": "txn",
"key": "thread_start_us",
"value": 201
},
{
"child_path": "txn",
"key": "num_ops",
"value": 2
},
{
"child_path": "txn",
"key": "replication_time_us",
"value": 579
}
Change-Id: I8bd1a249b2f09f8eb63bc64de55147b3d7738a9c
Reviewed-on: http://gerrit.cloudera.org:8080/3092
Tested-by: Kudu Jenkins
Reviewed-by: Jean-Daniel Cryans
Reviewed-by: Adar Dembo
(cherry picked from commit 08b66007c7eee301ba7e77ed11a7c48e3eefd72c)
Reviewed-on: http://gerrit.cloudera.org:8080/3195
---
M src/kudu/rpc/rpc-test-base.h
M src/kudu/rpc/rpc_introspection.proto
M src/kudu/rpc/rpc_stub-test.cc
M src/kudu/rpc/rpcz_store.cc
M src/kudu/util/trace.cc
M src/kudu/util/trace.h
6 files changed, 77 insertions(+), 3 deletions(-)
Approvals:
Jean-Daniel Cryans: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/3195
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I8bd1a249b2f09f8eb63bc64de55147b3d7738a9c
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: branch-0.9.x
Gerrit-Owner: Todd Lipcon
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Remove default table partitioning
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3131 to look at the new patch set (#8). Change subject: Remove default table partitioning .. Remove default table partitioning This commit removes the current default of creating tables with range partitioning over the primary key columns with no splits. This default is problematic because it results in a single tablet, which is a known anti-pattern. Kudu can't predict appropriate split rows without knowledge of the dataset, so creating default splits is not technically feasible. A better default to range partitioning would be to hash partition on the primary key columns with a number of buckets based on the number of tablet servers. Unfortunately, it's similarly difficult to predict an appopriate number of hash buckets with knowledge of the data set. Since changing the default would be a breaking change, and we don't currently have a bullet-proof default option, this commit changes the table creator in the C++ and Java clients to force users to explicitly specify at least range or hash partitioning. Users who really do want a table with no partitioning (a single tablet), can still explicitly set the range partition columns to an empty list and provide no split rows. Change-Id: I7021d7950f8dbb4918503ea6fab2e6ee35076064 --- M docs/release_notes.adoc M docs/schema_design.adoc M java/kudu-client-tools/src/main/java/org/kududb/mapreduce/tools/IntegrationTestBigLinkedList.java M java/kudu-client-tools/src/test/java/org/kududb/mapreduce/tools/ITImportCsv.java M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/kududb/client/CreateTableOptions.java M java/kudu-client/src/main/java/org/kududb/client/KuduClient.java M java/kudu-client/src/test/java/org/kududb/client/BaseKuduTest.java M java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduClient.java M java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduSession.java M java/kudu-client/src/test/java/org/kududb/client/TestFlexiblePartitioning.java M java/kudu-client/src/test/java/org/kududb/client/TestHybridTime.java M java/kudu-client/src/test/java/org/kududb/client/TestKuduClient.java M java/kudu-client/src/test/java/org/kududb/client/TestKuduSession.java M java/kudu-client/src/test/java/org/kududb/client/TestKuduTable.java M java/kudu-client/src/test/java/org/kududb/client/TestLeaderFailover.java M java/kudu-client/src/test/java/org/kududb/client/TestMasterFailover.java M java/kudu-client/src/test/java/org/kududb/client/TestRowErrors.java M java/kudu-client/src/test/java/org/kududb/client/TestRowResult.java M java/kudu-client/src/test/java/org/kududb/client/TestScanPredicate.java M java/kudu-client/src/test/java/org/kududb/client/TestScannerMultiTablet.java M java/kudu-client/src/test/java/org/kududb/client/TestStatistics.java M java/kudu-client/src/test/java/org/kududb/client/TestTimeouts.java M java/kudu-flume-sink/src/test/java/org/kududb/flume/sink/KuduSinkTest.java M java/kudu-mapreduce/src/test/java/org/kududb/mapreduce/ITKuduTableInputFormat.java M java/kudu-mapreduce/src/test/java/org/kududb/mapreduce/ITKuduTableOutputFormat.java M java/kudu-mapreduce/src/test/java/org/kududb/mapreduce/ITOutputFormatJob.java M java/kudu-spark/src/main/scala/org/kududb/spark/kudu/KuduContext.scala M java/kudu-spark/src/test/scala/org/kududb/spark/kudu/DefaultSourceTest.scala M java/kudu-spark/src/test/scala/org/kududb/spark/kudu/TestContext.scala M src/kudu/benchmarks/tpch/rpc_line_item_dao.cc M src/kudu/client/client-test.cc M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/predicate-test.cc M src/kudu/client/samples/sample.cc M src/kudu/integration-tests/all_types-itest.cc M src/kudu/integration-tests/alter_table-randomized-test.cc M src/kudu/integration-tests/alter_table-test.cc M src/kudu/integration-tests/create-table-itest.cc M src/kudu/integration-tests/create-table-stress-test.cc M src/kudu/integration-tests/delete_table-test.cc M src/kudu/integration-tests/full_stack-insert-scan-test.cc M src/kudu/integration-tests/fuzz-itest.cc M src/kudu/integration-tests/linked_list-test-util.h M src/kudu/integration-tests/master_failover-itest.cc M src/kudu/integration-tests/master_replication-itest.cc M src/kudu/integration-tests/test_workload.cc M src/kudu/integration-tests/ts_itest-base.h M src/kudu/integration-tests/ts_tablet_manager-itest.cc M src/kudu/integration-tests/update_scan_delta_compact-test.cc M src/kudu/integration-tests/write_throttling-itest.cc M src/kudu/tools/ksck_remote-test.cc 53 files changed, 215 insertions(+), 139 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/31/3131/8 -- To view, visit http://gerrit.cloudera.org:8080/3131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7021d79
[kudu-CR] Remove default table partitioning
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3131 to look at the new patch set (#7). Change subject: Remove default table partitioning .. Remove default table partitioning This commit removes the current default of creating tables with range partitioning over the primary key columns with no splits. This default is problematic because it results in a single tablet, which is a known anti-pattern. Kudu can't predict appropriate split rows without knowledge of the dataset, so creating default splits is not technically feasible. A better default to range partitioning would be to hash partition on the primary key columns with a number of buckets based on the number of tablet servers. Unfortunately, it's similarly difficult to predict an appopriate number of hash buckets with knowledge of the data set. Since changing the default would be a breaking change, and we don't currently have a bullet-proof default option, this commit changes the table creator in the C++ and Java clients to force users to explicitly specify at least range or hash partitioning. Users who really do want a table with no partitioning (a single tablet), can still explicitly set the range partition columns to an empty list and provide no split rows. Change-Id: I7021d7950f8dbb4918503ea6fab2e6ee35076064 --- M docs/release_notes.adoc M docs/schema_design.adoc M java/kudu-client-tools/src/main/java/org/kududb/mapreduce/tools/IntegrationTestBigLinkedList.java M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/kududb/client/CreateTableOptions.java M java/kudu-client/src/main/java/org/kududb/client/KuduClient.java M java/kudu-client/src/test/java/org/kududb/client/BaseKuduTest.java M java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduClient.java M java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduSession.java M java/kudu-client/src/test/java/org/kududb/client/TestFlexiblePartitioning.java M java/kudu-client/src/test/java/org/kududb/client/TestHybridTime.java M java/kudu-client/src/test/java/org/kududb/client/TestKuduClient.java M java/kudu-client/src/test/java/org/kududb/client/TestKuduSession.java M java/kudu-client/src/test/java/org/kududb/client/TestKuduTable.java M java/kudu-client/src/test/java/org/kududb/client/TestLeaderFailover.java M java/kudu-client/src/test/java/org/kududb/client/TestMasterFailover.java M java/kudu-client/src/test/java/org/kududb/client/TestRowErrors.java M java/kudu-client/src/test/java/org/kududb/client/TestRowResult.java M java/kudu-client/src/test/java/org/kududb/client/TestScanPredicate.java M java/kudu-client/src/test/java/org/kududb/client/TestScannerMultiTablet.java M java/kudu-client/src/test/java/org/kududb/client/TestStatistics.java M java/kudu-client/src/test/java/org/kududb/client/TestTimeouts.java M java/kudu-flume-sink/src/test/java/org/kududb/flume/sink/KuduSinkTest.java M java/kudu-mapreduce/src/test/java/org/kududb/mapreduce/ITKuduTableInputFormat.java M java/kudu-mapreduce/src/test/java/org/kududb/mapreduce/ITKuduTableOutputFormat.java M java/kudu-mapreduce/src/test/java/org/kududb/mapreduce/ITOutputFormatJob.java M java/kudu-spark/src/main/scala/org/kududb/spark/kudu/KuduContext.scala M java/kudu-spark/src/test/scala/org/kududb/spark/kudu/DefaultSourceTest.scala M java/kudu-spark/src/test/scala/org/kududb/spark/kudu/TestContext.scala M src/kudu/benchmarks/tpch/rpc_line_item_dao.cc M src/kudu/client/client-test.cc M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/predicate-test.cc M src/kudu/client/samples/sample.cc M src/kudu/integration-tests/all_types-itest.cc M src/kudu/integration-tests/alter_table-randomized-test.cc M src/kudu/integration-tests/alter_table-test.cc M src/kudu/integration-tests/create-table-itest.cc M src/kudu/integration-tests/create-table-stress-test.cc M src/kudu/integration-tests/delete_table-test.cc M src/kudu/integration-tests/full_stack-insert-scan-test.cc M src/kudu/integration-tests/fuzz-itest.cc M src/kudu/integration-tests/linked_list-test-util.h M src/kudu/integration-tests/master_failover-itest.cc M src/kudu/integration-tests/master_replication-itest.cc M src/kudu/integration-tests/test_workload.cc M src/kudu/integration-tests/ts_itest-base.h M src/kudu/integration-tests/ts_tablet_manager-itest.cc M src/kudu/integration-tests/update_scan_delta_compact-test.cc M src/kudu/integration-tests/write_throttling-itest.cc M src/kudu/tools/ksck_remote-test.cc 52 files changed, 211 insertions(+), 138 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/31/3131/7 -- To view, visit http://gerrit.cloudera.org:8080/3131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7021d7950f8dbb4918503ea6fab2e6ee35076064 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Bra
[kudu-CR] Remove default table partitioning
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3131 to look at the new patch set (#6). Change subject: Remove default table partitioning .. Remove default table partitioning This commit removes the current default of creating tables with range partitioning over the primary key columns with no splits. This default is problematic because it results in a single tablet, which is a known anti-pattern. Kudu can't predict appropriate split rows without knowledge of the dataset, so creating default splits is not technically feasible. A better default to range partitioning would be to hash partition on the primary key columns with a number of buckets based on the number of tablet servers. Unfortunately, it's similarly difficult to predict an appopriate number of hash buckets with knowledge of the data set. Since changing the default would be a breaking change, and we don't currently have a bullet-proof default option, this commit changes the table creator in the C++ and Java clients to force users to explicitly specify at least range or hash partitioning. Users who really do want a table with no partitioning (a single tablet), can still explicitly set the range partition columns to an empty list and provide no split rows. Change-Id: I7021d7950f8dbb4918503ea6fab2e6ee35076064 --- M docs/release_notes.adoc M docs/schema_design.adoc M java/kudu-client-tools/src/main/java/org/kududb/mapreduce/tools/IntegrationTestBigLinkedList.java M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/kududb/client/CreateTableOptions.java M java/kudu-client/src/main/java/org/kududb/client/KuduClient.java M java/kudu-client/src/test/java/org/kududb/client/BaseKuduTest.java M java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduClient.java M java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduSession.java M java/kudu-client/src/test/java/org/kududb/client/TestFlexiblePartitioning.java M java/kudu-client/src/test/java/org/kududb/client/TestHybridTime.java M java/kudu-client/src/test/java/org/kududb/client/TestKuduClient.java M java/kudu-client/src/test/java/org/kududb/client/TestKuduSession.java M java/kudu-client/src/test/java/org/kududb/client/TestKuduTable.java M java/kudu-client/src/test/java/org/kududb/client/TestLeaderFailover.java M java/kudu-client/src/test/java/org/kududb/client/TestMasterFailover.java M java/kudu-client/src/test/java/org/kududb/client/TestRowErrors.java M java/kudu-client/src/test/java/org/kududb/client/TestRowResult.java M java/kudu-client/src/test/java/org/kududb/client/TestScanPredicate.java M java/kudu-client/src/test/java/org/kududb/client/TestScannerMultiTablet.java M java/kudu-client/src/test/java/org/kududb/client/TestStatistics.java M java/kudu-client/src/test/java/org/kududb/client/TestTimeouts.java M java/kudu-flume-sink/src/test/java/org/kududb/flume/sink/KuduSinkTest.java M java/kudu-mapreduce/src/test/java/org/kududb/mapreduce/ITKuduTableInputFormat.java M java/kudu-mapreduce/src/test/java/org/kududb/mapreduce/ITKuduTableOutputFormat.java M java/kudu-mapreduce/src/test/java/org/kududb/mapreduce/ITOutputFormatJob.java M java/kudu-spark/src/main/scala/org/kududb/spark/kudu/KuduContext.scala M java/kudu-spark/src/test/scala/org/kududb/spark/kudu/DefaultSourceTest.scala M java/kudu-spark/src/test/scala/org/kududb/spark/kudu/TestContext.scala M src/kudu/benchmarks/tpch/rpc_line_item_dao.cc M src/kudu/client/client-test.cc M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/predicate-test.cc M src/kudu/client/samples/sample.cc M src/kudu/integration-tests/all_types-itest.cc M src/kudu/integration-tests/alter_table-randomized-test.cc M src/kudu/integration-tests/alter_table-test.cc M src/kudu/integration-tests/create-table-itest.cc M src/kudu/integration-tests/delete_table-test.cc M src/kudu/integration-tests/full_stack-insert-scan-test.cc M src/kudu/integration-tests/fuzz-itest.cc M src/kudu/integration-tests/linked_list-test-util.h M src/kudu/integration-tests/master_failover-itest.cc M src/kudu/integration-tests/master_replication-itest.cc M src/kudu/integration-tests/test_workload.cc M src/kudu/integration-tests/ts_itest-base.h M src/kudu/integration-tests/ts_tablet_manager-itest.cc M src/kudu/integration-tests/update_scan_delta_compact-test.cc M src/kudu/integration-tests/write_throttling-itest.cc M src/kudu/tools/ksck_remote-test.cc 51 files changed, 210 insertions(+), 138 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/31/3131/6 -- To view, visit http://gerrit.cloudera.org:8080/3131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7021d7950f8dbb4918503ea6fab2e6ee35076064 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: A
[kudu-CR] Remove default table partitioning
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3131 to look at the new patch set (#5). Change subject: Remove default table partitioning .. Remove default table partitioning This commit removes the current default of creating tables with range partitioning over the primary key columns with no splits. This default is problematic because it results in a single tablet, which is a known anti-pattern. Kudu can't predict appropriate split rows without knowledge of the dataset, so creating default splits is not technically feasible. A better default to range partitioning would be to hash partition on the primary key columns with a number of buckets based on the number of tablet servers. Unfortunately, it's similarly difficult to predict an appopriate number of hash buckets with knowledge of the data set. Since changing the default would be a breaking change, and we don't currently have a bullet-proof default option, this commit changes the table creator in the C++ and Java clients to force users to explicitly specify at least range or hash partitioning. Users who really do want a table with no partitioning (a single tablet), can still explicitly set the range partition columns to an empty list and provide no split rows. Change-Id: I7021d7950f8dbb4918503ea6fab2e6ee35076064 --- M docs/release_notes.adoc M docs/schema_design.adoc M java/kudu-client-tools/src/main/java/org/kududb/mapreduce/tools/IntegrationTestBigLinkedList.java M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/kududb/client/CreateTableOptions.java M java/kudu-client/src/main/java/org/kududb/client/KuduClient.java M java/kudu-client/src/test/java/org/kududb/client/BaseKuduTest.java M java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduClient.java M java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduSession.java M java/kudu-client/src/test/java/org/kududb/client/TestFlexiblePartitioning.java M java/kudu-client/src/test/java/org/kududb/client/TestHybridTime.java M java/kudu-client/src/test/java/org/kududb/client/TestKuduClient.java M java/kudu-client/src/test/java/org/kududb/client/TestKuduSession.java M java/kudu-client/src/test/java/org/kududb/client/TestKuduTable.java M java/kudu-client/src/test/java/org/kududb/client/TestLeaderFailover.java M java/kudu-client/src/test/java/org/kududb/client/TestMasterFailover.java M java/kudu-client/src/test/java/org/kududb/client/TestRowErrors.java M java/kudu-client/src/test/java/org/kududb/client/TestRowResult.java M java/kudu-client/src/test/java/org/kududb/client/TestScanPredicate.java M java/kudu-client/src/test/java/org/kududb/client/TestScannerMultiTablet.java M java/kudu-client/src/test/java/org/kududb/client/TestStatistics.java M java/kudu-client/src/test/java/org/kududb/client/TestTimeouts.java M java/kudu-flume-sink/src/test/java/org/kududb/flume/sink/KuduSinkTest.java M java/kudu-mapreduce/src/test/java/org/kududb/mapreduce/ITKuduTableInputFormat.java M java/kudu-mapreduce/src/test/java/org/kududb/mapreduce/ITKuduTableOutputFormat.java M java/kudu-mapreduce/src/test/java/org/kududb/mapreduce/ITOutputFormatJob.java M java/kudu-spark/src/main/scala/org/kududb/spark/kudu/KuduContext.scala M java/kudu-spark/src/test/scala/org/kududb/spark/kudu/DefaultSourceTest.scala M java/kudu-spark/src/test/scala/org/kududb/spark/kudu/TestContext.scala M src/kudu/benchmarks/tpch/rpc_line_item_dao.cc M src/kudu/client/client-test.cc M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/predicate-test.cc M src/kudu/client/samples/sample.cc M src/kudu/integration-tests/all_types-itest.cc M src/kudu/integration-tests/alter_table-randomized-test.cc M src/kudu/integration-tests/alter_table-test.cc M src/kudu/integration-tests/create-table-itest.cc M src/kudu/integration-tests/delete_table-test.cc M src/kudu/integration-tests/full_stack-insert-scan-test.cc M src/kudu/integration-tests/fuzz-itest.cc M src/kudu/integration-tests/linked_list-test-util.h M src/kudu/integration-tests/master_replication-itest.cc M src/kudu/integration-tests/test_workload.cc M src/kudu/integration-tests/ts_itest-base.h M src/kudu/integration-tests/ts_tablet_manager-itest.cc M src/kudu/integration-tests/update_scan_delta_compact-test.cc M src/kudu/integration-tests/write_throttling-itest.cc M src/kudu/tools/ksck_remote-test.cc 50 files changed, 209 insertions(+), 138 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/31/3131/5 -- To view, visit http://gerrit.cloudera.org:8080/3131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7021d7950f8dbb4918503ea6fab2e6ee35076064 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Review
[kudu-CR] Bump maven-assembly-plugin version
Dan Burkert has submitted this change and it was merged. Change subject: Bump maven-assembly-plugin version .. Bump maven-assembly-plugin version The previous version spewed useless log lines at INFO level. With this change, a simple `mvn clean verify -DskipTests` build is reduced from 4103 to 547 lines of output. Change-Id: I18760502ebe947a1757706c5dd4fdf5d5772efca Reviewed-on: http://gerrit.cloudera.org:8080/3193 Reviewed-by: Jean-Daniel Cryans Tested-by: Kudu Jenkins --- M java/pom.xml 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Jean-Daniel Cryans: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/3193 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I18760502ebe947a1757706c5dd4fdf5d5772efca Gerrit-PatchSet: 3 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] Move the PbTracer in rpc_context.cc to pb_util
Adar Dembo has posted comments on this change.
Change subject: Move the PbTracer in rpc_context.cc to pb_util
..
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/3194/1/src/kudu/util/pb_util.h
File src/kudu/util/pb_util.h:
Line 431: class PbTracer : public debug::ConvertableToTraceFormat {
Since you're making this more "public" than it was before (i.e. not buried in
rpc_context.cc), could you add comments to the methods explaining what they do,
and perhaps a class comment explaining how to use this thing?
--
To view, visit http://gerrit.cloudera.org:8080/3194
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib601deea9fe0dcc6e2428416a6993c7567ff7335
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] Add a ResultTracker class that will track server side results
Adar Dembo has posted comments on this change.
Change subject: Add a ResultTracker class that will track server side results
..
Patch Set 3:
(25 comments)
I did a light first pass.
Overall I think this needs more documentation; I'd have absolutely no idea how
to use a ResultTracker without 1) looking at the implementation, and 2) looking
at the other patches. Ideally the header should give enough information for
someone to use the class safely.
http://gerrit.cloudera.org:8080/#/c/3190/3//COMMIT_MSG
Commit Message:
Line 14: will addressing the missing functionality.
Nit: will address
Line 21: and it's not clear waht it would do) which became problematic as the
Nit: what
http://gerrit.cloudera.org:8080/#/c/3190/3/src/kudu/rpc/result_tracker.cc
File src/kudu/rpc/result_tracker.cc:
Line 40: lock_guard l(&lock_);
AFAICT lock_ only protects clients_, so don't we only need to hold it between
L41 and L45?
Same comment applies to other usages of lock_, more or less.
Line 46: client_state->last_heard_from = MonoTime::Now(MonoTime::COARSE);
This suggests that ClientState's constructor should not initialize
last_heard_from, since we're guaranteed to set it here any time a new one is
created.
Also, use FINE.
Line 55: completion_record->ongoing_rpcs.push_back({response, context});
DCHECK that context isn't null?
Line 78: LOG(FATAL) << "Wrong state";
Seems pretty easy to end up here if I call CheckDuplicate() again after getting
an RpcState::NEW response. Shouldn't we handle that more gracefully than
inducing a crash?
Line 119: ClientState* client_state = FindOrDie(clients_, client_id);
: CompletionRecord* completion_record =
FindOrDie(client_state->completion_records,
: sequence_number);
Seems heavy-handed to die instead of returning a bad status if client_id or
sequence_number can't be found.
Line 139: {
Nit: don't need an extra scope here.
Line 163:
Nit: got an extra empty line here. Check below too.
Line 188: void ResultTracker::ProcessAck(
So why include it in the patch at all?
http://gerrit.cloudera.org:8080/#/c/3190/3/src/kudu/rpc/result_tracker.h
File src/kudu/rpc/result_tracker.h:
Line 34: typedef rpc::RequestTracker::SequenceNumber SequenceNumber;
Hmm, doing this here means it'll leak into any includer of this header, and may
collide with other definitions of SequenceNumber. Can we do without it? AFAICT
you'll have to use RequestTracker::SequenceNumber below.
Line 36: // A ResultTracker for RPC results.
No real class-level comments? What about some design considerations? Or usage
information?
Line 38: RpcContext is a template argument to allow this class to be tested
without having to setup the
: // whole RPC infrastructure.
Not the case anymore, right?
Line 45: enum RpcState {
Nit: would be nice to explain in comments what the different states mean.
Line 51: // Checks and returns the status of the RPC.
Nit: empty line before.
Line 53: // If the RpcState is anything else it will taked care of internally.
Nit: "it will taked care of internally" --> should be "taken care of", but
beyond that it's not really clear what it means to "take care of" the RPC.
Could you clarify?
Line 54: // If 'response' and 'context' are non null, they are stored so that
we can later reply
How can they be null? Isn't that invalid?
Line 68: // Responds to all RPCs identified 'client_id' and 'sequence_number'
with the same response,
Nit: identified by
Line 73: void FailAndRespond(const std::string& client_id,
Nit: overloads are typically frowned on, could you find a naming scheme for
these three variants?
Line 86: // Trims the state for a given client.
This is vague, can you add more detail?
Line 104: ClientState() : last_heard_from(MonoTime::Now(MonoTime::COARSE))
{}
Nit: we generally default to FINE, saving COARSE for only those rare situations
where it's needed.
Line 106: std::map completion_records;
Does this map own the completion records? If so, should use unique_ptr. If not,
should explain ownership semantics somewhere.
Line 109: ResultTracker::CompletionRecord* GetAndDeleteCompletionRecord(
Shouldn't this return a unique_ptr, so it's clear ownership of the
CompletionRecord is being passed to the caller?
Line 114: void LogAndTraceFailure(RpcContext* context, const
google::protobuf::Message& msg);
Nit: can you rename the variants to avoid overloading?
Line 119: std::map clients_;
unique_ptr, I think clients_ owns ClientStates, no?
--
To view, visit http://gerrit.cloudera.org:8080/3190
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6718951a9998a6c9b0db35e8f09ff8304591e8b1
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves
Gerrit-Reviewer: Adar Dembo
[kudu-CR] rpcz: break out trace metrics into separate PB fields
Todd Lipcon has submitted this change and it was merged.
Change subject: rpcz: break out trace metrics into separate PB fields
..
rpcz: break out trace metrics into separate PB fields
This removes the trace metrics from the textual trace dump and instead adds
some structured metrics information to the /rpcz page. This should be easier
to read in the /rpcz output, and also easier to parse from any future
analysis tools we might build.
I updated the test and also manually verified the new output on a
CreateTableRequestPB on a local instance. Here's the metrics from that call:
"metrics": [
{
"child_path": "txn",
"key": "apply.queue_time_us",
"value": 168
},
{
"child_path": "txn",
"key": "apply.run_wall_time_us",
"value": 584
},
{
"child_path": "txn",
"key": "apply.run_cpu_time_us",
"value": 584
},
{
"child_path": "txn",
"key": "prepare.run_cpu_time_us",
"value": 684
},
{
"child_path": "txn",
"key": "prepare.queue_time_us",
"value": 168
},
{
"child_path": "txn",
"key": "prepare.run_wall_time_us",
"value": 683
},
{
"child_path": "txn",
"key": "threads_started",
"value": 2
},
{
"child_path": "txn",
"key": "thread_start_us",
"value": 201
},
{
"child_path": "txn",
"key": "num_ops",
"value": 2
},
{
"child_path": "txn",
"key": "replication_time_us",
"value": 579
}
Change-Id: I8bd1a249b2f09f8eb63bc64de55147b3d7738a9c
Reviewed-on: http://gerrit.cloudera.org:8080/3092
Tested-by: Kudu Jenkins
Reviewed-by: Jean-Daniel Cryans
Reviewed-by: Adar Dembo
---
M src/kudu/rpc/rpc-test-base.h
M src/kudu/rpc/rpc_introspection.proto
M src/kudu/rpc/rpc_stub-test.cc
M src/kudu/rpc/rpcz_store.cc
M src/kudu/util/trace.cc
M src/kudu/util/trace.h
6 files changed, 77 insertions(+), 3 deletions(-)
Approvals:
Jean-Daniel Cryans: Looks good to me, but someone else must approve
Adar Dembo: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/3092
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I8bd1a249b2f09f8eb63bc64de55147b3d7738a9c
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
[kudu-CR](branch-0.9.x) rpcz: break out trace metrics into separate PB fields
Todd Lipcon has uploaded a new change for review.
http://gerrit.cloudera.org:8080/3195
Change subject: rpcz: break out trace metrics into separate PB fields
..
rpcz: break out trace metrics into separate PB fields
This removes the trace metrics from the textual trace dump and instead adds
some structured metrics information to the /rpcz page. This should be easier
to read in the /rpcz output, and also easier to parse from any future
analysis tools we might build.
I updated the test and also manually verified the new output on a
CreateTableRequestPB on a local instance. Here's the metrics from that call:
"metrics": [
{
"child_path": "txn",
"key": "apply.queue_time_us",
"value": 168
},
{
"child_path": "txn",
"key": "apply.run_wall_time_us",
"value": 584
},
{
"child_path": "txn",
"key": "apply.run_cpu_time_us",
"value": 584
},
{
"child_path": "txn",
"key": "prepare.run_cpu_time_us",
"value": 684
},
{
"child_path": "txn",
"key": "prepare.queue_time_us",
"value": 168
},
{
"child_path": "txn",
"key": "prepare.run_wall_time_us",
"value": 683
},
{
"child_path": "txn",
"key": "threads_started",
"value": 2
},
{
"child_path": "txn",
"key": "thread_start_us",
"value": 201
},
{
"child_path": "txn",
"key": "num_ops",
"value": 2
},
{
"child_path": "txn",
"key": "replication_time_us",
"value": 579
}
Change-Id: I8bd1a249b2f09f8eb63bc64de55147b3d7738a9c
Reviewed-on: http://gerrit.cloudera.org:8080/3092
Tested-by: Kudu Jenkins
Reviewed-by: Jean-Daniel Cryans
Reviewed-by: Adar Dembo
(cherry picked from commit 08b66007c7eee301ba7e77ed11a7c48e3eefd72c)
---
M src/kudu/rpc/rpc-test-base.h
M src/kudu/rpc/rpc_introspection.proto
M src/kudu/rpc/rpc_stub-test.cc
M src/kudu/rpc/rpcz_store.cc
M src/kudu/util/trace.cc
M src/kudu/util/trace.h
6 files changed, 77 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/95/3195/1
--
To view, visit http://gerrit.cloudera.org:8080/3195
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8bd1a249b2f09f8eb63bc64de55147b3d7738a9c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-0.9.x
Gerrit-Owner: Todd Lipcon
[kudu-CR] Make BuildLog() return Status
Adar Dembo has posted comments on this change. Change subject: Make BuildLog() return Status .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3134 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icfa8ddba74b909f5697ce03b45e97c1733082b07 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] KUDU-1386 NaN float and double values are not handled correctly
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3142 to look at the new patch set (#3). Change subject: KUDU-1386 NaN float and double values are not handled correctly .. KUDU-1386 NaN float and double values are not handled correctly Previously, TypeInfo::Compare always returned 0 for comparisons with NaN. This meant that an equality predicate on a floating point column would always return all NaN values for the column. This patch changes Compare so that NaN compares > all non-NaN floating point values, and == to itself (contrary to IEEE754 standard), based on the suggestion from Todd Lipcon that we follow Postgres's example. Change-Id: I194dcddeb8eabcc67699661b9cc9362a99f2f4ae --- M src/kudu/client/predicate-test.cc M src/kudu/common/key_util.cc M src/kudu/common/types-test.cc M src/kudu/common/types.h 4 files changed, 101 insertions(+), 18 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/42/3142/3 -- To view, visit http://gerrit.cloudera.org:8080/3142 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I194dcddeb8eabcc67699661b9cc9362a99f2f4ae Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-1386 NaN float and double values are not handled correctly
Will Berkeley has posted comments on this change.
Change subject: KUDU-1386 NaN float and double values are not handled correctly
..
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/3142/2/src/kudu/common/types.h
File src/kudu/common/types.h:
Line 303: } else if (isnan(*lhs_n)) {
> given that nans are less common than anything else, I think it would be bes
Done.
Line 334: const double *lhs_n = reinterpret_cast(lhs);
> can you extract this code into a FloatCompare templated method?
Done.
--
To view, visit http://gerrit.cloudera.org:8080/3142
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I194dcddeb8eabcc67699661b9cc9362a99f2f4ae
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Will Berkeley
Gerrit-HasComments: Yes
[kudu-CR] Add unique id generation to the client
David Ribeiro Alves has posted comments on this change.
Change subject: Add unique id generation to the client
..
Patch Set 11:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/3077/11/src/kudu/integration-tests/client-stress-test.cc
File src/kudu/integration-tests/client-stress-test.cc:
Line 49: class ClientStressTest: public KuduTest {
> This syntax is quite a bit more rare in the Kudu codebase than the original
pressed the auto format key by mistake, sorry about that, reverting these
changes.
--
To view, visit http://gerrit.cloudera.org:8080/3077
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie752ab8337cc76c3be1536958c24a8b6c5794650
Gerrit-PatchSet: 11
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: David Ribeiro Alves
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] Bump maven-assembly-plugin version
Jean-Daniel Cryans has posted comments on this change. Change subject: Bump maven-assembly-plugin version .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3193 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I18760502ebe947a1757706c5dd4fdf5d5772efca Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] KUDU-1386 NaN float and double values are not handled correctly
Todd Lipcon has posted comments on this change. Change subject: KUDU-1386 NaN float and double values are not handled correctly .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/3142/1/src/kudu/common/types.h File src/kudu/common/types.h: Line 53: NONCOMPARABLE = 12 > I agree more with Dan, though I think the Todd/Postgres position is reasona > I'm not following, why would you want NaNs to show up in a per-block min/max > statistic? Imagine a column for a time series value which is a float with lots of NaNs. The user wants to 'select * where !isNan(val)'. In that case, with min/max statistics, you can just convert this to '< NaN', and quickly use the min/max to determine whether a given block has any NaNs. The whole concept of min/max gets a bit messed up if there is no total order. In terms of compatibility, Oracle also does like Postgres: "Comparison operators conform, except for comparisons with NaN. Oracle orders NaN greatest with respect to all other values, and evaluates NaN equal to NaN. See "Floating-Point Conditions"." (http://docs.oracle.com/cd/B19306_01/server.102/b14200/sql_elements001.htm) Best I can tell, MySQL doesn't support NaN at all. SQL Server before 2000 would apparently corrupt the database if you inserted NaN (https://www.simple-talk.com/blogs/2010/04/13/from-nan-to-infinity-and-beyond/). According to that article, SQL Server 2008 just disallowed them. SQLite seems to coerce NaN to NULL and prevents insertion into a NOT NULL column (which is also a reasonable position IMO). DB2 apparently lives in another universe (the IBM one) where there is both a positive and negative NaN as well as two variants, "signalling" and "quiet" (what???). But they do assign a total order: The ordering among the different special values is as follows: -NAN < -SNAN < -INFINITY < 0 < INFINITY < SNAN https://www.ibm.com/support/knowledgecenter/SSEPEK_10.0.0/com.ibm.db2z10.doc.sqlref/src/tpc/db2z_numericcomparisions.dita) So, I think despite there being an IEEE standard around float comparison, I can't seem to find any database engines which behave that way. -- To view, visit http://gerrit.cloudera.org:8080/3142 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I194dcddeb8eabcc67699661b9cc9362a99f2f4ae Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: Yes
[kudu-CR] Bump maven-assembly-plugin version
Hello Jean-Daniel Cryans, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3193 to look at the new patch set (#2). Change subject: Bump maven-assembly-plugin version .. Bump maven-assembly-plugin version The previous version spewed useless log lines at INFO level. With this change, a simple `mvn clean verify -DskipTests` build is reduced from 4103 to 547 lines of output. Change-Id: I18760502ebe947a1757706c5dd4fdf5d5772efca --- M java/pom.xml 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/93/3193/2 -- To view, visit http://gerrit.cloudera.org:8080/3193 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I18760502ebe947a1757706c5dd4fdf5d5772efca Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] java: use truncated randomized exponential backoff for retries
Adar Dembo has posted comments on this change. Change subject: java: use truncated randomized exponential backoff for retries .. Patch Set 1: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/3184/1/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java File java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java: Line 924: // Randomized exponential backoff, truncated at 4096ms. > The Java client uses this somewhat indiscriminately including things like w > Looking at the C++ client, there are a number of different strategies > sprinkled around. Yeah, and it's made me rather grumpy. Makes thinking about backoff a very unpredictable affair. Anyway, I guess this isn't any worse, since the Java client did its own thing previously too. -- To view, visit http://gerrit.cloudera.org:8080/3184 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I79256e00cf35c072c60cd2b9034e6c1b10c18b38 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Remove default table partitioning
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3131 to look at the new patch set (#4). Change subject: Remove default table partitioning .. Remove default table partitioning This commit removes the current default of creating tables with range partitioning over the primary key columns with no splits. This default is problematic because it results in a single tablet, which is a known anti-pattern. Kudu can't predict appropriate split rows without knowledge of the dataset, so creating default splits is not technically feasible. A better default to range partitioning would be to hash partition on the primary key columns with a number of buckets based on the number of tablet servers. Unfortunately, it's similarly difficult to predict an appopriate number of hash buckets with knowledge of the data set. Since changing the default would be a breaking change, and we don't currently have a bullet-proof default option, this commit changes the table creator in the C++ and Java clients to force users to explicitly specify at least range or hash partitioning. Users who really do want a table with no partitioning (a single tablet), can still explicitly set the range partition columns to an empty list and provide no split rows. Change-Id: I7021d7950f8dbb4918503ea6fab2e6ee35076064 --- M docs/release_notes.adoc M docs/schema_design.adoc M java/kudu-client-tools/src/main/java/org/kududb/mapreduce/tools/IntegrationTestBigLinkedList.java M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/kududb/client/CreateTableOptions.java M java/kudu-client/src/main/java/org/kududb/client/KuduClient.java M java/kudu-client/src/test/java/org/kududb/client/BaseKuduTest.java M java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduClient.java M java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduSession.java M java/kudu-client/src/test/java/org/kududb/client/TestFlexiblePartitioning.java M java/kudu-client/src/test/java/org/kududb/client/TestHybridTime.java M java/kudu-client/src/test/java/org/kududb/client/TestKuduClient.java M java/kudu-client/src/test/java/org/kududb/client/TestKuduSession.java M java/kudu-client/src/test/java/org/kududb/client/TestKuduTable.java M java/kudu-client/src/test/java/org/kududb/client/TestLeaderFailover.java M java/kudu-client/src/test/java/org/kududb/client/TestMasterFailover.java M java/kudu-client/src/test/java/org/kududb/client/TestRowErrors.java M java/kudu-client/src/test/java/org/kududb/client/TestRowResult.java M java/kudu-client/src/test/java/org/kududb/client/TestScanPredicate.java M java/kudu-client/src/test/java/org/kududb/client/TestScannerMultiTablet.java M java/kudu-client/src/test/java/org/kududb/client/TestStatistics.java M java/kudu-client/src/test/java/org/kududb/client/TestTimeouts.java M java/kudu-flume-sink/src/test/java/org/kududb/flume/sink/KuduSinkTest.java M java/kudu-mapreduce/src/test/java/org/kududb/mapreduce/ITKuduTableInputFormat.java M java/kudu-mapreduce/src/test/java/org/kududb/mapreduce/ITKuduTableOutputFormat.java M java/kudu-mapreduce/src/test/java/org/kududb/mapreduce/ITOutputFormatJob.java M java/kudu-spark/src/main/scala/org/kududb/spark/kudu/KuduContext.scala M java/kudu-spark/src/test/scala/org/kududb/spark/kudu/DefaultSourceTest.scala M java/kudu-spark/src/test/scala/org/kududb/spark/kudu/TestContext.scala M src/kudu/benchmarks/tpch/rpc_line_item_dao.cc M src/kudu/client/client-test.cc M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/predicate-test.cc M src/kudu/client/samples/sample.cc M src/kudu/integration-tests/all_types-itest.cc M src/kudu/integration-tests/alter_table-randomized-test.cc M src/kudu/integration-tests/alter_table-test.cc M src/kudu/integration-tests/create-table-itest.cc M src/kudu/integration-tests/delete_table-test.cc M src/kudu/integration-tests/full_stack-insert-scan-test.cc M src/kudu/integration-tests/fuzz-itest.cc M src/kudu/integration-tests/linked_list-test-util.h M src/kudu/integration-tests/master_replication-itest.cc M src/kudu/integration-tests/test_workload.cc M src/kudu/integration-tests/ts_itest-base.h M src/kudu/integration-tests/ts_tablet_manager-itest.cc M src/kudu/integration-tests/update_scan_delta_compact-test.cc M src/kudu/integration-tests/write_throttling-itest.cc M src/kudu/tools/ksck_remote-test.cc 50 files changed, 209 insertions(+), 138 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/31/3131/4 -- To view, visit http://gerrit.cloudera.org:8080/3131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7021d7950f8dbb4918503ea6fab2e6ee35076064 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Review
[kudu-CR] Add a generic retriable rpc class
David Ribeiro Alves has submitted this change and it was merged. Change subject: Add a generic retriable rpc class .. Add a generic retriable rpc class This patch adds a new, generic, class for retriable Rpcs: RetriableRpc. This class will handle retry logic, such as setting id/sequence numbers on the rpc header, as well as number of attempts. Derived classes of RetriableRpc no longer have to deal with logic regarding replica selection, proxy initialization or retrying. They just have to implement: Try() - Actually sends the rpc to a server. AnalyzeResponse() - Buckets the response into a few, common, categories. Finish() - Handle (final) success or failure. This also refactors WriteRpc to use the new class. Change-Id: Iaa58bdc5656a5d4d9172885a67363f74718a0c8e Reviewed-on: http://gerrit.cloudera.org:8080/3064 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo --- M src/kudu/client/batcher.cc A src/kudu/rpc/retriable_rpc.h 2 files changed, 213 insertions(+), 114 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/3064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Iaa58bdc5656a5d4d9172885a67363f74718a0c8e Gerrit-PatchSet: 13 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Move the MetaCacheServerPicker into meta_cache.h/cc
David Ribeiro Alves has submitted this change and it was merged. Change subject: Move the MetaCacheServerPicker into meta_cache.h/cc .. Move the MetaCacheServerPicker into meta_cache.h/cc This moves the MetaCacheServerPicker into meta_cache.h/cc. Previously this would also make the picker a member of the corresponding RemoteTablet, but turns out this is problematic due to the lifecycle of KuduTable, a user controlled object that is used internally on every lookup. Specifically I tried two options: Making the picker hold a reference to KuduTable, which resulted in a memory leak due to cyclic dependencies; Just holding a plain pointer, at which time I would get a SIGSEGV due to the user deleting the object. We can likely sort the lifecycle issues in some way but I didn't look too deep into it. Change-Id: I5344e9556f7042743458efbf1a5dc8be1e45f8da Reviewed-on: http://gerrit.cloudera.org:8080/3028 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo --- M src/kudu/client/batcher.cc M src/kudu/client/meta_cache.cc M src/kudu/client/meta_cache.h 3 files changed, 205 insertions(+), 181 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/3028 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I5344e9556f7042743458efbf1a5dc8be1e45f8da Gerrit-PatchSet: 16 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Integrate the ResultTracker into the rpc subsystem and add a test
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3192 to look at the new patch set (#4). Change subject: Integrate the ResultTracker into the rpc subsystem and add a test .. Integrate the ResultTracker into the rpc subsystem and add a test This integrates the ResultTracker into the rpc subsystem by allowing to specify a new mehtod option when defining rpc service methods. When this method option is specificed _and_ the call's rpc header has a RequestIdPB the results for the call will be tracked and the call may have exactly once semantics. This allows to have the simplest form of exactly once semantics for single server calls. Of course limitations apply, like response persistence across restarts is not supported, neither is propagating/rebuilding responses to/on replicas for replicated calls. If support for this is needed it will have to be done ad-hoc, case by case. Change-Id: I1d624810350feceefe244e0319e22cac241bf0d6 --- M src/kudu/common/common.proto M src/kudu/rpc/CMakeLists.txt M src/kudu/rpc/inbound_call.cc M src/kudu/rpc/protoc-gen-krpc.cc A src/kudu/rpc/rpc-stress-test.cc M src/kudu/rpc/rpc-test-base.h M src/kudu/rpc/rpc_context.cc M src/kudu/rpc/rpc_context.h M src/kudu/rpc/rtest.proto M src/kudu/rpc/service_if.cc M src/kudu/rpc/service_if.h 11 files changed, 324 insertions(+), 36 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/3192/4 -- To view, visit http://gerrit.cloudera.org:8080/3192 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1d624810350feceefe244e0319e22cac241bf0d6 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
