[kudu-CR] tool: new actions for adding and removing data directories

[Adar Dembo (Code Review)]

Hello Kudu Jenkins, Andrew Wong, Todd Lipcon,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8352

to look at the new patch set (#4).

Change subject: tool: new actions for adding and removing data directories
..

tool: new actions for adding and removing data directories

To add a data directory, a new FS root is created and within it, a new data
directory. Then, the set of all_uuids is updated to include the uuid of the
new data directory, and all of the on-disk instance files are updated with
the new value of all_uuids.

To remove a data directory, some sanity checks are performed, all_uuids is
updated to exclude the uuid of the data directory being removed, and all of
the on-disk instance files are updated with the new value of all_uuids.
Needless to say, removing a data directory will cause any tablet with data
in it to fail at the next startup.

In terms of the overall approach, I waffled between fully encapsulating the
new logic in FsManager/DataDirManager, and separating it out entirely.
Eventually I settled on this hybrid model where existing code in the
FsManager and DataDirManager created the roots, directories, and instance
files, while tool-specific code was responsible for updating existing
instances, fsyncing directories, and stringing it all together.

Change-Id: I6ddbdd6cc6231996e7802a622a8b4691527a0643
---
M src/kudu/fs/block_manager_util.h
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
M src/kudu/fs/fs_manager-test.cc
M src/kudu/fs/fs_manager.cc
M src/kudu/fs/fs_manager.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action.cc
M src/kudu/tools/tool_action_fs.cc
9 files changed, 785 insertions(+), 109 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/52/8352/4
--
To view, visit http://gerrit.cloudera.org:8080/8352
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6ddbdd6cc6231996e7802a622a8b4691527a0643
Gerrit-Change-Number: 8352
Gerrit-PatchSet: 4
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] fs: defer failure from metadata load to bootstrap when data dir is missing

[Adar Dembo (Code Review)]

Adar Dembo has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/8376


Change subject: fs: defer failure from metadata load to bootstrap when data dir 
is missing
..

fs: defer failure from metadata load to bootstrap when data dir is missing

An upcoming patch adds CLI tool actions to add and remove data directories.
When a data dir is removed, all tablets with data on it will fail. Today
that failure manifests as a FindOrDie in DataDirGroup::FromPB; we need to
make that a little bit more graceful.

This patch modifies the DataDirGroup FromPB/CopyToPB methods to return a
failure when a data dir is missing. It further changes TabletMetadata to
treat such failures as non-fatal, and adds checks to TabletBootstrap so that
the failures manifest there instead.

No tests in this patch because:
1. Andrew is already working on tablet-level tests for failed disks, and
2. The CLI tool patch adds coverage at the itest-level.

Change-Id: I1e8d5697c2bb08287cce11fbdab6fb8d6e37d1ad
---
M src/kudu/fs/block_manager-stress-test.cc
M src/kudu/fs/block_manager-test.cc
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/error_manager.h
M src/kudu/fs/log_block_manager-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/ts_tablet_manager.cc
11 files changed, 128 insertions(+), 70 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/8376/1
--
To view, visit http://gerrit.cloudera.org:8080/8376
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1e8d5697c2bb08287cce11fbdab6fb8d6e37d1ad
Gerrit-Change-Number: 8376
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 

[kudu-CR] tool: new actions for adding and removing data directories

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8352 )

Change subject: tool: new actions for adding and removing data directories
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8352/2/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/8352/2/src/kudu/tools/kudu-tool-test.cc@2330
PS2, Line 2330:   NO_FATALS(RunActionStdoutNone(Substitute(
> Interesting problem, hadn't thought of that..
After consideration, discussion, and some experimentation, I settled on a 
completely different approach wherein:
1. Missing data dirs are propoagated as errors.
2. If a TabletMetadata encounters an error while loading its data dir group 
from disk, that error is non-fatal.
3. Bootstrapping fails if a loaded data dir group cannot be found.



--
To view, visit http://gerrit.cloudera.org:8080/8352
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ddbdd6cc6231996e7802a622a8b4691527a0643
Gerrit-Change-Number: 8352
Gerrit-PatchSet: 3
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 25 Oct 2017 01:59:23 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2162 Expose stats about scan filters

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8375 )

Change subject: KUDU-2162 Expose stats about scan filters
..


Patch Set 1:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/8375/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8375/1//COMMIT_MSG@11
PS1, Line 11: - elapsed time processing the scan request, in nanoseconds
we should also expose a client-calculated metric which is the amount of time 
spent waiting for responses from the server -- that should give us a good sense 
of whether there were delays in the network or RPC stack.

(I wonder if we could expose the RPC's queue time as a trace metric as well, so 
we could even get that level of detail)


http://gerrit.cloudera.org:8080/#/c/8375/1/java/kudu-client/src/main/java/org/apache/kudu/client/ResourceMetrics.java
File java/kudu-client/src/main/java/org/apache/kudu/client/ResourceMetrics.java:

http://gerrit.cloudera.org:8080/#/c/8375/1/java/kudu-client/src/main/java/org/apache/kudu/client/ResourceMetrics.java@34
PS1, Line 34: ResourceMetricsPB
if this is a public API we should not include the implementation details.


http://gerrit.cloudera.org:8080/#/c/8375/1/java/kudu-client/src/main/java/org/apache/kudu/client/ResourceMetrics.java@38
PS1, Line 38: public class ResourceMetrics {
needs interface annotation


http://gerrit.cloudera.org:8080/#/c/8375/1/java/kudu-client/src/main/java/org/apache/kudu/client/ResourceMetrics.java@60
PS1, Line 60: return metrics.containsKey(name) ? metrics.get(name) : 0L;
nit: can you use get() and then check the result vs null to avoid the double 
lookup?


http://gerrit.cloudera.org:8080/#/c/8375/1/java/kudu-client/src/main/java/org/apache/kudu/client/ResourceMetrics.java@63
PS1, Line 63:   // Updates this instance's metrics with those found in 
'resourceMetricsPb'.
javadoc style
also worth specifying that this *adds* not replaces


http://gerrit.cloudera.org:8080/#/c/8375/1/java/kudu-client/src/main/java/org/apache/kudu/client/ResourceMetrics.java@77
PS1, Line 77: Long oldAmount = metrics.containsKey(name) ? 
metrics.get(name) : 0L;
same


http://gerrit.cloudera.org:8080/#/c/8375/1/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:

http://gerrit.cloudera.org:8080/#/c/8375/1/src/kudu/tserver/tablet_service.cc@1246
PS1, Line 1246:   metrics->set_elapsed_time_nanos(sw.elapsed().wall);
I think we should also include CPU time, because that can help determine if 
we're IO bound or not


http://gerrit.cloudera.org:8080/#/c/8375/1/src/kudu/tserver/tserver.proto
File src/kudu/tserver/tserver.proto:

http://gerrit.cloudera.org:8080/#/c/8375/1/src/kudu/tserver/tserver.proto@334
PS1, Line 334:   optional int64 bytes_read = 4;
doc this to be more specific on whether this includes data that was filtered 
out, etc



--
To view, visit http://gerrit.cloudera.org:8080/8375
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id30a7e82357fe2fc28f6d316378a612af43d8c96
Gerrit-Change-Number: 8375
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 25 Oct 2017 00:08:14 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2055 [part 5]: Coalesce hole punch for LBM

[Hao Hao (Code Review)]

Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8162 )

Change subject: KUDU-2055 [part 5]: Coalesce hole punch for LBM
..


Patch Set 11:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/8162/10/src/kudu/fs/log_block_manager-test.cc
File src/kudu/fs/log_block_manager-test.cc:

http://gerrit.cloudera.org:8080/#/c/8162/10/src/kudu/fs/log_block_manager-test.cc@260
PS10, Line 260: (*prototyp
> I think it'd be better to do a FindOrDie here to be more explicit about wha
It seems MetricEntity does not have FindOrDie, so dereference here.


http://gerrit.cloudera.org:8080/#/c/8162/10/src/kudu/fs/log_block_manager-test.cc@266
PS10, Line 266:int expected_value, const 
MetricPrototype* prototype) {
> likewise
Done


http://gerrit.cloudera.org:8080/#/c/8162/10/src/kudu/fs/log_block_manager-test.cc@1069
PS10, Line 1069: TEST_F(LogBlockManagerTest, TestMisalignedBlocksFuzz) {
> Are these explicit blocks necessary for correctness, or is it just to add a
Yeah, it is necessary for correctness.


http://gerrit.cloudera.org:8080/#/c/8162/10/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:

http://gerrit.cloudera.org:8080/#/c/8162/10/src/kudu/fs/log_block_manager.cc@231
PS10, Line 231: ction>& tr
> should this be committed?
As you noticed, I think using 'destructed' is more accurate.


http://gerrit.cloudera.org:8080/#/c/8162/10/src/kudu/fs/log_block_manager.cc@1244
PS10, Line 1244:
> What is the reason to do this work in the destructor instead of in CommitDe
Done



--
To view, visit http://gerrit.cloudera.org:8080/8162
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7ccc9538b8184f8f7ba8f6118713b82fef433275
Gerrit-Change-Number: 8162
Gerrit-PatchSet: 11
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Tue, 24 Oct 2017 23:05:35 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2055 [part 5]: Coalesce hole punch for LBM

[Hao Hao (Code Review)]

Hello Tidy Bot, Dan Burkert, Adar Dembo,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8162

to look at the new patch set (#11).

Change subject: KUDU-2055 [part 5]: Coalesce hole punch for LBM
..

KUDU-2055 [part 5]: Coalesce hole punch for LBM

This patch extends the implementation of BlockDeletionTransaction to
actually coalesce hole punch in LBM, so that blocks in one container
that are contiguous are grouped together in a hole punch operation.

It also adds a new metric 'holes_punched' in log block manager to track
the number of hole punching operations. And another two metrics
'blocks_created' and 'blocks_deleted' in block manager to track blocks
that were created and deleted since service start respectively.

It updates unit test LogBlockManagerTest.MetricsTest, to verify that
coalescing hole punching works as expected by checking the value of
'holes_punched' metric.

Change-Id: I7ccc9538b8184f8f7ba8f6118713b82fef433275
---
M src/kudu/fs/block_manager-test.cc
M src/kudu/fs/block_manager_metrics.cc
M src/kudu/fs/block_manager_metrics.h
M src/kudu/fs/file_block_manager.cc
M src/kudu/fs/log_block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/fs/log_block_manager.h
M src/kudu/util/metrics.cc
M src/kudu/util/metrics.h
9 files changed, 436 insertions(+), 189 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/62/8162/11
--
To view, visit http://gerrit.cloudera.org:8080/8162
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7ccc9538b8184f8f7ba8f6118713b82fef433275
Gerrit-Change-Number: 8162
Gerrit-PatchSet: 11
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Tidy Bot

[kudu-CR] KUDU-2162 Expose stats about scan filters

[Will Berkeley (Code Review)]

Will Berkeley has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/8375


Change subject: KUDU-2162 Expose stats about scan filters
..

KUDU-2162 Expose stats about scan filters

This patch adds a couple of resource metrics to scanners.
- bytes read, from disk or cache
- elapsed time processing the scan request, in nanoseconds
These metrics can be used to roughly compare the amount of work done by
scan operations, and could be useful for runtime optimizations in query
planners like Impala or Spark.

Change-Id: Id30a7e82357fe2fc28f6d316378a612af43d8c96
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanner.java
A java/kudu-client/src/main/java/org/apache/kudu/client/ResourceMetrics.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/TestScannerMultiTablet.java
M src/kudu/client/client-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tserver.proto
7 files changed, 154 insertions(+), 6 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/75/8375/1
--
To view, visit http://gerrit.cloudera.org:8080/8375
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id30a7e82357fe2fc28f6d316378a612af43d8c96
Gerrit-Change-Number: 8375
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley 

[kudu-CR] KUDU-2191 (4/n): WIP: Hive Metastore catalog manager integration

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8312 )

Change subject: KUDU-2191 (4/n): WIP: Hive Metastore catalog manager integration
..


Patch Set 3:

(11 comments)

curious about timeouts - I dont see any timeouts set here but I'm guessing 
they're necessary (what if you kill -STOP the minihms, what happens to our 
calls?)

http://gerrit.cloudera.org:8080/#/c/8312/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8312/3//COMMIT_MSG@23
PS3, Line 23: master_hms-itest before this is committed:
any plan for fault tests? eg crashing either the HMS or the master at some 
point in the middle of these sequences?


http://gerrit.cloudera.org:8080/#/c/8312/3/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/8312/3/src/kudu/master/catalog_manager.cc@231
PS3, Line 231:   "Addresses of Hive Metastore");
a bit more docs here, eg what the multiple addrs mean, etc. Also does the HMS 
expose more than one port? (eg an HTTP interface?) If so we should clarify that 
it's the Thrift addr


http://gerrit.cloudera.org:8080/#/c/8312/3/src/kudu/master/catalog_manager.cc@1419
PS3, Line 1419:   SetupError(resp->mutable_error(), 
MasterErrorPB::HIVE_METASTORE_ERROR, s);
I think it's worth logging such errors as well, since it may be an end user 
that gets the response,  and then they'll probably complain to the admin who 
will look in the logs for more details.

We should probably be especially verbose in these logs because some failures at 
this point can cause orphaned pointers in the HMS (eg a timeout may be thrown 
even if the op succeeded)

(same below)


http://gerrit.cloudera.org:8080/#/c/8312/3/src/kudu/master/hms_catalog.cc
File src/kudu/master/hms_catalog.cc:

http://gerrit.cloudera.org:8080/#/c/8312/3/src/kudu/master/hms_catalog.cc@88
PS3, Line 88:const Schema& schema) {
worth a CHECK(running_) in these functions? otherwise it seems like they would 
just hang forever


http://gerrit.cloudera.org:8080/#/c/8312/3/src/kudu/master/hms_catalog.cc@198
PS3, Line 198: "Kudu table name must contain only lower-case ASCII 
characters, "
no digits allowed? that's surprising


http://gerrit.cloudera.org:8080/#/c/8312/3/src/kudu/master/hms_catalog.cc@203
PS3, Line 203:   for (int idx = 0; idx < table.size(); idx++) {
I feel like this code could be written more concisely using some of the stuff 
from gutil/strings/util.h.

For example, you could use Split() on '.' and then ensure that it has 1 or 2 
results, and IsIdentifier() returns true for all parts. Or use 
AdvanceIdentifier up to twice? Or match the whole string against a class that 
includes '.' and then use strcount('.') to  see that there is at most one dot?

I think the 'split' is probably most straight forward


http://gerrit.cloudera.org:8080/#/c/8312/3/src/kudu/master/hms_catalog.cc@228
PS3, Line 228:   for (const HostPort& address : addresses_) {
I'd think we might need to store an index in the class so that we swap to the 
new HMS when one has an issue. Otherwise what if an HMS is "up" (ie accepts 
thrift connections) but times out all calls, or "up" but for some reason is 
unable to talk to its backing RDBMS or whatever? I'd guess we'd want to 
reconnect to the other one.


http://gerrit.cloudera.org:8080/#/c/8312/3/src/kudu/master/hms_catalog.cc@289
PS3, Line 289: cond_.TimedWait(MonoDelta::FromMilliseconds(wait));
worth some logging here?


http://gerrit.cloudera.org:8080/#/c/8312/3/src/kudu/master/hms_catalog.cc@304
PS3, Line 304:   Status s = rpc.run(_.get());
I think wrapping this  in a TRACE_EVENT scope and a   LOG_IF_SLOW and such 
would  be helpful. Perhaps a TRACE message and/or a   TRACE_COUNTER_INCREMENT 
too to keep track of how  much time is spent in calls to HMS for a given 
CreateTable call. That will also require propagating the current Trace object 
into the 'Rpc' object.

If you'd rather defer this work please add a TODO. I'm happy to do it if you 
want to assign me the todo


http://gerrit.cloudera.org:8080/#/c/8312/3/src/kudu/master/hms_catalog.cc@308
PS3, Line 308: client_ = boost::none;
is  there a more explicit Shutdown we should do on the client? or does calling 
the dtor like this suffice?


http://gerrit.cloudera.org:8080/#/c/8312/3/src/kudu/util/net/net_util.h
File src/kudu/util/net/net_util.h:

http://gerrit.cloudera.org:8080/#/c/8312/3/src/kudu/util/net/net_util.h@133
PS3, Line 133: bool ValidateAddressListFlag(const char* flag_name, const 
std::string& addr_list);
doc this? specifically I think this is meant to be used as a gflag VALIDATOR 
and thus the somewhat odd signature



--
To view, visit http://gerrit.cloudera.org:8080/8312
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment

[kudu-CR] KUDU-2198. Allow disregarding system-wide auth-to-local mapping

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/8373 )

Change subject: KUDU-2198. Allow disregarding system-wide auth-to-local mapping
..

KUDU-2198. Allow disregarding system-wide auth-to-local mapping

This adds a workaround for an issue reported on the user mailing list.
Some systems are configured such that the auth_to_local mapping provided
by the krb5 library doesn't work properly for service accounts.

This patch adds a new configuration which allows Kudu to disregard the
system auth_to_local rules and instead just map kerberos principals to
their first component, which is typically the username.

Change-Id: I2e893493f52965ea54d2ceaac83d375285b49486
Reviewed-on: http://gerrit.cloudera.org:8080/8373
Reviewed-by: Alexey Serbin 
Reviewed-by: Dan Burkert 
Tested-by: Kudu Jenkins
---
M src/kudu/security/init.cc
1 file changed, 24 insertions(+), 11 deletions(-)

Approvals:
  Alexey Serbin: Looks good to me, approved
  Dan Burkert: Looks good to me, approved
  Kudu Jenkins: Verified

--
To view, visit http://gerrit.cloudera.org:8080/8373
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I2e893493f52965ea54d2ceaac83d375285b49486
Gerrit-Change-Number: 8373
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2198. Allow disregarding system-wide auth-to-local mapping

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8373 )

Change subject: KUDU-2198. Allow disregarding system-wide auth-to-local mapping
..


Patch Set 2: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/8373
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e893493f52965ea54d2ceaac83d375285b49486
Gerrit-Change-Number: 8373
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 24 Oct 2017 20:21:19 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2198. Allow disregarding system-wide auth-to-local mapping

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8373 )

Change subject: KUDU-2198. Allow disregarding system-wide auth-to-local mapping
..


Patch Set 2: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/8373
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e893493f52965ea54d2ceaac83d375285b49486
Gerrit-Change-Number: 8373
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 24 Oct 2017 20:18:10 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2198. Allow disregarding system-wide auth-to-local mapping

[Todd Lipcon (Code Review)]

Hello Alexey Serbin, Dan Burkert, Kudu Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8373

to look at the new patch set (#2).

Change subject: KUDU-2198. Allow disregarding system-wide auth-to-local mapping
..

KUDU-2198. Allow disregarding system-wide auth-to-local mapping

This adds a workaround for an issue reported on the user mailing list.
Some systems are configured such that the auth_to_local mapping provided
by the krb5 library doesn't work properly for service accounts.

This patch adds a new configuration which allows Kudu to disregard the
system auth_to_local rules and instead just map kerberos principals to
their first component, which is typically the username.

Change-Id: I2e893493f52965ea54d2ceaac83d375285b49486
---
M src/kudu/security/init.cc
1 file changed, 24 insertions(+), 11 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/73/8373/2
--
To view, visit http://gerrit.cloudera.org:8080/8373
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2e893493f52965ea54d2ceaac83d375285b49486
Gerrit-Change-Number: 8373
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2198. Allow disregarding system-wide auth-to-local mapping

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8373 )

Change subject: KUDU-2198. Allow disregarding system-wide auth-to-local mapping
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8373/1/src/kudu/security/init.cc
File src/kudu/security/init.cc:

http://gerrit.cloudera.org:8080/#/c/8373/1/src/kudu/security/init.cc@75
PS1, Line 75: #define DEFAULT_SYSTEM_AUTH_TO_LOCAL true
> consider using a constant here, e.g. https://github.com/apache/kudu/blob/ma
Done


http://gerrit.cloudera.org:8080/#/c/8373/1/src/kudu/security/init.cc@84
PS1, Line 84: componnt
> component
Done



--
To view, visit http://gerrit.cloudera.org:8080/8373
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e893493f52965ea54d2ceaac83d375285b49486
Gerrit-Change-Number: 8373
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 24 Oct 2017 20:06:11 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2198. Allow disregarding system-wide auth-to-local mapping

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8373 )

Change subject: KUDU-2198. Allow disregarding system-wide auth-to-local mapping
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8373/1/src/kudu/security/init.cc
File src/kudu/security/init.cc:

http://gerrit.cloudera.org:8080/#/c/8373/1/src/kudu/security/init.cc@75
PS1, Line 75: #define DEFAULT_SYSTEM_AUTH_TO_LOCAL true
consider using a constant here, e.g. 
https://github.com/apache/kudu/blob/master/src/kudu/mini-cluster/mini_cluster.h#L93-L97.
  I don't feel too strongly if you want to keep it as-is, though.


http://gerrit.cloudera.org:8080/#/c/8373/1/src/kudu/security/init.cc@84
PS1, Line 84: componnt
component



--
To view, visit http://gerrit.cloudera.org:8080/8373
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e893493f52965ea54d2ceaac83d375285b49486
Gerrit-Change-Number: 8373
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 24 Oct 2017 19:54:23 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2198. Allow disregarding system-wide auth-to-local mapping

[Todd Lipcon (Code Review)]

Hello Alexey Serbin, Dan Burkert,

I'd like you to do a code review. Please visit

http://gerrit.cloudera.org:8080/8373

to review the following change.


Change subject: KUDU-2198. Allow disregarding system-wide auth-to-local mapping
..

KUDU-2198. Allow disregarding system-wide auth-to-local mapping

This adds a workaround for an issue reported on the user mailing list.
Some systems are configured such that the auth_to_local mapping provided
by the krb5 library doesn't work properly for service accounts.

This patch adds a new configuration which allows Kudu to disregard the
system auth_to_local rules and instead just map kerberos principals to
their first component, which is typically the username.

Change-Id: I2e893493f52965ea54d2ceaac83d375285b49486
---
M src/kudu/security/init.cc
1 file changed, 24 insertions(+), 11 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/73/8373/1
--
To view, visit http://gerrit.cloudera.org:8080/8373
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2e893493f52965ea54d2ceaac83d375285b49486
Gerrit-Change-Number: 8373
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 

[kudu-CR] consensus: support changing between VOTER and NON VOTER

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/8297 )

Change subject: consensus: support changing between VOTER and NON_VOTER
..

consensus: support changing between VOTER and NON_VOTER

This patch implements changing from VOTER to NON_VOTER and vice-versa.

Added an integration test scenario for NON_VOTER --> VOTER,
VOTER --> NON_VOTER changes and other use cases.

Change-Id: I978cbed683b0e95a903f2bd2b57496aee71cb33a
Reviewed-on: http://gerrit.cloudera.org:8080/8297
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
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/consensus/raft_consensus.cc
M src/kudu/consensus/raft_consensus.h
M src/kudu/integration-tests/cluster_itest_util.cc
M src/kudu/integration-tests/cluster_itest_util.h
M src/kudu/integration-tests/raft_consensus_nonvoter-itest.cc
8 files changed, 345 insertions(+), 14 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Todd Lipcon: Looks good to me, approved

--
To view, visit http://gerrit.cloudera.org:8080/8297
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I978cbed683b0e95a903f2bd2b57496aee71cb33a
Gerrit-Change-Number: 8297
Gerrit-PatchSet: 6
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] consensus: support changing between VOTER and NON VOTER

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8297 )

Change subject: consensus: support changing between VOTER and NON_VOTER
..


Patch Set 5: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/8297
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I978cbed683b0e95a903f2bd2b57496aee71cb33a
Gerrit-Change-Number: 8297
Gerrit-PatchSet: 5
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 24 Oct 2017 18:16:32 +
Gerrit-HasComments: No

[kudu-CR] [webui] Add templates for tserver webui

[Will Berkeley (Code Review)]

Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8307 )

Change subject: [webui] Add templates for tserver webui
..


Patch Set 6:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/8307/6/src/kudu/tserver/tserver_path_handlers.h
File src/kudu/tserver/tserver_path_handlers.h:

http://gerrit.cloudera.org:8080/#/c/8307/6/src/kudu/tserver/tserver_path_handlers.h@27
PS6, Line 27: template  class scoped_refptr;
> warning: invalid case style for class 'scoped_refptr' [readability-identifi
Not my fault, tidy bot.


http://gerrit.cloudera.org:8080/#/c/8307/6/src/kudu/tserver/tserver_path_handlers.cc
File src/kudu/tserver/tserver_path_handlers.cc:

http://gerrit.cloudera.org:8080/#/c/8307/6/src/kudu/tserver/tserver_path_handlers.cc@a156
PS6, Line 156:
> so did we lose the ?raw ability here? I actually use this pretty often with
We did, because I wasn't sure anyone used it. Since someone does, I'll keep it.


http://gerrit.cloudera.org:8080/#/c/8307/6/src/kudu/tserver/tserver_path_handlers.cc@169
PS6, Line 169: transaction_json["trace"] = inflight_tx.trace_buffer();
> Perhaps we should be std::move()ing the large strings out of the PB into th
Done


http://gerrit.cloudera.org:8080/#/c/8307/6/src/kudu/tserver/tserver_path_handlers.cc@210
PS6, Line 210: Subst
> std::to_string
Done


http://gerrit.cloudera.org:8080/#/c/8307/6/src/kudu/tserver/tserver_path_handlers.cc@220
PS6, Line 220: percentage
> do you want to be using StringPrintf here to get a nice printout? it seems
Done


http://gerrit.cloudera.org:8080/#/c/8307/6/src/kudu/tserver/tserver_path_handlers.cc@225
PS6, Line 225: kArray
> I'm surprised this isn't kObject.. how does this work?
Yeah, that's a mistake. I think it works because EasyJson replaces the array 
value with an object value because I'm doing object-y things to it.


http://gerrit.cloudera.org:8080/#/c/8307/6/src/kudu/tserver/tserver_path_handlers.cc@235
PS6, Line 235:  if (replica->tablet() != nullptr) {
 :   tablet_detail_json["target"] = Substitute("/tablet?id=$0", 
status.tablet_id());
 :   tablet_detail_json["mem_bytes"] = 
HumanReadableNumBytes::ToString(
 :   replica->tablet()
> isn't there a race here where tablet() can become null in between, if it's
Done


http://gerrit.cloudera.org:8080/#/c/8307/6/src/kudu/tserver/tserver_path_handlers.cc@413
PS6, Line 413: Substitute("$0",
> std::to_string() here and a few other spots
Done



--
To view, visit http://gerrit.cloudera.org:8080/8307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99e08be9aa8abddd51ada61683b6f75190a00b5c
Gerrit-Change-Number: 8307
Gerrit-PatchSet: 6
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Tue, 24 Oct 2017 17:56:32 +
Gerrit-HasComments: Yes

[kudu-CR] [webui] Add templates for tserver webui

[Will Berkeley (Code Review)]

Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Todd Lipcon,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8307

to look at the new patch set (#7).

Change subject: [webui] Add templates for tserver webui
..

[webui] Add templates for tserver webui

This converts all the tablet server web ui endpoints to use a template,
unless it doesn't make sense to. This includes
- /scans
- /tablets
- /tablet
- /transactions
- /tablet-rowsetlayout-svg
- /tablet-consensus-status
- /log-anchors
- /dashboards

Change-Id: I99e08be9aa8abddd51ada61683b6f75190a00b5c
---
M src/kudu/server/webserver.cc
M src/kudu/server/webui_util.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tserver_path_handlers.cc
M src/kudu/tserver/tserver_path_handlers.h
A www/dashboards.mustache
A www/log-anchors.mustache
A www/scans.mustache
M www/table.mustache
A www/tablet-consensus-status.mustache
A www/tablet-rowsetlayout-svg.mustache
A www/tablet.mustache
A www/tablets.mustache
A www/transactions.mustache
14 files changed, 602 insertions(+), 384 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/8307/7
--
To view, visit http://gerrit.cloudera.org:8080/8307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I99e08be9aa8abddd51ada61683b6f75190a00b5c
Gerrit-Change-Number: 8307
Gerrit-PatchSet: 7
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] KUDU-2191 (3/n): Add mini HMS option to external mini cluster

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8304 )

Change subject: KUDU-2191 (3/n): Add mini HMS option to external mini cluster
..


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8304/6/src/kudu/mini-cluster/external_mini_cluster.h
File src/kudu/mini-cluster/external_mini_cluster.h:

http://gerrit.cloudera.org:8080/#/c/8304/6/src/kudu/mini-cluster/external_mini_cluster.h@138
PS6, Line 138:   // If true, set up a Hive Metastore as part of this 
ExternalMiniCluster.
> It's not so simple due to cases like data_root, block_manager_type, or daem
sure, that makes sense. but for the ones that are simple bools like this it 
seems like it might be more readable.



--
To view, visit http://gerrit.cloudera.org:8080/8304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaa1da05b13a120f2d4b739b6ab709e6315c955c0
Gerrit-Change-Number: 8304
Gerrit-PatchSet: 6
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 24 Oct 2017 17:43:20 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2184. arena: fix max buffer size to just less than 1MB

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/8324 )

Change subject: KUDU-2184. arena: fix max buffer size to just less than 1MB
..

KUDU-2184. arena: fix max buffer size to just less than 1MB

eb5c7e2f0c73729a4486bc71512739b0788b1574 changed the max buffer size for
arenas to 1MB to try to sync up with the max size of an exact free-list
in tcmalloc. However it turns out that tcmalloc's max size class is
sized at 8kb less than 1MB rather than exactly at 1MB.

So, this patch adjusts the arena buffer size to match.

I verified under a workload that the memz stats on a tablet server now
show heavy usage of the 127-page size class whereas before they did not.

Change-Id: I6549fefe8cd82e4e890aed339fea34a227f94a5b
Reviewed-on: http://gerrit.cloudera.org:8080/8324
Tested-by: Kudu Jenkins
Reviewed-by: David Ribeiro Alves 
---
M src/kudu/util/memory/arena.cc
M src/kudu/util/memory/arena.h
2 files changed, 15 insertions(+), 5 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  David Ribeiro Alves: Looks good to me, approved

--
To view, visit http://gerrit.cloudera.org:8080/8324
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I6549fefe8cd82e4e890aed339fea34a227f94a5b
Gerrit-Change-Number: 8324
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2184. arena: fix max buffer size to just less than 1MB

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8324 )

Change subject: KUDU-2184. arena: fix max buffer size to just less than 1MB
..


Patch Set 1: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/8324
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6549fefe8cd82e4e890aed339fea34a227f94a5b
Gerrit-Change-Number: 8324
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 24 Oct 2017 17:22:40 +
Gerrit-HasComments: No

[kudu-CR] consensus: support changing between VOTER and NON VOTER

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8297 )

Change subject: consensus: support changing between VOTER and NON_VOTER
..


Patch Set 4:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/8297/4/src/kudu/consensus/raft_consensus.h
File src/kudu/consensus/raft_consensus.h:

http://gerrit.cloudera.org:8080/#/c/8297/4/src/kudu/consensus/raft_consensus.h@569
PS4, Line 569:  boost::optional delta = 
boost::none);
> can you explain what 'delta' is here? I know it's just passing through  to
Done


http://gerrit.cloudera.org:8080/#/c/8297/4/src/kudu/consensus/raft_consensus.cc
File src/kudu/consensus/raft_consensus.cc:

http://gerrit.cloudera.org:8080/#/c/8297/4/src/kudu/consensus/raft_consensus.cc@2424
PS4, Line 2424:   ToggleFailureDetector(cmeta_->CommittedConfig());
> can't you remove the config param from  the ToggleFailureDetector function
Indeed -- that's a good observation, thanks!


http://gerrit.cloudera.org:8080/#/c/8297/4/src/kudu/consensus/raft_consensus.cc@2622
PS4, Line 2622:   ToggleFailureDetector(new_config);
> same here, we just set the pending config, so if the function just uses Act
Done


http://gerrit.cloudera.org:8080/#/c/8297/4/src/kudu/integration-tests/raft_consensus_nonvoter-itest.cc
File src/kudu/integration-tests/raft_consensus_nonvoter-itest.cc:

http://gerrit.cloudera.org:8080/#/c/8297/4/src/kudu/integration-tests/raft_consensus_nonvoter-itest.cc@103
PS4, Line 103:   // Promote non-voter replica to voter one.
 :   Status PromoteReplica(const string& tablet_id,
 : const TServerDetails* replica,
 : const MonoDelta& timeout) {
 : return ChangeReplicaMembership(tablet_id, replica, 
RaftPeerPB::VOTER, timeout);
 :   }
 :
 :   // Demote voter replica to non-voter one.
 :   Status DemoteReplica(const string& tablet_id,
 :const TServerDetails* replica,
 :const MonoDelta& timeout) {
 : return ChangeReplicaMembership(tablet_id, replica, 
RaftPeerPB::NON_VOTER, timeout);
 :   }
> I'd go  one further and just inline these ChangeReplicaMembership calls dow
Done



--
To view, visit http://gerrit.cloudera.org:8080/8297
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I978cbed683b0e95a903f2bd2b57496aee71cb33a
Gerrit-Change-Number: 8297
Gerrit-PatchSet: 4
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 24 Oct 2017 06:43:25 +
Gerrit-HasComments: Yes

[kudu-CR] consensus: support changing between VOTER and NON VOTER

[Alexey Serbin (Code Review)]

Alexey Serbin has uploaded a new patch set (#5) to the change originally 
created by Mike Percy. ( http://gerrit.cloudera.org:8080/8297 )

Change subject: consensus: support changing between VOTER and NON_VOTER
..

consensus: support changing between VOTER and NON_VOTER

This patch implements changing from VOTER to NON_VOTER and vice-versa.

Added an integration test scenario for NON_VOTER --> VOTER,
VOTER --> NON_VOTER changes and other use cases.

Change-Id: I978cbed683b0e95a903f2bd2b57496aee71cb33a
---
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/consensus/raft_consensus.cc
M src/kudu/consensus/raft_consensus.h
M src/kudu/integration-tests/cluster_itest_util.cc
M src/kudu/integration-tests/cluster_itest_util.h
M src/kudu/integration-tests/raft_consensus_nonvoter-itest.cc
8 files changed, 345 insertions(+), 14 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/97/8297/5
--
To view, visit http://gerrit.cloudera.org:8080/8297
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I978cbed683b0e95a903f2bd2b57496aee71cb33a
Gerrit-Change-Number: 8297
Gerrit-PatchSet: 5
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon