[kudu-CR] client-test: remove an unnecessary manual leader election
Mike Percy has submitted this change and it was merged. Change subject: client-test: remove an unnecessary manual leader election .. client-test: remove an unnecessary manual leader election One test case predates the addition of automatic leader election to Kudu, and was manually triggering a leader election after killing a node. This is no longer necessary, so the test can be simplified. Change-Id: I943d331d4d8afed9d7d929df1df05aa2f3cf454e Reviewed-on: http://gerrit.cloudera.org:8080/4141 Tested-by: Kudu Jenkins Reviewed-by: Mike Percy --- M src/kudu/client/client-test.cc 1 file changed, 0 insertions(+), 40 deletions(-) Approvals: Mike Percy: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4141 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I943d331d4d8afed9d7d929df1df05aa2f3cf454e Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR] raft consensus-itest: workaround flakiness due to KUDU-1580
Mike Percy has submitted this change and it was merged. Change subject: raft_consensus-itest: workaround flakiness due to KUDU-1580 .. raft_consensus-itest: workaround flakiness due to KUDU-1580 KUDU-1580 is a bug in which the client doesn't properly fail over in the case that the RPC connection negotiation times out. In the MultiThreadedInsertWithFailovers test case, the client is frequently timing out with such errors causing flakiness. This workaround just bumps the connection negotiation timeout. Change-Id: If8f375654b954701ab8d23c07bd133ba393e222d Reviewed-on: http://gerrit.cloudera.org:8080/4142 Tested-by: Kudu Jenkins Reviewed-by: Mike Percy --- M src/kudu/integration-tests/raft_consensus-itest.cc 1 file changed, 5 insertions(+), 0 deletions(-) Approvals: Mike Percy: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4142 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: If8f375654b954701ab8d23c07bd133ba393e222d Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR] raft consensus-itest: inserter thread should FATAL instead of FAIL
Mike Percy has submitted this change and it was merged. Change subject: raft_consensus-itest: inserter thread should FATAL instead of FAIL .. raft_consensus-itest: inserter thread should FATAL instead of FAIL This test has a thread which inserts rows and is supposed to fail the test case if it sees any row errors. Failing the test using FAIL() from a non-main thread is not thread-safe in gtest. Furthermore, FAIL() acts as a 'return' and thus the latch used to communicate the thread completion never gets fired. So, when this assertion failed, the test would hang forever. This was a regression caused by d0cff255f84e75b70c0c39ccd34a35f348e3c722 which changed the code from a CHECK(...) to a FAIL(). In order to correct this behavior, this patch switches to using the utility method FlushSessionOrDie() which has an identical implementation. Change-Id: I9dbe1e551b7f1b8b81ce0156627deb096db5112b Reviewed-on: http://gerrit.cloudera.org:8080/4140 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin Reviewed-by: Mike Percy --- M src/kudu/integration-tests/raft_consensus-itest.cc 1 file changed, 1 insertion(+), 21 deletions(-) Approvals: Mike Percy: 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/4140 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I9dbe1e551b7f1b8b81ce0156627deb096db5112b Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR] raft consensus-itest: workaround flakiness due to KUDU-1580
Mike Percy has posted comments on this change. Change subject: raft_consensus-itest: workaround flakiness due to KUDU-1580 .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4142 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If8f375654b954701ab8d23c07bd133ba393e222d Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] client-test: remove an unnecessary manual leader election
Mike Percy has posted comments on this change. Change subject: client-test: remove an unnecessary manual leader election .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4141 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I943d331d4d8afed9d7d929df1df05aa2f3cf454e Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] raft consensus-itest: inserter thread should FATAL instead of FAIL
Mike Percy has posted comments on this change. Change subject: raft_consensus-itest: inserter thread should FATAL instead of FAIL .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4140 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9dbe1e551b7f1b8b81ce0156627deb096db5112b Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] KUDU-1586. consensus: always send at least one op
Hello Dan Burkert, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/4168 to review the following change. Change subject: KUDU-1586. consensus: always send at least one op .. KUDU-1586. consensus: always send at least one op This fixes a bug in which the LogCache would not return any operations in the case of a cache miss on an operation larger than the configured batch size. This would cause consensus to get "stuck": the leader would repeatedly send status-only messages (i.e. no operations) to the follower in a tight loop, never making any progress. The new unit test reproduces the problem. Change-Id: Ibde0e833a3bf02a5f09f2b73b6ab03b188c1e697 --- M src/kudu/consensus/log_cache-test.cc M src/kudu/consensus/log_cache.cc 2 files changed, 7 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/68/4168/1 -- To view, visit http://gerrit.cloudera.org:8080/4168 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ibde0e833a3bf02a5f09f2b73b6ab03b188c1e697 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Dan Burkert
[kudu-CR] KUDU-1586. consensus: always send at least one op
Kudu Jenkins has posted comments on this change. Change subject: KUDU-1586. consensus: always send at least one op .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/3146/ -- To view, visit http://gerrit.cloudera.org:8080/4168 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibde0e833a3bf02a5f09f2b73b6ab03b188c1e697 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] KUDU-1500: Data race in RaftConsensusITest.TestCorruptReplicaMetadata
Mike Percy has posted comments on this change. Change subject: KUDU-1500: Data race in RaftConsensusITest.TestCorruptReplicaMetadata .. Patch Set 15: (1 comment) http://gerrit.cloudera.org:8080/#/c/3823/15/src/kudu/tablet/tablet_metadata.cc File src/kudu/tablet/tablet_metadata.cc: PS15, Line 313: DCHECK_EQ(table_id_, superblock.table_id()); : PartitionSchemaPB partition_schema_pb; : partition_schema_.ToPB(&partition_schema_pb); : DCHECK_EQ(superblock.partition_schema().SerializeAsString(), : partition_schema_pb.SerializeAsString()); : PartitionPB partition_pb; : partition_.ToPB(&partition_pb); : DCHECK_EQ(superblock.partition().SerializeAsString(), : partition_pb.SerializeAsString()); > I liked this idea, but it still doesn't prevent compat break if the wire fo I suppose we could change this to CHECK and just rely on upgrade / downgrade tests for coverage on this stuff, if we don't want release and debug builds to act differently. After all, this isn't a hot path. Dinesh, to your point: providing an Equals() method for the non-PB objects doesn't prevent forward-compatibility problems, you are right. These are just inherent issues we have to deal with and as far as I can tell, having upgrade tests are really the only way to avoid unforeseen compatibility problems. -- 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: 15 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [docs] added Kudu version into the doxygen footer
Alexey Serbin has posted comments on this change. Change subject: [docs] added Kudu version into the doxygen footer .. Patch Set 1: (6 comments) http://gerrit.cloudera.org:8080/#/c/4165/1/CMakeLists.txt File CMakeLists.txt: PS1, Line 79: execute_process(COMMAND git rev-parse HEAD : WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} : OUTPUT_VARIABLE KUDU_SRC_GIT_HASH) > What happens if this runs outside of a git repo, or if git isn't installed? I didn't know that: was assuming all builds work against git-cloned repo. Will re-use the generated info in version_defines.h http://gerrit.cloudera.org:8080/#/c/4165/1/docs/support/doxygen/client_api.doxy.in File docs/support/doxygen/client_api.doxy.in: PS1, Line 55: share/doc/kuduClient/samples/sample.cc > Can we substitute this in as a list? Better yet if we can somehow set it fr Certainly, good idea! Not sure whether it's possible to set it via src/kudu/client/CMakeLists.txt, but as for the list from the top-level CMakeLists.txt -- for sure. Will change. http://gerrit.cloudera.org:8080/#/c/4165/1/docs/support/doxygen/client_api.footer.in File docs/support/doxygen/client_api.footer.in: PS1, Line 6: : > Presumably these were opened in the header? Yes, this is how doxygen wants it: http://www.stack.nl/~dimitri/doxygen/manual/config.html#cfg_html_footer http://gerrit.cloudera.org:8080/#/c/4165/1/src/kudu/client/shared_ptr.h File src/kudu/client/shared_ptr.h: PS1, Line 21: /// @file shared_ptr.h > Hmm, I don't recall seeing @file tags in the other files you doxygenized. W The @file command/directive is needed here because this file contains only 'global' entities, so without the @file they are not included into the generated documentation. For details, take a look at the 'Important' note: http://www.stack.nl/~dimitri/doxygen/manual/commands.html#cmdfile PS1, Line 22: pointers > Nit: pointer Done PS1, Line 39: defined(__clang__) > You can build Kudu with gcc on macOS? Didn't know that was possible. Ah, it seems that's from my attempts to do that. I worked some asm issues, but I stopped with glog libraries. I'll remove this -- thanks. -- To view, visit http://gerrit.cloudera.org:8080/4165 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3861aafa1e615cc991647704d9a4ce8d44ad678f Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] compaction policy: fix bound calculation
Alexey Serbin has posted comments on this change. Change subject: compaction_policy: fix bound calculation .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/4152/1/src/kudu/tablet/compaction_policy.cc File src/kudu/tablet/compaction_policy.cc: PS1, Line 160: fractional_solution_ Nit: why not to use priority_queue for that as a handy wrapper? PS1, Line 161: >= So, if the resulting weight is equal to the specified max_weight, then it's not a solution? -- To view, visit http://gerrit.cloudera.org:8080/4152 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I76364c17debd7293b36884d64c7747f8c604ae12 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Anonymous Coward #174 Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: Yes
[kudu-CR] Cleanup/refactor tracking of consensus watermarks
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4133 to look at the new patch set (#4). Change subject: Cleanup/refactor tracking of consensus watermarks .. Cleanup/refactor tracking of consensus watermarks This is a fairly invasive cleanup/refactor to consensus in preparation for propagating replication watermarks from the leader to the followers. The key item here is to address a long-standing TODO that the PeerMessageQueue should itself calculate the commit index, rather than delegate that job to ReplicaState. The flow used to be something like this on the leader upon receiving a response from a peer that it has replicated some new operations: - PeerMessageQueue::ResponseFromPeer - updates peer last_received - calls PeerMessageQueue::AdvanceQueueWatermark - updates PeerMessageQueue::majority_replicated to the new majority watermark - calls NotifyObserversOfMajorityReplOp --> submits an async task to the raft threadpool - for each observer (best I can tell there is currently always only one, but that's a separate issue) -- observer->UpdateMajorityReplicated() to grab committed index (the observer is always the RaftConsensus instance) --- RaftConsensus::UpdateMajorityReplicated ReplicaState::UpdateMajorityReplicatedUnlocked - this checks the condition that the majority-replicated watermark is within the leader's term before advancing the commit index. - calls AdvanceCommittedIndexUnlocked --- commits stuff, etc - sets *committed_index out-param -- PeerMessageQueue picks up this out-param and sets the new value within the PeerMessageQueue This was very difficult to follow, given that the "observer" in fact was passing data back to the "notifier" through an out-parameter. Moreover, it wasn't obvious to see which class was "authoritative" for the tracking and advancement of watermarks. The new design makes the PeerMessageQueue itself fully responsible for calculating when the commit index can advance. This required a fairly invasive surgery because the PeerMessageQueue itself doesn't remember the terms of pending operations, and thus the watermarks became indexes instead of OpIds. This is itself also a simplification -- we previously were very messy about using the word "index" when in fact we had an OpId type. In almost every case, we only used the index of those OpIds. In the Raft paper itself, these watermarks are just indexes and not OpIds, so this change brings us closer to the implementation described in the original design. Change-Id: I2aa294472f018013d88f36a9358e9ebd9d5ed8f8 --- M docs/release_notes.adoc M src/kudu/consensus/consensus-test-util.h M src/kudu/consensus/consensus.proto M src/kudu/consensus/consensus_peers-test.cc M src/kudu/consensus/consensus_peers.cc M src/kudu/consensus/consensus_queue-test.cc M src/kudu/consensus/consensus_queue.cc M src/kudu/consensus/consensus_queue.h M src/kudu/consensus/raft_consensus-test.cc M src/kudu/consensus/raft_consensus.cc M src/kudu/consensus/raft_consensus.h M src/kudu/consensus/raft_consensus_quorum-test.cc M src/kudu/consensus/raft_consensus_state.cc M src/kudu/consensus/raft_consensus_state.h M src/kudu/integration-tests/raft_consensus-itest.cc 15 files changed, 400 insertions(+), 569 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/33/4133/4 -- To view, visit http://gerrit.cloudera.org:8080/4133 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2aa294472f018013d88f36a9358e9ebd9d5ed8f8 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Cleanup/refactor tracking of consensus watermarks
Kudu Jenkins has posted comments on this change. Change subject: Cleanup/refactor tracking of consensus watermarks .. Patch Set 4: Build Started http://104.196.14.100/job/kudu-gerrit/3145/ -- To view, visit http://gerrit.cloudera.org:8080/4133 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2aa294472f018013d88f36a9358e9ebd9d5ed8f8 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] tool: port log-dump
Todd Lipcon has posted comments on this change. Change subject: tool: port log-dump .. Patch Set 1: Code lgtm. Can you add this to the release note with a pointer to 'kudu wal dump' and 'kudu tablet dump_wals'? -- To view, visit http://gerrit.cloudera.org:8080/4167 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I14f048169c0b211e3c72fcd255c76dd59cbb05c9 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1231. Add "unlock" flag for experimental and unsafe flags
Todd Lipcon has submitted this change and it was merged. 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 Reviewed-on: http://gerrit.cloudera.org:8080/4100 Reviewed-by: Adar Dembo Tested-by: Todd Lipcon --- 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, 136 insertions(+), 5 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Todd Lipcon: Verified -- To view, visit http://gerrit.cloudera.org:8080/4100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Iec49e77fca604a7c5ee7501121a6263b7ee590d6 Gerrit-PatchSet: 10 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[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 9: Verified+1 Hit a jenkins bug in the lint build (but previous lint builds passed) -- 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: 9 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: No
[kudu-CR] Predicate evaluation pushdown
Todd Lipcon has posted comments on this change. Change subject: Predicate evaluation pushdown .. Patch Set 7: (31 comments) http://gerrit.cloudera.org:8080/#/c/3990/7/src/kudu/cfile/binary_plain_block.cc File src/kudu/cfile/binary_plain_block.cc: Line 306: Status BinaryPlainBlockDecoder::CopyNextValues(size_t *n, ColumnDataView *dst) { I wonder whether this function could share code with the new one using some templating (to force that they're separately instantiated) http://gerrit.cloudera.org:8080/#/c/3990/7/src/kudu/cfile/binary_prefix_block.h File src/kudu/cfile/binary_prefix_block.h: Line 99: cur_idx_ += *n; surprised this doesn't need to update cur_val_ and next_ptr_ http://gerrit.cloudera.org:8080/#/c/3990/7/src/kudu/cfile/cfile_reader.cc File src/kudu/cfile/cfile_reader.cc: Line 488: ctx->SetDecoderEvalNotSupported(); shouldn't this be done in PrepareEvalContext? PS7, Line 489: ctx->block() given the number of times you use ctx->block() in this function I think it's worth just doing: ColumnBlock* dst = ctx->block() http://gerrit.cloudera.org:8080/#/c/3990/7/src/kudu/cfile/cfile_reader.h File src/kudu/cfile/cfile_reader.h: Line 355: // Sets context if needed. This must happen before a call to no need for this line (since it's already described in the interface above) PS7, Line 390: ; nit: space after ; PS7, Line 463: or>cod nit: space after > Line 480: // Predicate and selection vector associated with the iterator would this ever be null? worth a comment explaining when it might be http://gerrit.cloudera.org:8080/#/c/3990/7/src/kudu/cfile/rle_block.h File src/kudu/cfile/rle_block.h: Line 190: void SeekForward(size_t *n) override { hrm, I'm skeptical of this function, I'd think it would need to update the rle_decoder_ as well. Maybe update encoding-test so that this is covered? Line 413: void SeekForward(size_t *n) override { same http://gerrit.cloudera.org:8080/#/c/3990/7/src/kudu/common/column_eval_context.h File src/kudu/common/column_eval_context.h: Line 26: // get passed down to the decoders during predicate push-down. this is now more generally the set of objects passed down to the column iterator and decoder during column materialization, right? Line 31: public: nit: indent one Line 43: // Column index in the parent CFileSet. I think this is incorrect and it's actually the index within the projection schema, rather than within the CFileSet. Perhaps we should call it projection_idx or something to clarify? Line 51: // I.e. BinaryDictBlockDecoder.CopyNextAndEval(), CFileIterator.Scan() not quite following this comment. when is this used outside of scans? Line 54: // Selection vector reflecting the result of the predicate evaluation. should clarify whether this is an "out" or an "in-out" type parameter. In other words, if the scanner sees that a column is already non-selected in here, is it free to skip reading that row? I'm guessing so, but good to clarify. Line 57: // Must be greater than size 0 and must be large enough to cover the number isn't "greater than size 0" redundant with "large enough to cover the number of rows"? Line 63: // materializing_iterator_decoder_eval to false, forcing evaluation to fall I think this comment needs an update Line 74: if (decoder_eval_status_ == kNotSet) { it seems weird that this function is a silent no-op in the case that it's already been set. If it's set in a "conflicting" manner, what happens? seems like it should either be a DCHECK, or by a 'TrySetDecoderEvalSupported' type function that returns a bool about whether it successfully changed modes or not. Line 79: void SetDecoderEvalNotSupported() { particularly seems like here it should be a CHECK that it's kNotSet. Otherwise you might try to switch from supported to non-supported, and only partially have filled in the selection vector http://gerrit.cloudera.org:8080/#/c/3990/7/src/kudu/common/generic_iterators.cc File src/kudu/common/generic_iterators.cc: Line 26: #include "kudu/common/column_eval_context.h" nit: sort the includes PS7, Line 522: = ColumnEvalContext can just be: ColumnEvalContext ctx(...) instead of the extra temporary construction and assignment Also, given that we now use ColumnEvalContext for all materialization, and not just the one with predicate evaluation, we should rename to ColumnMaterializationContext Line 526: if (ctx.pred()->predicate_type() == PredicateType::None) { does a None predicate ever make it this far into the code? I would think that the None would be handled way up above this layer (and this could be a DCHECK) Line 549: ColumnEvalContext ctx = ColumnEvalContext(col_idx, same Line 553: ctx.SetDecoderEvalNotSupported(); why's this necessary? shouldn't the nullptr predicate be sufficient for the code to know it doesn't need to evaluate any predicate? http://gerrit.cloudera
[kudu-CR] tool: port log-dump
Hello Todd Lipcon, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/4167 to review the following change. Change subject: tool: port log-dump .. tool: port log-dump This one was more complicated, because log-dump can run against a single file or an entire tablet. So I put all the common functionality in one place and referenced it from both modes. I consolidated similar gflags where it made sense to do so, and I tweaked the endline handling for help generation so that each parameter is separated from the next with an empty line. Semantic changes from log-dump: - If called with print_entries=no, will also print the footer. - The print_headers flag is now print_meta, to be more consistent with 'kudu cfile dump'. Change-Id: I14f048169c0b211e3c72fcd255c76dd59cbb05c9 --- M src/kudu/consensus/CMakeLists.txt M src/kudu/tools/CMakeLists.txt M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action.cc M src/kudu/tools/tool_action.h R src/kudu/tools/tool_action_common.cc A src/kudu/tools/tool_action_common.h M src/kudu/tools/tool_action_fs.cc M src/kudu/tools/tool_action_tablet.cc A src/kudu/tools/tool_action_wal.cc M src/kudu/tools/tool_main.cc M src/kudu/util/test_macros.h 12 files changed, 434 insertions(+), 160 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/67/4167/1 -- To view, visit http://gerrit.cloudera.org:8080/4167 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I14f048169c0b211e3c72fcd255c76dd59cbb05c9 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Todd Lipcon
[kudu-CR] tool: port log-dump
Kudu Jenkins has posted comments on this change. Change subject: tool: port log-dump .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/3144/ -- To view, visit http://gerrit.cloudera.org:8080/4167 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I14f048169c0b211e3c72fcd255c76dd59cbb05c9 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-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 9: Code-Review+2 -- 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: 9 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-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: 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 9: arg, one more conflict resolution! -- 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: 9 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-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: No
[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 9: Build Started http://104.196.14.100/job/kudu-gerrit/3143/ -- 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: 9 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-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: No
[kudu-CR] KUDU-1231. Add "unlock" flag for experimental and unsafe flags
Hello Mike Percy, Adar Dembo, Alexey Serbin, 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 (#9). 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, 136 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/00/4100/9 -- 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: 9 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] Inlined dispatch for predicate evaluation
Kudu Jenkins has posted comments on this change. Change subject: Inlined dispatch for predicate evaluation .. Patch Set 2: Build Started http://104.196.14.100/job/kudu-gerrit/3142/ -- To view, visit http://gerrit.cloudera.org:8080/4164 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] Inlined dispatch for predicate evaluation
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4164 to look at the new patch set (#2). Change subject: Inlined dispatch for predicate evaluation .. Inlined dispatch for predicate evaluation In order to evaluate a predicate, we must first determine the correct comparator to handle the physical type of interest. Currently, this occurs once per batch when ColumnPredicate::Evaluate() is called. A call to this will dispatch the comparator as a lambda to be applied in a loop over all rows in the batch. For ColumnPredicate::EvaluateCell(), rather than occuring in batches, dispatch occurs for each cell. This particularly slows down decoder-level evaluation, which falls back on evaluating cell-by-cell rather than in batches. To remediate this, the dispatch has been templatized here to remediate the cost of repeated dispatch. This figure shows the performance of dictionary encoding for decoder-level evaluation without this templating adjustment. The query selects a single value out of a dictionary-encoded column containing 1M unique strings. https://raw.githubusercontent.com/anjuwong/kudu/300f1e8bdf05bc16bdacacca22482e579e49de67/docs/images/SELECT_OUTSIDE_OF_RANGE_non_inlined.png Compare the above with the plot on the right, which is the result of the same query, but with inlined dispatch. https://raw.githubusercontent.com/anjuwong/kudu/300f1e8bdf05bc16bdacacca22482e579e49de67/docs/images/SELECT_OUTSIDE_OF_RANGE_inlined.png Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 --- M src/kudu/cfile/binary_plain_block.cc M src/kudu/cfile/cfile_reader.cc M src/kudu/common/column_predicate.cc M src/kudu/common/column_predicate.h 4 files changed, 83 insertions(+), 44 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/64/4164/2 -- To view, visit http://gerrit.cloudera.org:8080/4164 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] raft consensus-itest: inserter thread should FATAL instead of FAIL
Alexey Serbin has posted comments on this change. Change subject: raft_consensus-itest: inserter thread should FATAL instead of FAIL .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/4140 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9dbe1e551b7f1b8b81ce0156627deb096db5112b Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] transaction tracker: back-off when logging in-flight transactions
Alexey Serbin has posted comments on this change. Change subject: transaction_tracker: back-off when logging in-flight transactions .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/4159 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7d1a2563dd57c59ae8c130a5b26966f4964048de Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] [docs] added Kudu version into the doxygen footer
Adar Dembo has posted comments on this change. Change subject: [docs] added Kudu version into the doxygen footer .. Patch Set 2: (6 comments) http://gerrit.cloudera.org:8080/#/c/4165/1/CMakeLists.txt File CMakeLists.txt: PS1, Line 79: execute_process(COMMAND git rev-parse HEAD : WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} : OUTPUT_VARIABLE KUDU_SRC_GIT_HASH) What happens if this runs outside of a git repo, or if git isn't installed? Some of our builds don't work directly against git. Also, I think it would be better if we could rely on version_defines.h (see util/CMakeLists.txt) for this information instead of generating it again. http://gerrit.cloudera.org:8080/#/c/4165/1/docs/support/doxygen/client_api.doxy.in File docs/support/doxygen/client_api.doxy.in: PS1, Line 55: share/doc/kuduClient/samples/sample.cc Can we substitute this in as a list? Better yet if we can somehow set it from within src/kudu/client/CMakeLists.txt, which is where sample.cc is placed in the installation output. http://gerrit.cloudera.org:8080/#/c/4165/1/docs/support/doxygen/client_api.footer.in File docs/support/doxygen/client_api.footer.in: PS1, Line 6: : Presumably these were opened in the header? http://gerrit.cloudera.org:8080/#/c/4165/1/src/kudu/client/shared_ptr.h File src/kudu/client/shared_ptr.h: PS1, Line 21: /// @file shared_ptr.h Hmm, I don't recall seeing @file tags in the other files you doxygenized. What is its purpose here? PS1, Line 22: pointers Nit: pointer PS1, Line 39: defined(__clang__) You can build Kudu with gcc on macOS? Didn't know that was possible. -- To view, visit http://gerrit.cloudera.org:8080/4165 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3861aafa1e615cc991647704d9a4ce8d44ad678f Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] [docs] added Kudu version into the doxygen footer
Kudu Jenkins has posted comments on this change. Change subject: [docs] added Kudu version into the doxygen footer .. Patch Set 2: Build Started http://104.196.14.100/job/kudu-gerrit/3141/ -- To view, visit http://gerrit.cloudera.org:8080/4165 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3861aafa1e615cc991647704d9a4ce8d44ad678f Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] [docs] added Kudu version into the doxygen footer
Alexey Serbin has uploaded a new patch set (#2). Change subject: [docs] added Kudu version into the doxygen footer .. [docs] added Kudu version into the doxygen footer Added information on the Kudu source version into the HTML footer of the auto-generated doxygen docs of the Kudu C++ API. Removed sample.cc file from the list of files to process with doxygen. Change-Id: I3861aafa1e615cc991647704d9a4ce8d44ad678f --- M CMakeLists.txt M docs/support/doxygen/client_api.doxy.in A docs/support/doxygen/client_api.footer.in M src/kudu/client/shared_ptr.h 4 files changed, 42 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/65/4165/2 -- To view, visit http://gerrit.cloudera.org:8080/4165 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3861aafa1e615cc991647704d9a4ce8d44ad678f Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [docs] added Kudu version into the doxygen footer
Kudu Jenkins has posted comments on this change. Change subject: [docs] added Kudu version into the doxygen footer .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/3140/ -- To view, visit http://gerrit.cloudera.org:8080/4165 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3861aafa1e615cc991647704d9a4ce8d44ad678f Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] [docs] added Kudu version into the doxygen footer
Alexey Serbin has uploaded a new change for review. http://gerrit.cloudera.org:8080/4165 Change subject: [docs] added Kudu version into the doxygen footer .. [docs] added Kudu version into the doxygen footer Added information on the Kudu source version into the HTML footer of the auto-generated doxygen docs of the Kudu C++ API. Removed sample.cc file from the list of files to process with doxygen. Change-Id: I3861aafa1e615cc991647704d9a4ce8d44ad678f --- M CMakeLists.txt M docs/support/doxygen/client_api.doxy.in A docs/support/doxygen/client_api.footer.in M src/kudu/client/shared_ptr.h 4 files changed, 46 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/65/4165/1 -- To view, visit http://gerrit.cloudera.org:8080/4165 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I3861aafa1e615cc991647704d9a4ce8d44ad678f Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin
[kudu-CR] Inlined dispatch for predicate evaluation
Kudu Jenkins has posted comments on this change. Change subject: Inlined dispatch for predicate evaluation .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/3139/ -- To view, visit http://gerrit.cloudera.org:8080/4164 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] Inlined dispatch for predicate evaluation
Andrew Wong has uploaded a new change for review. http://gerrit.cloudera.org:8080/4164 Change subject: Inlined dispatch for predicate evaluation .. Inlined dispatch for predicate evaluation In order to evaluate a predicate, we must first determine the correct comparator to handle the physical type of interest. Currently, this occurs once per batch when ColumnPredicate::Evaluate() is called. A call to this will dispatch the comparator as a lambda to be applied in a loop over all rows in the batch. For ColumnPredicate::EvaluateCell(), rather than occuring in batches, dispatch occurs for each cell. This particularly slows down decoder-level evaluation, which falls back on evaluating cell-by-cell rather than in batches. To remediate this, the dispatch has been templatized here to remediate the cost of repeated dispatch. This figure shows the performance of dictionary encoding for decoder-level evaluation without this templating adjustment. The query selects a single value out of a dictionary-encoded column containing 1M unique strings. https://raw.githubusercontent.com/anjuwong/kudu/300f1e8bdf05bc16bdacacca22482e579e49de67/docs/images/SELECT_OUTSIDE_OF_RANGE_non_inlined.png Compare the above with the plot on the right, which is the result of the same query, but with inlined dispatch. https://raw.githubusercontent.com/anjuwong/kudu/300f1e8bdf05bc16bdacacca22482e579e49de67/docs/images/SELECT_OUTSIDE_OF_RANGE_inlined.png Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 --- M src/kudu/cfile/binary_plain_block.cc M src/kudu/cfile/cfile_reader.cc M src/kudu/common/column_predicate.cc M src/kudu/common/column_predicate.h 4 files changed, 86 insertions(+), 48 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/64/4164/1 -- To view, visit http://gerrit.cloudera.org:8080/4164 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong
[kudu-CR] compaction policy: fix bound calculation
Will Berkeley has posted comments on this change. Change subject: compaction_policy: fix bound calculation .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/4152 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I76364c17debd7293b36884d64c7747f8c604ae12 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Anonymous Coward #174 Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: No
[kudu-CR] KUDU-1048 master should show versions of tservers, version summary
Todd Lipcon has submitted this change and it was merged. Change subject: KUDU-1048 master should show versions of tservers, version summary .. KUDU-1048 master should show versions of tservers, version summary This patch adds a version summary table and a total count of registered tablet servers to /tablet-servers. It also fixes the display of the registration, which was printing in red font. Sample: https://raw.githubusercontent.com/wdberkeley/kudu-cr/master/KUDU-1048-improved.png Change-Id: Idd203209e3d99292018801b94ec2904b6634854f Reviewed-on: http://gerrit.cloudera.org:8080/4104 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon --- M src/kudu/master/master-path-handlers.cc M src/kudu/master/ts_descriptor.cc M src/kudu/master/ts_descriptor.h M src/kudu/master/ts_manager.cc 4 files changed, 46 insertions(+), 14 deletions(-) Approvals: Todd Lipcon: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4104 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Idd203209e3d99292018801b94ec2904b6634854f 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] KUDU-1048 master should show versions of tservers, version summary
Todd Lipcon has posted comments on this change. Change subject: KUDU-1048 master should show versions of tservers, version summary .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4104 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idd203209e3d99292018801b94ec2904b6634854f 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-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 8: Code-Review+2 -- 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: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-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: No
[kudu-CR] tool: port cfile-dump to 'kudu fs dump cfile'
Adar Dembo has submitted this change and it was merged. Change subject: tool: port cfile-dump to 'kudu fs dump_cfile' .. tool: port cfile-dump to 'kudu fs dump_cfile' Some non-cosmetic changes: - I changed the block_id conversion into something nicer than a CHECK. - The block_id parameter is expected in base 10, not base 16. To be honest, cfile-dump should have used base 10 for quite some time, because that's how they're printed in dumped PBs. - I dropped the num_iterations parameter because it didn't seem useful. Change-Id: I30cbaa6552e88348cebbf3059390a4c252eb7f8e Reviewed-on: http://gerrit.cloudera.org:8080/4151 Reviewed-by: Todd Lipcon Tested-by: Kudu Jenkins --- M src/kudu/cfile/CMakeLists.txt D src/kudu/cfile/cfile-dump.cc M src/kudu/cfile/cfile-test-base.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_fs.cc 5 files changed, 131 insertions(+), 106 deletions(-) Approvals: Todd Lipcon: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4151 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I30cbaa6552e88348cebbf3059390a4c252eb7f8e Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] tool: port cfile-dump to 'kudu fs dump cfile'
Todd Lipcon has posted comments on this change. Change subject: tool: port cfile-dump to 'kudu fs dump_cfile' .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4151 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I30cbaa6552e88348cebbf3059390a4c252eb7f8e Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[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 8: Build Started http://104.196.14.100/job/kudu-gerrit/3138/ -- 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: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-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: No
[kudu-CR] KUDU-1231. Add "unlock" flag for experimental and unsafe flags
Hello Mike Percy, Adar Dembo, Alexey Serbin, 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 (#8). 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, 136 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/00/4100/8 -- 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: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-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 7: Build Started http://104.196.14.100/job/kudu-gerrit/3135/ -- 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: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-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: No
[kudu-CR] KUDU-1231. Add "unlock" flag for experimental and unsafe flags
Hello Mike Percy, Alexey Serbin, 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 (#7). 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, 136 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/00/4100/7 -- 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: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] tool: port cfile-dump to 'kudu fs dump cfile'
Kudu Jenkins has posted comments on this change. Change subject: tool: port cfile-dump to 'kudu fs dump_cfile' .. Patch Set 3: Build Started http://104.196.14.100/job/kudu-gerrit/3137/ -- To view, visit http://gerrit.cloudera.org:8080/4151 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I30cbaa6552e88348cebbf3059390a4c252eb7f8e Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-456 Implement AUTO FLUSH BACKGROUND flush mode
Kudu Jenkins has posted comments on this change. Change subject: KUDU-456 Implement AUTO_FLUSH_BACKGROUND flush mode .. Patch Set 23: Build Started http://104.196.14.100/job/kudu-gerrit/3136/ -- 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: 23 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-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: No
[kudu-CR] KUDU-456 Implement AUTO FLUSH BACKGROUND flush mode
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3952 to look at the new patch set (#23). Change subject: KUDU-456 Implement AUTO_FLUSH_BACKGROUND flush mode .. KUDU-456 Implement AUTO_FLUSH_BACKGROUND flush mode Implemented AUTO_FLUSH_BACKGROUND for the Kudu C++ client library. In AUTO_FLUSH_BACKGROUND mode, the KuduSession::Apply() method blocks if total amount of data for pending operations reaches the buffer size limit. The limit on the buffer size can be set by the KuduSession::SetMutationBufferSpace() method. The background flush logic checks whether at least one of the following two conditions is satisfied to determine whether it's time to flush the accumulated write operations: * The over-the-watermark criterion: check whether the total size of the freshly submitted (i.e. not-yet-scheduled-for-flush) write operations is over the threshold. The threshold can be set as the percentage of the total buffer size using the KuduSession::SetMutationBufferFlushWatermark() method. * The maximum wait time criterion: check whether the current batch of operations has been accumulating for more than the maximum wait time. The maximum wait time can be specified in milliseconds using the KuduSession::SetMutationBufferFlushInterval() method. A KuduSession object uses RPC messenger's thread pool to monitor batches' maximum wait time. Added functionality to control the maximum number of batchers per session. If number of batchers is at the limit already, KuduSession::Apply() blocks until it's possible to add a new batcher to accommodate the incoming operation. Modified behavior of the KuduSession::Flush(): now it waits until all batchers are flushed before returning. This change also addresses the following JIRA issue: KUDU-1376 KuduSession::SetMutationBufferSpace is not defined Change-Id: I34905c30b3aad96f53cf7a1822b1cde6d25f33a8 --- M python/kudu/tests/test_client.py M src/kudu/client/batcher.cc M src/kudu/client/batcher.h M src/kudu/client/client-internal.h M src/kudu/client/client-test.cc M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/error_collector.cc M src/kudu/client/error_collector.h M src/kudu/client/session-internal.cc M src/kudu/client/session-internal.h M src/kudu/client/write_op.cc M src/kudu/client/write_op.h 13 files changed, 1,440 insertions(+), 209 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/52/3952/23 -- To view, visit http://gerrit.cloudera.org:8080/3952 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I34905c30b3aad96f53cf7a1822b1cde6d25f33a8 Gerrit-PatchSet: 23 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] tool: split up action descriptions
Adar Dembo has submitted this change and it was merged. Change subject: tool: split up action descriptions .. tool: split up action descriptions With ksck we have a use case for "short" and "long" action descriptions: the former appears in mode help and action help, while the latter is only in action help. This patch splits action descriptions into "short" and "extra", refactoring all actions accordingly. While I was there I got rid of the label abstraction, which I felt was producing too much unfluent code. I also added some more tests to kudu-tool-test. Change-Id: Ie90b6228bba54575933fd9ac2f4f0bebf579ecd7 Reviewed-on: http://gerrit.cloudera.org:8080/4148 Reviewed-by: Todd Lipcon Tested-by: Kudu Jenkins --- M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action.cc M src/kudu/tools/tool_action.h M src/kudu/tools/tool_action_cluster.cc M src/kudu/tools/tool_action_fs.cc M src/kudu/tools/tool_action_pbc.cc M src/kudu/tools/tool_action_tablet.cc M src/kudu/tools/tool_main.cc 8 files changed, 261 insertions(+), 97 deletions(-) Approvals: Todd Lipcon: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4148 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ie90b6228bba54575933fd9ac2f4f0bebf579ecd7 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[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 7: Code-Review+2 -- 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: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-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: 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 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. Done 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. Done Line 75: ASSERT_DEATH({ HandleCommonFlags();}, > Nit: { HandleCommonFlags(); } Done PS6, Line 82: StringVectorSink sink; : ScopedRegisterSink reg(&sink); > You know you've made it as a project when you've got unit tests that check \_o_/ 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 Done -- 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 Lipcon Gerrit-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] Fix kudu-ts-cli crash when there is no data in tablet
Todd Lipcon has posted comments on this change. Change subject: Fix kudu-ts-cli crash when there is no data in tablet .. Patch Set 6: Code change looks good, but how about a test in kudu-ts-cli-test? -- To view, visit http://gerrit.cloudera.org:8080/4134 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8470377400047236127dd409b4825c5c6ea6bbae Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] tool: port cfile-dump to 'kudu fs dump cfile'
Todd Lipcon has posted comments on this change. Change subject: tool: port cfile-dump to 'kudu fs dump_cfile' .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4151 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I30cbaa6552e88348cebbf3059390a4c252eb7f8e Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] tool: split up action descriptions
Kudu Jenkins has posted comments on this change. Change subject: tool: split up action descriptions .. Patch Set 4: Build Started http://104.196.14.100/job/kudu-gerrit/3134/ -- To view, visit http://gerrit.cloudera.org:8080/4148 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie90b6228bba54575933fd9ac2f4f0bebf579ecd7 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] cfile: replace DumpIteratorOptions with number of rows
Todd Lipcon has submitted this change and it was merged. Change subject: cfile: replace DumpIteratorOptions with number of rows .. cfile: replace DumpIteratorOptions with number of rows As of commit 9884fab, DumpIterator() doesn't print anything at all when print_rows is false. That makes the option rather useless, so I'm replacing it here with the raw number of rows. Change-Id: I5d9e80e50926a71d22de4f88a7af6a8091bb2063 Reviewed-on: http://gerrit.cloudera.org:8080/4150 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon --- M src/kudu/cfile/cfile-dump.cc M src/kudu/cfile/cfile_util.cc M src/kudu/cfile/cfile_util.h M src/kudu/tools/fs_tool.cc 4 files changed, 27 insertions(+), 52 deletions(-) Approvals: Todd Lipcon: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4150 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I5d9e80e50926a71d22de4f88a7af6a8091bb2063 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] cfile: replace DumpIteratorOptions with number of rows
Todd Lipcon has posted comments on this change. Change subject: cfile: replace DumpIteratorOptions with number of rows .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4150 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5d9e80e50926a71d22de4f88a7af6a8091bb2063 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] tool: split up action descriptions
Todd Lipcon has posted comments on this change. Change subject: tool: split up action descriptions .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4148 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie90b6228bba54575933fd9ac2f4f0bebf579ecd7 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-687: expose additional tablet metadata in C++ client
Todd Lipcon has submitted this change and it was merged. Change subject: KUDU-687: expose additional tablet metadata in C++ client .. KUDU-687: expose additional tablet metadata in C++ client This patch adds new data-only KuduReplica and KuduTablet classes. Along with KuduTabletServer, the C++ client now has more parity with the Java client w.r.t. exposing tablet metadata. For now, this is only exposed via KuduScanToken, because it's already doing the work to figure out which replicas exist where. That's an odd fit for ksck (which doesn't scan, at least not using the client), but it should stave off any controversy stemming from adding dubious public APIs. In the future, it wouldn't be unreasonable for these classes to be exposed via KuduTable in some way. There are four public signature changes: - Addition of KuduScanToken::tablet(): this is backwards compatible. - Addition of KuduTabletServer::port(): this is backwards compatible. - Change to the KuduScanToken constructor: this should be backwards compatible, because it's private and so shouldn't used by applications. - Removal of KuduScanToken::TabletServers(): this is an incompatible change. I think it's OK because Impala is the only significant C++ client user and it's not even using scan tokens yet. Change-Id: I3cbbb1df8d3adf60425541b57e68595bbf6e92ff Reviewed-on: http://gerrit.cloudera.org:8080/4146 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon --- M docs/release_notes.adoc M src/kudu/client/CMakeLists.txt M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/meta_cache.cc M src/kudu/client/meta_cache.h A src/kudu/client/replica-internal.cc A src/kudu/client/replica-internal.h M src/kudu/client/scan_token-internal.cc M src/kudu/client/scan_token-internal.h M src/kudu/client/scan_token-test.cc A src/kudu/client/tablet-internal.cc A src/kudu/client/tablet-internal.h M src/kudu/client/tablet_server-internal.cc M src/kudu/client/tablet_server-internal.h 15 files changed, 445 insertions(+), 95 deletions(-) Approvals: Todd Lipcon: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I3cbbb1df8d3adf60425541b57e68595bbf6e92ff Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-687: use client in ksck for master operations
Todd Lipcon has posted comments on this change. Change subject: KUDU-687: use client in ksck for master operations .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4147 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icbd6b28ea1b2c88e02cd451095e4dec94b0ebfc3 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-687: use client in ksck for master operations
Todd Lipcon has submitted this change and it was merged. Change subject: KUDU-687: use client in ksck for master operations .. KUDU-687: use client in ksck for master operations This patch modifies ksck to use the client for all master operations, restricting direct access only for tserver operations. In doing so, ksck can now work with multi-master clusters, retrying operations when the leader master dies. Interesting things of note: - I went back and forth on what the semantics of KsckMaster::Connect() ought to be. At first I thought we should ping every master, but in the end I settled on building the client, which just verifies the existence of a leader master. My justification: today's ksck isn't really concerned with master health; the master merely provides information that is consumed during the real checks: of table and tablet integrity. - I relented on commit cf009d4 and restored a MiniCluster method to find the leader master. It's got the same signature as the ExternalMiniCluster variant so that Mike's common cluster interface (a work in progress) can expose it. Change-Id: Icbd6b28ea1b2c88e02cd451095e4dec94b0ebfc3 Reviewed-on: http://gerrit.cloudera.org:8080/4147 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon --- M src/kudu/client/schema.h M src/kudu/integration-tests/cluster_verifier.cc M src/kudu/integration-tests/mini_cluster.cc M src/kudu/integration-tests/mini_cluster.h M src/kudu/tools/CMakeLists.txt M src/kudu/tools/ksck-test.cc M src/kudu/tools/ksck.h M src/kudu/tools/ksck_remote-test.cc M src/kudu/tools/ksck_remote.cc M src/kudu/tools/ksck_remote.h M src/kudu/tools/tool_action_cluster.cc 11 files changed, 163 insertions(+), 166 deletions(-) Approvals: Todd Lipcon: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4147 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Icbd6b28ea1b2c88e02cd451095e4dec94b0ebfc3 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] transaction tracker: back-off when logging in-flight transactions
Kudu Jenkins has posted comments on this change. Change subject: transaction_tracker: back-off when logging in-flight transactions .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/3133/ -- To view, visit http://gerrit.cloudera.org:8080/4159 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7d1a2563dd57c59ae8c130a5b26966f4964048de Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] transaction tracker: back-off when logging in-flight transactions
Hello Mike Percy, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/4159 to review the following change. Change subject: transaction_tracker: back-off when logging in-flight transactions .. transaction_tracker: back-off when logging in-flight transactions On a test cluster pushed into overload, I ended up with thousands of in-flight transactions which were only being drained at the rate of one or two per second. Dumping the entire list of transactions once a second ended up flooding the glog with many MB per second work of transaction strings. This changes the dumping to back-off exponentially. Change-Id: I7d1a2563dd57c59ae8c130a5b26966f4964048de --- M src/kudu/tablet/transactions/transaction_tracker.cc 1 file changed, 16 insertions(+), 13 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/59/4159/1 -- To view, visit http://gerrit.cloudera.org:8080/4159 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I7d1a2563dd57c59ae8c130a5b26966f4964048de Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Mike Percy
[kudu-CR] KUDU-687: use client in ksck for master operations
Adar Dembo has posted comments on this change. Change subject: KUDU-687: use client in ksck for master operations .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/4147/1/src/kudu/tools/ksck_remote.cc File src/kudu/tools/ksck_remote.cc: Line 284: for (auto s : servers) { > ah. I think 'const auto*' is nice, then. Makes it clearer it's a pointer. OK, added * where appropriate. -- To view, visit http://gerrit.cloudera.org:8080/4147 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icbd6b28ea1b2c88e02cd451095e4dec94b0ebfc3 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-687: use client in ksck for master operations
Kudu Jenkins has posted comments on this change. Change subject: KUDU-687: use client in ksck for master operations .. Patch Set 4: Build Started http://104.196.14.100/job/kudu-gerrit/3132/ -- To view, visit http://gerrit.cloudera.org:8080/4147 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icbd6b28ea1b2c88e02cd451095e4dec94b0ebfc3 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-687: use client in ksck for master operations
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4147 to look at the new patch set (#4). Change subject: KUDU-687: use client in ksck for master operations .. KUDU-687: use client in ksck for master operations This patch modifies ksck to use the client for all master operations, restricting direct access only for tserver operations. In doing so, ksck can now work with multi-master clusters, retrying operations when the leader master dies. Interesting things of note: - I went back and forth on what the semantics of KsckMaster::Connect() ought to be. At first I thought we should ping every master, but in the end I settled on building the client, which just verifies the existence of a leader master. My justification: today's ksck isn't really concerned with master health; the master merely provides information that is consumed during the real checks: of table and tablet integrity. - I relented on commit cf009d4 and restored a MiniCluster method to find the leader master. It's got the same signature as the ExternalMiniCluster variant so that Mike's common cluster interface (a work in progress) can expose it. Change-Id: Icbd6b28ea1b2c88e02cd451095e4dec94b0ebfc3 --- M src/kudu/client/schema.h M src/kudu/integration-tests/cluster_verifier.cc M src/kudu/integration-tests/mini_cluster.cc M src/kudu/integration-tests/mini_cluster.h M src/kudu/tools/CMakeLists.txt M src/kudu/tools/ksck-test.cc M src/kudu/tools/ksck.h M src/kudu/tools/ksck_remote-test.cc M src/kudu/tools/ksck_remote.cc M src/kudu/tools/ksck_remote.h M src/kudu/tools/tool_action_cluster.cc 11 files changed, 163 insertions(+), 166 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/47/4147/4 -- To view, visit http://gerrit.cloudera.org:8080/4147 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Icbd6b28ea1b2c88e02cd451095e4dec94b0ebfc3 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Fix kudu-ts-cli crash when there is no data in tablet
Kudu Jenkins has posted comments on this change. Change subject: Fix kudu-ts-cli crash when there is no data in tablet .. Patch Set 6: Build Started http://104.196.14.100/job/kudu-gerrit/3131/ -- To view, visit http://gerrit.cloudera.org:8080/4134 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8470377400047236127dd409b4825c5c6ea6bbae Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Fix kudu-ts-cli crash when there is no data in tablet
Hello Dan Burkert, Todd Lipcon, Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4134 to look at the new patch set (#6). Change subject: Fix kudu-ts-cli crash when there is no data in tablet .. Fix kudu-ts-cli crash when there is no data in tablet NULL pointer was being sent in as an argument to RowwiseRowBlockPB::Swap routine, fixing the callsite here. ./bin/kudu-ts-cli dump_tablet f32f6e0b1f354643b6b030599a359fa6 --server_address=127.108.26.1:33958 previously segfaulted to: $0 0x009c9867 in std::swap (__a=@0x7ffc75c76af8, __b=@0x18) at /opt/rh/devtoolset-3/root/usr/include/c++/4.9.2/bits/move.h:176 $1 0x00b5bf1f in kudu::RowwiseRowBlockPB::Swap (this=0x7ffc75c76ae0, other=0x0) at /data/10/dinesh/kudu/build/debug/src/kudu/common/wire_protocol.pb.cc:1916 $2 0x0085de72 in kudu::client::KuduScanBatch::Data::Reset (this=0x7ffc75c76a80, controller=0x7ffc75c76a20, projection=0x7ffc75c76b40, client_projection=0x7ffc75c76ca0, data=...) at /data/10/dinesh/kudu/src/kudu/client/scanner-internal.cc:484 $3 0x0084d5d9 in kudu::tools::TsAdminClient::DumpTablet (this=0x7ffc75c76e20, tablet_id="1f7a6bd64c604f7fada909bd575708cd") at /data/10/dinesh/kudu/src/kudu/tools/ts-cli.cc:278 $4 0x0084f4bd in kudu::tools::TsCliMain (argc=3, argv=0x7ffc75c773f0) at /data/10/dinesh/kudu/src/kudu/tools/ts-cli.cc:455 $5 0x00850079 in main (argc=4, argv=0x7ffc75c773e8) at /data/10/dinesh/kudu/src/kudu/tools/ts-cli.cc:492 Change-Id: I8470377400047236127dd409b4825c5c6ea6bbae --- M src/kudu/tools/ts-cli.cc 1 file changed, 17 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/34/4134/6 -- To view, visit http://gerrit.cloudera.org:8080/4134 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8470377400047236127dd409b4825c5c6ea6bbae Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Fix kudu-ts-cli crash when there is no data in tablet
Dinesh Bhat has posted comments on this change. Change subject: Fix kudu-ts-cli crash when there is no data in tablet .. Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/4134/2/src/kudu/tools/ts-cli.cc File src/kudu/tools/ts-cli.cc: Line 278: req.set_scanner_id(resp.scanner_id()); > yea, even without filters, you could have a bunch of deleted rows, and it's Got it, thanks, also updated the scan to 'continue' instead of break. I will pump some small data into tablet to check this patch doesn't break the regular usage of dump_tablet. -- To view, visit http://gerrit.cloudera.org:8080/4134 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8470377400047236127dd409b4825c5c6ea6bbae Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat 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] Fix kudu-ts-cli crash when there is no data in tablet
Hello Dan Burkert, Todd Lipcon, Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4134 to look at the new patch set (#5). Change subject: Fix kudu-ts-cli crash when there is no data in tablet .. Fix kudu-ts-cli crash when there is no data in tablet NULL pointer was being sent in as an argument to RowwiseRowBlockPB::Swap routine, fixing the callsite here. ./bin/kudu-ts-cli dump_tablet f32f6e0b1f354643b6b030599a359fa6 --server_address=127.108.26.1:33958 previously segfaulted to: $0 0x009c9867 in std::swap (__a=@0x7ffc75c76af8, __b=@0x18) at /opt/rh/devtoolset-3/root/usr/include/c++/4.9.2/bits/move.h:176 $1 0x00b5bf1f in kudu::RowwiseRowBlockPB::Swap (this=0x7ffc75c76ae0, other=0x0) at /data/10/dinesh/kudu/build/debug/src/kudu/common/wire_protocol.pb.cc:1916 $2 0x0085de72 in kudu::client::KuduScanBatch::Data::Reset (this=0x7ffc75c76a80, controller=0x7ffc75c76a20, projection=0x7ffc75c76b40, client_projection=0x7ffc75c76ca0, data=...) at /data/10/dinesh/kudu/src/kudu/client/scanner-internal.cc:484 $3 0x0084d5d9 in kudu::tools::TsAdminClient::DumpTablet (this=0x7ffc75c76e20, tablet_id="1f7a6bd64c604f7fada909bd575708cd") at /data/10/dinesh/kudu/src/kudu/tools/ts-cli.cc:278 $4 0x0084f4bd in kudu::tools::TsCliMain (argc=3, argv=0x7ffc75c773f0) at /data/10/dinesh/kudu/src/kudu/tools/ts-cli.cc:455 $5 0x00850079 in main (argc=4, argv=0x7ffc75c773e8) at /data/10/dinesh/kudu/src/kudu/tools/ts-cli.cc:492 Change-Id: I8470377400047236127dd409b4825c5c6ea6bbae --- M src/kudu/tools/ts-cli.cc 1 file changed, 8 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/34/4134/5 -- To view, visit http://gerrit.cloudera.org:8080/4134 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8470377400047236127dd409b4825c5c6ea6bbae Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Fix kudu-ts-cli crash when there is no data in tablet
Kudu Jenkins has posted comments on this change. Change subject: Fix kudu-ts-cli crash when there is no data in tablet .. Patch Set 5: Build Started http://104.196.14.100/job/kudu-gerrit/3130/ -- To view, visit http://gerrit.cloudera.org:8080/4134 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8470377400047236127dd409b4825c5c6ea6bbae Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] tool: split up action descriptions
Todd Lipcon has posted comments on this change. Change subject: tool: split up action descriptions .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4148 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie90b6228bba54575933fd9ac2f4f0bebf579ecd7 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-687: use client in ksck for master operations
Todd Lipcon has posted comments on this change. Change subject: KUDU-687: use client in ksck for master operations .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/4147/1/src/kudu/tools/ksck_remote.cc File src/kudu/tools/ksck_remote.cc: Line 284: for (const auto s : servers) { > I'll add const, but why ref? All of these containers are of raw pointers. ah. I think 'const auto*' is nice, then. Makes it clearer it's a pointer. -- To view, visit http://gerrit.cloudera.org:8080/4147 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icbd6b28ea1b2c88e02cd451095e4dec94b0ebfc3 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-687: expose additional tablet metadata in C++ client
Todd Lipcon has posted comments on this change. Change subject: KUDU-687: expose additional tablet metadata in C++ client .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3cbbb1df8d3adf60425541b57e68595bbf6e92ff Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1500: Data race in RaftConsensusITest.TestCorruptReplicaMetadata
Dinesh Bhat has posted comments on this change. Change subject: KUDU-1500: Data race in RaftConsensusITest.TestCorruptReplicaMetadata .. Patch Set 16: (1 comment) http://gerrit.cloudera.org:8080/#/c/3823/15/src/kudu/tablet/tablet_metadata.cc File src/kudu/tablet/tablet_metadata.cc: PS15, Line 313: } else { : DCHECK_EQ(table_id_, superblock.table_id()); : PartitionSchemaPB partition_schema_pb; : partition_schema_.ToPB(&partition_schema_pb); : DCHECK_EQ(superblock.partition_schema().SerializeAsString(), : partition_schema_pb.SerializeAsString()); : PartitionPB partition_pb; : partition_.ToPB(&partition_pb); : DCHECK_EQ(superblock.partition().SerializeAs > Well, it looks like we already have PartitionSchema::Equals() so maybe we j I liked this idea, but it still doesn't prevent compat break if the wire format differs from the local copy if the destination version is ahead of source during tablet-copy. In general, it seems this is a question of : if the node is accepting a remote copy, is it a good idea to accept a copy from a server with release (N-1) ? -- 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: 16 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-1048 master should show versions of tservers, version summary
Kudu Jenkins has posted comments on this change. Change subject: KUDU-1048 master should show versions of tservers, version summary .. Patch Set 3: Build Started http://104.196.14.100/job/kudu-gerrit/3129/ -- To view, visit http://gerrit.cloudera.org:8080/4104 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idd203209e3d99292018801b94ec2904b6634854f 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-1048 master should show versions of tservers, version summary
Will Berkeley has posted comments on this change. Change subject: KUDU-1048 master should show versions of tservers, version summary .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/4104/2/src/kudu/master/ts_manager.cc File src/kudu/master/ts_manager.cc: Line 103: if (ts->TimeSinceHeartbeat().ToMilliseconds() < FLAGS_tserver_unresponsive_timeout_ms) { > can you use PresumedDead here now? then you shouldn't need the DECLARE in t Done -- To view, visit http://gerrit.cloudera.org:8080/4104 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idd203209e3d99292018801b94ec2904b6634854f Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: Yes
[kudu-CR] KUDU-1048 master should show versions of tservers, version summary
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4104 to look at the new patch set (#3). Change subject: KUDU-1048 master should show versions of tservers, version summary .. KUDU-1048 master should show versions of tservers, version summary This patch adds a version summary table and a total count of registered tablet servers to /tablet-servers. It also fixes the display of the registration, which was printing in red font. Sample: https://raw.githubusercontent.com/wdberkeley/kudu-cr/master/KUDU-1048-improved.png Change-Id: Idd203209e3d99292018801b94ec2904b6634854f --- M src/kudu/master/master-path-handlers.cc M src/kudu/master/ts_descriptor.cc M src/kudu/master/ts_descriptor.h M src/kudu/master/ts_manager.cc 4 files changed, 46 insertions(+), 14 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/4104/3 -- To view, visit http://gerrit.cloudera.org:8080/4104 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Idd203209e3d99292018801b94ec2904b6634854f 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
[kudu-CR] KUDU-1581 Fix DataFrame read failure when table has Binary Col
Will Berkeley has posted comments on this change. Change subject: KUDU-1581 Fix DataFrame read failure when table has Binary Col .. Patch Set 3: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/4145 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3e3926d85fab7efb407325c9992c3fccdab04bad Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Ram Mettu Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Ram Mettu Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: No
[kudu-CR] KUDU-1581 Fix DataFrame read failure when table has Binary Col
Hello Will Berkeley, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4145 to look at the new patch set (#3). Change subject: KUDU-1581 Fix DataFrame read failure when table has Binary Col .. KUDU-1581 Fix DataFrame read failure when table has Binary Col For Binary Cols, kudu-spark is returning a ByteBuffer object when Spark expects to receive Array[Byte], so change is to return a copy of the byte array. Modified testcase to add missing col types and verify the values read back. Change-Id: I3e3926d85fab7efb407325c9992c3fccdab04bad --- M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduRDD.scala M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduContextTest.scala M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala 4 files changed, 57 insertions(+), 20 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/45/4145/3 -- To view, visit http://gerrit.cloudera.org:8080/4145 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3e3926d85fab7efb407325c9992c3fccdab04bad Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Ram Mettu Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Ram Mettu Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-1581 Fix DataFrame read failure when table has Binary Col
Kudu Jenkins has posted comments on this change. Change subject: KUDU-1581 Fix DataFrame read failure when table has Binary Col .. Patch Set 3: Build Started http://104.196.14.100/job/kudu-gerrit/3128/ -- To view, visit http://gerrit.cloudera.org:8080/4145 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3e3926d85fab7efb407325c9992c3fccdab04bad Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Ram Mettu Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Ram Mettu Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: No
[kudu-CR] KUDU-1581 Fix DataFrame read failure when table has Binary Col For Binary Cols, kudu-spark is returning a ByteBuffer object when Spark expects to receive Array[Byte], so change is to return
Ram Mettu has posted comments on this change. Change subject: KUDU-1581 Fix DataFrame read failure when table has Binary Col For Binary Cols, kudu-spark is returning a ByteBuffer object when Spark expects to receive Array[Byte], so change is to return a copy of the byte array. Modified testcase to add missing col ty .. Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/4145/2//COMMIT_MSG Commit Message: PS2, Line 8: For Binary > Nit: empty line here to conform to usual style Done http://gerrit.cloudera.org:8080/#/c/4145/2/java/kudu-spark/pom.xml File java/kudu-spark/pom.xml: PS2, Line 57: : > Nit: extra line break introduced here-- remove. Actually line break was removed in my change, it was reverted back to the original http://gerrit.cloudera.org:8080/#/c/4145/2/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala File java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala: PS2, Line 54: "c9_Timestamp" > Nit: c9_timestamp -- just to have consistent case Done PS2, Line 55: "c10_Byte" > Ditto Done -- To view, visit http://gerrit.cloudera.org:8080/4145 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3e3926d85fab7efb407325c9992c3fccdab04bad Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Ram Mettu Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Ram Mettu Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: Yes
[kudu-CR] KUDU-1500: Data race in RaftConsensusITest.TestCorruptReplicaMetadata
Mike Percy has posted comments on this change. Change subject: KUDU-1500: Data race in RaftConsensusITest.TestCorruptReplicaMetadata .. Patch Set 15: (1 comment) http://gerrit.cloudera.org:8080/#/c/3823/15/src/kudu/tablet/tablet_metadata.cc File src/kudu/tablet/tablet_metadata.cc: PS15, Line 313: DCHECK_EQ(table_id_, superblock.table_id()); : PartitionSchemaPB partition_schema_pb; : partition_schema_.ToPB(&partition_schema_pb); : DCHECK_EQ(superblock.partition_schema().SerializeAsString(), : partition_schema_pb.SerializeAsString()); : PartitionPB partition_pb; : partition_.ToPB(&partition_pb); : DCHECK_EQ(superblock.partition().SerializeAsString(), : partition_pb.SerializeAsString()); > I understand that this code isn't executed in release mode, but I feel fair Well, it looks like we already have PartitionSchema::Equals() so maybe we just need to implement Partition::Equals() to do this without comparing protobuf encodings. We could have something like: PartitionNSchema s; RETURN_NOT_OK(PartitionSchema::FromPB(superblock.partition_schema(), &s)); DCHECK(partition_schema_.Equals(s)); And something similar for Partition. In general I'm mostly concerned with forward and backward compatibility in release mode, although also maintaining it for DEBUG builds clearly would be nice. I think writing automated upgrade / downgrade tests would also help detect problems before release. -- 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: 15 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes