[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 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/4134/2/src/kudu/tools/ts-cli.cc File src/kudu/tools/ts-cli.cc: Line 278: break; > Todd, is this true for a scan without any filters ? doesn't 'ts-cli dump_ta yea, even without filters, you could have a bunch of deleted rows, and it's possible to "return early" after only reading batches of deleted rows. You can think of every scanner as having an implicit filter which performs deletion (and MVCC, more generally) -- 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: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Fix kudu-ts-cli crash when there is no data in tablet
Dinesh Bhat has posted comments on this change.
Change subject: Fix kudu-ts-cli crash when there is no data in tablet
..
Patch Set 4:
(5 comments)
TFTR Alexey/Todd, please see response inlined.
http://gerrit.cloudera.org:8080/#/c/4134/2/src/kudu/tools/ts-cli.cc
File src/kudu/tools/ts-cli.cc:
Line 265: do {
> Consider moving this into the scope of the while() loop -- there there will
Done
Line 266: RpcController rpc;
> Style nit: consider re-writing the cycle like
Done
Line 277: break;
> style nit: consider having the closure brackets for the if(). It's good fo
Done
Line 278: }
> What if the response has more results? I.e., is it guaranteed that if !has
Good point, a quick look at TabletServiceImpl::Scan tells me that Scan could
result in an empty batch, but my guess was that that could happen if
predicate-filters were specified. For a sequential scan like the one generated
here, I thought empty data means end of tablet.
Line 278: }
> nope, that's not guaranteed - you can get an empty batch in the "middle" of
Todd, is this true for a scan without any filters ? doesn't 'ts-cli dump_tablet
' output everything sequentially ? which means empty batch may mean
end-of-results ? I changed current logic to !has_data && !has_more_results if
that sounds more accurate.
--
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: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Dinesh Bhat
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] Fix kudu-ts-cli crash when there is no data in tablet
Kudu Jenkins has posted comments on this change. Change subject: Fix kudu-ts-cli crash when there is no data in tablet .. Patch Set 4: Build Started http://104.196.14.100/job/kudu-gerrit/3127/ -- 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: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert 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 (#4). Change subject: Fix kudu-ts-cli crash when there is no data in tablet .. Fix kudu-ts-cli crash when there is no data in tablet NULL pointer was being sent in as an argument to RowwiseRowBlockPB::Swap routine, fixing the callsite here. ./bin/kudu-ts-cli dump_tablet f32f6e0b1f354643b6b030599a359fa6 --server_address=127.108.26.1:33958 previously segfaulted to: $0 0x009c9867 in std::swap (__a=@0x7ffc75c76af8, __b=@0x18) at /opt/rh/devtoolset-3/root/usr/include/c++/4.9.2/bits/move.h:176 $1 0x00b5bf1f in kudu::RowwiseRowBlockPB::Swap (this=0x7ffc75c76ae0, other=0x0) at /data/10/dinesh/kudu/build/debug/src/kudu/common/wire_protocol.pb.cc:1916 $2 0x0085de72 in kudu::client::KuduScanBatch::Data::Reset (this=0x7ffc75c76a80, controller=0x7ffc75c76a20, projection=0x7ffc75c76b40, client_projection=0x7ffc75c76ca0, data=...) at /data/10/dinesh/kudu/src/kudu/client/scanner-internal.cc:484 $3 0x0084d5d9 in kudu::tools::TsAdminClient::DumpTablet (this=0x7ffc75c76e20, tablet_id="1f7a6bd64c604f7fada909bd575708cd") at /data/10/dinesh/kudu/src/kudu/tools/ts-cli.cc:278 $4 0x0084f4bd in kudu::tools::TsCliMain (argc=3, argv=0x7ffc75c773f0) at /data/10/dinesh/kudu/src/kudu/tools/ts-cli.cc:455 $5 0x00850079 in main (argc=4, argv=0x7ffc75c773e8) at /data/10/dinesh/kudu/src/kudu/tools/ts-cli.cc:492 Change-Id: I8470377400047236127dd409b4825c5c6ea6bbae --- M src/kudu/tools/ts-cli.cc 1 file changed, 8 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/34/4134/4 -- 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: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Alexey Serbin 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
Kudu Jenkins has posted comments on this change. Change subject: Fix kudu-ts-cli crash when there is no data in tablet .. Patch Set 3: Build Started http://104.196.14.100/job/kudu-gerrit/3126/ -- 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: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert 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 (#3). 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/common/row_operations.cc M src/kudu/tools/ts-cli.cc 2 files changed, 10 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/34/4134/3 -- 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: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1048 master should show versions of tservers, version summary
Todd Lipcon has posted comments on this change.
Change subject: KUDU-1048 master should show versions of tservers, version
summary
..
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4104/2/src/kudu/master/ts_manager.cc
File src/kudu/master/ts_manager.cc:
Line 103: if (ts->TimeSinceHeartbeat().ToMilliseconds() <
FLAGS_tserver_unresponsive_timeout_ms) {
can you use PresumedDead here now? then you shouldn't need the DECLARE in the
header either, since the only access to that flag would be from the
ts_descriptor.cc
--
To view, visit http://gerrit.cloudera.org:8080/4104
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Idd203209e3d99292018801b94ec2904b6634854f
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Will Berkeley
Gerrit-HasComments: Yes
[kudu-CR] tracing: gzip the trace JSON ajax response
Kudu Jenkins has posted comments on this change. Change subject: tracing: gzip the trace JSON ajax response .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/3125/ -- To view, visit http://gerrit.cloudera.org:8080/4154 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia8d5966e65b51576bf1460cb1ea8c2240ea88cbc 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] tracing: gzip the trace JSON ajax response
Hello Mike Percy, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/4154 to review the following change. Change subject: tracing: gzip the trace JSON ajax response .. tracing: gzip the trace JSON ajax response This enables gzipping of the actual trace which is sent back over HTTP when using the tracing web page. I noticed that the traces can be quite slow to receive, but given they often have a lot of repetitive text, they are quite compressible. The original Chrome implementation already did this, but I had ripped it out for simplicity when importing the code. This simply adds it back on the server side. A corresponding commit on the JS side adds a slight tweak so that we can send back binary gzipped data (whereas the Chrome version uses base64) Change-Id: Ia8d5966e65b51576bf1460cb1ea8c2240ea88cbc --- M src/kudu/server/tracing-path-handlers.cc M src/kudu/tserver/tablet_server-test.cc M thirdparty/vars.sh 3 files changed, 103 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/4154/1 -- To view, visit http://gerrit.cloudera.org:8080/4154 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ia8d5966e65b51576bf1460cb1ea8c2240ea88cbc Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Mike Percy
[kudu-CR] KUDU-687: use client in ksck for master operations
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4147 to look at the new patch set (#3). Change subject: KUDU-687: use client in ksck for master operations .. KUDU-687: use client in ksck for master operations This patch modifies ksck to use the client for all master operations, restricting direct access only for tserver operations. In doing so, ksck can now work with multi-master clusters, retrying operations when the leader master dies. Interesting things of note: - I went back and forth on what the semantics of KsckMaster::Connect() ought to be. At first I thought we should ping every master, but in the end I settled on building the client, which just verifies the existence of a leader master. My justification: today's ksck isn't really concerned with master health; the master merely provides information that is consumed during the real checks: of table and tablet integrity. - I relented on commit cf009d4 and restored a MiniCluster method to find the leader master. It's got the same signature as the ExternalMiniCluster variant so that Mike's common cluster interface (a work in progress) can expose it. Change-Id: Icbd6b28ea1b2c88e02cd451095e4dec94b0ebfc3 --- M src/kudu/client/schema.h M src/kudu/integration-tests/cluster_verifier.cc M src/kudu/integration-tests/mini_cluster.cc M src/kudu/integration-tests/mini_cluster.h M src/kudu/tools/CMakeLists.txt M src/kudu/tools/ksck-test.cc M src/kudu/tools/ksck.h M src/kudu/tools/ksck_remote-test.cc M src/kudu/tools/ksck_remote.cc M src/kudu/tools/ksck_remote.h M src/kudu/tools/tool_action_cluster.cc 11 files changed, 161 insertions(+), 164 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/47/4147/3 -- To view, visit http://gerrit.cloudera.org:8080/4147 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Icbd6b28ea1b2c88e02cd451095e4dec94b0ebfc3 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-687: use client in ksck for master operations
Kudu Jenkins has posted comments on this change. Change subject: KUDU-687: use client in ksck for master operations .. Patch Set 3: Build Started http://104.196.14.100/job/kudu-gerrit/3124/ -- To view, visit http://gerrit.cloudera.org:8080/4147 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icbd6b28ea1b2c88e02cd451095e4dec94b0ebfc3 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-687: expose additional tablet metadata in C++ client
Kudu Jenkins has posted comments on this change. Change subject: KUDU-687: expose additional tablet metadata in C++ client .. Patch Set 2: Build Started http://104.196.14.100/job/kudu-gerrit/3119/ -- To view, visit http://gerrit.cloudera.org:8080/4146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3cbbb1df8d3adf60425541b57e68595bbf6e92ff Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] tool: split up action descriptions
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4148 to look at the new patch set (#3). Change subject: tool: split up action descriptions .. tool: split up action descriptions With ksck we have a use case for "short" and "long" action descriptions: the former appears in mode help and action help, while the latter is only in action help. This patch splits action descriptions into "short" and "extra", refactoring all actions accordingly. While I was there I got rid of the label abstraction, which I felt was producing too much unfluent code. I also added some more tests to kudu-tool-test. Change-Id: Ie90b6228bba54575933fd9ac2f4f0bebf579ecd7 --- M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action.cc M src/kudu/tools/tool_action.h M src/kudu/tools/tool_action_cluster.cc M src/kudu/tools/tool_action_fs.cc M src/kudu/tools/tool_action_pbc.cc M src/kudu/tools/tool_action_tablet.cc M src/kudu/tools/tool_main.cc 8 files changed, 261 insertions(+), 97 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/48/4148/3 -- To view, visit http://gerrit.cloudera.org:8080/4148 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie90b6228bba54575933fd9ac2f4f0bebf579ecd7 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] tool: port cfile-dump to 'kudu fs dump cfile'
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4151 to look at the new patch set (#2). Change subject: tool: port cfile-dump to 'kudu fs dump_cfile' .. tool: port cfile-dump to 'kudu fs dump_cfile' Some non-cosmetic changes: - I changed the block_id conversion into something nicer than a CHECK. - The block_id parameter is expected in base 10, not base 16. To be honest, cfile-dump should have used base 10 for quite some time, because that's how they're printed in dumped PBs. - I dropped the num_iterations parameter because it didn't seem useful. Change-Id: I30cbaa6552e88348cebbf3059390a4c252eb7f8e --- M src/kudu/cfile/CMakeLists.txt D src/kudu/cfile/cfile-dump.cc M src/kudu/cfile/cfile-test-base.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_fs.cc 5 files changed, 131 insertions(+), 106 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/51/4151/2 -- To view, visit http://gerrit.cloudera.org:8080/4151 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I30cbaa6552e88348cebbf3059390a4c252eb7f8e Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] cfile: replace DumpIteratorOptions with number of rows
Kudu Jenkins has posted comments on this change. Change subject: cfile: replace DumpIteratorOptions with number of rows .. Patch Set 2: Build Started http://104.196.14.100/job/kudu-gerrit/3121/ -- To view, visit http://gerrit.cloudera.org:8080/4150 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5d9e80e50926a71d22de4f88a7af6a8091bb2063 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-687: expose additional tablet metadata in C++ client
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4146 to look at the new patch set (#2). Change subject: KUDU-687: expose additional tablet metadata in C++ client .. KUDU-687: expose additional tablet metadata in C++ client This patch adds new data-only KuduReplica and KuduTablet classes. Along with KuduTabletServer, the C++ client now has more parity with the Java client w.r.t. exposing tablet metadata. For now, this is only exposed via KuduScanToken, because it's already doing the work to figure out which replicas exist where. That's an odd fit for ksck (which doesn't scan, at least not using the client), but it should stave off any controversy stemming from adding dubious public APIs. In the future, it wouldn't be unreasonable for these classes to be exposed via KuduTable in some way. There are four public signature changes: - Addition of KuduScanToken::tablet(): this is backwards compatible. - Addition of KuduTabletServer::port(): this is backwards compatible. - Change to the KuduScanToken constructor: this should be backwards compatible, because it's private and so shouldn't used by applications. - Removal of KuduScanToken::TabletServers(): this is an incompatible change. I think it's OK because Impala is the only significant C++ client user and it's not even using scan tokens yet. Change-Id: I3cbbb1df8d3adf60425541b57e68595bbf6e92ff --- M docs/release_notes.adoc M src/kudu/client/CMakeLists.txt M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/meta_cache.cc M src/kudu/client/meta_cache.h A src/kudu/client/replica-internal.cc A src/kudu/client/replica-internal.h M src/kudu/client/scan_token-internal.cc M src/kudu/client/scan_token-internal.h M src/kudu/client/scan_token-test.cc A src/kudu/client/tablet-internal.cc A src/kudu/client/tablet-internal.h M src/kudu/client/tablet_server-internal.cc M src/kudu/client/tablet_server-internal.h 15 files changed, 445 insertions(+), 95 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/46/4146/2 -- To view, visit http://gerrit.cloudera.org:8080/4146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3cbbb1df8d3adf60425541b57e68595bbf6e92ff Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-687: use client in ksck for master operations
Kudu Jenkins has posted comments on this change. Change subject: KUDU-687: use client in ksck for master operations .. Patch Set 2: Build Started http://104.196.14.100/job/kudu-gerrit/3120/ -- To view, visit http://gerrit.cloudera.org:8080/4147 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icbd6b28ea1b2c88e02cd451095e4dec94b0ebfc3 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] tool: split up action descriptions
Kudu Jenkins has posted comments on this change. Change subject: tool: split up action descriptions .. Patch Set 3: Build Started http://104.196.14.100/job/kudu-gerrit/3123/ -- To view, visit http://gerrit.cloudera.org:8080/4148 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie90b6228bba54575933fd9ac2f4f0bebf579ecd7 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-687: use client in ksck for master operations
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4147 to look at the new patch set (#2). Change subject: KUDU-687: use client in ksck for master operations .. KUDU-687: use client in ksck for master operations This patch modifies ksck to use the client for all master operations, restricting direct access only for tserver operations. In doing so, ksck can now work with multi-master clusters, retrying operations when the leader master dies. Interesting things of note: - For 'kudu cluster ksck', I opted to make the master addresses a variadic (i.e. space-delimited) list, but I can also see the case for making it a comma-separated list, to be more consistent with --master_addresses. - I went back and forth on what the semantics of KsckMaster::Connect() ought to be. At first I thought we should ping every master, but in the end I settled on building the client, which just verifies the existence of a leader master. My justification: today's ksck isn't really concerned with master health; the master merely provides information that is consumed during the real checks: of table and tablet integrity. - I relented on commit cf009d4 and restored a MiniCluster method to find the leader master. It's got the same signature as the ExternalMiniCluster variant so that Mike's common cluster interface (a work in progress) can expose it. Change-Id: Icbd6b28ea1b2c88e02cd451095e4dec94b0ebfc3 --- M src/kudu/client/schema.h M src/kudu/integration-tests/cluster_verifier.cc M src/kudu/integration-tests/mini_cluster.cc M src/kudu/integration-tests/mini_cluster.h M src/kudu/tools/CMakeLists.txt M src/kudu/tools/ksck-test.cc M src/kudu/tools/ksck.h M src/kudu/tools/ksck_remote-test.cc M src/kudu/tools/ksck_remote.cc M src/kudu/tools/ksck_remote.h M src/kudu/tools/tool_action_cluster.cc 11 files changed, 161 insertions(+), 164 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/47/4147/2 -- To view, visit http://gerrit.cloudera.org:8080/4147 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Icbd6b28ea1b2c88e02cd451095e4dec94b0ebfc3 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] tool: port cfile-dump to 'kudu fs dump cfile'
Kudu Jenkins has posted comments on this change. Change subject: tool: port cfile-dump to 'kudu fs dump_cfile' .. Patch Set 2: Build Started http://104.196.14.100/job/kudu-gerrit/3122/ -- To view, visit http://gerrit.cloudera.org:8080/4151 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I30cbaa6552e88348cebbf3059390a4c252eb7f8e Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-687: expose additional tablet metadata in C++ client
Adar Dembo has posted comments on this change.
Change subject: KUDU-687: expose additional tablet metadata in C++ client
..
Patch Set 1:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/4146/1/src/kudu/client/client.h
File src/kudu/client/client.h:
Line 488: NON_PARTICIPANT = 3,
> how come this one has a value but not the others?
My bad. I had values in there originally, then removed them all (or so I
thought).
Line 495: Role role() const;
> what's the intended user use case for this one? maybe we can offer somethin
ksck keeps track of replicas that are leaders and those that are followers.
AFAICT it only actually uses is_leader (see Ksck::VerifyTablet).
I'll replace this with an is_leader() boolean instead.
http://gerrit.cloudera.org:8080/#/c/4146/1/src/kudu/client/meta_cache.cc
File src/kudu/client/meta_cache.cc:
Line 243: void RemoteTablet::GetRemoteReplicas(vector* replicas)
const {
> should clear replicas first
Done
http://gerrit.cloudera.org:8080/#/c/4146/1/src/kudu/client/replica-internal.cc
File src/kudu/client/replica-internal.cc:
PS1, Line 39: encoding
> 'role type'
Whoops, copy/pasta.
Line 49: default: LOG(FATAL) << "Unexpected encoding type: " << role;
> not sure this is a good idea, since if we added a new role, clients would s
We need to return something; what should we do? Or should all of these be
modified to return a Status in the event of an unknown input type?
I guess this is a moot point as I'm going to remove the new enum in favor of a
boolean for is_leader.
http://gerrit.cloudera.org:8080/#/c/4146/1/src/kudu/client/scan_token-internal.cc
File src/kudu/client/scan_token-internal.cc:
Line 258: // they've been moved.
> you could use ElementDeleter and just call client_replicas.clear(); after s
Dan convinced me that following std::move(), it is undefined behavior to access
an object. But, I see SO posts that agree with your assessment, so I'll switch
this back.
--
To view, visit http://gerrit.cloudera.org:8080/4146
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3cbbb1df8d3adf60425541b57e68595bbf6e92ff
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] tool: port cfile-dump to 'kudu fs dump cfile'
Adar Dembo has posted comments on this change. Change subject: tool: port cfile-dump to 'kudu fs dump_cfile' .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/4151/1/src/kudu/tools/tool_action_fs.cc File src/kudu/tools/tool_action_fs.cc: PS1, Line 78: numeric string > numeric string? maybe 'could not parse as numeric block ID?" Done -- To view, visit http://gerrit.cloudera.org:8080/4151 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I30cbaa6552e88348cebbf3059390a4c252eb7f8e Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] cfile: replace DumpIteratorOptions with number of rows
Adar Dembo has posted comments on this change. Change subject: cfile: replace DumpIteratorOptions with number of rows .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/4150/1/src/kudu/cfile/cfile-dump.cc File src/kudu/cfile/cfile-dump.cc: Line 31: DEFINE_int32(num_iterations, 1, "number of times to iterate the file"); > the original reason to have --noprint_rows not print anything was so that t Yep, the next patch in this series gets rid of it. http://gerrit.cloudera.org:8080/#/c/4150/1/src/kudu/cfile/cfile_util.h File src/kudu/cfile/cfile_util.h: Line 86: int num_rows, > do we actually use this other than with 0? kudu-fs_dump-tool allows users to customize it via FLAGS_nrows. That hasn't been ported to the new tool, but I imagine the flag will survive when it is. -- To view, visit http://gerrit.cloudera.org:8080/4150 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5d9e80e50926a71d22de4f88a7af6a8091bb2063 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] tool: split up action descriptions
Adar Dembo has posted comments on this change.
Change subject: tool: split up action descriptions
..
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4148/2/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
Line 71: void RunTestAction(const string& arg_str) const {
> maybe name this RunTestActionNoOutput or something so that the implied asse
Done
--
To view, visit http://gerrit.cloudera.org:8080/4148
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie90b6228bba54575933fd9ac2f4f0bebf579ecd7
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1500: Data race in RaftConsensusITest.TestCorruptReplicaMetadata
Adar Dembo 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()); > #1 is a good idea I understand that this code isn't executed in release mode, but I feel fairly strongly that forward (or backward, for that matter) compatibility behavior shouldn't vary between Kudu build types. That's really unexpected, and it's the kind of thing that may show up when a user encounters a real bug and tries to reproduce it with a debug build to get more information. -- 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] KUDU-687: use client in ksck for master operations
Adar Dembo has posted comments on this change.
Change subject: KUDU-687: use client in ksck for master operations
..
Patch Set 1:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/4147/1//COMMIT_MSG
Commit Message:
Line 17: comma-separated list, to be more consistent with --master_addresses.
> yea, I think I prefer comma-separated since it's the same way we specify it
Okay.
http://gerrit.cloudera.org:8080/#/c/4147/1/src/kudu/integration-tests/mini_cluster.cc
File src/kudu/integration-tests/mini_cluster.cc:
PS1, Line 294: *
> style
Done
http://gerrit.cloudera.org:8080/#/c/4147/1/src/kudu/tools/ksck_remote.cc
File src/kudu/tools/ksck_remote.cc:
Line 284: for (auto s : servers) {
> const auto&
I'll add const, but why ref? All of these containers are of raw pointers.
Line 324: for (auto t : tokens) {
> const auto&
See above.
Line 328: for (const auto r : t->tablet().replicas()) {
> auto&
See above.
--
To view, visit http://gerrit.cloudera.org:8080/4147
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Icbd6b28ea1b2c88e02cd451095e4dec94b0ebfc3
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1500: Data race in RaftConsensusITest.TestCorruptReplicaMetadata
Mike Percy has posted comments on this change.
Change subject: KUDU-1500: Data race in
RaftConsensusITest.TestCorruptReplicaMetadata
..
Patch Set 15:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/3823/15/src/kudu/tablet/tablet_metadata.cc
File src/kudu/tablet/tablet_metadata.cc:
PS15, Line 313: DCHECK_EQ(table_id_, superblock.table_id());
: PartitionSchemaPB partition_schema_pb;
: partition_schema_.ToPB(&partition_schema_pb);
:
DCHECK_EQ(superblock.partition_schema().SerializeAsString(),
: partition_schema_pb.SerializeAsString());
: PartitionPB partition_pb;
: partition_.ToPB(&partition_pb);
: DCHECK_EQ(superblock.partition().SerializeAsString(),
: partition_pb.SerializeAsString());
> 1. I actually thought about NDEBUG, but forgot to materialize in between th
#1 is a good idea
Regarding #2, since it's a DCHECK we ignore it in release mode.
Below is what the PBs look like. I suppose we could create equality functions
for them to try to future proof them from PB changes but I'm not sure it's
worth it for a DCHECK.
message PartitionPB {
// The hash buckets of the partition. The number of hash buckets must match
// the number of hash bucket components in the partition's schema.
repeated int32 hash_buckets = 1 [packed = true];
// The encoded start partition key (inclusive).
optional bytes partition_key_start = 2;
// The encoded end partition key (exclusive).
optional bytes partition_key_end = 3;
}
// The serialized format of a Kudu table partition schema.
message PartitionSchemaPB {
// A column identifier for partition schemas. In general, the name will be
// used when a client creates the table since column IDs are assigned by the
// master. All other uses of partition schemas will use the numeric column ID.
message ColumnIdentifierPB {
oneof identifier {
int32 id = 1;
string name = 2;
}
}
message RangeSchemaPB {
// Column identifiers of columns included in the range. All columns must be
// a component of the primary key.
repeated ColumnIdentifierPB columns = 1;
}
message HashBucketSchemaPB {
// Column identifiers of columns included in the hash. Every column must be
// a component of the primary key.
repeated ColumnIdentifierPB columns = 1;
// Number of buckets into which columns will be hashed. Must be at least 2.
required int32 num_buckets = 2;
// Seed value for hash calculation. Administrators may set a seed value
// on a per-table basis in order to randomize the mapping of rows to
// buckets. Setting a seed provides some amount of protection against denial
// of service attacks when the hash bucket columns contain user provided
// input.
optional uint32 seed = 3;
enum HashAlgorithm {
UNKNOWN = 0;
MURMUR_HASH_2 = 1;
}
// The hash algorithm to use for calculating the hash bucket.
optional HashAlgorithm hash_algorithm = 4;
}
repeated HashBucketSchemaPB hash_bucket_schemas = 1;
optional RangeSchemaPB range_schema = 2;
}
--
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] 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 16: Build Started http://104.196.14.100/job/kudu-gerrit/3118/ -- To view, visit http://gerrit.cloudera.org:8080/3823 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If57617e22b41296b8d4e8ad131220f1ebb235019 Gerrit-PatchSet: 16 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: 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 (#16). 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/integration-tests/tablet_copy-itest.cc M src/kudu/tablet/tablet_metadata.cc 2 files changed, 120 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/23/3823/16 -- 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: 16 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[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 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/4134/2/src/kudu/tools/ts-cli.cc File src/kudu/tools/ts-cli.cc: Line 278: break; > What if the response has more results? I.e., is it guaranteed that if !has nope, that's not guaranteed - you can get an empty batch in the "middle" of a scan -- 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: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [docs] added tip on generic git commit guidelines
Todd Lipcon has submitted this change and it was merged. Change subject: [docs] added tip on generic git commit guidelines .. [docs] added tip on generic git commit guidelines Change-Id: I5493723f7f0b337b73e3ef5ed4a7621fe5249927 Reviewed-on: http://gerrit.cloudera.org:8080/4115 Reviewed-by: Todd Lipcon Tested-by: Kudu Jenkins --- M docs/contributing.adoc 1 file changed, 3 insertions(+), 0 deletions(-) Approvals: Todd Lipcon: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4115 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I5493723f7f0b337b73e3ef5ed4a7621fe5249927 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] tool: port cfile-dump to 'kudu fs dump cfile'
Todd Lipcon has posted comments on this change. Change subject: tool: port cfile-dump to 'kudu fs dump_cfile' .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/4151/1/src/kudu/tools/tool_action_fs.cc File src/kudu/tools/tool_action_fs.cc: PS1, Line 78: numeric string numeric string? maybe 'could not parse as numeric block ID?" -- To view, visit http://gerrit.cloudera.org:8080/4151 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I30cbaa6552e88348cebbf3059390a4c252eb7f8e Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] cfile: replace DumpIteratorOptions with number of rows
Todd Lipcon has posted comments on this change. Change subject: cfile: replace DumpIteratorOptions with number of rows .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/4150/1/src/kudu/cfile/cfile-dump.cc File src/kudu/cfile/cfile-dump.cc: Line 31: DEFINE_int32(num_iterations, 1, "number of times to iterate the file"); the original reason to have --noprint_rows not print anything was so that this tool could be used for benchmarking raw read performance of a cfile, which also explains num_iterations. I agree that we have better (and more relevant) benchmarks now, but let's also get rid of num_iterations while we're at it http://gerrit.cloudera.org:8080/#/c/4150/1/src/kudu/cfile/cfile_util.h File src/kudu/cfile/cfile_util.h: Line 86: int num_rows, do we actually use this other than with 0? -- To view, visit http://gerrit.cloudera.org:8080/4150 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5d9e80e50926a71d22de4f88a7af6a8091bb2063 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] tool: split up action descriptions
Todd Lipcon has posted comments on this change.
Change subject: tool: split up action descriptions
..
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4148/2/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
Line 71: void RunTestAction(const string& arg_str) const {
maybe name this RunTestActionNoOutput or something so that the implied
assertion is more clear?
--
To view, visit http://gerrit.cloudera.org:8080/4148
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie90b6228bba54575933fd9ac2f4f0bebf579ecd7
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] KUDU-687: use client in ksck for master operations
Todd Lipcon has posted comments on this change.
Change subject: KUDU-687: use client in ksck for master operations
..
Patch Set 1:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/4147/1//COMMIT_MSG
Commit Message:
Line 17: comma-separated list, to be more consistent with --master_addresses.
yea, I think I prefer comma-separated since it's the same way we specify it
everywhere else (and in other tools)
http://gerrit.cloudera.org:8080/#/c/4147/1/src/kudu/integration-tests/mini_cluster.cc
File src/kudu/integration-tests/mini_cluster.cc:
PS1, Line 294: *
style
http://gerrit.cloudera.org:8080/#/c/4147/1/src/kudu/tools/ksck_remote.cc
File src/kudu/tools/ksck_remote.cc:
Line 284: for (auto s : servers) {
const auto&
Line 324: for (auto t : tokens) {
const auto&
Line 328: for (const auto r : t->tablet().replicas()) {
auto&
--
To view, visit http://gerrit.cloudera.org:8080/4147
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Icbd6b28ea1b2c88e02cd451095e4dec94b0ebfc3
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] KUDU-687: expose additional tablet metadata in C++ client
Todd Lipcon has posted comments on this change.
Change subject: KUDU-687: expose additional tablet metadata in C++ client
..
Patch Set 1:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/4146/1/src/kudu/client/client.h
File src/kudu/client/client.h:
Line 488: NON_PARTICIPANT = 3,
how come this one has a value but not the others?
Line 495: Role role() const;
what's the intended user use case for this one? maybe we can offer something a
bit more 'semantic'? otherwise if we add other roles like 'PRE_VOTER' it'll be
hard to do compatibly in this API
http://gerrit.cloudera.org:8080/#/c/4146/1/src/kudu/client/meta_cache.cc
File src/kudu/client/meta_cache.cc:
Line 243: void RemoteTablet::GetRemoteReplicas(vector* replicas)
const {
should clear replicas first
http://gerrit.cloudera.org:8080/#/c/4146/1/src/kudu/client/replica-internal.cc
File src/kudu/client/replica-internal.cc:
PS1, Line 39: encoding
'role type'
Line 49: default: LOG(FATAL) << "Unexpected encoding type: " << role;
not sure this is a good idea, since if we added a new role, clients would start
crashing
http://gerrit.cloudera.org:8080/#/c/4146/1/src/kudu/client/scan_token-internal.cc
File src/kudu/client/scan_token-internal.cc:
Line 258: // they've been moved.
you could use ElementDeleter and just call client_replicas.clear(); after
std::moveing it (std::move guarantees that the moved-from object is in a "valid
but undefined state", in which case clear() on it is safe
--
To view, visit http://gerrit.cloudera.org:8080/4146
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3cbbb1df8d3adf60425541b57e68595bbf6e92ff
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] compaction policy: fix bound calculation
Kudu Jenkins has posted comments on this change. Change subject: compaction_policy: fix bound calculation .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/3116/ -- To view, visit http://gerrit.cloudera.org:8080/4152 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I76364c17debd7293b36884d64c7747f8c604ae12 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Anonymous Coward #174 Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] compaction policy: fix bound calculation
Hello Anonymous Coward #174, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/4152 to review the following change. 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/1 -- To view, visit http://gerrit.cloudera.org:8080/4152 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I76364c17debd7293b36884d64c7747f8c604ae12 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Anonymous Coward #174
[kudu-CR] KUDU-1582. Optimize budgeted compaction policy with an approximation
Hello Anonymous Coward #174, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/4153 to review the following change. Change subject: KUDU-1582. Optimize budgeted compaction policy with an approximation .. KUDU-1582. Optimize budgeted compaction policy with an approximation On a server with ~5.5TB of data, I noticed that the maintenance manager was spending tens of seconds computing optimal compactions, and most of the time was in knapsack solving. This patch implements a "first pass" solution to the compaction policy using an approximation algorithm that is significantly faster than the actual knapsack solver. The approximation algorithm gives a lower bound solution which is at least 50% as good as the optimal solution (i.e. it is a '2-approximation'). In practice, it is often optimal or nearly optimal. To test, I dumped the bounds and sizes of rowsets from a 200+GB YCSB tablet from a real cluster that has been inserting for a few days, and wrote a test which imported that layout back in as mock rowsets in order to run the policy. The result (with the default 5% max approximation error) is between a 6x and 20x speedup (depending on the budget size). The resulting loss in solution quality is less than 1%. Before: Time spent Computing compaction with 128MB budget: real 0.965suser 0.964s sys 0.000s quality=0.118006 Time spent Computing compaction with 256MB budget: real 1.202suser 1.204s sys 0.000s quality=0.254289 Time spent Computing compaction with 512MB budget: real 2.233suser 2.232s sys 0.000s quality=0.524391 Time spent Computing compaction with 1024MB budget: real 4.202s user 4.200s sys 0.000s quality=1.06674 After: Time spent Computing compaction with 128MB budget: real 0.148suser 0.148s sys 0.000s quality=0.117985 Time spent Computing compaction with 256MB budget: real 0.162suser 0.160s sys 0.000s quality=0.252118 Time spent Computing compaction with 512MB budget: real 0.174suser 0.176s sys 0.000s quality=0.524391 Time spent Computing compaction with 1024MB budget: real 0.206s user 0.208s sys 0.000s quality=1.06674 Change-Id: I4e611f161d66ddc47e97e3b5328bc1778a4ac958 --- M src/kudu/tablet/CMakeLists.txt M src/kudu/tablet/compaction_policy-test.cc M src/kudu/tablet/compaction_policy.cc A src/kudu/tablet/ycsb-test-rowsets.tsv 4 files changed, 7,063 insertions(+), 83 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/4153/1 -- To view, visit http://gerrit.cloudera.org:8080/4153 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I4e611f161d66ddc47e97e3b5328bc1778a4ac958 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Anonymous Coward #174
[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 1: Build Started http://104.196.14.100/job/kudu-gerrit/3117/ -- 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: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Anonymous Coward #174 Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] tool: split up action descriptions
Hello Todd Lipcon, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4148 to look at the new patch set (#2). Change subject: tool: split up action descriptions .. tool: split up action descriptions With ksck we have a use case for "short" and "long" action descriptions: the former appears in mode help and action help, while the latter is only in action help. This patch splits action descriptions into "short" and "extra", refactoring all actions accordingly. While I was there I got rid of the label abstraction, which I felt was producing too much unfluent code. I also added some more tests to kudu-tool-test. Change-Id: Ie90b6228bba54575933fd9ac2f4f0bebf579ecd7 --- M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action.cc M src/kudu/tools/tool_action.h M src/kudu/tools/tool_action_cluster.cc M src/kudu/tools/tool_action_fs.cc M src/kudu/tools/tool_action_pbc.cc M src/kudu/tools/tool_action_tablet.cc M src/kudu/tools/tool_main.cc 8 files changed, 261 insertions(+), 97 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/48/4148/2 -- To view, visit http://gerrit.cloudera.org:8080/4148 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie90b6228bba54575933fd9ac2f4f0bebf579ecd7 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] tool: port cfile-dump to 'kudu fs dump cfile'
Hello Todd Lipcon, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/4151 to review the following change. Change subject: tool: port cfile-dump to 'kudu fs dump_cfile' .. tool: port cfile-dump to 'kudu fs dump_cfile' Some non-cosmetic changes: - I changed the block_id conversion into something nicer than a CHECK. - The block_id parameter is expected in base 10, not base 16. To be honest, cfile-dump should have used base 10 for quite some time, because that's how they're printed in dumped PBs. - I dropped the num_iterations parameter because it didn't seem useful. Change-Id: I30cbaa6552e88348cebbf3059390a4c252eb7f8e --- M src/kudu/cfile/CMakeLists.txt D src/kudu/cfile/cfile-dump.cc M src/kudu/cfile/cfile-test-base.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_fs.cc 5 files changed, 131 insertions(+), 106 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/51/4151/1 -- To view, visit http://gerrit.cloudera.org:8080/4151 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I30cbaa6552e88348cebbf3059390a4c252eb7f8e Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Todd Lipcon
[kudu-CR] tool: port cfile-dump to 'kudu fs dump cfile'
Kudu Jenkins has posted comments on this change. Change subject: tool: port cfile-dump to 'kudu fs dump_cfile' .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/3115/ -- To view, visit http://gerrit.cloudera.org:8080/4151 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I30cbaa6552e88348cebbf3059390a4c252eb7f8e Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] cfile: replace DumpIteratorOptions with number of rows
Hello Todd Lipcon, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/4150 to review the following change. Change subject: cfile: replace DumpIteratorOptions with number of rows .. cfile: replace DumpIteratorOptions with number of rows As of commit 9884fab, DumpIterator() doesn't print anything at all when print_rows is false. That makes the option rather useless, so I'm replacing it here with the raw number of rows. Change-Id: I5d9e80e50926a71d22de4f88a7af6a8091bb2063 --- M src/kudu/cfile/cfile-dump.cc M src/kudu/cfile/cfile_util.cc M src/kudu/cfile/cfile_util.h M src/kudu/tools/fs_tool.cc 4 files changed, 27 insertions(+), 52 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/50/4150/1 -- To view, visit http://gerrit.cloudera.org:8080/4150 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I5d9e80e50926a71d22de4f88a7af6a8091bb2063 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Todd Lipcon
[kudu-CR] tool: split up action descriptions
Kudu Jenkins has posted comments on this change. Change subject: tool: split up action descriptions .. Patch Set 2: Build Started http://104.196.14.100/job/kudu-gerrit/3113/ -- To view, visit http://gerrit.cloudera.org:8080/4148 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie90b6228bba54575933fd9ac2f4f0bebf579ecd7 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] cfile: replace DumpIteratorOptions with number of rows
Kudu Jenkins has posted comments on this change. Change subject: cfile: replace DumpIteratorOptions with number of rows .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/3114/ -- To view, visit http://gerrit.cloudera.org:8080/4150 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5d9e80e50926a71d22de4f88a7af6a8091bb2063 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1581 Fix DataFrame read failure when table has Binary Col For Binary Cols, kudu-spark is returning a ByteBuffer object when Spark expects to receive Array[Byte], so change is to return
Will Berkeley has posted comments on this change. Change subject: KUDU-1581 Fix DataFrame read failure when table has Binary Col For Binary Cols, kudu-spark is returning a ByteBuffer object when Spark expects to receive Array[Byte], so change is to return a copy of the byte array. Modified testcase to add missing col ty .. Patch Set 2: Code-Review+1 (4 comments) Couple whitespace and casing nits. Otherwise LGTM. http://gerrit.cloudera.org:8080/#/c/4145/2//COMMIT_MSG Commit Message: PS2, Line 8: For Binary Nit: empty line here to conform to usual style http://gerrit.cloudera.org:8080/#/c/4145/2/java/kudu-spark/pom.xml File java/kudu-spark/pom.xml: PS2, Line 57: : Nit: extra line break introduced here-- remove. http://gerrit.cloudera.org:8080/#/c/4145/2/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala File java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala: PS2, Line 54: "c9_Timestamp" Nit: c9_timestamp -- just to have consistent case PS2, Line 55: "c10_Byte" Ditto -- To view, visit http://gerrit.cloudera.org:8080/4145 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3e3926d85fab7efb407325c9992c3fccdab04bad Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Ram Mettu Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Ram Mettu Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: Yes
[kudu-CR](gh-pages) kudu flume sink blog post
Ara Ebrahimi has uploaded a new change for review. http://gerrit.cloudera.org:8080/4149 Change subject: kudu flume sink blog post .. kudu flume sink blog post Change-Id: I33166f0c3a0f739527d009808d4433342f9a95a8 --- M _posts/2016-07-06-flume.md 1 file changed, 144 insertions(+), 103 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/49/4149/1 -- To view, visit http://gerrit.cloudera.org:8080/4149 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I33166f0c3a0f739527d009808d4433342f9a95a8 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Ara Ebrahimi
[kudu-CR] tool: split up action descriptions
Kudu Jenkins has posted comments on this change. Change subject: tool: split up action descriptions .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/3112/ -- To view, visit http://gerrit.cloudera.org:8080/4148 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie90b6228bba54575933fd9ac2f4f0bebf579ecd7 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] tool: split up action descriptions
Hello Todd Lipcon, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/4148 to review the following change. Change subject: tool: split up action descriptions .. tool: split up action descriptions With ksck we have a use case for "short" and "long" action descriptions: the former appears in mode help and action help, while the latter is only in action help. This patch splits action descriptions into "short" and "extra", refactoring all actions accordingly. While I was there I got rid of the label abstraction, which I felt was producing too much unfluent code. I also added some more tests to kudu-tool-test. Change-Id: Ie90b6228bba54575933fd9ac2f4f0bebf579ecd7 --- M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action.cc M src/kudu/tools/tool_action.h M src/kudu/tools/tool_action_cluster.cc M src/kudu/tools/tool_action_fs.cc M src/kudu/tools/tool_action_pbc.cc M src/kudu/tools/tool_action_tablet.cc M src/kudu/tools/tool_main.cc 8 files changed, 255 insertions(+), 97 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/48/4148/1 -- To view, visit http://gerrit.cloudera.org:8080/4148 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ie90b6228bba54575933fd9ac2f4f0bebf579ecd7 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Todd Lipcon
