[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 25: Build Started http://104.196.14.100/job/kudu-gerrit/3167/ -- 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: 25 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 (#25). 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,516 insertions(+), 238 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/52/3952/25 -- 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: 25 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] 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 (#24). 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,517 insertions(+), 238 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/52/3952/24 -- 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: 24 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] 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 24: Build Started http://104.196.14.100/job/kudu-gerrit/3166/ -- 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: 24 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] ksck: add --checksum scan concurrency to optional flags
Kudu Jenkins has posted comments on this change. Change subject: ksck: add --checksum_scan_concurrency to optional flags .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/3164/ -- To view, visit http://gerrit.cloudera.org:8080/4183 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iaaa76215e0c30860eebd99db0548b3b37dd2e60e 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] ksck: add --checksum scan concurrency to optional flags
Hello Dan Burkert, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/4183 to review the following change. Change subject: ksck: add --checksum_scan_concurrency to optional flags .. ksck: add --checksum_scan_concurrency to optional flags Change-Id: Iaaa76215e0c30860eebd99db0548b3b37dd2e60e --- M src/kudu/tools/tool_action_cluster.cc 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/83/4183/1 -- To view, visit http://gerrit.cloudera.org:8080/4183 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Iaaa76215e0c30860eebd99db0548b3b37dd2e60e Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Dan Burkert
[kudu-CR] consensus: improve log messages for lagging or tablet-copying peers
Hello Dan Burkert, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/4184 to review the following change. Change subject: consensus: improve log messages for lagging or tablet-copying peers .. consensus: improve log messages for lagging or tablet-copying peers This patch improves the log throttling utility code so that it supports a user-defined "throttler" instance. It then uses a throttler for each consensus peer in order to improve the logging: - we used to spew "Successfully read ops from disk" messages out of log_cache.cc when we were reading messages for a lagging peer. This message was not that useful, considering it didn't tell us which peer was lagging. This is now replaced by a specific (and throttled) message letting us know which peer is lagging and how far behind they are. Unfortunately the "how far behind" is only in terms of op indexes and not time, since we don't attach any wall time to ReplicateMsgs. Still, this is better than we had before. - we used to spew "Sending request to start Tablet Copy" if a peer was already in the process of copying. This log message is relocated and also now throttled to once every few seconds. Change-Id: I4dd560309841ba738b031ea87b12f8f612e6c674 --- M src/kudu/consensus/consensus_peers.cc M src/kudu/consensus/consensus_queue.cc M src/kudu/consensus/consensus_queue.h M src/kudu/consensus/log_cache.cc M src/kudu/util/logging-test.cc M src/kudu/util/logging.h M src/kudu/util/logging_test_util.h 7 files changed, 116 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/4184/1 -- To view, visit http://gerrit.cloudera.org:8080/4184 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I4dd560309841ba738b031ea87b12f8f612e6c674 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Dan Burkert
[kudu-CR] consensus: improve log messages for lagging or tablet-copying peers
Kudu Jenkins has posted comments on this change. Change subject: consensus: improve log messages for lagging or tablet-copying peers .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/3165/ -- To view, visit http://gerrit.cloudera.org:8080/4184 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4dd560309841ba738b031ea87b12f8f612e6c674 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] docs: updates to kudu impala integration.adoc
Alexey Serbin has posted comments on this change. Change subject: docs: updates to kudu_impala_integration.adoc .. Patch Set 2: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/4138 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I02a91f1c33be3f0d6fbffaef5e03832e21b6db70 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] c++ client: expose private GetTablet API
Kudu Jenkins has posted comments on this change. Change subject: c++ client: expose private GetTablet API .. Patch Set 1: -Verified Build Started http://104.196.14.100/job/kudu-gerrit/3162/ -- To view, visit http://gerrit.cloudera.org:8080/4179 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If798820e5d790d07f554aaa6f89b31aaf360a3a5 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] tool: port kudu-admin to 'kudu cluster'
Kudu Jenkins has posted comments on this change. Change subject: tool: port kudu-admin to 'kudu cluster' .. Patch Set 1: -Verified Build Started http://104.196.14.100/job/kudu-gerrit/3163/ -- To view, visit http://gerrit.cloudera.org:8080/4180 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id6e96fc5b0106946f29a2faee8e2667be738b740 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[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 7: > Subprocess::Call has optional out-parameters for stdout/stderr so > you can capture it and assert Thanks Todd, updated diffs. sorry, it took a while for me to go incorporate injection of data and dumping the tablet data with and without workload. I was able to capture small tablet data ouput in the logfile itself as you suggested earlier with this change. -- 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: 7 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
Kudu Jenkins has posted comments on this change. Change subject: Fix kudu-ts-cli crash when there is no data in tablet .. Patch Set 7: Build Started http://104.196.14.100/job/kudu-gerrit/3161/ -- 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: 7 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 (#7). 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/kudu-ts-cli-test.cc M src/kudu/tools/ts-cli.cc 2 files changed, 48 insertions(+), 20 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/34/4134/7 -- 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: 7 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](gh-pages) kudu flume sink blog post
Todd Lipcon has posted comments on this change. Change subject: kudu flume sink blog post .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3510 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I810146ab24c88bc6cc562d81746b9bf5303396ed Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Ara Ebrahimi Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Misty Stanley-Jones Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Add AvroKuduEventProducer to Kudu-Flume integration
Will Berkeley has posted comments on this change. Change subject: Add AvroKuduEventProducer to Kudu-Flume integration .. Patch Set 5: > Build Failed > > http://104.196.14.100/job/kudu-gerrit/3158/ : FAILURE All the KuduSink tests passed. I think this one is on Jenkins. -- To view, visit http://gerrit.cloudera.org:8080/4034 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6715df72e447e72f4801a2e026f6840d09b401e1 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: No
[kudu-CR] KUDU-456 Implement AUTO FLUSH BACKGROUND flush mode
Alexey Serbin has posted comments on this change. Change subject: KUDU-456 Implement AUTO_FLUSH_BACKGROUND flush mode .. Patch Set 23: (2 comments) http://gerrit.cloudera.org:8080/#/c/3952/23/src/kudu/client/session-internal.cc File src/kudu/client/session-internal.cc: Line 118: // Thread-safety note: the buffer_bytes_limit_ is not supposed to be accessed > I think it's important to have safety notes like this in the header so that But from the method caller's perspective all what's needed to know is that it's not supposed to be called from multiple threads, plus generic info on the parameters. I don't see more. Am I missing something? I can put notion regarding the thread-safety constraint into the header. Would it be enough? http://gerrit.cloudera.org:8080/#/c/3952/23/src/kudu/client/session-internal.h File src/kudu/client/session-internal.h: Line 191: std::unordered_set flushed_batchers_; > Isn't the count of bytes in currently pending operations kept in the buffer Correct. As a matter of fact, I've removed flished_batchers_ already in the patchset I'm preparing. I think now it's looks better. Probably, it would be nice to get some feedback on that from the original author of the Batcher class (Todd and/or David?). -- 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: Yes
[kudu-CR] KUDU-456 Implement AUTO FLUSH BACKGROUND flush mode
Dan Burkert has posted comments on this change.
Change subject: KUDU-456 Implement AUTO_FLUSH_BACKGROUND flush mode
..
Patch Set 23:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/3952/23/src/kudu/client/batcher.cc
File src/kudu/client/batcher.cc:
Line 474: Status s = had_errors_ ? Status::Incomplete("Some errors
occurred")
> OK, that reasoning makes sense.
Yah, I agree IOError isn't a great fit, but the Status codes are pretty general.
http://gerrit.cloudera.org:8080/#/c/3952/23/src/kudu/client/session-internal.cc
File src/kudu/client/session-internal.cc:
Line 118: // Thread-safety note: the buffer_bytes_limit_ is not supposed to
be accessed
> I tried that, but from the reading perspective it looked worse -- there is
I think it's important to have safety notes like this in the header so that
callers can see these invariants without going to the implementation. This is,
after all, an invariant that is imposed on the caller.
http://gerrit.cloudera.org:8080/#/c/3952/23/src/kudu/client/session-internal.h
File src/kudu/client/session-internal.h:
Line 191: std::unordered_set flushed_batchers_;
> That's a good question! Besides using those for current implementation of
Isn't the count of bytes in currently pending operations kept in the
buffer_bytes_used_ counter?
--
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: Yes
[kudu-CR] c++ client: expose private GetTablet API
Kudu Jenkins has posted comments on this change. Change subject: c++ client: expose private GetTablet API .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/3159/ -- To view, visit http://gerrit.cloudera.org:8080/4179 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If798820e5d790d07f554aaa6f89b31aaf360a3a5 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] c++ client: expose private GetTablet API
Hello Mike Percy, Todd Lipcon, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/4179 to review the following change. Change subject: c++ client: expose private GetTablet API .. c++ client: expose private GetTablet API The new API can be used to look up tablet information (i.e. replicas and roles) using a tablet ID. I'm going to use it in kudu-admin; the "change config" operation needs to know which tserver hosts the tablet's leader replica. Without this new API, we'd have to go through the scan token API, which means either forcing the user to also provide the table name, or abusing the scan token API by creating tokens for _all_ tables, then hunting for the matching tablet. As such, I've opted to make this API "private". I've done so via KUDU_NO_EXPORT, though I could have just as easily declared it a private method and used friendship. This way it can more easily be tested (as tests do not use the "exported" client library). Change-Id: If798820e5d790d07f554aaa6f89b31aaf360a3a5 --- M src/kudu/client/client-internal.cc M src/kudu/client/client-test.cc M src/kudu/client/client.cc M src/kudu/client/client.h 4 files changed, 137 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/79/4179/1 -- To view, visit http://gerrit.cloudera.org:8080/4179 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: If798820e5d790d07f554aaa6f89b31aaf360a3a5 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] tool: port kudu-admin to 'kudu cluster'
Kudu Jenkins has posted comments on this change. Change subject: tool: port kudu-admin to 'kudu cluster' .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/3160/ -- To view, visit http://gerrit.cloudera.org:8080/4180 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id6e96fc5b0106946f29a2faee8e2667be738b740 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] tool: port kudu-admin to 'kudu cluster'
Hello Mike Percy, Todd Lipcon, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/4180 to review the following change. Change subject: tool: port kudu-admin to 'kudu cluster' .. tool: port kudu-admin to 'kudu cluster' In doing so, the ported functionality can now also work in multi-master mode; previously, the ClusterAdminClient refused to accept multiple masters. I opted to group the config change operations as a common mode rather than an additional parameter. I think this makes them more discoverable. Change-Id: Id6e96fc5b0106946f29a2faee8e2667be738b740 --- M build-support/dist_test.py M docs/release_notes.adoc M src/kudu/tools/CMakeLists.txt M src/kudu/tools/kudu-admin-test.cc D src/kudu/tools/kudu-admin.cc M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/kudu-ts-cli-test.cc M src/kudu/tools/tool_action_cluster.cc 8 files changed, 327 insertions(+), 480 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/80/4180/1 -- To view, visit http://gerrit.cloudera.org:8080/4180 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Id6e96fc5b0106946f29a2faee8e2667be738b740 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR](gh-pages) kudu flume sink blog post
Hello Mike Percy, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3510 to look at the new patch set (#7). Change subject: kudu flume sink blog post .. kudu flume sink blog post Change-Id: I810146ab24c88bc6cc562d81746b9bf5303396ed --- A _posts/2016-08-31-intro-flume-kudu-sink.md 1 file changed, 261 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/3510/7 -- To view, visit http://gerrit.cloudera.org:8080/3510 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I810146ab24c88bc6cc562d81746b9bf5303396ed Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Ara Ebrahimi Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Misty Stanley-Jones Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Add AvroKuduEventProducer to Kudu-Flume integration
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4034 to look at the new patch set (#5). Change subject: Add AvroKuduEventProducer to Kudu-Flume integration .. Add AvroKuduEventProducer to Kudu-Flume integration This adds a KuduEventProducer that accepts events with Avro-encoded bodies and allows the KuduSink to push them to Kudu. It only accepts schemas of Record type that hold values of types compatible with Kudu's types. The schema is provided to the EventProducer ahead of time or in the event header, as a URL or as a literal. Change-Id: I6715df72e447e72f4801a2e026f6840d09b401e1 --- M java/kudu-flume-sink/pom.xml A java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/AvroKuduEventProducer.java A java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/AvroKuduEventProducerTest.java A java/kudu-flume-sink/src/test/resources/testAvroEventProducer.avsc 4 files changed, 538 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/34/4034/5 -- To view, visit http://gerrit.cloudera.org:8080/4034 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6715df72e447e72f4801a2e026f6840d09b401e1 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] Add AvroKuduEventProducer to Kudu-Flume integration
Kudu Jenkins has posted comments on this change. Change subject: Add AvroKuduEventProducer to Kudu-Flume integration .. Patch Set 5: Build Started http://104.196.14.100/job/kudu-gerrit/3158/ -- To view, visit http://gerrit.cloudera.org:8080/4034 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6715df72e447e72f4801a2e026f6840d09b401e1 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: No
[kudu-CR] Add AvroKuduEventProducer to Kudu-Flume integration
Will Berkeley has posted comments on this change.
Change subject: Add AvroKuduEventProducer to Kudu-Flume integration
..
Patch Set 4:
(7 comments)
Did a significant rewrite to make it more similar to the DataSet Sink
implementation. Sorry if this pushes it further from the finish line but I
think it will make it better in the end. It'll get another rewrite when the
KuduEventProducer API changes, too.
http://gerrit.cloudera.org:8080/#/c/4034/4/java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/AvroKuduEventProducer.java
File
java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/AvroKuduEventProducer.java:
Line 60: * producer.schema.path
> should be schemaPath
Done
Line 63: * The location of the Avro schema file used to deserialize
the Avro-encoded event bodies.
> Add: If not specified, the schema must be specified on a per-event basis.
Done
Line 73: private static final String SCHEMA_HEADER = "schemaPath";
> Let's use the same property as the Kite sink for the event headers so that
Done
Line 110: String.format("No schema for event! Specify either property
%s or event header %s",
> s/property/configuration property/
Done
Line 121: payloadReader = new DataFileReader<>(new
SeekableByteArrayInput(payload), reader);
> We should not treat each Flume Event as an Avro DataFile. We should be trea
Did a significant rewrite based on AvroParser.
Line 150: if (!col.isNullable()) {
> I think this should be:
Done. Made a note to myself fix later.
Line 202: private DatumReader openSchema(String schemaPath) {
> It would be nice to support both URL and literal. See how the DatasetSink i
Done
--
To view, visit http://gerrit.cloudera.org:8080/4034
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6715df72e447e72f4801a2e026f6840d09b401e1
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Will Berkeley
Gerrit-HasComments: Yes
[kudu-CR] KUDU-456 Implement AUTO FLUSH BACKGROUND flush mode
Alexey Serbin has posted comments on this change.
Change subject: KUDU-456 Implement AUTO_FLUSH_BACKGROUND flush mode
..
Patch Set 23:
(27 comments)
Dan & Adar, thank you for the review. I'll post updated version as soon as I
make sure the tests are passing OK.
http://gerrit.cloudera.org:8080/#/c/3952/23/src/kudu/client/batcher.cc
File src/kudu/client/batcher.cc:
Line 474: Status s = had_errors_ ? Status::Incomplete("Some errors
occurred")
> I don't think Incomplete is the right status here; the flush is complete.
OK, that reasoning makes sense.
However, I thought IOError might be irrelevant here as well. Or this is the
best match among all possible status codes? There is seemingly more generic
RunTimeError, but I'm not sure that's the right case to use it.
I'll try to return IOError back to get back to the original, and meanwhile we
can discuss it separately.
http://gerrit.cloudera.org:8080/#/c/3952/23/src/kudu/client/client-test.cc
File src/kudu/client/client-test.cc:
PS23, Line 2360: there the current batcher is at place
> Not sure what this is trying to say, maybe:
ops, my bad.
Should have been something like 'OK, now the current batcher should be at
place.'
Will fix.
http://gerrit.cloudera.org:8080/#/c/3952/23/src/kudu/client/client.cc
File src/kudu/client/client.cc:
Line 322:hp);
> this can go on the line above.
Done
http://gerrit.cloudera.org:8080/#/c/3952/23/src/kudu/client/session-internal.cc
File src/kudu/client/session-internal.cc:
Line 63: TimeBasedFlushInit(session);
> How about checking if we are in AUTO_FLUSH_BACKGROUND first? I know this i
The check you mentioned is done by TimeBasedFlushTask() method _before_
scheduling a task (i.e. no task scheduled if current mode is not
AUTO_FLUSH_BACKGROUND). So, I see no need to perform that check here.
I put all those ugly guts (synchronization, etc.) into that method with
intention to avoid doing those checks elsewhere.
Line 63: TimeBasedFlushInit(session);
> The funny thing is that, by definition, we cannot be in AUTO_FLUSH_BACKGROU
TimeBasedFlushInit() is no-op in other than AUTO_FLUSH_BACKGROUND mode.
Yes, it might be a bad name for the method.
What name would you recommend instead?
PS23, Line 105:std::lock_guard l(lock_);
> Since this isn't acquired on any hot path, what do you think of removing it
I thought it was better to keep those locks separate to avoid having one giant
lock for all the methods. Those locks based on simple_spinlock look lighter
than based on mutex. But as you mentioned, since we already have that for the
most critical paths, and the rest with simple_spinlock are just for methods
which almost never called, I think it's a good idea.
Will do, thanks!
Line 118: // Thread-safety note: the buffer_bytes_limit_ is not supposed to
be accessed
> These thread safety notes (here and below) would be better in the header de
I tried that, but from the reading perspective it looked worse -- there is not
much of context when looking at the signature of a function/method. However,
in the body of a method, there is much more relevant context since you can see
the logic of the method, more comments, corresponding variables/members, etc.
That's said, I'm hesitant to move those in the header.
However, if you feel strong about that and have a compelling reasoning which
addresses my concern regarding the lack of context, I'll move those.
Line 156: Status KuduSession::Data::SetMaxBatchersNum(unsigned int max_num) {
> How come this one doesn't check HasPendingOperations?
I didn't see a restriction that would arise from the current design.
Actually, we can remove that for SetBufferFlushInterval() and other except for
the KuduSession::SetFlushMode().
But as I see from our recent HipChat discussion, we want to keep us more space
for the future, because once published, it's easier to make those restrictions
less strict. But making these more strict could be almost impossible in the
future.
So, I added the artificial restriction here as well.
Line 181: FlushCurrentBatcher(1, cb);
> Nit: add a comment explaining the significance of 1 (i.e. why not 0?)
Done
PS23, Line 188: nullptr
> I spent a lot of time wondering if the loss of the synchronizer and RETURN_
I did a research on that prior to using nullptr as a callback in the new code
and I didn't see any cases where an error would be dropped on the floor (i.e.
in all cases where Batcher::had_errors_ is set to true, the error is registered
into the Batcher::error_collector_). So, it's safe as far as I can see. But
I'll add corresponding comments into the batcher and for the
FlushCurrentBatcher() methods to clarify on that.
I think it's a good idea to document the difference. I even thought I had done
it, but re-visiting the client.h comments revealed that I missed that part.
Thank you for the remin
[kudu-CR] KUDU-668. log block manager should tolerate empty metadata
Todd Lipcon has posted comments on this change. Change subject: KUDU-668. log block manager should tolerate empty metadata .. Patch Set 1: fwiw I also just tried this patch on a test cluster which had hit this issue, and some servers that previously couldn't start now started OK -- To view, visit http://gerrit.cloudera.org:8080/4178 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie4fd9d0f510835d100d35efaf541d34829118f56 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-668. log block manager should tolerate empty metadata
Hello Adar Dembo, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/4178 to review the following change. Change subject: KUDU-668. log block manager should tolerate empty metadata .. KUDU-668. log block manager should tolerate empty metadata This solves a commonly seen issue after a server crashes due to "Too many open files". In many cases, this type of crash would leave an empty metadata file and a missing data file. This isn't a full fix for KUDU-668, but handles the above common case by just checking the size and existence of these files. If the metadata is too small to be valid, and the data is empty, then we just remove the container and continue with startup. Change-Id: Ie4fd9d0f510835d100d35efaf541d34829118f56 --- M src/kudu/fs/block_manager-test.cc M src/kudu/fs/log_block_manager.cc M src/kudu/util/pb_util.cc M src/kudu/util/pb_util.h 4 files changed, 70 insertions(+), 12 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/78/4178/1 -- To view, visit http://gerrit.cloudera.org:8080/4178 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ie4fd9d0f510835d100d35efaf541d34829118f56 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo
[kudu-CR] KUDU-668. log block manager should tolerate empty metadata
Kudu Jenkins has posted comments on this change. Change subject: KUDU-668. log block manager should tolerate empty metadata .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/3157/ -- To view, visit http://gerrit.cloudera.org:8080/4178 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie4fd9d0f510835d100d35efaf541d34829118f56 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] Cleanup/refactor tracking of consensus watermarks
Mike Percy has posted comments on this change. Change subject: Cleanup/refactor tracking of consensus watermarks .. Patch Set 4: (4 comments) I am still working my way through this patch. There are several TODOs, not sure if you know the answers to those yet. But I will investigate when I pick up again on reviewing the logic changes in this patch. http://gerrit.cloudera.org:8080/#/c/4133/4/src/kudu/consensus/consensus.proto File src/kudu/consensus/consensus.proto: Line 333: // The id of the last committed operation in the configuration. This is the s/id/index/ Line 334: // id of the last operation the leader deemed committed from a consensus s/id/index/ http://gerrit.cloudera.org:8080/#/c/4133/4/src/kudu/consensus/consensus_peers-test.cc File src/kudu/consensus/consensus_peers-test.cc: Line 142: message_queue_->SetLeaderMode(0, 0, BuildRaftConfigPBForTests(3)); Maybe better to use kMinimumTerm and kMinimumOpIdIndex from opid_util.h here and elsewhere http://gerrit.cloudera.org:8080/#/c/4133/4/src/kudu/consensus/consensus_queue.h File src/kudu/consensus/consensus_queue.h: Line 415: // TODO: doc me todo -- 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: Yes
[kudu-CR](gh-pages) kudu flume sink blog post
Mike Percy has posted comments on this change. Change subject: kudu flume sink blog post .. Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/3510/6/_posts/2016-08-31-intro-flume-kudu-sink.md File _posts/2016-08-31-intro-flume-kudu-sink.md: PS6, Line 68: Cloudera > sorry, one nit: This should be Apache Impala (incubating) We already mentioned Apache Impala (incubating) above, so this can just read "Impala". Yes, we should remove Cloudera from this line. -- To view, visit http://gerrit.cloudera.org:8080/3510 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I810146ab24c88bc6cc562d81746b9bf5303396ed Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Ara Ebrahimi Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Misty Stanley-Jones Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR](gh-pages) kudu flume sink blog post
Todd Lipcon has posted comments on this change. Change subject: kudu flume sink blog post .. Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/3510/6/_posts/2016-08-31-intro-flume-kudu-sink.md File _posts/2016-08-31-intro-flume-kudu-sink.md: PS6, Line 68: Cloudera sorry, one nit: This should be Apache Impala (incubating) -- To view, visit http://gerrit.cloudera.org:8080/3510 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I810146ab24c88bc6cc562d81746b9bf5303396ed Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Ara Ebrahimi Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Misty Stanley-Jones Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR](gh-pages) kudu flume sink blog post
Mike Percy has posted comments on this change. Change subject: kudu flume sink blog post .. Patch Set 6: Code-Review+2 Verified+1 Thanks Ara! I'll push this live tomorrow morning. -- To view, visit http://gerrit.cloudera.org:8080/3510 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I810146ab24c88bc6cc562d81746b9bf5303396ed Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Ara Ebrahimi Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Misty Stanley-Jones 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 2: Code-Review+2 -- 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: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] tool: port log-dump
Todd Lipcon has submitted this change and it was merged. 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 Reviewed-on: http://gerrit.cloudera.org:8080/4167 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon --- M docs/release_notes.adoc 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 13 files changed, 440 insertions(+), 160 deletions(-) Approvals: Todd Lipcon: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4167 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I14f048169c0b211e3c72fcd255c76dd59cbb05c9 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR](gh-pages) kudu flume sink blog post
Ara Ebrahimi has uploaded a new patch set (#6). Change subject: kudu flume sink blog post .. kudu flume sink blog post Change-Id: I810146ab24c88bc6cc562d81746b9bf5303396ed --- A _posts/2016-08-31-intro-flume-kudu-sink.md 1 file changed, 261 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/3510/6 -- To view, visit http://gerrit.cloudera.org:8080/3510 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I810146ab24c88bc6cc562d81746b9bf5303396ed Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Ara Ebrahimi Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Misty Stanley-Jones
[kudu-CR] WIP: tie log retention to consensus watermarks
Kudu Jenkins has posted comments on this change. Change subject: WIP: tie log retention to consensus watermarks .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/3156/ -- To view, visit http://gerrit.cloudera.org:8080/4177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icfc071270510f3dc3c65f88d615e93c6ffb26b12 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] WIP: tie log retention to consensus watermarks
Hello Mike Percy, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/4177 to review the following change. Change subject: WIP: tie log retention to consensus watermarks .. WIP: tie log retention to consensus watermarks This changes the calculation of log retention to consult consensus. Consensus returns a struct which indicates the watermark necessary for durability (the committed index) as well as the watermark necessary to catch up other peers. This replaces the old single "must be retained" watermark that the log GC code used before. The new struct is passed down into the log, and we use the following policy: - we always maintain any logs necessary for durability - beyond that, we try to retain logs to catch up lagging peers, however we never maintain more than --log_max_segments_to_retain (a new configuration) I removed the old flag --log_min_seconds_to_retain, since its main purpose was for dealing with lagging peers, and that is now handled by directly consulting consensus. The one tricky bit of the policy is that, even though the peer catch-up figures into log retention, we do _not_ want it to impact the calculation of flush priority. In other words, even if the user is OK retaining 10GB of logs to catch up trailing peers, they probably still want to flush more aggressively than that so they can avoid very long startup times. So, the peer-based watermark is not used during the mapping of log anchors to retention amounts. Note that the above is only relevant once we have implemented KUDU-38: we currently will replay all of the retained logs even though we are aggressively flushing to keep the durability-related retention bounded. Manually tested for now as follows: - started a three-node cluster (locally), set to roll logs at 1MB segments, but otherwise default - started an insert workload against a single-tablet table - I could see that the three servers were maintaining 2 WAL segments in their WAL directory. - I kill -STOPped a random server while continuing to insert. I saw that the WALs in this tablet server's directory froze as is (obviously), and the other two kept rolling. However, because of this change, the other servers started retaining wals starting from the point where I had stopped the follower. - If I let the insert workload continue, the live servers kept rolling up until they had 10 segments (default --log_max_segments_to_retain) at which point they dropped the oldest log. - I verified that, during this period while the extra segments were retained, the servers continued to flush frequently so that their recovery time would be bounded. - I also verified that, if I un-paused the follower before the others had evicted it, it was able to catch up, at which point the other servers GCed those extra logs they had been retaining. WIP because this needs some automated tests for the above behavior, and need to sweep for comments to update, clean-up, etc. Change-Id: Icfc071270510f3dc3c65f88d615e93c6ffb26b12 --- M src/kudu/consensus/consensus.h M src/kudu/consensus/consensus.proto 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/log-test-base.h M src/kudu/consensus/log-test.cc M src/kudu/consensus/log.cc M src/kudu/consensus/log.h M src/kudu/consensus/log_reader.cc M src/kudu/consensus/log_reader.h M src/kudu/consensus/raft_consensus.cc M src/kudu/consensus/raft_consensus.h M src/kudu/integration-tests/raft_consensus-itest.cc M src/kudu/tablet/tablet_peer-test.cc M src/kudu/tablet/tablet_peer.cc M src/kudu/tablet/tablet_peer.h 17 files changed, 193 insertions(+), 225 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/4177/1 -- To view, visit http://gerrit.cloudera.org:8080/4177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Icfc071270510f3dc3c65f88d615e93c6ffb26b12 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Mike Percy
[kudu-CR](gh-pages) kudu flume sink blog post
Mike Percy has posted comments on this change. Change subject: kudu flume sink blog post .. Patch Set 5: > Let's publish tomorrow. > > Please rename the file to 2016-08-07-intro-flume-kudu-sink.md Oops, my mistake. Please rename to 2016-08-31-intro-flume-kudu-sink.md -- To view, visit http://gerrit.cloudera.org:8080/3510 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I810146ab24c88bc6cc562d81746b9bf5303396ed Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Ara Ebrahimi Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Misty Stanley-Jones Gerrit-HasComments: No
[kudu-CR](gh-pages) kudu flume sink blog post
Mike Percy has posted comments on this change. Change subject: kudu flume sink blog post .. Patch Set 5: Let's publish tomorrow. Please rename the file to 2016-08-07-intro-flume-kudu-sink.md -- To view, visit http://gerrit.cloudera.org:8080/3510 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I810146ab24c88bc6cc562d81746b9bf5303396ed Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Ara Ebrahimi Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Misty Stanley-Jones Gerrit-HasComments: No
[kudu-CR](gh-pages) kudu flume sink blog post
Mike Percy has posted comments on this change. Change subject: kudu flume sink blog post .. Patch Set 5: (1 comment) We need to rename the file to the date it will get posted. Let me collect some opinions and get back to you on the publish date. http://gerrit.cloudera.org:8080/#/c/3510/5/_posts/2016-07-06-flume.md File _posts/2016-07-06-flume.md: Line 48: - low-latency and relatively high throughput, both for ingest and query this seems to duplicate "fast ingest and fast query" -- To view, visit http://gerrit.cloudera.org:8080/3510 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I810146ab24c88bc6cc562d81746b9bf5303396ed Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Ara Ebrahimi Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Misty Stanley-Jones Gerrit-HasComments: Yes
[kudu-CR] Add trace metrics for maintenance ops, LBM writes
Todd Lipcon has submitted this change and it was merged.
Change subject: Add trace metrics for maintenance ops, LBM writes
..
Add trace metrics for maintenance ops, LBM writes
This adds a trace-scoped counter for LogBlockManager writes, and also
adds traces and counter printing to the maintenance manager.
With this patch, flushes and compactions now output log messages like:
I0830 13:42:28.809852 2208 maintenance_manager.cc:356] Time spent running
FlushMRSOp(4301e24175a2469fba37184430567acc): real 9.510s user 4.432s sys
0.860s
I0830 13:42:28.810760 2208 maintenance_manager.cc:362]
FlushMRSOp(4301e24175a2469fba37184430567acc) metrics:
{"cfile_init":14,"compiler_manager_pool.queue_time_us":106,"compiler_manager_pool.run_cpu_time_us":219218,"compiler_manager_pool.run_wall_time_us":232004,"fdatasync":533,"fdatasync_us":67137,"lbm
root 0.queue_time_us":1605,"lbm root 0.run_cpu_time_us":1280,"lbm root
0.run_wall_time_us":344957,"lbm_read_time_us":94,"lbm_reads_lt_1ms":56,"lbm_write_time_us":709833,"lbm_writes_gt_100_ms":1,"lbm_writes_lt_1ms":11564,"mutex_wait_us":343945,"tcmalloc_contention_cycles":2759552,"thread_start_us":1304,"threads_started":13}
This can be handy for understanding why a particular compaction or flush
was slow.
Change-Id: I39cd9438071aabbd2d7bdeca3269af8b83f2d55b
Reviewed-on: http://gerrit.cloudera.org:8080/4172
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert
---
M src/kudu/fs/log_block_manager.cc
M src/kudu/util/maintenance_manager.cc
M src/kudu/util/trace.h
3 files changed, 30 insertions(+), 10 deletions(-)
Approvals:
Dan Burkert: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/4172
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I39cd9438071aabbd2d7bdeca3269af8b83f2d55b
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Add trace metrics for maintenance ops, LBM writes
Todd Lipcon has posted comments on this change. Change subject: Add trace metrics for maintenance ops, LBM writes .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/4172/1/src/kudu/util/maintenance_manager.cc File src/kudu/util/maintenance_manager.cc: Line 357: ADOPT_TRACE(trace.get()); > What does the ADOPT_TRACE change? Did this work before? the maintenance manager doesn't run in the context of an RPC, so there was no trace associated with the thread when it was running. So we have to manually construct one and adopt it here, or else the counters don't end up going anywhere. -- To view, visit http://gerrit.cloudera.org:8080/4172 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I39cd9438071aabbd2d7bdeca3269af8b83f2d55b Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-456 Implement AUTO FLUSH BACKGROUND flush mode
Adar Dembo has posted comments on this change. Change subject: KUDU-456 Implement AUTO_FLUSH_BACKGROUND flush mode .. Patch Set 23: (11 comments) http://gerrit.cloudera.org:8080/#/c/3952/23/src/kudu/client/session-internal.cc File src/kudu/client/session-internal.cc: Line 63: TimeBasedFlushInit(session); > How about checking if we are in AUTO_FLUSH_BACKGROUND first? I know this i The funny thing is that, by definition, we cannot be in AUTO_FLUSH_BACKGROUND, because the default flush mode is AUTO_FLUSH_SYNC. But, that can change in the future. In any case, I think TimeBasedFlushInit() is a no-op in AUTO_FLUSH_BACKGROUND mode. PS23, Line 105:std::lock_guard l(lock_); Since this isn't acquired on any hot path, what do you think of removing it in favor of batch_ctl_mutex_? I think it's a little easier to reason about the flow if we can minimize the number of locks. If you do end up doing this, probably best to rename batch_ctl_mutex_ to lock_ (and maybe rename batch_ctl_cond_ too). Line 181: FlushCurrentBatcher(1, cb); Nit: add a comment explaining the significance of 1 (i.e. why not 0?) PS23, Line 188: nullptr I spent a lot of time wondering if the loss of the synchronizer and RETURN_NOT_OK(s.Wait()) meant we were dropping errors on the floor. Meaning, is there any case where the batcher invokes the flush callback with !Status::OK() and does NOT write errors to the error collector? I don't think there are, but the flow is tricky enough that I thought I'd ask you the same question. BTW, it would be worth documenting in client.h the difference in behavior between Flush() and FlushAsync() w.r.t. waiting for outstanding batches to flush. Or do you think it's implicitly understood? PS23, Line 296: The flush_mode_ is accessed : // from the background time-based flush task, but it accesses it : // only for reading. TSAN may complain about this. ISTR it doesn't like accesses to the same members with a mixed set of locks (i.e. one lock held in one access, and no locks in another). Line 354: batch_ctl_cond_.Wait(); This isn't going to work for ApplyAsync(). Now, we haven't actually implemented ApplyAsync() yet, so there's no reason to do anything. But we should start thinking about how ApplyWriteOp() would work in an ApplyAsync() world, hopefully without the creation of two separate apply code paths. http://gerrit.cloudera.org:8080/#/c/3952/23/src/kudu/client/session-internal.h File src/kudu/client/session-internal.h: PS23, Line 42: It's Nit: Its PS23, Line 43: implemented Nit: don't need this word. PS23, Line 128: // The self-rescheduling task to flush write operations which have been : // accumulating for too long (controlled by flush_interval_). : // This does real work only in case of AUTO_FLUSH_BACKGROUND mode. Should document what 'do_startup_check' means. PS23, Line 172: which Nit: "whose" PS23, Line 172: requries Nit: "requires" -- 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: Yes
[kudu-CR] Add trace metrics for maintenance ops, LBM writes
Dan Burkert has posted comments on this change. Change subject: Add trace metrics for maintenance ops, LBM writes .. Patch Set 1: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/4172/1/src/kudu/util/maintenance_manager.cc File src/kudu/util/maintenance_manager.cc: Line 357: ADOPT_TRACE(trace.get()); What does the ADOPT_TRACE change? Did this work before? -- To view, visit http://gerrit.cloudera.org:8080/4172 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I39cd9438071aabbd2d7bdeca3269af8b83f2d55b Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR](gh-pages) kudu flume sink blog post
Ara Ebrahimi has uploaded a new patch set (#5). Change subject: kudu flume sink blog post .. kudu flume sink blog post Change-Id: I810146ab24c88bc6cc562d81746b9bf5303396ed --- A _posts/2016-07-06-flume.md 1 file changed, 262 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/3510/5 -- To view, visit http://gerrit.cloudera.org:8080/3510 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I810146ab24c88bc6cc562d81746b9bf5303396ed Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Ara Ebrahimi Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Misty Stanley-Jones
[kudu-CR](gh-pages) kudu flume sink blog post
Mike Percy has posted comments on this change. Change subject: kudu flume sink blog post .. Patch Set 4: (32 comments) Thanks for the update, Ara! I've provided a bunch of feedback. Also, if you don't mind, please remove the trailing spaces in the file (they are highlighted in red on Gerrit) http://gerrit.cloudera.org:8080/#/c/3510/4/_posts/2016-07-06-flume.md File _posts/2016-07-06-flume.md: Line 25: So, in a nutshell, _batch processing_ is: s/So, in a nutshell/To summarize/ Line 27: - primitive I don't really understand what you mean by primitive here. I would consider removing this dimension of the comparison. An example is Spark, which is batch oriented but quite rich in its APIs and capabilities, I think. Line 29: - batch-oriented How about: s/batch-oriented/a paradigm that processes large chunks of data as a group/ Line 30: - fast ingest, slow query high latency and high throughput, both for ingest and query Line 31: - easy to program, but hard to orchestrate typically easy to program Line 32: - easy to write ad-hoc, though slow, queries well suited for writing ad-hoc queries, although they are typically high latency Line 36: - not primitive, has rich constructs such as time windows s/not primitive, has rich constructs such as time windows/a totally different paradigm, which involves single events and time windows instead of large groups of events/ Line 37: - still file-based and not a long-term database typically still file-oriented, instead of table-oriented Line 39: - ultra-fast ingest and ultra-fast query (query results basically pre-calculated) very low latency and often low throughput both for ingest and query (query results are typically pre-calculated at ingest time) Line 40: - not so easy to program, relatively easy to orchestrate often difficult to program for Line 45: - not primitive, thanks to SQL support via Impala flexible and expressive, thanks to SQL support via Apache Impala (incubating) Line 46: - a real long-term database, with SQL support via Impala a table-oriented, mutable data store that feels like a traditional relational database Line 47: - neither batch nor streaming I'm not sure what you mean by neither batch or streaming. maybe just remove this? Line 48: - fast ingest and fast query low-latency and relatively high throughput, both for ingest and query Line 83: can see, nowhere Hadoop is mentioned but Flume is typically used for ingesting data to Hadoop s/nowhere Hadoop is mentioned/nowhere is Hadoop mentioned/ PS4, Line 88: _agent_ add a comma after _agent_ PS4, Line 129: vmstat `vmstat` PS4, Line 132: SimpleKuduEventProducer `SimpleKuduEventProducer` PS4, Line 135: KuduSink `KuduSink` PS4, Line 136: from Kudu distributio from the Kudu distribution Line 137: `$FLUME_HOME/plugins.d/kudu-sink/lib` in the Flume installation. The jar file contains KuduSink missing word: `$FLUME_HOME/plugins.d/kudu-sink/lib` directory PS4, Line 137: KuduSink `KuduSink` Line 138: and all the dependencies of it (including Kudu java client classes). s/and all the dependencies of it/and all of its dependencies/ PS4, Line 142: Kudu Flume Sink The Kudu Flume Sink PS4, Line 143: it runs before the Kudu Flume Sink is started. PS4, Line 205: SimpleKuduEventProducer `SimpleKuduEventProducer` PS4, Line 236: from from the PS4, Line 238: SimpleKuduEventProducer `SimpleKuduEventProducer` Line 248: on the built-in ones. Add: In the future, we plan to add more flexible event producer implementations so that creation of a custom event producer is not required to write data to Kudu. See here for a work-in-progress generic event producer for Avro-encoded Events: https://gerrit.cloudera.org/#/c/4034/ Line 252: Kudu is a scalable database which lets us ingest insane amounts of data per second. Apache Flume s/database/data store/ PS4, Line 253: library s/library// Line 256 At the end, if you want to, consider adding a bio paragraph about yourself in italics. Maybe something like this? Ara Abrahamian is a software engineer at Argyle Data building X. . Ara is the original author of the Flume Kudu Sink that is included in the Kudu distribution. -- To view, visit http://gerrit.cloudera.org:8080/3510 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I810146ab24c88bc6cc562d81746b9bf5303396ed Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Ara Ebrahimi Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Misty Stanley-Jones Gerrit-HasComments: Yes
[kudu-CR] [docs] added Kudu version into the doxygen footer
Adar Dembo has submitted this change and it was merged. 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 Reviewed-on: http://gerrit.cloudera.org:8080/4165 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo --- 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(+), 6 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4165 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I3861aafa1e615cc991647704d9a4ce8d44ad678f Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Add AvroKuduEventProducer to Kudu-Flume integration
Mike Percy has posted comments on this change.
Change subject: Add AvroKuduEventProducer to Kudu-Flume integration
..
Patch Set 4:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/4034/4/java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/AvroKuduEventProducer.java
File
java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/AvroKuduEventProducer.java:
Line 60: * producer.schema.path
should be schemaPath
Line 63: * The location of the Avro schema file used to deserialize
the Avro-encoded event bodies.
Add: If not specified, the schema must be specified on a per-event basis.
Line 73: private static final String SCHEMA_HEADER = "schemaPath";
Let's use the same property as the Kite sink for the event headers so that both
Flume sinks can operate on the same events and provide the same behavior.
That means we should use "flume.avro.schema.url" for the header property.
Line 110: String.format("No schema for event! Specify either property
%s or event header %s",
s/property/configuration property/
Line 121: payloadReader = new DataFileReader<>(new
SeekableByteArrayInput(payload), reader);
We should not treat each Flume Event as an Avro DataFile. We should be treating
each Flume Event as an Avro record.
I recommend looking at the Flume DatasetSink implementation and doing the same
thing. Here is how they parse the events:
https://github.com/apache/flume/blob/trunk/flume-ng-sinks/flume-dataset-sink/src/main/java/org/apache/flume/sink/kite/parser/AvroParser.java#L152
Line 150: if (!col.isNullable()) {
I think this should be:
if (value == null) {
if (col.isNullable()) {
row.setNull(name);
} else {
// leave unset for possible Kudu default
}
...actually, it seems like we are missing something from the client API like
ColumnSchema.hasDefaultValue() because if we could check that then we could
throw if the field is null and a default does not exist. We should probably fix
that in a follow-up.
Line 202: private DatumReader openSchema(String schemaPath) {
It would be nice to support both URL and literal. See how the DatasetSink
implements this here:
https://github.com/apache/flume/blob/trunk/flume-ng-sinks/flume-dataset-sink/src/main/java/org/apache/flume/sink/kite/parser/AvroParser.java#L175
--
To view, visit http://gerrit.cloudera.org:8080/4034
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6715df72e447e72f4801a2e026f6840d09b401e1
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Will Berkeley
Gerrit-HasComments: Yes
[kudu-CR] [java-client] Add ScanToken.stringifySerializedToken
Hello Matthew Jacobs, Adar Dembo, Todd Lipcon,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/4173
to review the following change.
Change subject: [java-client] Add ScanToken.stringifySerializedToken
..
[java-client] Add ScanToken.stringifySerializedToken
stringifySerializedToken takes a serialized scan token, and returns a
String suitable for debug printing. The string contains information
sufficient to determine which range of tablet(s) the scan token will
cover, and which rows within the tablets. Namely, the range partition
bounds and primary key bounds. Example output, wrapped for readability
(a, b, and c are column names with type STRING):
ScanToken{table=org.apache.kudu.client.TestKuduClient-1472595465767,
lower-bound-primary-key=(string a=1, string b=3, string c=5),
upper-bound-primary-key=(string a=2, string b=4, string c=),
hash-partition-buckets: [2],
range-partition: [(string a=0, string b=0, string c=0),
(string a=3, string b=5, string c=6))}
The Java client did not have any method of deserializing encoded primary
or partition keys, so most of the work in this commit is introducing
utility methods for that purpose. I haven't added tests of the specific
format of the strings, but I have added the printing to many of the
existing ScanToken tests in order to make sure that the formatting code
itself doesn't fail. I've also verified that the output looks good. The
format doesn't include information like predicates or consistency, but
that could be added in the future if so desired.
Change-Id: I42014da270e66c370cc6d6ed68fb38331130ff6d
---
M java/kudu-client/src/main/java/org/apache/kudu/client/DeadlineTracker.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KeyEncoder.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java
M java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowResult.java
M
java/kudu-client/src/test/java/org/apache/kudu/client/TestFlexiblePartitioning.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKeyEncoding.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestOperation.java
M
java/kudu-mapreduce/src/main/java/org/apache/kudu/mapreduce/KuduTableInputFormat.java
10 files changed, 452 insertions(+), 81 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/73/4173/1
--
To view, visit http://gerrit.cloudera.org:8080/4173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I42014da270e66c370cc6d6ed68fb38331130ff6d
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Matthew Jacobs
Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [java-client] Add ScanToken.stringifySerializedToken
Kudu Jenkins has posted comments on this change. Change subject: [java-client] Add ScanToken.stringifySerializedToken .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/3155/ -- To view, visit http://gerrit.cloudera.org:8080/4173 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I42014da270e66c370cc6d6ed68fb38331130ff6d Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Todd Lipcon 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 3: Code-Review+2 -- 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: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins 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 17: (2 comments) TFTRs Mike/Dan/Adar, updated the patch to reflect the discussions we have had on this. Please take a look. http://gerrit.cloudera.org:8080/#/c/3823/15/src/kudu/tablet/tablet_metadata.cc File src/kudu/tablet/tablet_metadata.cc: PS15, Line 313: CHECK_EQ(table_id_, superblock.table_id()); : PartitionSchema partition_schema; : RETURN_NOT_OK(PartitionSchema::FromPB(superblock.partition_schema(), : *schema_, &partition_schema)); : CHECK(partition_schema_.Equals(partition_schema)); : : Partition partition; : Partition::FromPB(superblock.partition(), &partition); : CHECK(partition_.Equals(partition)); > Providing equals on partition should be easy, it just needs to check that t also checked for hash_buckets equality Dan, assuming that's immutable as well. PS15, Line 313: CHECK_EQ(table_id_, superblock.table_id()); : PartitionSchema partition_schema; : RETURN_NOT_OK(PartitionSchema::FromPB(superblock.partition_schema(), : *schema_, &partition_schema)); : CHECK(partition_schema_.Equals(partition_schema)); : : Partition partition; : Partition::FromPB(superblock.partition(), &partition); : CHECK(partition_.Equals(partition)); > I suppose we could change this to CHECK and just rely on upgrade / downgrad Cool, thanks Mike, updated the code with Equals() for both partition and partition_schema, and moving to CHECK to keep the code same for all build types. I agree with catching the compat issues with white/black box automated tests for upgrade/downgrade. -- 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: 17 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: update installation with new OS support
Hello Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4128 to look at the new patch set (#3). Change subject: docs: update installation with new OS support .. docs: update installation with new OS support At the time of writing, a couple things are broken: 1. The SLES 12, Jessie, and Xenial Cloudera repo files contain unsubstituted template variables. 2. The SLES 12 Cloudera kudu package has a dependency (cyrus-sasl-lib) that does not exist on SLES 12. I'm testing a patch to fix #2 and we're trying to fix #1 live. I did verify that the SLES 12 packages work if installation is forced. Change-Id: I2f51b55561f18fbe28d6bf4ed507dfba65947dc0 --- M docs/installation.adoc 1 file changed, 66 insertions(+), 20 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/4128/3 -- To view, visit http://gerrit.cloudera.org:8080/4128 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2f51b55561f18fbe28d6bf4ed507dfba65947dc0 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Todd Lipcon
[kudu-CR] docs: updates to kudu impala integration.adoc
Kudu Jenkins has posted comments on this change. Change subject: docs: updates to kudu_impala_integration.adoc .. Patch Set 2: Build Started http://104.196.14.100/job/kudu-gerrit/3154/ -- To view, visit http://gerrit.cloudera.org:8080/4138 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I02a91f1c33be3f0d6fbffaef5e03832e21b6db70 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] docs: update installation with new OS support
Kudu Jenkins has posted comments on this change. Change subject: docs: update installation with new OS support .. Patch Set 3: Build Started http://104.196.14.100/job/kudu-gerrit/3153/ -- To view, visit http://gerrit.cloudera.org:8080/4128 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2f51b55561f18fbe28d6bf4ed507dfba65947dc0 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] docs: updates to kudu impala integration.adoc
Hello Todd Lipcon, Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4138 to look at the new patch set (#2). Change subject: docs: updates to kudu_impala_integration.adoc .. docs: updates to kudu_impala_integration.adoc Note that this doc has diverged pretty substantially from its downstream Cloudera counterpart, which is far more featureful. Change-Id: I02a91f1c33be3f0d6fbffaef5e03832e21b6db70 --- M docs/kudu_impala_integration.adoc 1 file changed, 11 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/38/4138/2 -- To view, visit http://gerrit.cloudera.org:8080/4138 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I02a91f1c33be3f0d6fbffaef5e03832e21b6db70 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1500: Data race in RaftConsensusITest.TestCorruptReplicaMetadata
Kudu Jenkins has posted comments on this change. Change subject: KUDU-1500: Data race in RaftConsensusITest.TestCorruptReplicaMetadata .. Patch Set 17: Build Started http://104.196.14.100/job/kudu-gerrit/3152/ -- 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: 17 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: No
[kudu-CR] KUDU-1500: Data race in RaftConsensusITest.TestCorruptReplicaMetadata
Hello Dan Burkert, Mike Percy, Adar Dembo, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3823 to look at the new patch set (#17). Change subject: KUDU-1500: Data race in RaftConsensusITest.TestCorruptReplicaMetadata .. KUDU-1500: Data race in RaftConsensusITest.TestCorruptReplicaMetadata The test intends to corrupt the metadata of one of the tserver tablets. While the cluster is in the process of resurrecting the failed tserver, the Partition and PartitionSchema on that tserver is accessed using TabletPeer in an unguarded manner via ListTablets RPCs. The proposed fix here keeps the Partition and PartitionSchema immutable after it is loaded for the very first time. These fields are never overwritten again during tablet-copy or any other workflow. Also a unit test is added to exercise this data race window. i.e, the unit test aims to issue ListTablets RPCs during a follower's tablet copy stage and provides an optimistic coverage for the fix. DCHECKs are added to make sure the oncoming protobuf fields match the immutable fields not updated during tablet-copy. Testing: Spun a test TabletCopyITest.TestDeleteTabletDuringTabletCopy which exercises this data race window, also ran original test which caught the race in raft_consensus-itest.TestCorruptReplicaMetadata. Change-Id: If57617e22b41296b8d4e8ad131220f1ebb235019 --- M src/kudu/common/partition.cc M src/kudu/common/partition.h M src/kudu/integration-tests/tablet_copy-itest.cc M src/kudu/tablet/tablet_metadata.cc 4 files changed, 129 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/23/3823/17 -- To view, visit http://gerrit.cloudera.org:8080/3823 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: If57617e22b41296b8d4e8ad131220f1ebb235019 Gerrit-PatchSet: 17 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
[kudu-CR] docs: updates to kudu impala integration.adoc
Adar Dembo has posted comments on this change. Change subject: docs: updates to kudu_impala_integration.adoc .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/4138/1/docs/kudu_impala_integration.adoc File docs/kudu_impala_integration.adoc: Line 241: | RHEL | link:http://archive.cloudera.com/beta/impala-kudu/redhat/6/x86_64/impala-kudu/cloudera-impala-kudu.repo[RHEL 6], > BTW, what about CentOS? I would expect it to work on CentOS. Or we don't These packages work on CentOS., it's just not explicitly documented here. I'll fix that. -- To view, visit http://gerrit.cloudera.org:8080/4138 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I02a91f1c33be3f0d6fbffaef5e03832e21b6db70 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Add trace metrics for maintenance ops, LBM writes
Kudu Jenkins has posted comments on this change. Change subject: Add trace metrics for maintenance ops, LBM writes .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/3150/ -- To view, visit http://gerrit.cloudera.org:8080/4172 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I39cd9438071aabbd2d7bdeca3269af8b83f2d55b 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] tool: port log-dump
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4167 to look at the new patch set (#2). 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 docs/release_notes.adoc 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 13 files changed, 440 insertions(+), 160 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/67/4167/2 -- To view, visit http://gerrit.cloudera.org:8080/4167 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I14f048169c0b211e3c72fcd255c76dd59cbb05c9 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Add trace metrics for maintenance ops, LBM writes
Hello Dan Burkert,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/4172
to review the following change.
Change subject: Add trace metrics for maintenance ops, LBM writes
..
Add trace metrics for maintenance ops, LBM writes
This adds a trace-scoped counter for LogBlockManager writes, and also
adds traces and counter printing to the maintenance manager.
With this patch, flushes and compactions now output log messages like:
I0830 13:42:28.809852 2208 maintenance_manager.cc:356] Time spent running
FlushMRSOp(4301e24175a2469fba37184430567acc): real 9.510s user 4.432s sys
0.860s
I0830 13:42:28.810760 2208 maintenance_manager.cc:362]
FlushMRSOp(4301e24175a2469fba37184430567acc) metrics:
{"cfile_init":14,"compiler_manager_pool.queue_time_us":106,"compiler_manager_pool.run_cpu_time_us":219218,"compiler_manager_pool.run_wall_time_us":232004,"fdatasync":533,"fdatasync_us":67137,"lbm
root 0.queue_time_us":1605,"lbm root 0.run_cpu_time_us":1280,"lbm root
0.run_wall_time_us":344957,"lbm_read_time_us":94,"lbm_reads_lt_1ms":56,"lbm_write_time_us":709833,"lbm_writes_gt_100_ms":1,"lbm_writes_lt_1ms":11564,"mutex_wait_us":343945,"tcmalloc_contention_cycles":2759552,"thread_start_us":1304,"threads_started":13}
This can be handy for understanding why a particular compaction or flush
was slow.
Change-Id: I39cd9438071aabbd2d7bdeca3269af8b83f2d55b
---
M src/kudu/fs/log_block_manager.cc
M src/kudu/util/maintenance_manager.cc
M src/kudu/util/trace.h
3 files changed, 30 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/4172/1
--
To view, visit http://gerrit.cloudera.org:8080/4172
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I39cd9438071aabbd2d7bdeca3269af8b83f2d55b
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon
Gerrit-Reviewer: Dan Burkert
[kudu-CR] tool: port log-dump
Kudu Jenkins has posted comments on this change. Change subject: tool: port log-dump .. Patch Set 2: Build Started http://104.196.14.100/job/kudu-gerrit/3151/ -- 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: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Inlined dispatch for predicate evaluation
Dan Burkert has posted comments on this change. Change subject: Inlined dispatch for predicate evaluation .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/4164/2/src/kudu/common/column_predicate.h File src/kudu/common/column_predicate.h: Line 142: bool EvaluateCell(DataType physical_type, const void *cell) const; I think you may want to define this method in the header, so that you can be absolutely sure that the switch gets inlined away (since you are always calling it with a statically known DataType). The other option is to just expose it as a templatized method, but you will have to add public instantiations, I think. Todd, what do you think? -- 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: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-456 Implement AUTO FLUSH BACKGROUND flush mode
Dan Burkert has posted comments on this change.
Change subject: KUDU-456 Implement AUTO_FLUSH_BACKGROUND flush mode
..
Patch Set 23:
(16 comments)
Nice. This is much more understandable than previously.
http://gerrit.cloudera.org:8080/#/c/3952/23/src/kudu/client/batcher.cc
File src/kudu/client/batcher.cc:
Line 474: Status s = had_errors_ ? Status::Incomplete("Some errors
occurred")
I don't think Incomplete is the right status here; the flush is complete.
http://gerrit.cloudera.org:8080/#/c/3952/23/src/kudu/client/client-test.cc
File src/kudu/client/client-test.cc:
PS23, Line 2360: there the current batcher is at place
Not sure what this is trying to say, maybe:
there is a current batch
http://gerrit.cloudera.org:8080/#/c/3952/23/src/kudu/client/client.cc
File src/kudu/client/client.cc:
Line 322:hp);
this can go on the line above.
http://gerrit.cloudera.org:8080/#/c/3952/23/src/kudu/client/session-internal.cc
File src/kudu/client/session-internal.cc:
Line 63: TimeBasedFlushInit(session);
How about checking if we are in AUTO_FLUSH_BACKGROUND first? I know this is
basically a no-op if not, but it does schedule a background task.
Line 118: // Thread-safety note: the buffer_bytes_limit_ is not supposed to
be accessed
These thread safety notes (here and below) would be better in the header
declaration doc.
Line 156: Status KuduSession::Data::SetMaxBatchersNum(unsigned int max_num) {
How come this one doesn't check HasPendingOperations?
PS23, Line 283: nil
NULL
PS23, Line 356: }
: if (
should this be an else if?
Line 368: buffer_bytes_used_ += required_size;
hmm, this seems a little early. I can't think of a reason it would matter, but
it seems to me this counter shouldn't be updated until the batcher is available
and we are just about to or just have added the op to the batcher. Otherwise
we are updating the counter, potentially dropping the lock via the cond_.Wait()
call, and then later actually adding the op.
Line 379: if (!batcher_) {
Will the batcher_ ever be non-null here? I can't think of a reason why it
would be. Maybe a DCHECK is in order instead of the conditional.
Line 399: buffer_bytes_used_ -= required_size;
Another good reason not to increment this counter until after the op is
successfully added to the batcher_.
http://gerrit.cloudera.org:8080/#/c/3952/23/src/kudu/client/session-internal.h
File src/kudu/client/session-internal.h:
PS23, Line 46: Aply
Apply
PS23, Line 91: Return once no more pending operations are left.
this sentence seems redundant.
PS23, Line 91: finished their flushing
finish flushing
Line 191: std::unordered_set flushed_batchers_;
Why is it necessary to hold on to these batchers? I see that it's used for
calling HasPendingOperations, but wouldn't it be enough to check that
batches_num_ > 0 in that case?
PS23, Line 199: // protected by batch_ctl_mutex_
the setter doesn't take batch_ctl_mutex_
--
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: Yes
[kudu-CR](gh-pages) kudu flume sink blog post
Ara Ebrahimi has uploaded a new patch set (#4). Change subject: kudu flume sink blog post .. kudu flume sink blog post Change-Id: I810146ab24c88bc6cc562d81746b9bf5303396ed --- A _posts/2016-07-06-flume.md 1 file changed, 255 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/3510/4 -- To view, visit http://gerrit.cloudera.org:8080/3510 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I810146ab24c88bc6cc562d81746b9bf5303396ed Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Ara Ebrahimi Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Misty Stanley-Jones
[kudu-CR](gh-pages) kudu flume sink blog post
Ara Ebrahimi has uploaded a new patch set (#3). Change subject: kudu flume sink blog post .. kudu flume sink blog post Change-Id: I810146ab24c88bc6cc562d81746b9bf5303396ed --- A _posts/2016-07-06-flume.md 1 file changed, 255 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/3510/3 -- To view, visit http://gerrit.cloudera.org:8080/3510 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I810146ab24c88bc6cc562d81746b9bf5303396ed Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Ara Ebrahimi Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Misty Stanley-Jones
[kudu-CR](gh-pages) kudu flume sink blog post
Mike Percy has abandoned this change. Change subject: kudu flume sink blog post .. Abandoned Squashed into https://gerrit.cloudera.org/3510 -- To view, visit http://gerrit.cloudera.org:8080/4149 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: abandon Gerrit-Change-Id: I33166f0c3a0f739527d009808d4433342f9a95a8 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Ara Ebrahimi Gerrit-Reviewer: Mike Percy
[kudu-CR] compaction policy: fix bound calculation
Alexey Serbin has posted comments on this change. Change subject: compaction_policy: fix bound calculation .. Patch Set 2: 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: 2 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: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: No
[kudu-CR](gh-pages) kudu flume sink blog post
Ara Ebrahimi has uploaded a new patch set (#2). Change subject: kudu flume sink blog post .. kudu flume sink blog post Change-Id: I810146ab24c88bc6cc562d81746b9bf5303396ed --- A _posts/2016-07-06-flume.md 1 file changed, 182 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/3510/2 -- To view, visit http://gerrit.cloudera.org:8080/3510 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I810146ab24c88bc6cc562d81746b9bf5303396ed Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Ara Ebrahimi Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Misty Stanley-Jones
[kudu-CR] docs: updates to kudu impala integration.adoc
Alexey Serbin has posted comments on this change. Change subject: docs: updates to kudu_impala_integration.adoc .. Patch Set 1: Code-Review+1 (1 comment) Just one nice-to-know question regarding CentOS. Otherwise lgtm. http://gerrit.cloudera.org:8080/#/c/4138/1/docs/kudu_impala_integration.adoc File docs/kudu_impala_integration.adoc: Line 241: | RHEL | link:http://archive.cloudera.com/beta/impala-kudu/redhat/6/x86_64/impala-kudu/cloudera-impala-kudu.repo[RHEL 6], BTW, what about CentOS? I would expect it to work on CentOS. Or we don't support CentOS because there is not enough interest in impala-kudu on that platform? -- To view, visit http://gerrit.cloudera.org:8080/4138 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I02a91f1c33be3f0d6fbffaef5e03832e21b6db70 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] make election timeout jitter more aggressive
Todd Lipcon has posted comments on this change. Change subject: make election timeout jitter more aggressive .. Patch Set 1: Let's try and close this one out soon? Not sure where the conversation got left. -- To view, visit http://gerrit.cloudera.org:8080/3828 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2c9dad820c2b7d4bc4b9e791b78222559cdf63c8 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert 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
Dan Burkert 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 suppose we could change this to CHECK and just rely on upgrade / downgrad Providing equals on partition should be easy, it just needs to check that the lower and upper bound partition key strings are equal. -- 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: updates to kudu impala integration.adoc
Todd Lipcon has posted comments on this change. Change subject: docs: updates to kudu_impala_integration.adoc .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4138 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I02a91f1c33be3f0d6fbffaef5e03832e21b6db70 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] docs: update installation with new OS support
Todd Lipcon has posted comments on this change. Change subject: docs: update installation with new OS support .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4128 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2f51b55561f18fbe28d6bf4ed507dfba65947dc0 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1582. Optimize budgeted compaction policy with an approximation
Kudu Jenkins has posted comments on this change. Change subject: KUDU-1582. Optimize budgeted compaction policy with an approximation .. Patch Set 2: Build Started http://104.196.14.100/job/kudu-gerrit/3149/ -- To view, visit http://gerrit.cloudera.org:8080/4153 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4e611f161d66ddc47e97e3b5328bc1778a4ac958 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Anonymous Coward #174 Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] compaction policy: fix bound calculation
Hello Will Berkeley, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4152 to look at the new patch set (#2). Change subject: compaction_policy: fix bound calculation .. compaction_policy: fix bound calculation The upper-bound calculator for the knapsack problem maintains a heap which is supposed to keep no more than one item beyond the maximum weight allowed. This invariant got broken in some cases when an item was added to the heap -- in some cases a single addition actually requires removing multiple old items. Change-Id: I76364c17debd7293b36884d64c7747f8c604ae12 --- M src/kudu/tablet/compaction_policy.cc 1 file changed, 6 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/52/4152/2 -- To view, visit http://gerrit.cloudera.org:8080/4152 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I76364c17debd7293b36884d64c7747f8c604ae12 Gerrit-PatchSet: 2 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: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] compaction policy: fix bound calculation
Kudu Jenkins has posted comments on this change. Change subject: compaction_policy: fix bound calculation .. Patch Set 2: Build Started http://104.196.14.100/job/kudu-gerrit/3148/ -- 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: 2 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: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: No
[kudu-CR] compaction policy: fix bound calculation
Todd Lipcon 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? good point, although in the next patch I'll need to actually access all the non-top elements of the heap, and priority_queue<> doesn't seem to let you access non-top elements without popping. PS1, Line 161: >= > So, if the resulting weight is equal to the specified max_weight, then it's Done -- 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: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: Yes
[kudu-CR] KUDU-1586. consensus: always send at least one op
Todd Lipcon has submitted this change and it was merged. 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 Reviewed-on: http://gerrit.cloudera.org:8080/4168 Tested-by: Kudu Jenkins Reviewed-by: Dan Burkert --- M src/kudu/consensus/log_cache-test.cc M src/kudu/consensus/log_cache.cc 2 files changed, 7 insertions(+), 1 deletion(-) Approvals: Dan Burkert: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4168 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ibde0e833a3bf02a5f09f2b73b6ab03b188c1e697 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Fix kudu-ts-cli crash when there is no data in tablet
Dan Burkert has posted comments on this change. Change subject: Fix kudu-ts-cli crash when there is no data in tablet .. Patch Set 6: Code-Review+1 Code LGTM, but I agree that a test would be nice. -- 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] KUDU-1586. consensus: always send at least one op
Dan Burkert has posted comments on this change. Change subject: KUDU-1586. consensus: always send at least one op .. Patch Set 1: Code-Review+2 -- 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] 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: Subprocess::Call has optional out-parameters for stdout/stderr so you can capture it and assert -- 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] [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:
(1 comment)
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)
> I didn't know that: was assuming all builds work against git-cloned repo.
After some consideration I think adding git hash from version_defines.h
deserves a separate commit. For now it will be just sem-version, no git hash
info in the doxygen footer.
--
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] [docs] added Kudu version into the doxygen footer
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4165 to look at the new patch set (#3). 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(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/65/4165/3 -- 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: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: 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 3: Build Started http://104.196.14.100/job/kudu-gerrit/3147/ -- 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: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[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:
> Code change looks good, but how about a test in kudu-ts-cli-test?
Yess, thought about same after updating diffs; small piece of code lets me test
this path, however I am wondering how can we output a small dump_tablet output
to the test logfile(instead of usual stdout):
string exe_path = GetTsCliToolPath();
vector argv;
argv.push_back(exe_path);
argv.push_back("--server_address");
argv.push_back(cluster_->tablet_server(0)->bound_rpc_addr().ToString());
argv.push_back("dump_tablet");
argv.push_back(tablet_id);
ASSERT_OK(Subprocess::Call(argv));
--
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](gh-pages) kudu flume sink blog post
Mike Percy has posted comments on this change. Change subject: kudu flume sink blog post .. Patch Set 1: Thanks for posting this update! Please squash this into the previous commit and re-push to Gerrit: https://gerrit.cloudera.org/3510 -- To view, visit http://gerrit.cloudera.org:8080/4149 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I33166f0c3a0f739527d009808d4433342f9a95a8 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Ara Ebrahimi Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
