[kudu-CR] Remove pessimizing std::move() in return statement
Kudu Jenkins has posted comments on this change. Change subject: Remove pessimizing std::move() in return statement .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/3064/ -- To view, visit http://gerrit.cloudera.org:8080/4122 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ida46588398ca5f5982cb86aa03825530565c0923 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike PercyGerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] tool: add ksck
Todd Lipcon has posted comments on this change. Change subject: tool: add ksck .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4121 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iad9aaffa5c0c77080dcaeb2cdfa572dcbeeb1407 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar DemboGerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] tool: add ksck
Kudu Jenkins has posted comments on this change. Change subject: tool: add ksck .. Patch Set 2: Build Started http://104.196.14.100/job/kudu-gerrit/3063/ -- To view, visit http://gerrit.cloudera.org:8080/4121 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iad9aaffa5c0c77080dcaeb2cdfa572dcbeeb1407 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar DemboGerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Rolling-upgrades compat for changing TSRegistrationPB
Adar Dembo has posted comments on this change. Change subject: Rolling-upgrades compat for changing TSRegistrationPB .. Patch Set 5: (3 comments) http://gerrit.cloudera.org:8080/#/c/4062/5/src/kudu/master/ts_descriptor.cc File src/kudu/master/ts_descriptor.cc: PS5, Line 60: static bool HostPortPBsEqual(const ::google::protobuf::RepeatedPtrField<::kudu::HostPortPB>& pb1, : const ::google::protobuf::RepeatedPtrField<::kudu::HostPortPB>& pb2) Not related to your change, but can you get away with HostPortPB instead of ::kudu::HostPortPB? I thought namespace resolution automatically checked parent namespaces. http://gerrit.cloudera.org:8080/#/c/4062/5/src/kudu/util/net/net_util.h File src/kudu/util/net/net_util.h: PS5, Line 55: bool Equals(const HostPort& other) const { : return port_ == other.port_ && host_ == other.host_; : } Since this is new, consider dropping it in favor of just operator==, which is pretty self-explanatory. Line 59: bool operator==(const HostPort& other) const { Google style guide says const binary operators should be defined as free functions, so that implicit constructors (if any) can apply to both operands. -- To view, visit http://gerrit.cloudera.org:8080/4062 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I794517d200b1c9ad74b3897b17aa808d68d7924b Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike PercyGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Rolling-upgrades compat for changing TSRegistrationPB
Kudu Jenkins has posted comments on this change. Change subject: Rolling-upgrades compat for changing TSRegistrationPB .. Patch Set 5: Build Started http://104.196.14.100/job/kudu-gerrit/3062/ -- To view, visit http://gerrit.cloudera.org:8080/4062 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I794517d200b1c9ad74b3897b17aa808d68d7924b Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike PercyGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Rolling-upgrades compat for changing TSRegistrationPB
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4062 to look at the new patch set (#5). Change subject: Rolling-upgrades compat for changing TSRegistrationPB .. Rolling-upgrades compat for changing TSRegistrationPB This should allow for adding fields to TSRegistrationPB. Only the host/port is checked. Change-Id: I794517d200b1c9ad74b3897b17aa808d68d7924b --- M src/kudu/master/master-test.cc M src/kudu/master/ts_descriptor.cc M src/kudu/util/net/net_util.cc M src/kudu/util/net/net_util.h 4 files changed, 72 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/62/4062/5 -- To view, visit http://gerrit.cloudera.org:8080/4062 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I794517d200b1c9ad74b3897b17aa808d68d7924b Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike PercyGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Rolling-upgrades compat for changing TSRegistrationPB
Mike Percy has posted comments on this change. Change subject: Rolling-upgrades compat for changing TSRegistrationPB .. Patch Set 4: (2 comments) http://gerrit.cloudera.org:8080/#/c/4062/4/src/kudu/master/ts_descriptor.cc File src/kudu/master/ts_descriptor.cc: PS4, Line 68: using HostPortSet = std::unordered_set; : HostPortSet hostports; > Interesting, but why not just: Ah, I was planning on having to refer to the type and ended up never having to. Removed. PS4, Line 70: for (int i = 0; i < pb1.size(); i++) { : HostPort hp1; : if (!HostPortFromPB(pb1.Get(i), ).ok()) return false; : hostports.insert(hp1); : } : for (int i = 0; i < pb1.size(); i++) { : HostPort hp2; : if (!HostPortFromPB(pb2.Get(i), ).ok()) return false; : if (!ContainsKey(hostports, hp2)) { : return false; : } : } > You may be able to get away with unordered_sets of HostPortPB (maybe the ge This would have been really nice, but it doesn't work. I think it's probably easy with proto3. I partially took your advice with the unordered_set equality thing though. -- To view, visit http://gerrit.cloudera.org:8080/4062 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I794517d200b1c9ad74b3897b17aa808d68d7924b Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-456 Implement AUTO FLUSH BACKGROUND flush mode
Adar Dembo has posted comments on this change. Change subject: KUDU-456 Implement AUTO_FLUSH_BACKGROUND flush mode .. Patch Set 15: (8 comments) http://gerrit.cloudera.org:8080/#/c/3952/15/src/kudu/client/client.h File src/kudu/client/client.h: Line 1146: Status SetMutationBufferFlushWatermark(double watermark_pct) > Will do. BTW, do we need getters for those parameters? I think getters mi I don't think it's a big deal either way. Sessions are cheap (especially now that they don't have a dedicated thread to flush in the background) and so I tend to think they're short-lived, which means it's easy for the application to remember what value was provided here. http://gerrit.cloudera.org:8080/#/c/3952/15/src/kudu/client/session-internal.cc File src/kudu/client/session-internal.cc: PS15, Line 64: -1, > This is to force getting a new batcher and flushing the prior, if any, even I don't think the existing behavior is necessarily worth preserving; an empty flush doesn't actually accomplish anything. In any case, that wasn't what my comment was about; I was asking about the semantic difference between '-1' and '0' as the watermark value. Line 66: PeriodicFlushTask(Status::OK(), messenger_, session); > OK, I'll change it then. Sorry, I don't understand what you're suggesting. Can you explain it in more detail? Line 109: flow_control_cond_.Broadcast(); > The original idea was to get rid of those limitations like empty buffer whe Nope, simplifying the implementation is the underlying motivation. But I think it's enough; the clients are generally complicated code and aren't as well tested as the rest of Kudu. Line 135: CHECK_GE(timeout_ms, 0); > Yep. What we can do instead is to set it to 0 if the specified parameter w I think that's reasonable provided it's documented. PS15, Line 230: RETURN_NOT_OK(TryApplyWriteOp(write_op, _size, : _flush_mode, _flush_watermark)); > Yep, having those facts simplifies this. Again, I was under impression the I don't understand why friendship factors into this: both ApplyWriteOp and TryApplyWriteOp are members of KuduSession::Data, so they should be equivalent w.r.t. calling Batcher::SizeInBuffer(). Line 398: data->NextBatcher(session, 0, nullptr); > Nope. It would, however, if the second parameter were -1 (as you noticed t I find the semantics of -1, 0, and >0 to be non-obvious. Perhaps we can convey the desired behavior from caller to PeriodicFlushTask() in another way? Through an additional parameter, an enum, or something like that? http://gerrit.cloudera.org:8080/#/c/3952/15/src/kudu/client/session-internal.h File src/kudu/client/session-internal.h: PS15, Line 87: On successful return, the output flush_mode parameter is set : // to the effective flush mode. > This is because the flush_mode_ can change in the middle and the code which I don't understand how flush_mode_ can change in the middle of ApplyWriteOp if the session may only be used by a single thread. Or is this a holdover from before, when it was OK for multiple threads to write to a session? -- To view, visit http://gerrit.cloudera.org:8080/3952 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I34905c30b3aad96f53cf7a1822b1cde6d25f33a8 Gerrit-PatchSet: 15 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey SerbinGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-1500: Data race in RaftConsensusITest.TestCorruptReplicaMetadata
Kudu Jenkins has posted comments on this change. Change subject: KUDU-1500: Data race in RaftConsensusITest.TestCorruptReplicaMetadata .. Patch Set 10: Build Started http://104.196.14.100/job/kudu-gerrit/3061/ -- To view, visit http://gerrit.cloudera.org:8080/3823 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If57617e22b41296b8d4e8ad131220f1ebb235019 Gerrit-PatchSet: 10 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh BhatGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] tool: add ksck
Kudu Jenkins has posted comments on this change. Change subject: tool: add ksck .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/3060/ -- To view, visit http://gerrit.cloudera.org:8080/4121 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iad9aaffa5c0c77080dcaeb2cdfa572dcbeeb1407 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar DemboGerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] subprocess: allow Call() to read both stdout and stderr
Hello Dan Burkert, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4057 to look at the new patch set (#2). Change subject: subprocess: allow Call() to read both stdout and stderr .. subprocess: allow Call() to read both stdout and stderr I'm going to use this in a new integration test for the tool. Since the parent is now reading from two pipes, it needs to do so more carefully. For example, if it read from stdout fully before looking at stderr, both it and the child would deadlock if the child wrote 64k bytes to the stderr pipe, hit the kernel limit, and got blocked. I played around with an implementation based on poll(), but ultimately found this one (based on libev) to be simpler. Change-Id: If5f2be94c2e5cc0644a5bb2340adc4a71d844247 --- M src/kudu/util/CMakeLists.txt M src/kudu/util/subprocess-test.cc M src/kudu/util/subprocess.cc M src/kudu/util/subprocess.h 4 files changed, 163 insertions(+), 64 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/4057/2 -- To view, visit http://gerrit.cloudera.org:8080/4057 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: If5f2be94c2e5cc0644a5bb2340adc4a71d844247 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar DemboGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] tool: basic integration test
Kudu Jenkins has posted comments on this change. Change subject: tool: basic integration test .. Patch Set 4: Build Started http://104.196.14.100/job/kudu-gerrit/3058/ -- To view, visit http://gerrit.cloudera.org:8080/4058 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib386882c1874e987d5824cfe742cc86627cd9eaa Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar DemboGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] tool: basic integration test
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4058 to look at the new patch set (#4). Change subject: tool: basic integration test .. tool: basic integration test So far all it does is spot check some help pages, but in the future we should augment it to test functionality too. For now that's not a big deal because every tool function is covered in either master_migration-itest or master_failover-itest. Change-Id: Ib386882c1874e987d5824cfe742cc86627cd9eaa --- M build-support/dist_test.py M src/kudu/tools/CMakeLists.txt A src/kudu/tools/kudu-tool-test.cc M src/kudu/util/test_macros.h 4 files changed, 188 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/4058/4 -- To view, visit http://gerrit.cloudera.org:8080/4058 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib386882c1874e987d5824cfe742cc86627cd9eaa Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar DemboGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] tool: basic integration test
Adar Dembo has posted comments on this change. Change subject: tool: basic integration test .. Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/4058/3/build-support/dist_test.py File build-support/dist_test.py: Line 249: deps.extend(ldd_deps(d)) > do we need to worry about de-duping deps now? Since all of the tests passed I can't tell whether we actually shipped duplicate dependencies, but I'll deduplicate here. http://gerrit.cloudera.org:8080/#/c/4058/3/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: PS3, Line 70: expected_prefixes > don't really understand the usage here Replaced this with regexes, so it's a moot point now. Line 91: // Strip away everything up to the usage string to test for prefixes. > a little skeptical of this fancy verification vs just hard-coding a regex f Done Line 99: if (l.find(" " + m) != string::npos) { > this isn't checking for a prefix Done -- To view, visit http://gerrit.cloudera.org:8080/4058 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib386882c1874e987d5824cfe742cc86627cd9eaa Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar DemboGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Rolling-upgrades compat for changing TSRegistrationPB
Adar Dembo has posted comments on this change. Change subject: Rolling-upgrades compat for changing TSRegistrationPB .. Patch Set 4: (2 comments) http://gerrit.cloudera.org:8080/#/c/4062/4/src/kudu/master/ts_descriptor.cc File src/kudu/master/ts_descriptor.cc: PS4, Line 68: using HostPortSet = std::unordered_set; : HostPortSet hostports; Interesting, but why not just: unordered_set hostports; PS4, Line 70: for (int i = 0; i < pb1.size(); i++) { : HostPort hp1; : if (!HostPortFromPB(pb1.Get(i), ).ok()) return false; : hostports.insert(hp1); : } : for (int i = 0; i < pb1.size(); i++) { : HostPort hp2; : if (!HostPortFromPB(pb2.Get(i), ).ok()) return false; : if (!ContainsKey(hostports, hp2)) { : return false; : } : } You may be able to get away with unordered_sets of HostPortPB (maybe the generated code does the right thing): unordered_set hps1(pb1.begin(), pb1.end()); unordered_set hps2(pb2.begin(), pb2.end()); return hps1 == hps2; -- To view, visit http://gerrit.cloudera.org:8080/4062 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I794517d200b1c9ad74b3897b17aa808d68d7924b Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Rolling-upgrades compat for changing TSRegistrationPB
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4062 to look at the new patch set (#4). Change subject: Rolling-upgrades compat for changing TSRegistrationPB .. Rolling-upgrades compat for changing TSRegistrationPB This should allow for adding fields to TSRegistrationPB. Only the host/port is checked. Change-Id: I794517d200b1c9ad74b3897b17aa808d68d7924b --- M src/kudu/master/master-test.cc M src/kudu/master/ts_descriptor.cc M src/kudu/util/net/net_util.cc M src/kudu/util/net/net_util.h 4 files changed, 78 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/62/4062/4 -- To view, visit http://gerrit.cloudera.org:8080/4062 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I794517d200b1c9ad74b3897b17aa808d68d7924b Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike PercyGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Actually support downgrade to version that has LocalConsensus
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4059 to look at the new patch set (#3). Change subject: Actually support downgrade to version that has LocalConsensus .. Actually support downgrade to version that has LocalConsensus Commit 74210b2546df9fd5dec7bb926eeb524362d2da90 attempted to support downgrade from 0.10.x to 0.9.x however there is specific validation in VerifyRaftConfig() in Kudu 0.9.x that requires the optional field "local" to be set (checked using has_local()). So we must not rely on the default, rather, we must always set that field. This patch was manually tested as follows: A 3-way replicated table was created in 0.10.0-RC1 plus this patch, using a single master and 3 tablet servers. The services were killed, then restarted using 0.9.1 binaries. Change-Id: Icded76c6400b4802581671e2e41efb74a47c5ec4 --- M src/kudu/master/catalog_manager.cc M src/kudu/master/sys_catalog.cc 2 files changed, 4 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/59/4059/3 -- To view, visit http://gerrit.cloudera.org:8080/4059 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Icded76c6400b4802581671e2e41efb74a47c5ec4 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike PercyGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Rolling-upgrades compat for changing TSRegistrationPB
Kudu Jenkins has posted comments on this change. Change subject: Rolling-upgrades compat for changing TSRegistrationPB .. Patch Set 4: Build Started http://104.196.14.100/job/kudu-gerrit/3057/ -- To view, visit http://gerrit.cloudera.org:8080/4062 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I794517d200b1c9ad74b3897b17aa808d68d7924b Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike PercyGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Actually support downgrade to version that has LocalConsensus
Kudu Jenkins has posted comments on this change. Change subject: Actually support downgrade to version that has LocalConsensus .. Patch Set 3: Build Started http://104.196.14.100/job/kudu-gerrit/3056/ -- To view, visit http://gerrit.cloudera.org:8080/4059 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icded76c6400b4802581671e2e41efb74a47c5ec4 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike PercyGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Forward compat for changing TSRegistrationPB
Mike Percy has posted comments on this change. Change subject: Forward compat for changing TSRegistrationPB .. Patch Set 3: (2 comments) > How about a unit test showing a successful reregistration when > something other than the addresses has changed? Done http://gerrit.cloudera.org:8080/#/c/4062/3/src/kudu/master/ts_descriptor.cc File src/kudu/master/ts_descriptor.cc: Line 58: // Compares two repeated HostPortPB fields. Returns true if equal, false otherwise. > oh, i see, yea that makes sense Done http://gerrit.cloudera.org:8080/#/c/4062/3/src/kudu/util/net/net_util.h File src/kudu/util/net/net_util.h: Line 56: return port() == other.port() && host() == other.host(); > Any particular reason to compare the port before the host? I'd think the la Comparing integers is cheaper. -- To view, visit http://gerrit.cloudera.org:8080/4062 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I794517d200b1c9ad74b3897b17aa808d68d7924b Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike PercyGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] java: fix leak of TabletClient objects in client2tablets map
Adar Dembo has posted comments on this change. Change subject: java: fix leak of TabletClient objects in client2tablets map .. Patch Set 1: Forgot to add: I was under the impression that after a client reconnects, the client2tablets entry is reused. I see now that's not the case at all. -- To view, visit http://gerrit.cloudera.org:8080/4119 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I302650f2a6526e7c51537264137a4f00cbbda073 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd LipconGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] java: fix leak of TabletClient objects in client2tablets map
Adar Dembo has posted comments on this change. Change subject: java: fix leak of TabletClient objects in client2tablets map .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/4119/1/java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java File java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java: Line 317: for (int port : tserverPorts) { I don't think we're managing this list very well right now. We never actually remove entries from it. -- To view, visit http://gerrit.cloudera.org:8080/4119 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I302650f2a6526e7c51537264137a4f00cbbda073 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd LipconGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] java: fix leak of TabletClient objects in client2tablets map
Hello Dan Burkert, Adar Dembo, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/4119 to review the following change. Change subject: java: fix leak of TabletClient objects in client2tablets map .. java: fix leak of TabletClient objects in client2tablets map After running YCSB for a week, most of the clients had hit OOMEs. Some heap dump analysis showed that the client2tablets map had hundreds of thousands of leaked clients. It seems that we were neglecting to remove the client from the client2tablets map upon a disconnect. This fixes the issue and adds a regression test which reproduced the bug. Change-Id: I302650f2a6526e7c51537264137a4f00cbbda073 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.java 4 files changed, 52 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/19/4119/1 -- To view, visit http://gerrit.cloudera.org:8080/4119 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I302650f2a6526e7c51537264137a4f00cbbda073 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd LipconGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert
[kudu-CR] java: fix leak of TabletClient objects in client2tablets map
Kudu Jenkins has posted comments on this change. Change subject: java: fix leak of TabletClient objects in client2tablets map .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/3055/ -- To view, visit http://gerrit.cloudera.org:8080/4119 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I302650f2a6526e7c51537264137a4f00cbbda073 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd LipconGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] java: inherit from ASF parent pom
Dan Burkert has posted comments on this change. Change subject: java: inherit from ASF parent pom .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/4118/1/java/pom.xml File java/pom.xml: Line 34: 12 The latest version appears to be 18: https://repo.maven.apache.org/maven2/org/apache/apache/ -- To view, visit http://gerrit.cloudera.org:8080/4118 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icf4e46b9ba50c74fb7943aabd2065609e3c5e011 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd LipconGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] java: inherit from ASF parent pom
Dan Burkert has posted comments on this change. Change subject: java: inherit from ASF parent pom .. Patch Set 1: Oh sorry, just saw the associated email. -- To view, visit http://gerrit.cloudera.org:8080/4118 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icf4e46b9ba50c74fb7943aabd2065609e3c5e011 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd LipconGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] java: inherit from ASF parent pom
Dan Burkert has posted comments on this change. Change subject: java: inherit from ASF parent pom .. Patch Set 1: I'm fine with this as long as we keep publishing snapshots every night or every commit. Can we do that to the ASF repo? -- To view, visit http://gerrit.cloudera.org:8080/4118 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icf4e46b9ba50c74fb7943aabd2065609e3c5e011 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd LipconGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] java: inherit from ASF parent pom
Kudu Jenkins has posted comments on this change. Change subject: java: inherit from ASF parent pom .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/3054/ -- To view, visit http://gerrit.cloudera.org:8080/4118 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icf4e46b9ba50c74fb7943aabd2065609e3c5e011 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd LipconGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] java: inherit from ASF parent pom
Hello Dan Burkert, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/4118 to review the following change. Change subject: java: inherit from ASF parent pom .. java: inherit from ASF parent pom This removes the Cloudera distribution management section and adds the ASF parent pom to pick up the ASF nexus for distribution management. Change-Id: Icf4e46b9ba50c74fb7943aabd2065609e3c5e011 --- M java/pom.xml 1 file changed, 8 insertions(+), 13 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/18/4118/1 -- To view, visit http://gerrit.cloudera.org:8080/4118 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Icf4e46b9ba50c74fb7943aabd2065609e3c5e011 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd LipconGerrit-Reviewer: Dan Burkert
[kudu-CR] KUDU-1231. Add "unlock" flag for experimental and unsafe flags
Adar Dembo has posted comments on this change. Change subject: KUDU-1231. Add "unlock" flag for experimental and unsafe flags .. Patch Set 6: (5 comments) http://gerrit.cloudera.org:8080/#/c/4100/6/python/kudu/tests/common.py File python/kudu/tests/common.py: PS6, Line 56: "--unlock_unsafe_flags", : "--unlock_experimental_flags", Nit: use single dash for consistency with the other flags. Below too. http://gerrit.cloudera.org:8080/#/c/4100/6/src/kudu/util/flag_tags-test.cc File src/kudu/util/flag_tags-test.cc: Line 36: Nit: there's an extra empty line here. Line 75: ASSERT_DEATH({ HandleCommonFlags();}, Nit: { HandleCommonFlags(); } (space between semicolon and end brace). Below too. PS6, Line 82: StringVectorSink sink; : ScopedRegisterSink reg(); You know you've made it as a project when you've got unit tests that check log output. http://gerrit.cloudera.org:8080/#/c/4100/6/src/kudu/util/flags.cc File src/kudu/util/flags.cc: Line 312: void CheckFlagsUnlocked() { Nit: perhaps adjust the function name slightly? When I saw FooUnlocked() I assumed that meant a lock must be held when calling it. -- To view, visit http://gerrit.cloudera.org:8080/4100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iec49e77fca604a7c5ee7501121a6263b7ee590d6 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd LipconGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: Yes
[kudu-CR] KUDU-456 Implement AUTO FLUSH BACKGROUND flush mode
Alexey Serbin has posted comments on this change. Change subject: KUDU-456 Implement AUTO_FLUSH_BACKGROUND flush mode .. Patch Set 21: (7 comments) Thank you for the review. I addressed the issues you pointed at and will send the updated version with additional changes soon. http://gerrit.cloudera.org:8080/#/c/3952/21//COMMIT_MSG Commit Message: PS21, Line 27: va( > Interval Done http://gerrit.cloudera.org:8080/#/c/3952/21/src/kudu/client/batcher.h File src/kudu/client/batcher.h: PS21, Line 69: const scoped_refptr& error_collector, : const client::sp::weak_ptr& session, > both of these should be taken by value, and then moved into their field. Done http://gerrit.cloudera.org:8080/#/c/3952/21/src/kudu/client/client.h File src/kudu/client/client.h: PS21, Line 1143: when newly submitted : /// operations are about to overflow the buffer > when the newly applied operation would overflow the buffer Done http://gerrit.cloudera.org:8080/#/c/3952/21/src/kudu/client/session-internal.cc File src/kudu/client/session-internal.cc: PS21, Line 428: requied > required Done PS21, Line 451: Aplly > Apply Done http://gerrit.cloudera.org:8080/#/c/3952/21/src/kudu/client/session-internal.h File src/kudu/client/session-internal.h: PS21, Line 86: apply write operation > apply a write operation Done PS21, Line 86: Check whether it's possible > Does this not actually apply the op? If not I think the method should be r Oops, exactly. I messed up those comments, my bad. Will fix. -- To view, visit http://gerrit.cloudera.org:8080/3952 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I34905c30b3aad96f53cf7a1822b1cde6d25f33a8 Gerrit-PatchSet: 21 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey SerbinGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [c++-client] reduce in-flight-op partition key lifetime
Adar Dembo has submitted this change and it was merged. Change subject: [c++-client] reduce in-flight-op partition key lifetime .. [c++-client] reduce in-flight-op partition key lifetime This commit changes the C++ MetaCache to take the lookup partition key by value, and updates Batcher to pass the partition key by move when calling the MetaCache. Additionally, the partition key is no longer stored as a field in the InFlightOp. The effect is that the InFlightOp is smaller by the size of a std::string, and the lifetime of the partition key is reduced from when the InFlightOp is complete to when the meta cache lookup is complete. Change-Id: Ia949b65a9447979c9c5eff324448af307a1db1b7 Reviewed-on: http://gerrit.cloudera.org:8080/4114 Tested-by: Kudu Jenkins Reviewed-by: Alexey SerbinReviewed-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, 9 insertions(+), 11 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Alexey Serbin: Looks good to me, but someone else must approve Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4114 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ia949b65a9447979c9c5eff324448af307a1db1b7 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [c++-client] reduce in-flight-op partition key lifetime
Adar Dembo has posted comments on this change. Change subject: [c++-client] reduce in-flight-op partition key lifetime .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4114 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia949b65a9447979c9c5eff324448af307a1db1b7 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1500: Data race in RaftConsensusITest.TestCorruptReplicaMetadata
Kudu Jenkins has posted comments on this change. Change subject: KUDU-1500: Data race in RaftConsensusITest.TestCorruptReplicaMetadata .. Patch Set 9: Build Started http://104.196.14.100/job/kudu-gerrit/3053/ -- To view, visit http://gerrit.cloudera.org:8080/3823 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If57617e22b41296b8d4e8ad131220f1ebb235019 Gerrit-PatchSet: 9 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh BhatGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1500: Data race in RaftConsensusITest.TestCorruptReplicaMetadata
Hello Dan Burkert, Mike Percy, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3823 to look at the new patch set (#9). Change subject: KUDU-1500: Data race in RaftConsensusITest.TestCorruptReplicaMetadata .. KUDU-1500: Data race in RaftConsensusITest.TestCorruptReplicaMetadata The test intends to corrupt the metadata of one of the tserver tablets. While the cluster is in the process of resurrecting the failed tserver, the Partition and PartitionSchema on that tserver is accessed using TabletPeer in an unguarded manner via ListTablets RPCs. The proposed fix here keeps the Partition and PartitionSchema immutable once after it is loaded during replica bootstrap. These fields are never overwritten again during Tablet Copy or any other workflow. Also a unit test is added to exercise this data race window. i.e, the unit test aims to issue ListTablets RPCs during a follower's tablet copy stage and provides an optimistic coverage for the fix. Testing: Passed about 2000 iteration of the failing test raft_consensus-itest.TestCorruptReplicaMetadata Change-Id: If57617e22b41296b8d4e8ad131220f1ebb235019 --- M src/kudu/integration-tests/cluster_itest_util.h M src/kudu/integration-tests/tablet_copy-itest.cc M src/kudu/tablet/tablet_metadata.cc 3 files changed, 139 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/23/3823/9 -- To view, visit http://gerrit.cloudera.org:8080/3823 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: If57617e22b41296b8d4e8ad131220f1ebb235019 Gerrit-PatchSet: 9 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh BhatGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1231. Add "unlock" flag for experimental and unsafe flags
Alexey Serbin has posted comments on this change. Change subject: KUDU-1231. Add "unlock" flag for experimental and unsafe flags .. Patch Set 6: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/4100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iec49e77fca604a7c5ee7501121a6263b7ee590d6 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd LipconGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: No
[kudu-CR] [c++-client] reduce in-flight-op partition key lifetime
Alexey Serbin has posted comments on this change. Change subject: [c++-client] reduce in-flight-op partition key lifetime .. Patch Set 2: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/4114 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia949b65a9447979c9c5eff324448af307a1db1b7 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] [c++-client] reduce in-flight-op partition key lifetime
Kudu Jenkins has posted comments on this change. Change subject: [c++-client] reduce in-flight-op partition key lifetime .. Patch Set 2: Build Started http://104.196.14.100/job/kudu-gerrit/3052/ -- To view, visit http://gerrit.cloudera.org:8080/4114 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia949b65a9447979c9c5eff324448af307a1db1b7 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] [c++-client] reduce in-flight-op partition key lifetime
Hello Todd Lipcon, Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4114 to look at the new patch set (#2). Change subject: [c++-client] reduce in-flight-op partition key lifetime .. [c++-client] reduce in-flight-op partition key lifetime This commit changes the C++ MetaCache to take the lookup partition key by value, and updates Batcher to pass the partition key by move when calling the MetaCache. Additionally, the partition key is no longer stored as a field in the InFlightOp. The effect is that the InFlightOp is smaller by the size of a std::string, and the lifetime of the partition key is reduced from when the InFlightOp is complete to when the meta cache lookup is complete. Change-Id: Ia949b65a9447979c9c5eff324448af307a1db1b7 --- M src/kudu/client/batcher.cc M src/kudu/client/meta_cache.cc M src/kudu/client/meta_cache.h 3 files changed, 9 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/4114/2 -- To view, visit http://gerrit.cloudera.org:8080/4114 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia949b65a9447979c9c5eff324448af307a1db1b7 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1231. Add "unlock" flag for experimental and unsafe flags
Kudu Jenkins has posted comments on this change. Change subject: KUDU-1231. Add "unlock" flag for experimental and unsafe flags .. Patch Set 6: Build Started http://104.196.14.100/job/kudu-gerrit/3051/ -- To view, visit http://gerrit.cloudera.org:8080/4100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iec49e77fca604a7c5ee7501121a6263b7ee590d6 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd LipconGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: No
[kudu-CR] KUDU-1231. Add "unlock" flag for experimental and unsafe flags
Hello Mike Percy, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4100 to look at the new patch set (#6). Change subject: KUDU-1231. Add "unlock" flag for experimental and unsafe flags .. KUDU-1231. Add "unlock" flag for experimental and unsafe flags This adds two new flags: --unlock_experimental_flags --unlock_unsafe_flags If a flag is tagged as 'unsafe' or 'experimental', and the user tries to set this flag on the command line without the corresponding 'unlock' flag being set, then the process will exit at startup with an error. Example error output without flags unlocked: E0824 14:04:57.263624 14821 flags.cc:296] Flag --never_fsync is unsafe and unsupported. E0824 14:04:57.263749 14821 flags.cc:302] 1 unsafe flag(s) in use. E0824 14:04:57.263761 14821 flags.cc:303] Use --unlock_unsafe_flags to proceed at your own risk. E0824 14:04:57.264104 14821 flags.cc:296] Flag --local_ip_for_outbound_sockets is experimental and unsupported. E0824 14:04:57.264128 14821 flags.cc:302] 1 experimental flag(s) in use. E0824 14:04:57.264137 14821 flags.cc:303] Use --unlock_experimental_flags to proceed at your own risk. Example error output with flags unlocked: W0824 14:04:42.922560 14773 flags.cc:294] Enabled unsafe flag: --never_fsync=true W0824 14:04:42.923032 14773 flags.cc:294] Enabled experimental flag: --local_ip_for_outbound_sockets=127.0.0.1 Change-Id: Iec49e77fca604a7c5ee7501121a6263b7ee590d6 --- M docs/release_notes.adoc M java/kudu-client/src/test/resources/flags M python/kudu/tests/common.py M src/kudu/client/client_samples-test.sh M src/kudu/integration-tests/external_mini_cluster.cc M src/kudu/util/flag_tags-test.cc M src/kudu/util/flag_tags.h M src/kudu/util/flags.cc 8 files changed, 137 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/00/4100/6 -- To view, visit http://gerrit.cloudera.org:8080/4100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iec49e77fca604a7c5ee7501121a6263b7ee590d6 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd LipconGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-1231. Add "unlock" flag for experimental and unsafe flags
Kudu Jenkins has posted comments on this change. Change subject: KUDU-1231. Add "unlock" flag for experimental and unsafe flags .. Patch Set 5: Build Started http://104.196.14.100/job/kudu-gerrit/3050/ -- To view, visit http://gerrit.cloudera.org:8080/4100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iec49e77fca604a7c5ee7501121a6263b7ee590d6 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd LipconGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: No
[kudu-CR] KUDU-1231. Add "unlock" flag for experimental and unsafe flags
Todd Lipcon has posted comments on this change. Change subject: KUDU-1231. Add "unlock" flag for experimental and unsafe flags .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/4100/4/src/kudu/client/client_samples-test.sh File src/kudu/client/client_samples-test.sh: Line 125: --unlock_experimental_flags \ > What about unsafe flags? Not needed here? I think this sample only uses an experimental flag (--local_ip_for_outbound_sockets) -- To view, visit http://gerrit.cloudera.org:8080/4100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iec49e77fca604a7c5ee7501121a6263b7ee590d6 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd LipconGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: Yes
[kudu-CR] [c++-client] reduce in-flight-op partition key lifetime
Alexey Serbin has posted comments on this change. Change subject: [c++-client] reduce in-flight-op partition key lifetime .. Patch Set 1: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/4114/1//COMMIT_MSG Commit Message: Line 9: This commit changes the C++ MetaCache to take the lookup partition key by value, Nit: could you re-format the message a bit so the lines would be no longer than 72 symbols? -- To view, visit http://gerrit.cloudera.org:8080/4114 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia949b65a9447979c9c5eff324448af307a1db1b7 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KuduSession: do not advertise thread-safety
Dan Burkert has submitted this change and it was merged. Change subject: KuduSession: do not advertise thread-safety .. KuduSession: do not advertise thread-safety Do not advertise thread-safety of KuduSession methods for the Kudu C++ client, even if they are thread-safe de facto. This is to have one set of semantics for both C++ and Java Kudu client libraries (corresponding methods of the Java client are not thread-safe). Besides, current use cases for the Kudu client assume that operations with KuduSession object do not involve multiple threads. Change-Id: I6b3fcbd0d6446ccddce7a1812260c692eba18fcd Reviewed-on: http://gerrit.cloudera.org:8080/4105 Tested-by: Kudu Jenkins Reviewed-by: Dan Burkert--- M docs/release_notes.adoc M src/kudu/client/client.h 2 files changed, 4 insertions(+), 13 deletions(-) Approvals: Dan Burkert: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4105 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I6b3fcbd0d6446ccddce7a1812260c692eba18fcd Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Binglin Chang Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KuduSession: do not advertise thread-safety
Dan Burkert has posted comments on this change. Change subject: KuduSession: do not advertise thread-safety .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4105 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6b3fcbd0d6446ccddce7a1812260c692eba18fcd Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey SerbinGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Binglin Chang Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Replace incubator-kudu links with kudu in docs
Dan Burkert has posted comments on this change. Change subject: Replace incubator-kudu links with kudu in docs .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4107 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id9b0560f073735e55243643ddada6eac0001b9d4 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mladen KovacevicGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mladen Kovacevic Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: No
[kudu-CR] [c++-client] reduce in-flight-op partition key lifetime
Todd Lipcon has posted comments on this change. Change subject: [c++-client] reduce in-flight-op partition key lifetime .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/4114 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia949b65a9447979c9c5eff324448af307a1db1b7 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1157. Don't use array reference equality for EMPTY ARRAY
Todd Lipcon has submitted this change and it was merged. Change subject: KUDU-1157. Don't use array reference equality for EMPTY_ARRAY .. KUDU-1157. Don't use array reference equality for EMPTY_ARRAY It seems like the only way that the user can specify these byte[] bounds is some deprecated 'raw bounds' APIs. So, this isn't likely a fix for any real bugs. However, it's quite bad form to compare reference equality on arrays. Change-Id: I30163098926822aafbf23b03ba4c9e26a7c91349 Reviewed-on: http://gerrit.cloudera.org:8080/4101 Tested-by: Kudu Jenkins Reviewed-by: Dan Burkert--- M java/kudu-client/src/main/java/org/apache/kudu/client/AbstractKuduScannerBuilder.java M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java 3 files changed, 14 insertions(+), 18 deletions(-) Approvals: Dan Burkert: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4101 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I30163098926822aafbf23b03ba4c9e26a7c91349 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [docs] added tip on generic git commit guidelines
Todd Lipcon has posted comments on this change. Change subject: [docs] added tip on generic git commit guidelines .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4115 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5493723f7f0b337b73e3ef5ed4a7621fe5249927 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey SerbinGerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Replace incubator-kudu links with kudu in docs
Alexey Serbin has posted comments on this change. Change subject: Replace incubator-kudu links with kudu in docs .. Patch Set 4: Code-Review+1 Thanks, this looks good to me! P.S. I would give it +2 but I don't have the privilege :) -- To view, visit http://gerrit.cloudera.org:8080/4107 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id9b0560f073735e55243643ddada6eac0001b9d4 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mladen KovacevicGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mladen Kovacevic Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: No
[kudu-CR] [docs] added tip on generic git commit guidelines
Kudu Jenkins has posted comments on this change. Change subject: [docs] added tip on generic git commit guidelines .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/3049/ -- To view, visit http://gerrit.cloudera.org:8080/4115 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5493723f7f0b337b73e3ef5ed4a7621fe5249927 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey SerbinGerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] [docs] added tip on generic git commit guidelines
Alexey Serbin has uploaded a new change for review. http://gerrit.cloudera.org:8080/4115 Change subject: [docs] added tip on generic git commit guidelines .. [docs] added tip on generic git commit guidelines Change-Id: I5493723f7f0b337b73e3ef5ed4a7621fe5249927 --- M docs/contributing.adoc 1 file changed, 3 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/4115/1 -- To view, visit http://gerrit.cloudera.org:8080/4115 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I5493723f7f0b337b73e3ef5ed4a7621fe5249927 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin
[kudu-CR] Replace incubator-kudu links with kudu in docs
Mladen Kovacevic has posted comments on this change. Change subject: Replace incubator-kudu links with kudu in docs .. Patch Set 4: (3 comments) Alex and Will, thanks for the comments. Let me know if anything else is needed here. http://gerrit.cloudera.org:8080/#/c/4107/3//COMMIT_MSG Commit Message: Line 7: Replace incubator-kudu links with kudu in docs > Could you re-format the commit message a bit to follow the git guideline: Done http://gerrit.cloudera.org:8080/#/c/4107/3/docs/contributing.adoc File docs/contributing.adoc: Line 52: git clone https://github.com/apache/kudu > now that the repo name is kudu, I think this can be simplified to Done http://gerrit.cloudera.org:8080/#/c/4107/3/docs/installation.adoc File docs/installation.adoc: Line 242: $ git clone https://github.com/apache/kudu > same with all these; the kudu at the end can be left off. Done -- To view, visit http://gerrit.cloudera.org:8080/4107 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id9b0560f073735e55243643ddada6eac0001b9d4 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mladen KovacevicGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mladen Kovacevic Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: Yes
[kudu-CR] Replace incubator-kudu links with kudu in docs
Kudu Jenkins has posted comments on this change. Change subject: Replace incubator-kudu links with kudu in docs .. Patch Set 4: Build Started http://104.196.14.100/job/kudu-gerrit/3048/ -- To view, visit http://gerrit.cloudera.org:8080/4107 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id9b0560f073735e55243643ddada6eac0001b9d4 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mladen KovacevicGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mladen Kovacevic Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: No
[kudu-CR] Replace incubator-kudu links with kudu in docs
Hello Will Berkeley, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4107 to look at the new patch set (#4). Change subject: Replace incubator-kudu links with kudu in docs .. Replace incubator-kudu links with kudu in docs A number of docs were referring to the old incubator link: https://github.com/apache/incubator-kudu As opposed to the new, non-incubator link. https://github.com/apache/kudu We modify several of the documentation files to ensure that the links are consistent, and up-to-date with the apache/kudu url. Change-Id: Id9b0560f073735e55243643ddada6eac0001b9d4 --- M RELEASING.adoc M docs/contributing.adoc M docs/developing.adoc M docs/installation.adoc M docs/release_notes.adoc 5 files changed, 17 insertions(+), 17 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/4107/4 -- To view, visit http://gerrit.cloudera.org:8080/4107 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id9b0560f073735e55243643ddada6eac0001b9d4 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mladen KovacevicGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mladen Kovacevic Gerrit-Reviewer: Will Berkeley
[kudu-CR] tests: add --stress cpu threads argument
Mike Percy has submitted this change and it was merged. Change subject: tests: add --stress_cpu_threads argument .. tests: add --stress_cpu_threads argument This is equivalent to running 'stress -c ' except it's built into our test binaries. This is quite handy for reproducing race conditions, especially on dist-test where it isn't easy to just pop open another terminal and run 'stress' at the same time. Change-Id: I9214098a9f83fad78889b2ff9621b19109f92425 Reviewed-on: http://gerrit.cloudera.org:8080/4106 Tested-by: Kudu Jenkins Reviewed-by: Adar DemboReviewed-by: Mike Percy --- M src/kudu/util/test_main.cc 1 file changed, 19 insertions(+), 0 deletions(-) Approvals: Mike Percy: Looks good to me, approved Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I9214098a9f83fad78889b2ff9621b19109f92425 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR] tests: add --stress cpu threads argument
Mike Percy has posted comments on this change. Change subject: tests: add --stress_cpu_threads argument .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9214098a9f83fad78889b2ff9621b19109f92425 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd LipconGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] KUDU-1231. Add "unlock" flag for experimental and unsafe flags
Mike Percy has posted comments on this change. Change subject: KUDU-1231. Add "unlock" flag for experimental and unsafe flags .. Patch Set 4: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/4100/4/src/kudu/util/flags.cc File src/kudu/util/flags.cc: PS4, Line 293: exit(1) > Can you print out all enabled experimental and unsafe flags before exiting? This is a good idea, and pretty easy to do. -- To view, visit http://gerrit.cloudera.org:8080/4100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iec49e77fca604a7c5ee7501121a6263b7ee590d6 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd LipconGerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: Yes
[kudu-CR] Update contributing doc page with apache/kudu instead of apache/incubator-kudu
Alexey Serbin has posted comments on this change. Change subject: Update contributing doc page with apache/kudu instead of apache/incubator-kudu .. Patch Set 3: (1 comment) Could you also update the RELEASING.adoc accordingly? http://gerrit.cloudera.org:8080/#/c/4107/3//COMMIT_MSG Commit Message: Line 7: Update contributing doc page with apache/kudu instead Could you re-format the commit message a bit to follow the git guideline: https://git-scm.com/book/en/v2/Distributed-Git-Contributing-to-a-Project#_commit_guidelines -- To view, visit http://gerrit.cloudera.org:8080/4107 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id9b0560f073735e55243643ddada6eac0001b9d4 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mladen KovacevicGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mladen Kovacevic Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: Yes
[kudu-CR] Update contributing doc page with apache/kudu instead of apache/incubator-kudu
Dan Burkert has posted comments on this change. Change subject: Update contributing doc page with apache/kudu instead of apache/incubator-kudu .. Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/4107/3/docs/contributing.adoc File docs/contributing.adoc: Line 52: git clone https://github.com/apache/kudu.git kudu now that the repo name is kudu, I think this can be simplified to git clone https://github.com/apache/kudu http://gerrit.cloudera.org:8080/#/c/4107/3/docs/installation.adoc File docs/installation.adoc: Line 242: $ git clone https://github.com/apache/kudu kudu same with all these; the kudu at the end can be left off. -- To view, visit http://gerrit.cloudera.org:8080/4107 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id9b0560f073735e55243643ddada6eac0001b9d4 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mladen KovacevicGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mladen Kovacevic Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: Yes
[kudu-CR] [c++-client] reduce in-flight-op partition key lifetime
Hello Adar Dembo, Todd Lipcon, Alexey Serbin, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/4114 to review the following change. Change subject: [c++-client] reduce in-flight-op partition key lifetime .. [c++-client] reduce in-flight-op partition key lifetime This commit changes the C++ MetaCache to take the lookup partition key by value, and updates Batcher to pass the partition key by move when calling the MetaCache. Additionally, the partition key is no longer stored as a field in the InFlightOp. The effect is that the InFlightOp is smaller by the size of a std::string, and the lifetime of the partition key is reduced from when the InFlightOp is complete to when the meta cache lookup is complete. Change-Id: Ia949b65a9447979c9c5eff324448af307a1db1b7 --- M src/kudu/client/batcher.cc M src/kudu/client/meta_cache.cc M src/kudu/client/meta_cache.h 3 files changed, 9 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/4114/1 -- To view, visit http://gerrit.cloudera.org:8080/4114 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ia949b65a9447979c9c5eff324448af307a1db1b7 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [c++-client] reduce in-flight-op partition key lifetime
Kudu Jenkins has posted comments on this change. Change subject: [c++-client] reduce in-flight-op partition key lifetime .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/3047/ -- To view, visit http://gerrit.cloudera.org:8080/4114 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia949b65a9447979c9c5eff324448af307a1db1b7 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1157. Don't use array reference equality for EMPTY ARRAY
Dan Burkert has posted comments on this change. Change subject: KUDU-1157. Don't use array reference equality for EMPTY_ARRAY .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4101 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I30163098926822aafbf23b03ba4c9e26a7c91349 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd LipconGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] KuduSession: do not advertise thread-safety
Hello Adar Dembo, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4105 to look at the new patch set (#3). Change subject: KuduSession: do not advertise thread-safety .. KuduSession: do not advertise thread-safety Do not advertise thread-safety of KuduSession methods for the Kudu C++ client, even if they are thread-safe de facto. This is to have one set of semantics for both C++ and Java Kudu client libraries (corresponding methods of the Java client are not thread-safe). Besides, current use cases for the Kudu client assume that operations with KuduSession object do not involve multiple threads. Change-Id: I6b3fcbd0d6446ccddce7a1812260c692eba18fcd --- M docs/release_notes.adoc M src/kudu/client/client.h 2 files changed, 4 insertions(+), 13 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/05/4105/3 -- To view, visit http://gerrit.cloudera.org:8080/4105 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6b3fcbd0d6446ccddce7a1812260c692eba18fcd Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey SerbinGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Binglin Chang Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KuduSession: do not advertise thread-safety
Dan Burkert has posted comments on this change. Change subject: KuduSession: do not advertise thread-safety .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/4105/2/src/kudu/client/client.h File src/kudu/client/client.h: PS2, Line 1011: /// @note This class is not thread-safe except where otherwise specified. > Should this be removed too? or change it to @note This class is not thread-safe. -- To view, visit http://gerrit.cloudera.org:8080/4105 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6b3fcbd0d6446ccddce7a1812260c692eba18fcd Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey SerbinGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Binglin Chang Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-1534 : Added software version to ListMasters RPC
Adar Dembo has posted comments on this change. Change subject: KUDU-1534 : Added software_version to ListMasters RPC .. Patch Set 3: (13 comments) http://gerrit.cloudera.org:8080/#/c/4099/3//COMMIT_MSG Commit Message: Line 7: KUDU-1534 : Added software_version to ListMasters RPC Maybe add a note below about the cleanup you did? http://gerrit.cloudera.org:8080/#/c/4099/3/src/kudu/common/wire_protocol.proto File src/kudu/common/wire_protocol.proto: PS3, Line 82: // Sent by the TS when it first heartbeats with a master. This sends the : // master all of the necessary information about the current instance : // of the TS. This is also used by master during master bringup. The new comment describes how ServerRegistrationPB is used instead of what it means. The latter is more relevant here. http://gerrit.cloudera.org:8080/#/c/4099/3/src/kudu/integration-tests/cluster_itest_util.h File src/kudu/integration-tests/cluster_itest_util.h: Line 53: class ServerRegistrationPB; Nit: sort alphabetically. Line 78: ServerRegistrationPB registration; Hmm, how does the forward declaration help with this? The layout of ServerRegistrationPB must be known in order to satisfy this definition. http://gerrit.cloudera.org:8080/#/c/4099/3/src/kudu/master/master-path-handlers.h File src/kudu/master/master-path-handlers.h: Line 28: class ServerRegistrationPB; Why is this forward declaration needed? PS3, Line 68: MasterRegistrationPB Does this type even exist? Does this method still need to be templated? http://gerrit.cloudera.org:8080/#/c/4099/3/src/kudu/master/master-test.cc File src/kudu/master/master-test.cc: Line 54: class ServerRegistrationPB; Again, not seeing how this helps, given that to declare a ServerRegistrationPB on the stack you need the real class declaration. PS3, Line 72: ServerRegistrationPB reg; : master_->GetMasterRegistration(); : ASSERT_TRUE(reg.has_software_version()); This seems like an odd place to stash a test. Why not in its own TEST_F? Also, is this a better location than registration-test? http://gerrit.cloudera.org:8080/#/c/4099/3/src/kudu/master/master.h File src/kudu/master/master.h: Line 33: #include "kudu/common/wire_protocol.h" Nit: sort alphabetically. Line 41: class ServerRegistrationPB; I don't think this helps given L135. http://gerrit.cloudera.org:8080/#/c/4099/3/src/kudu/master/master.proto File src/kudu/master/master.proto: Line 26: Nit: why the added empty line? Line 234: optional ServerRegistrationPB registration = 2; I wonder if this is backwards compatible. The message types are clearly different, but the layout of both is compatible. Can you find out? Line 555: // TODO: Just use ServerRegistration here. Nit: is this comment still relevant? Looks like we're providing a ServerRegistrationPB now. -- To view, visit http://gerrit.cloudera.org:8080/4099 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I750eeb78c989daaa6fd5d9793b1218ae5993653c Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh BhatGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] tests: add --stress cpu threads argument
Adar Dembo has posted comments on this change. Change subject: tests: add --stress_cpu_threads argument .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9214098a9f83fad78889b2ff9621b19109f92425 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd LipconGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR](branch-0.10.x) Update version to 0.10.1-SNAPSHOT on branch
Alexey Serbin has posted comments on this change. Change subject: Update version to 0.10.1-SNAPSHOT on branch .. Patch Set 1: I looked which files were changed for 0.6.0-SNAPSHOT, and it seems the following files are stuck with 0.6.0 version: * java/kudu-csd/generate_mdl.py * java/kudu-csd/src/descriptor/service.sdl Is that OK? Probably, it's related to 0.9.x --> 0.10.0 change as well. -- To view, visit http://gerrit.cloudera.org:8080/4097 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8403d978605b5c5e385baa6ad6f7f4738b9c9f48 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: branch-0.10.x Gerrit-Owner: Todd LipconGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] tool: improve help output
Alexey Serbin has posted comments on this change. Change subject: tool: improve help output .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/4038/1/src/kudu/tools/tool_action.cc File src/kudu/tools/tool_action.cc: Line 96: msg << " can be one of the following:\n"; > but then it would be lined up with the subcommands. Or you think I should i I think that's perfectly fine, no need for the proposed indent. The idea of an extra-spacing was inspired by valgrind and some other programs. Moreover, this change has been already committed :) -- To view, visit http://gerrit.cloudera.org:8080/4038 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0b4fc2da2b03edf8ce6b9998eeabd06a4fcd216d Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd LipconGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes