[kudu-CR] [clock] introduce mini chronyd
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/13916 ) Change subject: [clock] introduce mini_chronyd .. Patch Set 5: Verified+1 Unrelated flake due to the issue reported in KUDU-2815 -- To view, visit http://gerrit.cloudera.org:8080/13916 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id9d06d218828240f2a2980ef5ec30428f86277f7 Gerrit-Change-Number: 13916 Gerrit-PatchSet: 5 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 23 Aug 2019 02:23:32 + Gerrit-HasComments: No
[kudu-CR] [clock] introduce mini chronyd
Alexey Serbin has removed a vote on this change. Change subject: [clock] introduce mini_chronyd .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/13916 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: Id9d06d218828240f2a2980ef5ec30428f86277f7 Gerrit-Change-Number: 13916 Gerrit-PatchSet: 5 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2069 pt 1: add a maintenance mode
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/14111 )
Change subject: KUDU-2069 pt 1: add a maintenance mode
..
Patch Set 2:
(13 comments)
A first quick pass. Will send an update soon.
http://gerrit.cloudera.org:8080/#/c/14111/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14111/2//COMMIT_MSG@10
PS2, Line 10: failures of T will not be considered when determining
: whether a given tablet is under-replicated
I'm curious whether it mandates to have some sort of special response error
code for CreateTable when it's not enough tablet servers to place tablet
replicas because tablet servers are put into maintenance mode. Probably, I
need to re-read the design doc.
http://gerrit.cloudera.org:8080/#/c/14111/2//COMMIT_MSG@23
PS2, Line 23: whitelist
nit: it sounds like rather a "blackgrey/list" or "the usual suspects". Just
kidding, but maybe it makes sense to call it some other way :)
http://gerrit.cloudera.org:8080/#/c/14111/2/src/kudu/master/maintenance_state-test.cc
File src/kudu/master/maintenance_state-test.cc:
http://gerrit.cloudera.org:8080/#/c/14111/2/src/kudu/master/maintenance_state-test.cc@84
PS2, Line 84: afiliated
affiliated
http://gerrit.cloudera.org:8080/#/c/14111/2/src/kudu/master/maintenance_state-test.cc@100
PS2, Line 100: void TearDown() override {
: mini_master_->Shutdown();
: KuduTest::TearDown();
: }
nit: move it up to be just after SetUp() -- that would look a bit better from
the readability standpoint
http://gerrit.cloudera.org:8080/#/c/14111/2/src/kudu/master/maintenance_state-test.cc@170
PS2, Line 170: and that it persists through restarts
looks like an incomplete sentence
http://gerrit.cloudera.org:8080/#/c/14111/2/src/kudu/master/master.proto
File src/kudu/master/master.proto:
http://gerrit.cloudera.org:8080/#/c/14111/2/src/kudu/master/master.proto@816
PS2, Line 816: optional string tserver_uuid = 2;
Maybe, it makes sense to allow setting the maintenance state in batches?
http://gerrit.cloudera.org:8080/#/c/14111/2/src/kudu/master/master_service.cc
File src/kudu/master/master_service.cc:
http://gerrit.cloudera.org:8080/#/c/14111/2/src/kudu/master/master_service.cc@282
PS2, Line 282: if (!req->tablet_report().is_incremental()) {
: ts_desc->UpdateNeedsFullTabletReport(false);
: }
Could you document the reasoning behind this code?
http://gerrit.cloudera.org:8080/#/c/14111/2/src/kudu/master/ts_descriptor.h
File src/kudu/master/ts_descriptor.h:
http://gerrit.cloudera.org:8080/#/c/14111/2/src/kudu/master/ts_descriptor.h@113
PS2, Line 113: boost::optional&
Why boost::optional? Would simple enum suffice?
enum MaintenanceState {
kRegularMode, // or kNone
kMaintenanceMode,
};
http://gerrit.cloudera.org:8080/#/c/14111/2/src/kudu/master/ts_descriptor.h@207
PS2, Line 207: needs
needs to send ?
http://gerrit.cloudera.org:8080/#/c/14111/2/src/kudu/master/ts_descriptor.h@207
PS2, Line 207: or not
nit: drop
http://gerrit.cloudera.org:8080/#/c/14111/2/src/kudu/master/ts_manager.h
File src/kudu/master/ts_manager.h:
http://gerrit.cloudera.org:8080/#/c/14111/2/src/kudu/master/ts_manager.h@90
PS2, Line 90: it
them ?
http://gerrit.cloudera.org:8080/#/c/14111/2/src/kudu/master/ts_manager.h@132
PS2, Line 132: unregistered_maintenance_states_
nit: could you follow the same naming notation that was used for
'servers_by_id_'?
http://gerrit.cloudera.org:8080/#/c/14111/2/src/kudu/master/ts_manager.cc
File src/kudu/master/ts_manager.cc:
http://gerrit.cloudera.org:8080/#/c/14111/2/src/kudu/master/ts_manager.cc@68
PS2, Line 68: string MaintenanceStateAsString(const
boost::optional& state) {
: if (!state) {
: return "NONE";
: }
: switch (*state) {
: case kMaintenanceMode:
: return "MAINTENANCE MODE";
: }
: return Substitute("UNKNOWN STATE ($0)", *state);
: }
nit: I think this method can return const reference to strings, returning
simply "UNKNOWN STATE" if it gets some unexpected state (plus DCHECK on that).
--
To view, visit http://gerrit.cloudera.org:8080/14111
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia857668b87560cdd451c2e7f90d72f8217ba5a4b
Gerrit-Change-Number: 14111
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 23 Aug 2019 02:03:17 +
Gerrit-HasComments: Yes
[kudu-CR] [clock] introduce mini chronyd
Hello Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13916 to look at the new patch set (#5). Change subject: [clock] introduce mini_chronyd .. [clock] introduce mini_chronyd This patch introduces a wrapper around chronyd, so it's possible to run multiple instances of chronyd reference NTP servers as part of Kudu mini-cluster, providing reference for NTP clients. In addition, this patch contains tests to cover the newly introduced functionality. Change-Id: Id9d06d218828240f2a2980ef5ec30428f86277f7 --- M build-support/dist_test.py M src/kudu/clock/CMakeLists.txt A src/kudu/clock/test/mini_chronyd-test.cc A src/kudu/clock/test/mini_chronyd.cc A src/kudu/clock/test/mini_chronyd.h M src/kudu/mini-cluster/CMakeLists.txt M src/kudu/mini-cluster/external_mini_cluster.cc M src/kudu/mini-cluster/external_mini_cluster.h 8 files changed, 798 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/13916/5 -- To view, visit http://gerrit.cloudera.org:8080/13916 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id9d06d218828240f2a2980ef5ec30428f86277f7 Gerrit-Change-Number: 13916 Gerrit-PatchSet: 5 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [clock] introduce mini chronyd
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/13916 )
Change subject: [clock] introduce mini_chronyd
..
Patch Set 4:
(17 comments)
http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/clock/CMakeLists.txt
File src/kudu/clock/CMakeLists.txt:
http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/clock/CMakeLists.txt@39
PS4, Line 39: # * symlinks would not work with dist-test
> But don't we use symlinks for the HMS/Sentry/Hadoop stuff? Why do they work
Yes, we do, but those are symlinks to directories, not regular files. I didn't
look deep into the internals of dist-test, but I know dist-test copies contents
of symlinked directories, but it doesn't follow symlinks in case of regular
files.
I tried symlinks for chronyd and chronyc first, but that didn't work. That's
why I switched to simply copying those files.
http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/clock/test/mini_chronyd-test.cc
File src/kudu/clock/test/mini_chronyd-test.cc:
PS4:
> Have you looped the new tests under dist-tes?
Yes, I did it some time ago and re-run again today:
http://dist-test.cloudera.org//job?job_id=aserbin.1566515381.147044
http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/clock/test/mini_chronyd-test.cc@143
PS4, Line 143: ASSERT_OK(servers[i]->SetTime(ref_time + 10));
> Was the intent to reconfigure each server to use the same time albeit one f
It doesn't matter from which point they are all offset, the important thing
here is that they are far from each other. Eventually, we want to make sure
the client does _not_ see these NTP servers as a reliable source of
synchronization.
In prior revisions of this patch, I did set the same time and it worked most of
the time because the it took some time per operation, and if everything goes
slow enough, servers seem far enough to the client. To make this more robust
and explicit, I changed this to use different, far enough from each other
points in time.
I updated the comment, hopefully it became clearer now.
http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/clock/test/mini_chronyd.h
File src/kudu/clock/test/mini_chronyd.h:
http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/clock/test/mini_chronyd.h@87
PS4, Line 87: // Structure to represent relevant information from output by
: // 'chronyc serverstats'.
> Nit: alignment
Done
http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/clock/test/mini_chronyd.h@94
PS4, Line 94: as good
> as a good
Done
http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/clock/test/mini_chronyd.h@99
PS4, Line 99: synchronize
> synchronization
Done
http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/clock/test/mini_chronyd.h@105
PS4, Line 105: of
> or
Done
http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/clock/test/mini_chronyd.cc
File src/kudu/clock/test/mini_chronyd.cc:
http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/clock/test/mini_chronyd.cc@72
PS4, Line 72: std::
> drop
Done
http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/clock/test/mini_chronyd.cc@75
PS4, Line 75: "server $0 port $1 maxpoll -1 minpoll -6 iburst burst
version 3\n";
> Could you rationalize the non-obvious stuff (i.e. everything besides server
Done
http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/clock/test/mini_chronyd.cc@88
PS4, Line 88: /dev/stdin
> Does "-" work? I think it's more idiomatic.
Yeah, I tried '-' first. Unfortunately, it doesn't work for chronyd. It
possible to put together an extra patch for chronyd, but I decided not to spend
time on that.
http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/clock/test/mini_chronyd.cc@196
PS4, Line 196: vector
> Can you use 'auto' here?
It does't compile:
kudu/src/kudu/clock/test/mini_chronyd.cc:216:14: error: no member named
'size' in 'strings::internal::Splitter'
if (kv.size() != 2) {
http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/clock/test/mini_chronyd.cc@204
PS4, Line 204: LOG(INFO) << kv[0] << ":" << kv[1];
> Should be removed?
Done
http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/clock/test/mini_chronyd.cc@206
PS4, Line 206: result.ntp_packets_received = std::atol(kv[1].c_str());
> I think we prefer our gutil/strings/numbers.h safe_* functions.
Done
http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/mini-cluster/external_mini_cluster.h
File src/kudu/mini-cluster/external_mini_cluster.h:
http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/mini-cluster/external_mini_cluster.h@195
PS4, Line 195: int num_ntp_servers;
> Should add test coverage for this in external_mini_cluster-test.
Yeap, this is coming in the follow-up changelist.
http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/mini-cluster/external_mini_cluster.cc
File src/kudu/mini-cluster/external_mini_cluster.cc:
http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/mini-cluster/external_mini_cluster.cc@597
[kudu-CR] Create kudu/rebalance subdirectory
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14110 to look at the new patch set (#9). Change subject: Create kudu/rebalance subdirectory .. Create kudu/rebalance subdirectory Moved and renamed structs used for rebalancing from tools/ksck_results into rebalance/cluster_status. Moved tools/rebalance_algo to rebalance/rebalance_algo. Moved class Rebalancer from tools/rebalancer to rebalance/rebalancer. Renamed tools/rebalancer to tools/rebalancer_tool. Created RebalancerTool in tools/rebalancer_tool that inherits from Rebalancer. Moved corresponding tests into kudu/rebalance subdirectory. Movement of code done to prepare for autorebalancer task in master. No functional changes made. Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 --- M CMakeLists.txt A src/kudu/rebalance/CMakeLists.txt A src/kudu/rebalance/cluster_status.cc A src/kudu/rebalance/cluster_status.h R src/kudu/rebalance/placement_policy_util-test.cc R src/kudu/rebalance/placement_policy_util.cc R src/kudu/rebalance/placement_policy_util.h R src/kudu/rebalance/rebalance-test.cc R src/kudu/rebalance/rebalance_algo-test.cc R src/kudu/rebalance/rebalance_algo.cc R src/kudu/rebalance/rebalance_algo.h A src/kudu/rebalance/rebalancer.cc A src/kudu/rebalance/rebalancer.h M src/kudu/tools/CMakeLists.txt M src/kudu/tools/ksck-test.cc M src/kudu/tools/ksck.cc M src/kudu/tools/ksck.h M src/kudu/tools/ksck_remote.cc M src/kudu/tools/ksck_remote.h M src/kudu/tools/ksck_results.cc M src/kudu/tools/ksck_results.h D src/kudu/tools/rebalancer.h R src/kudu/tools/rebalancer_tool.cc A src/kudu/tools/rebalancer_tool.h M src/kudu/tools/tool.proto M src/kudu/tools/tool_action_cluster.cc 26 files changed, 1,884 insertions(+), 1,610 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/14110/9 -- To view, visit http://gerrit.cloudera.org:8080/14110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 Gerrit-Change-Number: 14110 Gerrit-PatchSet: 9 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] Create kudu/rebalance subdirectory
Hannah Nguyen has posted comments on this change. (
http://gerrit.cloudera.org:8080/14110 )
Change subject: Create kudu/rebalance subdirectory
..
Patch Set 7:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/rebalance/placement_policy_util-test.cc
File src/kudu/rebalance/placement_policy_util-test.cc:
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/rebalance/placement_policy_util-test.cc@33
PS7, Line 33: #include "kudu/consensus/quorum_util.h"
> Hrm.. I might be missing something; why do we need this now?
oops
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/rebalance/placement_policy_util-test.cc@199
PS7, Line 199: Re
> nit: spacing
Done
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/rebalance/rebalancer.cc
File src/kudu/rebalance/rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/rebalance/rebalancer.cc@397
PS7, Line 397: ksck
> nit: maybe just "health report"? Same below. Or omit details of "ksck" in g
ah, these were copied comments from the original tools/rebalancer.cc
http://gerrit.cloudera.org:8080/#/c/14110/8/src/kudu/rebalance/rebalancer.cc
File src/kudu/rebalance/rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/14110/8/src/kudu/rebalance/rebalancer.cc@86
PS8, Line 86: }
> To leverage move semantics, pass 'config' to by copy, ie
Done
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/tools/rebalancer_tool.cc
File src/kudu/tools/rebalancer_tool.cc:
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/tools/rebalancer_tool.cc@339
PS7, Line 339: const ClusterRawInfo& raw_info,
: const ClusterInfo&
ci,
: ostream& out) const
{
> nit: spacing
Done
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/tools/rebalancer_tool.cc@750
PS7, Line 750:bool*
timed_out) {
> nit: spacing
Done
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 7
Gerrit-Owner: Hannah Nguyen
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Hannah Nguyen
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 22 Aug 2019 23:37:06 +
Gerrit-HasComments: Yes
[kudu-CR] Create kudu/rebalance subdirectory
Andrew Wong has posted comments on this change. (
http://gerrit.cloudera.org:8080/14110 )
Change subject: Create kudu/rebalance subdirectory
..
Patch Set 8:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/rebalance/placement_policy_util-test.cc
File src/kudu/rebalance/placement_policy_util-test.cc:
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/rebalance/placement_policy_util-test.cc@33
PS7, Line 33: #include "kudu/consensus/quorum_util.h"
Hrm.. I might be missing something; why do we need this now?
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/rebalance/placement_policy_util-test.cc@199
PS7, Line 199: Re
nit: spacing
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/rebalance/rebalancer.cc
File src/kudu/rebalance/rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/rebalance/rebalancer.cc@397
PS7, Line 397:
nit: maybe just "health report"? Same below. Or omit details of "ksck" in
general.
http://gerrit.cloudera.org:8080/#/c/14110/8/src/kudu/rebalance/rebalancer.cc
File src/kudu/rebalance/rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/14110/8/src/kudu/rebalance/rebalancer.cc@86
PS8, Line 86: : config_(std::move(config)) {
> warning: std::move of the const variable 'config' has no effect; remove std
To leverage move semantics, pass 'config' to by copy, ie
Rebalancer(Config config) {
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/tools/rebalancer_tool.cc
File src/kudu/tools/rebalancer_tool.cc:
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/tools/rebalancer_tool.cc@339
PS7, Line 339: "Location: " << location << endl;
: out << "--"
<< endl;
: }
nit: spacing
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/tools/rebalancer_tool.cc@750
PS7, Line 750:
nit: spacing
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 8
Gerrit-Owner: Hannah Nguyen
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Hannah Nguyen
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 22 Aug 2019 23:17:00 +
Gerrit-HasComments: Yes
[kudu-CR] Create kudu/rebalance subdirectory
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14110 to look at the new patch set (#8). Change subject: Create kudu/rebalance subdirectory .. Create kudu/rebalance subdirectory Moved and renamed structs used for rebalancing from tools/ksck_results into rebalance/cluster_status. Moved tools/rebalance_algo to rebalance/rebalance_algo. Moved class Rebalancer from tools/rebalancer to rebalance/rebalancer. Renamed tools/rebalancer to tools/rebalancer_tool. Created RebalancerTool in tools/rebalancer_tool that inherits from Rebalancer. Moved corresponding tests into kudu/rebalance subdirectory. Movement of code done to prepare for autorebalancer task in master. No functional changes made. Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 --- M CMakeLists.txt A src/kudu/rebalance/CMakeLists.txt A src/kudu/rebalance/cluster_status.cc A src/kudu/rebalance/cluster_status.h R src/kudu/rebalance/placement_policy_util-test.cc R src/kudu/rebalance/placement_policy_util.cc R src/kudu/rebalance/placement_policy_util.h R src/kudu/rebalance/rebalance-test.cc R src/kudu/rebalance/rebalance_algo-test.cc R src/kudu/rebalance/rebalance_algo.cc R src/kudu/rebalance/rebalance_algo.h A src/kudu/rebalance/rebalancer.cc A src/kudu/rebalance/rebalancer.h M src/kudu/tools/CMakeLists.txt M src/kudu/tools/ksck-test.cc M src/kudu/tools/ksck.cc M src/kudu/tools/ksck.h M src/kudu/tools/ksck_remote.cc M src/kudu/tools/ksck_remote.h M src/kudu/tools/ksck_results.cc M src/kudu/tools/ksck_results.h D src/kudu/tools/rebalancer.h R src/kudu/tools/rebalancer_tool.cc A src/kudu/tools/rebalancer_tool.h M src/kudu/tools/tool.proto M src/kudu/tools/tool_action_cluster.cc 26 files changed, 1,882 insertions(+), 1,606 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/14110/8 -- To view, visit http://gerrit.cloudera.org:8080/14110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 Gerrit-Change-Number: 14110 Gerrit-PatchSet: 8 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] Create kudu/rebalance subdirectory
Hannah Nguyen has posted comments on this change. ( http://gerrit.cloudera.org:8080/14110 ) Change subject: Create kudu/rebalance subdirectory .. Patch Set 7: (5 comments) http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/rebalance/placement_policy_util.cc File src/kudu/rebalance/placement_policy_util.cc: http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/rebalance/placement_policy_util.cc@458 PS7, Line 458: } // namespace tools > warning: namespace 'rebalance' ends with a comment that refers to a wrong n Done http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/tools/rebalancer_tool.cc File src/kudu/tools/rebalancer_tool.cc: http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/tools/rebalancer_tool.cc@79 PS7, Line 79: using std::multimap; > warning: using decl 'multimap' is unused [misc-unused-using-decls] Done http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/tools/rebalancer_tool.cc@80 PS7, Line 80: using std::numeric_limits; > warning: using decl 'numeric_limits' is unused [misc-unused-using-decls] Done http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/tools/rebalancer_tool.cc@82 PS7, Line 82: using std::set_difference; > warning: using decl 'set_difference' is unused [misc-unused-using-decls] Done http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/tools/rebalancer_tool.cc@99 PS7, Line 99: Fixed -- To view, visit http://gerrit.cloudera.org:8080/14110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 Gerrit-Change-Number: 14110 Gerrit-PatchSet: 7 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 22 Aug 2019 22:06:20 + Gerrit-HasComments: Yes
[kudu-CR] Create kudu/rebalance subdirectory
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14110 to look at the new patch set (#7). Change subject: Create kudu/rebalance subdirectory .. Create kudu/rebalance subdirectory Moved and renamed structs used for rebalancing from tools/ksck_results into rebalance/cluster_status. Moved tools/rebalance_algo to rebalance/rebalance_algo. Moved class Rebalancer from tools/rebalancer to rebalance/rebalancer. Renamed tools/rebalancer to tools/rebalancer_tool. Created RebalancerTool in tools/rebalancer_tool that inherits from Rebalancer. Moved corresponding tests into kudu/rebalance subdirectory. Movement of code done to prepare for autorebalancer task in master. No functional changes made. Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 --- M CMakeLists.txt A src/kudu/rebalance/CMakeLists.txt A src/kudu/rebalance/cluster_status.cc A src/kudu/rebalance/cluster_status.h R src/kudu/rebalance/placement_policy_util-test.cc R src/kudu/rebalance/placement_policy_util.cc R src/kudu/rebalance/placement_policy_util.h R src/kudu/rebalance/rebalance-test.cc R src/kudu/rebalance/rebalance_algo-test.cc R src/kudu/rebalance/rebalance_algo.cc R src/kudu/rebalance/rebalance_algo.h A src/kudu/rebalance/rebalancer.cc A src/kudu/rebalance/rebalancer.h M src/kudu/tools/CMakeLists.txt M src/kudu/tools/ksck-test.cc M src/kudu/tools/ksck.cc M src/kudu/tools/ksck.h M src/kudu/tools/ksck_remote.cc M src/kudu/tools/ksck_remote.h M src/kudu/tools/ksck_results.cc M src/kudu/tools/ksck_results.h D src/kudu/tools/rebalancer.h R src/kudu/tools/rebalancer_tool.cc A src/kudu/tools/rebalancer_tool.h M src/kudu/tools/tool.proto M src/kudu/tools/tool_action_cluster.cc 26 files changed, 1,884 insertions(+), 1,602 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/14110/7 -- To view, visit http://gerrit.cloudera.org:8080/14110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 Gerrit-Change-Number: 14110 Gerrit-PatchSet: 7 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] WIP: Create kudu/rebalance subdirectory
Hannah Nguyen has posted comments on this change. (
http://gerrit.cloudera.org:8080/14110 )
Change subject: WIP: Create kudu/rebalance subdirectory
..
Patch Set 6:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/14110/6/src/kudu/tools/ksck.h
File src/kudu/tools/ksck.h:
http://gerrit.cloudera.org:8080/#/c/14110/6/src/kudu/tools/ksck.h@301
PS6, Line 301: ClusterServerHealth
> Can we forward declare these?
Done
http://gerrit.cloudera.org:8080/#/c/14110/6/src/kudu/tools/ksck.h@613
PS6, Line 613: int table_num_replicas);
> nit: spacing
Done
http://gerrit.cloudera.org:8080/#/c/14110/6/src/kudu/tools/ksck_results.cc
File src/kudu/tools/ksck_results.cc:
http://gerrit.cloudera.org:8080/#/c/14110/6/src/kudu/tools/ksck_results.cc@87
PS6, Line 87: const map& label_by_uuid,
> nit: keep these on the same line
Done
http://gerrit.cloudera.org:8080/#/c/14110/6/src/kudu/tools/placement_policy_util.h
File src/kudu/tools/placement_policy_util.h:
http://gerrit.cloudera.org:8080/#/c/14110/6/src/kudu/tools/placement_policy_util.h@30
PS6, Line 30: namespace rebalance {
: struct ClusterLocalityInfo;
: } // namespace rebalance
> You can put this in the below kudu namespace.
Done
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 6
Gerrit-Owner: Hannah Nguyen
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Hannah Nguyen
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 22 Aug 2019 21:40:09 +
Gerrit-HasComments: Yes
[kudu-CR] WIP: Create kudu/rebalance subdirectory
Andrew Wong has posted comments on this change. (
http://gerrit.cloudera.org:8080/14110 )
Change subject: WIP: Create kudu/rebalance subdirectory
..
Patch Set 6:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/14110/6/src/kudu/tools/ksck.h
File src/kudu/tools/ksck.h:
http://gerrit.cloudera.org:8080/#/c/14110/6/src/kudu/tools/ksck.h@301
PS6, Line 301: ClusterServerHealth
Can we forward declare these?
http://gerrit.cloudera.org:8080/#/c/14110/6/src/kudu/tools/ksck.h@613
PS6, Line 613: int table_num_replicas);
nit: spacing
http://gerrit.cloudera.org:8080/#/c/14110/6/src/kudu/tools/ksck_results.cc
File src/kudu/tools/ksck_results.cc:
http://gerrit.cloudera.org:8080/#/c/14110/6/src/kudu/tools/ksck_results.cc@87
PS6, Line 87: const map& label_by_uuid,
nit: keep these on the same line
http://gerrit.cloudera.org:8080/#/c/14110/6/src/kudu/tools/placement_policy_util.h
File src/kudu/tools/placement_policy_util.h:
http://gerrit.cloudera.org:8080/#/c/14110/6/src/kudu/tools/placement_policy_util.h@30
PS6, Line 30: namespace rebalance {
: struct ClusterLocalityInfo;
: } // namespace rebalance
You can put this in the below kudu namespace.
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 6
Gerrit-Owner: Hannah Nguyen
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Hannah Nguyen
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 22 Aug 2019 21:00:04 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2907: add a dir to dir groups when full
Hello Tidy Bot, Kudu Jenkins, Adar Dembo, ZhangYao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14122 to look at the new patch set (#2). Change subject: KUDU-2907: add a dir to dir groups when full .. KUDU-2907: add a dir to dir groups when full This patch extends the directory-picking behavior when selecting directories for new blocks if all directories in the given tablet's group are full. It now allows the expansion of a tablet's data dir group if healthy directories are available. This doesn't change the policy for picking directories for groups; it only extends the usage of the policy to potentially add directories when getting a directory for a new block. Change-Id: Ic8e05bdfe1fc2ac0e7152d493f289d3ac6e850d7 --- M src/kudu/fs/data_dirs-test.cc M src/kudu/fs/data_dirs.cc M src/kudu/fs/data_dirs.h M src/kudu/fs/file_block_manager.cc M src/kudu/fs/log_block_manager.cc M src/kudu/tserver/tablet_server-test.cc 6 files changed, 318 insertions(+), 67 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/22/14122/2 -- To view, visit http://gerrit.cloudera.org:8080/14122 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic8e05bdfe1fc2ac0e7152d493f289d3ac6e850d7 Gerrit-Change-Number: 14122 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: ZhangYao
[kudu-CR] KUDU-2069 pt 1: add a maintenance mode
Hello Tidy Bot, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14111 to look at the new patch set (#2). Change subject: KUDU-2069 pt 1: add a maintenance mode .. KUDU-2069 pt 1: add a maintenance mode When tablet server T is put in maintenance mode, replicas will not be placed onto T, and failures of T will not be considered when determining whether a given tablet is under-replicated. This patch adds this mode with the following changes: - A new master-side endpoint that enters maintenance mode is added: - It plumbs in-memory maintenance states through the TSManager and the TSDescriptors. - It also writes a new kind of entry in the master system catalog for maintenance states (for now, there's only maintenance mode, but this could be useful for decommissioning). - When a master becomes leader, it scans the on-disk state and rebuilds the in-memory maintenance state. - When determining whether a replica needs to be added, we may now consider a "whitelist" of UUIDs that can be in a bad state while not counting towards being under-replicated. - When determining where to place new replicas, tablet servers in maintenance mode are not considered. - The same master-side endpoint is used to exit maintenance mode. - To ensure that replicas that actually need to be replicated get replicated upon finishing maintenance mode, when a tablet server is removed from maintenance mode, the master will mark all tablet servers as needing a full tablet report, triggering re-processing of tablet reports. This patch only introduces the master endpoints and the underlying behavior. A later patch will introduce a way to set maintenance mode via CLI. I considered implementing maintenance mode by blocking master->tserver RPCs, but opted to use this approach since it seems more intuitive for the stopping of replica movement to exist in placement logic, (i.e. what servers are available to host new replicas and what replicas needs to be replaced), rather than the placement mechanism, (i.e. the handful of RPCs that would need to be considered). Change-Id: Ia857668b87560cdd451c2e7f90d72f8217ba5a4b --- M src/kudu/consensus/consensus_peers.cc M src/kudu/consensus/quorum_util-test.cc M src/kudu/consensus/quorum_util.cc M src/kudu/consensus/quorum_util.h M src/kudu/integration-tests/CMakeLists.txt A src/kudu/integration-tests/maintenance_mode-itest.cc M src/kudu/master/CMakeLists.txt M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h A src/kudu/master/maintenance_state-test.cc M src/kudu/master/master.proto M src/kudu/master/master_service.cc M src/kudu/master/master_service.h M src/kudu/master/sys_catalog.cc M src/kudu/master/sys_catalog.h M src/kudu/master/ts_descriptor-test.cc M src/kudu/master/ts_descriptor.cc M src/kudu/master/ts_descriptor.h M src/kudu/master/ts_manager.cc M src/kudu/master/ts_manager.h 20 files changed, 1,095 insertions(+), 35 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/11/14111/2 -- To view, visit http://gerrit.cloudera.org:8080/14111 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia857668b87560cdd451c2e7f90d72f8217ba5a4b Gerrit-Change-Number: 14111 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2921: Exposing the table statistics to spark relation.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14107 ) Change subject: KUDU-2921: Exposing the table statistics to spark relation. .. Patch Set 2: > Actually, I have considered to put those statistics into existing message > such as getShema which will be used when opening table. But as for the table > statistics may contain more and more entries it may be better to be a > independent rpc. What's more, because this statistics may change when client > trying to write, so I think it will be useful to support a rpc for client to > get the statistics change. Although for query plan it will only use once and > save one round trip if we put it into existing rpc but it will be heavy if we > want to get statistic change in other use cases. How are the stats useful for a writing client (i.e. an Impala or Spark executor) vs. the query planner? What are the other use cases you're thinking about? If they're hypothetical, we could add the stats to GetTableLocations now, and also expose them via new RPC later, when those other use cases materialize. > > If doing this, let's make sure to make it optional, since the > > statistics could become larger later (eg with things like > > histograms, NDVs per column, etc) > > Will make it optional if do that :) Agreed. -- To view, visit http://gerrit.cloudera.org:8080/14107 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7742a76708f989b0ccc8ba417f3390013e260175 Gerrit-Change-Number: 14107 Gerrit-PatchSet: 2 Gerrit-Owner: ZhangYao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: ZhangYao Gerrit-Comment-Date: Thu, 22 Aug 2019 18:58:36 + Gerrit-HasComments: No
[kudu-CR] [clock] introduce mini chronyd
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13916 ) Change subject: [clock] introduce mini_chronyd .. Patch Set 4: (16 comments) http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/clock/CMakeLists.txt File src/kudu/clock/CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/clock/CMakeLists.txt@39 PS4, Line 39: # * symlinks would not work with dist-test But don't we use symlinks for the HMS/Sentry/Hadoop stuff? Why do they work there but not here? http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/clock/test/mini_chronyd-test.cc File src/kudu/clock/test/mini_chronyd-test.cc: PS4: Have you looped the new tests under dist-tes? http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/clock/test/mini_chronyd-test.cc@143 PS4, Line 143: ASSERT_OK(servers[i]->SetTime(ref_time + 10)); Was the intent to reconfigure each server to use the same time albeit one far removed from NOW? Or for each server to use a different time? Unclear from the comment. http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/clock/test/mini_chronyd.h File src/kudu/clock/test/mini_chronyd.h: http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/clock/test/mini_chronyd.h@87 PS4, Line 87: // Structure to represent relevant information from output by : // 'chronyc serverstats'. Nit: alignment http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/clock/test/mini_chronyd.h@94 PS4, Line 94: as good as a good http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/clock/test/mini_chronyd.h@99 PS4, Line 99: synchronize synchronization http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/clock/test/mini_chronyd.h@105 PS4, Line 105: of or http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/clock/test/mini_chronyd.cc File src/kudu/clock/test/mini_chronyd.cc: http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/clock/test/mini_chronyd.cc@72 PS4, Line 72: std:: drop http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/clock/test/mini_chronyd.cc@75 PS4, Line 75: "server $0 port $1 maxpoll -1 minpoll -6 iburst burst version 3\n"; Could you rationalize the non-obvious stuff (i.e. everything besides server and port) in a comment? http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/clock/test/mini_chronyd.cc@88 PS4, Line 88: /dev/stdin Does "-" work? I think it's more idiomatic. http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/clock/test/mini_chronyd.cc@196 PS4, Line 196: vector Can you use 'auto' here? http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/clock/test/mini_chronyd.cc@204 PS4, Line 204: LOG(INFO) << kv[0] << ":" << kv[1]; Should be removed? http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/clock/test/mini_chronyd.cc@206 PS4, Line 206: result.ntp_packets_received = std::atol(kv[1].c_str()); I think we prefer our gutil/strings/numbers.h safe_* functions. http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/mini-cluster/external_mini_cluster.h File src/kudu/mini-cluster/external_mini_cluster.h: http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/mini-cluster/external_mini_cluster.h@195 PS4, Line 195: int num_ntp_servers; Should add test coverage for this in external_mini_cluster-test. http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/mini-cluster/external_mini_cluster.cc File src/kudu/mini-cluster/external_mini_cluster.cc: http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/mini-cluster/external_mini_cluster.cc@597 PS4, Line 597: options.port = 10123 + idx; If we want this to work well on macOS, we'll need to bind to an ephemral port and then use lsof or whatever to figure out what port we got. Can we do that? http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/mini-cluster/internal_mini_cluster.h File src/kudu/mini-cluster/internal_mini_cluster.h: http://gerrit.cloudera.org:8080/#/c/13916/4/src/kudu/mini-cluster/internal_mini_cluster.h@111 PS4, Line 111: #if 0 Why commented out? Either remove or implement. -- To view, visit http://gerrit.cloudera.org:8080/13916 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id9d06d218828240f2a2980ef5ec30428f86277f7 Gerrit-Change-Number: 13916 Gerrit-PatchSet: 4 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 22 Aug 2019 18:49:07 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2907: add a dir to dir groups when full
Andrew Wong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14122 Change subject: KUDU-2907: add a dir to dir groups when full .. KUDU-2907: add a dir to dir groups when full This patch extends the directory-picking behavior when selecting directories for new blocks if all directories in the given tablet's group are full. It now allows the expansion of a tablet's data dir group if healthy directories are available. This doesn't change the policy for picking directories for groups; it only extends the usage of the policy to potentially add directories when getting a directory for a new block. Change-Id: Ic8e05bdfe1fc2ac0e7152d493f289d3ac6e850d7 --- M src/kudu/fs/data_dirs-test.cc M src/kudu/fs/data_dirs.cc M src/kudu/fs/data_dirs.h M src/kudu/fs/file_block_manager.cc M src/kudu/fs/log_block_manager.cc M src/kudu/tserver/tablet_server-test.cc 6 files changed, 317 insertions(+), 67 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/22/14122/1 -- To view, visit http://gerrit.cloudera.org:8080/14122 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ic8e05bdfe1fc2ac0e7152d493f289d3ac6e850d7 Gerrit-Change-Number: 14122 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong
[kudu-CR] WIP: Create kudu/rebalance subdirectory
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14110 to look at the new patch set (#6). Change subject: WIP: Create kudu/rebalance subdirectory .. WIP: Create kudu/rebalance subdirectory Moved structs used for rebalancing from tools/ksck_results into rebalance/cluster_status. Moved contents of tools/rebalance_algo into rebalance subdirectory too. TODO: move tools/placement_policy_util and tools/rebalancer. Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 --- M CMakeLists.txt A src/kudu/rebalance/CMakeLists.txt A src/kudu/rebalance/cluster_status.cc A src/kudu/rebalance/cluster_status.h R src/kudu/rebalance/rebalance_algo-test.cc R src/kudu/rebalance/rebalance_algo.cc R src/kudu/rebalance/rebalance_algo.h M src/kudu/tools/CMakeLists.txt M src/kudu/tools/ksck-test.cc M src/kudu/tools/ksck.cc M src/kudu/tools/ksck.h M src/kudu/tools/ksck_remote.cc M src/kudu/tools/ksck_remote.h M src/kudu/tools/ksck_results.cc M src/kudu/tools/ksck_results.h M src/kudu/tools/placement_policy_util-test.cc M src/kudu/tools/placement_policy_util.cc M src/kudu/tools/placement_policy_util.h M src/kudu/tools/rebalance-test.cc M src/kudu/tools/rebalancer.cc M src/kudu/tools/rebalancer.h M src/kudu/tools/tool.proto M src/kudu/tools/tool_action_cluster.cc 23 files changed, 857 insertions(+), 661 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/14110/6 -- To view, visit http://gerrit.cloudera.org:8080/14110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 Gerrit-Change-Number: 14110 Gerrit-PatchSet: 6 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] WIP: Create kudu/rebalance subdirectory
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14110 to look at the new patch set (#5). Change subject: WIP: Create kudu/rebalance subdirectory .. WIP: Create kudu/rebalance subdirectory Moved structs used for rebalancing from tools/ksck_results into rebalance/cluster_status. Moved contents of tools/rebalance_algo into rebalance subdirectory too. TODO: move tools/placement_policy_util and tools/rebalancer. Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 --- M CMakeLists.txt A src/kudu/rebalance/CMakeLists.txt A src/kudu/rebalance/cluster_status.cc A src/kudu/rebalance/cluster_status.h R src/kudu/rebalance/rebalance_algo-test.cc R src/kudu/rebalance/rebalance_algo.cc R src/kudu/rebalance/rebalance_algo.h M src/kudu/tools/CMakeLists.txt M src/kudu/tools/ksck-test.cc M src/kudu/tools/ksck.cc M src/kudu/tools/ksck.h M src/kudu/tools/ksck_remote.cc M src/kudu/tools/ksck_remote.h M src/kudu/tools/ksck_results.cc M src/kudu/tools/ksck_results.h M src/kudu/tools/placement_policy_util-test.cc M src/kudu/tools/placement_policy_util.cc M src/kudu/tools/placement_policy_util.h M src/kudu/tools/rebalance-test.cc M src/kudu/tools/rebalancer.cc M src/kudu/tools/rebalancer.h M src/kudu/tools/tool.proto M src/kudu/tools/tool_action_cluster.cc 23 files changed, 859 insertions(+), 658 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/14110/5 -- To view, visit http://gerrit.cloudera.org:8080/14110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 Gerrit-Change-Number: 14110 Gerrit-PatchSet: 5 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [docs]: Fix bad markdown formatting in tablet.md
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14120 to look at the new patch set (#3). Change subject: [docs]: Fix bad markdown formatting in tablet.md .. [docs]: Fix bad markdown formatting in tablet.md Use '```' for code block Change-Id: I06e04031c8b817b3a3d7f771705c85a161cbb08d --- M docs/design-docs/tablet.md 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/14120/3 -- To view, visit http://gerrit.cloudera.org:8080/14120 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I06e04031c8b817b3a3d7f771705c85a161cbb08d Gerrit-Change-Number: 14120 Gerrit-PatchSet: 3 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [docs]: Fix bad marddown formatting in tablet.md
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14120 to look at the new patch set (#2). Change subject: [docs]: Fix bad marddown formatting in tablet.md .. [docs]: Fix bad marddown formatting in tablet.md Use '```' for code block Change-Id: I06e04031c8b817b3a3d7f771705c85a161cbb08d --- M docs/design-docs/tablet.md 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/14120/2 -- To view, visit http://gerrit.cloudera.org:8080/14120 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I06e04031c8b817b3a3d7f771705c85a161cbb08d Gerrit-Change-Number: 14120 Gerrit-PatchSet: 2 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2847: Optimize iteration over selection vector in SerializeRowBlock
Hello Tidy Bot, Kudu Jenkins, Yao Xu, Andrew Wong, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13721 to look at the new patch set (#9). Change subject: KUDU-2847: Optimize iteration over selection vector in SerializeRowBlock .. KUDU-2847: Optimize iteration over selection vector in SerializeRowBlock This improves the performance of serializing RowBlocks to the wire by amortizing the cost of iterating over the set bits of the selection bitmap. Instead of using the bitmap once per column, we convert the bitmap to a list of set indices up front, and then use those indices for conversion. This changes the benchmarks to report cycles/cell instead of raw times, making it easier to see the effects of column count or sparsity. Benchmark results: column count 3 and row select rate 1: 5.12520529 -> 5.44280228 cycles/cell column count 3 and row select rate 0.8: 12.74473127 -> 7.04588262 cycles/cell column count 3 and row select rate 0.5: 23.98607461 -> 7.51201477 cycles/cell column count 3 and row select rate 0.2: 40.66053179 -> 8.30233998 cycles/cell column count 30 and row select rate 1:15.43040511 -> 15.97765642 cycles/cell column count 30 and row select rate 0.8: 23.7480557 -> 17.84433817 cycles/cell column count 30 and row select rate 0.5: 40.08323337 -> 17.67888749 cycles/cell column count 30 and row select rate 0.2: 48.62210244 -> 16.56884988 cycles/cell column count 300 and row select rate 1: 18.9223316 -> 20.90426976 cycles/cell column count 300 and row select rate 0.8: 27.50793008 -> 21.92481189 cycles/cell column count 300 and row select rate 0.5: 40.34367716 -> 21.32180024 cycles/cell column count 300 and row select rate 0.2: 52.7446843 -> 20.92634437 cycles/cell Patch co-authored by Zhang Yao. Change-Id: I19917d1875c46fd4cf98ef8a471b0340a76161e7 --- M src/kudu/common/rowblock-test.cc M src/kudu/common/rowblock.cc M src/kudu/common/rowblock.h M src/kudu/common/wire_protocol-test.cc M src/kudu/common/wire_protocol.cc 5 files changed, 107 insertions(+), 53 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/21/13721/9 -- To view, visit http://gerrit.cloudera.org:8080/13721 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I19917d1875c46fd4cf98ef8a471b0340a76161e7 Gerrit-Change-Number: 13721 Gerrit-PatchSet: 9 Gerrit-Owner: ZhangYao Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Yao Xu Gerrit-Reviewer: ZhangYao
[kudu-CR] KUDU-2847: Optimize iteration over selection vector in SerializeRowBlock
ZhangYao has posted comments on this change. (
http://gerrit.cloudera.org:8080/13721 )
Change subject: KUDU-2847: Optimize iteration over selection vector in
SerializeRowBlock
..
Patch Set 8:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/13721/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13721/7//COMMIT_MSG@10
PS7, Line 10: amortizing
> amortizing
Done
http://gerrit.cloudera.org:8080/#/c/13721/7/src/kudu/common/rowblock.cc
File src/kudu/common/rowblock.cc:
http://gerrit.cloudera.org:8080/#/c/13721/7/src/kudu/common/rowblock.cc@91
PS7, Line 91: the bit index to the output until none are set.
: for (int i = 0; i < n_bytes_; i++) {
: uint8_t bm = *bitmap++;
: while (bm != 0) {
: int bit = Bits::FindLSBSetNonZero(bm);
:
> nit: Jumping into this is can be intimidating with no comments. Maybe add a
Done
http://gerrit.cloudera.org:8080/#/c/13721/7/src/kudu/common/wire_protocol.cc
File src/kudu/common/wire_protocol.cc:
http://gerrit.cloudera.org:8080/#/c/13721/7/src/kudu/common/wire_protocol.cc@927
PS7, Line 927: * slice = reinterpret_cast(src);
: size_t offset_in_indirect = indirect_data->size();
: indirect_data->append(reinterpret_cast(slice->data()), slice->size());
:
: Slice*
> nit: fix ordering of *s
Done
--
To view, visit http://gerrit.cloudera.org:8080/13721
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I19917d1875c46fd4cf98ef8a471b0340a76161e7
Gerrit-Change-Number: 13721
Gerrit-PatchSet: 8
Gerrit-Owner: ZhangYao
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Yao Xu
Gerrit-Reviewer: ZhangYao
Gerrit-Comment-Date: Thu, 22 Aug 2019 09:14:20 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2847: Optimize iteration over selection vector in SerializeRowBlock
Hello Tidy Bot, Kudu Jenkins, Yao Xu, Andrew Wong, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13721 to look at the new patch set (#8). Change subject: KUDU-2847: Optimize iteration over selection vector in SerializeRowBlock .. KUDU-2847: Optimize iteration over selection vector in SerializeRowBlock This improves the performance of serializing RowBlocks to the wire by amortizing the cost of iterating over the set bits of the selection bitmap. Instead of using the bitmap once per column, we convert the bitmap to a list of set indices up front, and then use those indices for conversion. This changes the benchmarks to report cycles/cell instead of raw times, making it easier to see the effects of column count or sparsity. Benchmark results: column count 3 and row select rate 1: 5.12520529 -> 5.44280228 cycles/cell column count 3 and row select rate 0.8: 12.74473127 -> 7.04588262 cycles/cell column count 3 and row select rate 0.5: 23.98607461 -> 7.51201477 cycles/cell column count 3 and row select rate 0.2: 40.66053179 -> 8.30233998 cycles/cell column count 30 and row select rate 1:15.43040511 -> 15.97765642 cycles/cell column count 30 and row select rate 0.8: 23.7480557 -> 17.84433817 cycles/cell column count 30 and row select rate 0.5: 40.08323337 -> 17.67888749 cycles/cell column count 30 and row select rate 0.2: 48.62210244 -> 16.56884988 cycles/cell column count 300 and row select rate 1: 18.9223316 -> 20.90426976 cycles/cell column count 300 and row select rate 0.8: 27.50793008 -> 21.92481189 cycles/cell column count 300 and row select rate 0.5: 40.34367716 -> 21.32180024 cycles/cell column count 300 and row select rate 0.2: 52.7446843 -> 20.92634437 cycles/cell Patch co-authored by Zhang Yao. Change-Id: I19917d1875c46fd4cf98ef8a471b0340a76161e7 --- M src/kudu/common/rowblock-test.cc M src/kudu/common/rowblock.cc M src/kudu/common/rowblock.h M src/kudu/common/wire_protocol-test.cc M src/kudu/common/wire_protocol.cc 5 files changed, 107 insertions(+), 54 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/21/13721/8 -- To view, visit http://gerrit.cloudera.org:8080/13721 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I19917d1875c46fd4cf98ef8a471b0340a76161e7 Gerrit-Change-Number: 13721 Gerrit-PatchSet: 8 Gerrit-Owner: ZhangYao Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Yao Xu Gerrit-Reviewer: ZhangYao
[kudu-CR] [util] Add ExtractFloat method to JsonReader and improve ExtractDouble method
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14114 ) Change subject: [util] Add ExtractFloat method to JsonReader and improve ExtractDouble method .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/14114/2/src/kudu/util/jsonreader.cc File src/kudu/util/jsonreader.cc: http://gerrit.cloudera.org:8080/#/c/14114/2/src/kudu/util/jsonreader.cc@248 PS2, Line 248: > This seems to be one of the major differences from the rapidjson implementa There are some precision issues in rapidjson implementation. Some of tests in rapidjson are: EXPECT_TRUE(Value(12.25).IsLosslessFloat()); EXPECT_FALSE(Value(0.3).IsLosslessFloat()); 0.3 is an approximation, for it is not powers of two or integer multiples thereof, there may be precision lost in conversion between double and float. The comparison 'a >= b && a <=b' seems not reasonable, for we just want to extract a float value, I think 'fabs(a-b) <= 1e-7' is better. -- To view, visit http://gerrit.cloudera.org:8080/14114 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ica0a04fb21daceaa44233995a42a60686f59361b Gerrit-Change-Number: 14114 Gerrit-PatchSet: 3 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yifan Zhang Gerrit-Comment-Date: Thu, 22 Aug 2019 08:19:49 + Gerrit-HasComments: Yes
[kudu-CR] [docs]: Fix bad marddown formatting in tablets.md
lingbi...@gmail.com has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14120 Change subject: [docs]: Fix bad marddown formatting in tablets.md .. [docs]: Fix bad marddown formatting in tablets.md Use '```' for code block Change-Id: I06e04031c8b817b3a3d7f771705c85a161cbb08d --- M docs/design-docs/tablet.md 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/14120/1 -- To view, visit http://gerrit.cloudera.org:8080/14120 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I06e04031c8b817b3a3d7f771705c85a161cbb08d Gerrit-Change-Number: 14120 Gerrit-PatchSet: 1 Gerrit-Owner: Anonymous Coward
[kudu-CR] KUDU-2921: Exposing the table statistics to spark relation.
ZhangYao has posted comments on this change. (
http://gerrit.cloudera.org:8080/14107 )
Change subject: KUDU-2921: Exposing the table statistics to spark relation.
..
Patch Set 2:
(11 comments)
> Did you consider adding the statistics to the existing
> GetTableLocations RPC response instead of creating a brand new RPC?
> Query planners and job drivers already use it to open tables, so
> it'd mean one less round trip to get the information.
Actually, I have considered to put those statistics into existing message such
as getShema which will be used when opening table. But as for the table
statistics may contain more and more entries it may be better to be a
independent rpc. What's more, because this statistics may change when client
trying to write, so I think it will be useful to support a rpc for client to
get the statistics change. Although for query plan it will only use once and
save one round trip if we put it into existing rpc but it will be heavy if we
want to get statistic change in other use cases.
> > Patch Set 1:
> >
> > Did you consider adding the statistics to the existing
> GetTableLocations RPC response instead of creating a brand new RPC?
> Query planners and job drivers already use it to open tables, so
> it'd mean one less round trip to get the information.
>
> If doing this, let's make sure to make it optional, since the
> statistics could become larger later (eg with things like
> histograms, NDVs per column, etc)
Will make it optional if do that :)
http://gerrit.cloudera.org:8080/#/c/14107/1/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java:
http://gerrit.cloudera.org:8080/#/c/14107/1/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@816
PS1, Line 816: GetTableStatisticsRequest rpc = new
GetTableStatisticsRequest(this.masterTable,
> Are you planning to expose this in the c++ client too?
Yes, will do this :) maybe next part.
http://gerrit.cloudera.org:8080/#/c/14107/1/java/kudu-client/src/main/java/org/apache/kudu/client/GetTableStatisticsRequest.java
File
java/kudu-client/src/main/java/org/apache/kudu/client/GetTableStatisticsRequest.java:
http://gerrit.cloudera.org:8080/#/c/14107/1/java/kudu-client/src/main/java/org/apache/kudu/client/GetTableStatisticsRequest.java@11
PS1, Line 11: class GetTableStatisticsRequest extends
KuduRpc {
> This can be package private.
Done
http://gerrit.cloudera.org:8080/#/c/14107/1/java/kudu-client/src/main/java/org/apache/kudu/client/GetTableStatisticsResponse.java
File
java/kudu-client/src/main/java/org/apache/kudu/client/GetTableStatisticsResponse.java:
http://gerrit.cloudera.org:8080/#/c/14107/1/java/kudu-client/src/main/java/org/apache/kudu/client/GetTableStatisticsResponse.java@8
PS1, Line 8: private final long onDiskSize;
> If you see my other comment on defining a KuduTableStatistics object, then
Done
http://gerrit.cloudera.org:8080/#/c/14107/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java:
http://gerrit.cloudera.org:8080/#/c/14107/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java@227
PS1, Line 227: public KuduTableStatistics getTableStatistics(String name)
throws KuduException {
> Does it make sense for this method to be on the KuduTable object? The api w
I have implemented this method in KuduTable. Done.
http://gerrit.cloudera.org:8080/#/c/14107/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java:
http://gerrit.cloudera.org:8080/#/c/14107/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java@811
PS1, Line 811:
> Is there a more deterministic way to ensure this happens? Using sleep often
Currently I don't find a way to trigger the heartbeat between tserver and
master from java client side(This may need new rpc to implement the control).
Here I use update_tablet_stats_interval_ms and heartbeat_interval_ms to control
the heartbeat frequency and wait enought time for the statistics to be
collected on master. If the heartbeat and statistics work correctly it supposed
to be right.
Whatever it still flaky somehow. Anyone have other advices?
http://gerrit.cloudera.org:8080/#/c/14107/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java@814
PS1, Line 814: Thread.sleep(200 * 6);
> Can you test some edge cases? Maybe call this with 0 rows, and insert rows
0 rows has been test in this test. As for test inserting row increasingly, it
can do but the statistics are suppose to lagging so maybe test the
monotonically increasing of statistics and the final accuracy :)
[kudu-CR] KUDU-2921: Exposing the table statistics to spark relation.
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo, Grant Henke, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14107 to look at the new patch set (#2). Change subject: KUDU-2921: Exposing the table statistics to spark relation. .. KUDU-2921: Exposing the table statistics to spark relation. Exposing current table statistics to spark via rpc from client to master. Change-Id: I7742a76708f989b0ccc8ba417f3390013e260175 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java A java/kudu-client/src/main/java/org/apache/kudu/client/GetTableStatisticsRequest.java A java/kudu-client/src/main/java/org/apache/kudu/client/GetTableStatisticsResponse.java M java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/KuduTable.java A java/kudu-client/src/main/java/org/apache/kudu/client/KuduTableStatistics.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/master-test.cc M src/kudu/master/master.proto M src/kudu/master/master_service.cc M src/kudu/master/master_service.h 15 files changed, 414 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/14107/2 -- To view, visit http://gerrit.cloudera.org:8080/14107 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7742a76708f989b0ccc8ba417f3390013e260175 Gerrit-Change-Number: 14107 Gerrit-PatchSet: 2 Gerrit-Owner: ZhangYao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [util] Add ExtractFloat method to JsonReader and improve ExtractDouble method
Hello Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14114 to look at the new patch set (#3). Change subject: [util] Add ExtractFloat method to JsonReader and improve ExtractDouble method .. [util] Add ExtractFloat method to JsonReader and improve ExtractDouble method ExtractDouble method should not only parse double values, int values that can be losslessly converted to double should also be parsed. Though there is already IsLosslessFloat() in rapidjson library, the implementation has some precision issues. This patch re-implement the method. Change-Id: Ica0a04fb21daceaa44233995a42a60686f59361b --- M src/kudu/tools/tool_action_table.cc M src/kudu/util/jsonreader-test.cc M src/kudu/util/jsonreader.cc M src/kudu/util/jsonreader.h 4 files changed, 89 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/14114/3 -- To view, visit http://gerrit.cloudera.org:8080/14114 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ica0a04fb21daceaa44233995a42a60686f59361b Gerrit-Change-Number: 14114 Gerrit-PatchSet: 3 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120)
