[kudu-CR] WIP: threadpool: token-based sequencing

[Adar Dembo (Code Review)]

Hello Todd Lipcon,

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

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

to review the following change.

Change subject: WIP: threadpool: token-based sequencing
..

WIP: threadpool: token-based sequencing

Here's a really rough implementation of token-based sequencing for
util/threadpool. The idea is to allow multiple contexts to share a single
threadpool with a high number of threads while also ensuring that the pool
executes tasks belonging to each context in order. Previously this was only
achievable via one-singleton-threadpool-per-context, which grossly inflates
the total number of threads.

WIP because it's incomplete, is poorly documented, needs a lot more tests,
is suboptimal from an allocation perspective (too much map access?) and is
probably outright broken.

Change-Id: If46dc34212027b6ea5dbc2ead7c7af8be57f2c70
---
M src/kudu/util/threadpool-test.cc
M src/kudu/util/threadpool.cc
M src/kudu/util/threadpool.h
3 files changed, 158 insertions(+), 19 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: If46dc34212027b6ea5dbc2ead7c7af8be57f2c70
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2001 Add UNDO size to tablet on-disk size

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: KUDU-2001 Add UNDO size to tablet on-disk size
..


Patch Set 4: -Code-Review

Huh. It occurs to me that this needs tests. :)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: No

[kudu-CR] Refactor ConsensusStatePB to hold committed and pending configs

[Will Berkeley (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Refactor ConsensusStatePB to hold committed and pending configs
..

Refactor ConsensusStatePB to hold committed and pending configs

This patch refactors ConsensusStatePB so it contains both the
committed config and, if there is a pending config, the pending
config. Previously, consumers of the consensus state would ask
for either the committed or active (= pending if there is a pending
config, else the committed config) config. With this change, the
committed and active configs are always included.

The addition of pending info to the ConsensusStatePB will be used
by ksck in a follow-up patch. Additionally, the master may use
this info in the future to be more aware of the health of tablets.

Change-Id: I4bc4bdd9752fc29a7ce2cefcdc95c4366f5353af
---
M src/kudu/consensus/consensus.h
M src/kudu/consensus/consensus.proto
M src/kudu/consensus/consensus_meta-test.cc
M src/kudu/consensus/consensus_meta.cc
M src/kudu/consensus/consensus_meta.h
M src/kudu/consensus/metadata.proto
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/consensus/raft_consensus_state.cc
M src/kudu/consensus/raft_consensus_state.h
M src/kudu/integration-tests/cluster_itest_util.cc
M src/kudu/integration-tests/cluster_itest_util.h
M src/kudu/integration-tests/delete_table-itest.cc
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/integration-tests/ts_tablet_manager-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master-path-handlers.cc
M src/kudu/master/master.proto
M src/kudu/master/sys_catalog.cc
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tserver/tablet_copy.proto
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_client.h
M src/kudu/tserver/tablet_copy_service.cc
M src/kudu/tserver/tablet_copy_source_session.cc
M src/kudu/tserver/tablet_copy_source_session.h
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/tserver-path-handlers.cc
34 files changed, 331 insertions(+), 302 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/09/6809/10
-- 
To view, visit http://gerrit.cloudera.org:8080/6809
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4bc4bdd9752fc29a7ce2cefcdc95c4366f5353af
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] KUDU-2001 Add UNDO size to tablet on-disk size

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: KUDU-2001 Add UNDO size to tablet on-disk size
..


Patch Set 4: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: No

[kudu-CR] KUDU-2001 Add UNDO size to tablet on-disk size

[Will Berkeley (Code Review)]

Will Berkeley has posted comments on this change.

Change subject: KUDU-2001 Add UNDO size to tablet on-disk size
..


Patch Set 3:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/6850/3/src/kudu/tablet/delta_tracker.h
File src/kudu/tablet/delta_tracker.h:

PS3, Line 219: .
> of all delta blocks.
Done


PS3, Line 222: eligible for compaction.
 :   // This estimate includes REDO deltas, but not UNDO deltas
> of REDO deltas
Done


PS3, Line 224: EstimateOnDiskSizeForCompaction
> Rename to EstimateRedoDeltaOnDiskSize()
Done


http://gerrit.cloudera.org:8080/#/c/6850/3/src/kudu/tablet/diskrowset.h
File src/kudu/tablet/diskrowset.h:

PS3, Line 336: EstimateDeltaDiskSizeForCompaction
> I think a better name would be EstimateRedoDeltaDiskSize()
Done


Line 339:   // TODO(wdberkeley) Offer a version that has the real total disk 
space usage.
> Add: See KUDU-1755.
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2001 Add UNDO size to tablet on-disk size

[Will Berkeley (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-2001 Add UNDO size to tablet on-disk size
..

KUDU-2001 Add UNDO size to tablet on-disk size

To make the on-disk size metric more accurate, this
patch makes DeltaTracker::EstimateOnDiskSize() count
UNDO deltas and adds a new method
DeltaTracker::EstimateOnDiskSizeForCompaction() that
retains the original behavior, which only counts
REDOs and is used for compaction scoring.

Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
---
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/delta_tracker.h
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/diskrowset.h
M src/kudu/tablet/tablet.cc
5 files changed, 23 insertions(+), 10 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] Refactor ConsensusStatePB to hold committed and pending configs

[Will Berkeley (Code Review)]

Will Berkeley has posted comments on this change.

Change subject: Refactor ConsensusStatePB to hold committed and pending configs
..


Patch Set 6:

(7 comments)

Also hit all the comments that somehow got missed from PS6.

http://gerrit.cloudera.org:8080/#/c/6809/6/src/kudu/consensus/metadata.proto
File src/kudu/consensus/metadata.proto:

PS6, Line 103: There is a corner case in Raft where a node may be elected 
leader of a
 :   // pending (uncommitted) configuration. In such a case, the 
master will
 :   // detect that the node is not a member of the committed 
configuration.
> Not done yet in rev 8
Sorry. Done.


PS6, Line 113: 5
> Not done in rev 8
Sorry. Done.


http://gerrit.cloudera.org:8080/#/c/6809/8/src/kudu/consensus/quorum_util.cc
File src/kudu/consensus/quorum_util.cc:

PS8, Line 197:  return Status::IllegalState(
 :   Substitute("Leader with UUID $0 is not a VOTER in the 
committed or pending config! "
 :"Consensus state: $1",
> I actually don't think it's possible to hit this in the current production 
Done. Omitting redundant checks for required fields and the leader check, the 
only thing that remains is to check that the committed_config is valid and that 
we're not storing a pending_config.


Line 219: string DiffConsensusStates(const ConsensusStatePB& old_state,
> nit: don't indent inside namespace
Done


Line 221:   bool leader_changed = old_state.leader_uuid() != 
new_state.leader_uuid();
> Add a comment about the structure of this map (key and value contents?)
Done


Line 304: if (SecureShortDebugString(old_state) == 
SecureShortDebugString(new_state)) {
> nit: 4 line indentation for a continued line per https://google.github.io/s
Done


PS8, Line 332: 
> gained/lost is a bit strange terminology here, since it sounds sort of like
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4bc4bdd9752fc29a7ce2cefcdc95c4366f5353af
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes

[kudu-CR] Refactor ConsensusStatePB to hold committed and pending configs

[Will Berkeley (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Refactor ConsensusStatePB to hold committed and pending configs
..

Refactor ConsensusStatePB to hold committed and pending configs

This patch refactors ConsensusStatePB so it contains both the
committed config and, if there is a pending config, the pending
config. Previously, consumers of the consensus state would ask
for either the committed or active (= pending if there is a pending
config, else the committed config) config. With this change, the
committed and active configs are always included.

The addition of pending info to the ConsensusStatePB will be used
by ksck in a follow-up patch. Additionally, the master may use
this info in the future to be more aware of the health of tablets.

Change-Id: I4bc4bdd9752fc29a7ce2cefcdc95c4366f5353af
---
M src/kudu/consensus/consensus.h
M src/kudu/consensus/consensus.proto
M src/kudu/consensus/consensus_meta-test.cc
M src/kudu/consensus/consensus_meta.cc
M src/kudu/consensus/consensus_meta.h
M src/kudu/consensus/metadata.proto
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/consensus/raft_consensus_state.cc
M src/kudu/consensus/raft_consensus_state.h
M src/kudu/integration-tests/cluster_itest_util.cc
M src/kudu/integration-tests/cluster_itest_util.h
M src/kudu/integration-tests/delete_table-itest.cc
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/integration-tests/ts_tablet_manager-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master-path-handlers.cc
M src/kudu/master/master.proto
M src/kudu/master/sys_catalog.cc
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tserver/tablet_copy.proto
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_client.h
M src/kudu/tserver/tablet_copy_service.cc
M src/kudu/tserver/tablet_copy_source_session.cc
M src/kudu/tserver/tablet_copy_source_session.h
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/tserver-path-handlers.cc
34 files changed, 331 insertions(+), 300 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/09/6809/9
-- 
To view, visit http://gerrit.cloudera.org:8080/6809
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4bc4bdd9752fc29a7ce2cefcdc95c4366f5353af
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] KUDU-2001 Add UNDO size to tablet on-disk size

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: KUDU-2001 Add UNDO size to tablet on-disk size
..


Patch Set 3:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/6850/3/src/kudu/tablet/delta_tracker.h
File src/kudu/tablet/delta_tracker.h:

PS3, Line 219: .
of all delta blocks.


PS3, Line 222: eligible for compaction.
 :   // This estimate includes REDO deltas, but not UNDO deltas
of REDO deltas


PS3, Line 224: EstimateOnDiskSizeForCompaction
Rename to EstimateRedoDeltaOnDiskSize()


http://gerrit.cloudera.org:8080/#/c/6850/3/src/kudu/tablet/diskrowset.h
File src/kudu/tablet/diskrowset.h:

PS3, Line 336: EstimateDeltaDiskSizeForCompaction
I think a better name would be EstimateRedoDeltaDiskSize()


Line 339:   // TODO(wdberkeley) Offer a version that has the real total disk 
space usage.
Add: See KUDU-1755.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes

[kudu-CR] Refactor ConsensusStatePB to hold committed and pending configs

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: Refactor ConsensusStatePB to hold committed and pending configs
..


Patch Set 8:

(4 comments)

I only took a quick look at rev 8; there are a bunch of review comments missed 
in rev 8 marked as done in rev 6 so I think a rebase got messed up or something.

http://gerrit.cloudera.org:8080/#/c/6809/6/src/kudu/consensus/metadata.proto
File src/kudu/consensus/metadata.proto:

PS6, Line 103: There is a corner case in Raft where a node may be elected 
leader of a
 :   // pending (uncommitted) configuration. In such a case, the 
master will
 :   // detect that the node is not a member of the committed 
configuration.
> Done
Not done yet in rev 8


PS6, Line 113: 5
> Done
Not done in rev 8


http://gerrit.cloudera.org:8080/#/c/6809/8/src/kudu/consensus/quorum_util.cc
File src/kudu/consensus/quorum_util.cc:

PS8, Line 197:  return Status::IllegalState(
 :   Substitute("Leader with UUID $0 is not a VOTER in the 
committed or pending config! "
 :"Consensus state: $1",
> hrm, isn't this possible? eg when catching up a node that is more than one 
I actually don't think it's possible to hit this in the current production code 
because we run this right after the consensus meta is loaded from disk. However 
I talked to Will about this offline and suggested he define a different 
function to call from sys_catalog.cc and then we can leave this one for use by 
tests.


Line 304:   Substitute("term changed from $0 to $1",
nit: 4 line indentation for a continued line per 
https://google.github.io/styleguide/cppguide.html#Function_Calls


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4bc4bdd9752fc29a7ce2cefcdc95c4366f5353af
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes

[kudu-CR] Bump glog to 0.3.5

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: Bump glog to 0.3.5
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9f638970abfd1aa214cf0032b46064ff7edf4dc4
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] [webui] Improvements for when there's many tablets & cleanup

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: [webui] Improvements for when there's many tablets & cleanup
..


Patch Set 1:

(4 comments)

Do the pages look OK and load reasonably quickly when the number of tablets is 
high (say, 1000+)?

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

Line 17: 4. Detailed tablet tables use table-hover so it's easier to
5.


http://gerrit.cloudera.org:8080/#/c/6870/1/src/kudu/master/master-path-handlers.cc
File src/kudu/master/master-path-handlers.cc:

Line 256: TabletMetadataLock l(tablet.get(), TabletMetadataLock::READ);
Would it be possible to combine all of this with the work in L278+ to avoid 
locking the tablets twice?


http://gerrit.cloudera.org:8080/#/c/6870/1/src/kudu/server/webserver.cc
File src/kudu/server/webserver.cc:

Line 424: " "
I noticed we include this already in www/metrics.html. Should we remove it from 
there? Or does the page header not get applied to that page?


http://gerrit.cloudera.org:8080/#/c/6870/1/src/kudu/tserver/tserver-path-handlers.cc
File src/kudu/tserver/tserver-path-handlers.cc:

PS1, Line 243: replica->tablet()->mem_tracker()
Can this actually be null?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic3904a4b0fbb6446615cd46c8a6f30f81c832c53
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1952 Remove round-robin for block placement

[Andrew Wong (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1952 Remove round-robin for block placement
..

KUDU-1952 Remove round-robin for block placement

This is the first of a multi-patch patchset to mitigate the
single-disk failure. Throughout the code, the term "DataDir" refers to
a data directory, which is often mounted on a distinct disk. Thus,
"disks" and "data directories" will be used interchangeably.

This patch adds a mapping from tablet to a set of disks and uses it to
replace the existing round-robin placement of blocks. Tablets are
mapped to a fixed number of disks (i.e. a DataDirGroup). New blocks
are placed randomly in directories within each tablet's DataDirGroup.

Tablet-to-group mappings are generated and stored as metadata upon
tablet creation, or upon tablet replacement during a tablet copy.
During group creation, disks are added to groups by randomly selecting
two available directories and selecting the one with fewer tablets on
it. This avoids pigeonholing new tablets to disks with relatively few
tablets, while still trending towards filling underloaded disks.

Groups are maintained when restarting the server, as they are flushed
with metadata, and are deleted upon tablet deletion.  When loading
tablet data from a previous version of Kudu, the tablet's superblock
will not have a DataDirGroup. One will be generated containing all
data directories, as the tablet's data may already be spread across
any number of disks.

As this patch only addresses block placement, it does not itself
mitigate single-disk failure. Given this, and given the tradeoff
between I/O and failure disk-failure tolerance, the default behavior
will be to spread tablet data across all disks.

Testing is done at the block manager level in block_manager-test and
log_block_manager-test, as well as in the new data_dirs-test.

A design doc can be found here:
https://docs.google.com/document/d/1zZk-vb_ETKUuePcZ9ZqoSK2oPvAAaEV1sjDXes8Pxgk/edit?usp=sharing

Change-Id: I9828147f4fa5c4d7f6ed23441dca5a116b8cb11b
---
M src/kudu/cfile/bloomfile-test-base.h
M src/kudu/cfile/cfile-test-base.h
M src/kudu/cfile/cfile-test.cc
M src/kudu/fs/CMakeLists.txt
M src/kudu/fs/block_manager-stress-test.cc
M src/kudu/fs/block_manager-test.cc
M src/kudu/fs/block_manager.h
A 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/file_block_manager.h
M src/kudu/fs/fs.proto
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/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/tablet/delta_compaction-test.cc
M src/kudu/tablet/delta_compaction.cc
M src/kudu/tablet/delta_compaction.h
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/deltafile-test.cc
M src/kudu/tablet/deltamemstore-test.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/multi_column_writer.cc
M src/kudu/tablet/multi_column_writer.h
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tablet/tablet_metadata.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_client.h
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
38 files changed, 986 insertions(+), 190 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/6636/24
-- 
To view, visit http://gerrit.cloudera.org:8080/6636
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9828147f4fa5c4d7f6ed23441dca5a116b8cb11b
Gerrit-PatchSet: 24
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] cmake: fix protoc dependency typo

[Dan Burkert (Code Review)]

Dan Burkert has submitted this change and it was merged.

Change subject: cmake: fix protoc dependency typo
..


cmake: fix protoc dependency typo

The effect of this typo was that protoc would always be statically
linked. Since it's only a build-time dependency, we never noticed
previously.

Change-Id: Ide6f69332527f56347b2ee3e3fc6e5b4bb543fa3
Reviewed-on: http://gerrit.cloudera.org:8080/6869
Reviewed-by: Todd Lipcon 
Tested-by: Kudu Jenkins
---
M CMakeLists.txt
1 file changed, 1 insertion(+), 1 deletion(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ide6f69332527f56347b2ee3e3fc6e5b4bb543fa3
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR](branch-1.3.x) [util] updated output from TryRunLsof()

[Alexey Serbin (Code Review)]

Alexey Serbin has submitted this change and it was merged.

Change subject: [util] updated output from TryRunLsof()
..


[util] updated output from TryRunLsof()

TryRunLsof() should not conclude it's called because of an error
binding to the specified address: in some use-cases it might not be the
case.  The output is updated to remove the statement about a failure to
bind to the specified address, leaving just the output from lsof.

This is a pure cosmetic fix to improve error message readability
in the context of KUDU-2005.

Change-Id: I1ff0d28f631c7f392420040b132f7542f08b74d6
Reviewed-on: http://gerrit.cloudera.org:8080/6852
Reviewed-by: Adar Dembo 
Tested-by: Kudu Jenkins
(cherry picked from commit ae7cfa04a64e8f9a494db36c4f20e11cea311845)
Reviewed-on: http://gerrit.cloudera.org:8080/6863
Reviewed-by: Todd Lipcon 
---
M src/kudu/util/net/net_util.cc
1 file changed, 3 insertions(+), 4 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I1ff0d28f631c7f392420040b132f7542f08b74d6
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] log: shut down appender thread when idle

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: log: shut down appender thread when idle
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6856/1/src/kudu/consensus/log.cc
File src/kudu/consensus/log.cc:

Line 256:   _state_, WORKER_STOPPED, WORKER_ACTIVE);
> hard to repro in a test without injecting various sleeps. I did manually in
I looked at mt-log-test and I don't think it's geared to trigger this, because 
all of the writers are running all at once, and then not at all. That is, 
there's no staggering, so there's no window of time where the queue will be 
empty of entries for an entire second.

Maybe mt-log-test could reliably trigger this if you made kIdleThreshold 
configurable and turned it way down (say 1ms) for the test? That would probably 
be a good stress test in and of itself, since it'd trigger many active->stopped 
and stopped->active transitions.

Broadly speaking I don't find MAYBE... style fault injection to be polluting, 
but I don't want to ask to you spend hours building a test that, in the best of 
times, rarely triggers the race. So, if it's easily reproducible by tweaking 
the timeout (or injecting some latency), I'd support it, but not if it 
necessitates a new test with hundreds of lines of code.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id40a4cfcec96198b537c2f50be7ff1204faf96d2
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] spark: add support for fault tolerant scanner

[Hao Hao (Code Review)]

Hao Hao has posted comments on this change.

Change subject: spark: add support for fault tolerant scanner
..


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6782/6/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
File 
java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala:

Line 78: 
> That's true, and putting it last with a default value isn't something we'd 
Then does it make sense to pass those configuration to KuduContext? I know JD 
mentioned this may be too restrictive though..


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3f3192025ca5d74197600480fd3d040d70b4bbc2
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] Refactor ConsensusStatePB to hold committed and pending configs

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Refactor ConsensusStatePB to hold committed and pending configs
..


Patch Set 8:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/6809/8/src/kudu/consensus/quorum_util.cc
File src/kudu/consensus/quorum_util.cc:

PS8, Line 197:  return Status::IllegalState(
 :   Substitute("Leader with UUID $0 is not a VOTER in the 
committed or pending config! "
 :"Consensus state: $1",
hrm, isn't this possible? eg when catching up a node that is more than one 
config change behind, you might have a leader which isn't in its committed or 
pending config yet?


Line 219: namespace {
nit: don't indent inside namespace


Line 221:   typedef map> PeerInfoMap;
Add a comment about the structure of this map (key and value contents?)


PS8, Line 332: gained a
gained/lost is a bit strange terminology here, since it sounds sort of like 
there's a list. Maybe just "changed pending config" or "now has pending config" 
or somesuch


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4bc4bdd9752fc29a7ce2cefcdc95c4366f5353af
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes

[kudu-CR](branch-1.3.x) [util] updated output from TryRunLsof()

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: [util] updated output from TryRunLsof()
..


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ff0d28f631c7f392420040b132f7542f08b74d6
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR](branch-1.3.x) KUDU-1941: more validation for RPC auth flags

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: KUDU-1941: more validation for RPC auth flags
..


Patch Set 2: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3c088fd6d7a695234e2955e09ca53626078b4e51
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] cmake: fix protoc dependency typo

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: cmake: fix protoc dependency typo
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ide6f69332527f56347b2ee3e3fc6e5b4bb543fa3
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] cmake: fix protoc dependency typo

[Dan Burkert (Code Review)]

Hello Adar Dembo,

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

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

to review the following change.

Change subject: cmake: fix protoc dependency typo
..

cmake: fix protoc dependency typo

The effect of this typo was that protoc would always be statically
linked. Since it's only a build-time dependency, we never noticed
previously.

Change-Id: Ide6f69332527f56347b2ee3e3fc6e5b4bb543fa3
---
M CMakeLists.txt
1 file changed, 1 insertion(+), 1 deletion(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ide6f69332527f56347b2ee3e3fc6e5b4bb543fa3
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 

[kudu-CR] KUDU-1952 Remove round-robin for block placement

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: KUDU-1952 Remove round-robin for block placement
..


Patch Set 23:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/6636/23/src/kudu/fs/data_dirs.cc
File src/kudu/fs/data_dirs.cc:

Line 477:   // Use a different RNG than that used to create groups to avoid 
races.
This comment is no longer true.


Line 544:   // Use a separte RNG from that used by directory selection to avoid 
races.
No longer true?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9828147f4fa5c4d7f6ed23441dca5a116b8cb11b
Gerrit-PatchSet: 23
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1952 Remove round-robin for block placement

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: KUDU-1952 Remove round-robin for block placement
..


Patch Set 21:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6636/18/src/kudu/fs/data_dirs.cc
File src/kudu/fs/data_dirs.cc:

Line 540: }
> The output of std::default_random_engine() would be constant; it's the inpu
As we discussed, give ThreadSafeRandom a shot. It should let you share the same 
RNG.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9828147f4fa5c4d7f6ed23441dca5a116b8cb11b
Gerrit-PatchSet: 21
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1952 Remove round-robin for block placement

[Andrew Wong (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1952 Remove round-robin for block placement
..

KUDU-1952 Remove round-robin for block placement

This is the first of a multi-patch patchset to mitigate the
single-disk failure. Throughout the code, the term "DataDir" refers to
a data directory, which is often mounted on a distinct disk. Thus,
"disks" and "data directories" will be used interchangeably.

This patch adds a mapping from tablet to a set of disks and uses it to
replace the existing round-robin placement of blocks. Tablets are
mapped to a fixed number of disks (i.e. a DataDirGroup). New blocks
are placed randomly in directories within each tablet's DataDirGroup.

Tablet-to-group mappings are generated and stored as metadata upon
tablet creation, or upon tablet replacement during a tablet copy.
During group creation, disks are added to groups by randomly selecting
two available directories and selecting the one with fewer tablets on
it. This avoids pigeonholing new tablets to disks with relatively few
tablets, while still trending towards filling underloaded disks.

Groups are maintained when restarting the server, as they are flushed
with metadata, and are deleted upon tablet deletion.  When loading
tablet data from a previous version of Kudu, the tablet's superblock
will not have a DataDirGroup. One will be generated containing all
data directories, as the tablet's data may already be spread across
any number of disks.

As this patch only addresses block placement, it does not itself
mitigate single-disk failure. Given this, and given the tradeoff
between I/O and failure disk-failure tolerance, the default behavior
will be to spread tablet data across all disks.

Testing is done at the block manager level in block_manager-test and
log_block_manager-test, as well as in the new data_dirs-test.

A design doc can be found here:
https://docs.google.com/document/d/1zZk-vb_ETKUuePcZ9ZqoSK2oPvAAaEV1sjDXes8Pxgk/edit?usp=sharing

Change-Id: I9828147f4fa5c4d7f6ed23441dca5a116b8cb11b
---
M src/kudu/cfile/bloomfile-test-base.h
M src/kudu/cfile/cfile-test-base.h
M src/kudu/cfile/cfile-test.cc
M src/kudu/fs/CMakeLists.txt
M src/kudu/fs/block_manager-stress-test.cc
M src/kudu/fs/block_manager-test.cc
M src/kudu/fs/block_manager.h
A 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/file_block_manager.h
M src/kudu/fs/fs.proto
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/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/tablet/delta_compaction-test.cc
M src/kudu/tablet/delta_compaction.cc
M src/kudu/tablet/delta_compaction.h
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/deltafile-test.cc
M src/kudu/tablet/deltamemstore-test.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/multi_column_writer.cc
M src/kudu/tablet/multi_column_writer.h
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tablet/tablet_metadata.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_client.h
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
38 files changed, 978 insertions(+), 179 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/6636/23
-- 
To view, visit http://gerrit.cloudera.org:8080/6636
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9828147f4fa5c4d7f6ed23441dca5a116b8cb11b
Gerrit-PatchSet: 23
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] log: shut down appender thread when idle

[Todd Lipcon (Code Review)]

Hello Adar Dembo, Kudu Jenkins,

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

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

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

Change subject: log: shut down appender thread when idle
..

log: shut down appender thread when idle

This changes the log appender thread to be based on submitting tasks to a pool,
rather than a thread which is always running.

See the comments on Log::AppendThread in log.cc for notes on the design.

Change-Id: Id40a4cfcec96198b537c2f50be7ff1204faf96d2
---
M src/kudu/consensus/log-test.cc
M src/kudu/consensus/log.cc
M src/kudu/consensus/log.h
M src/kudu/util/blocking_queue-test.cc
M src/kudu/util/blocking_queue.h
M src/kudu/util/threadpool.h
6 files changed, 243 insertions(+), 86 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id40a4cfcec96198b537c2f50be7ff1204faf96d2
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] log: shut down appender thread when idle

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: log: shut down appender thread when idle
..


Patch Set 1:

(11 comments)

http://gerrit.cloudera.org:8080/#/c/6856/1/src/kudu/consensus/log-test.cc
File src/kudu/consensus/log-test.cc:

Line 1105:   ASSERT_TRUE(log_->append_thread_active_for_tests());
> Could be flaky if enough time passes between Append() and now, right?
yea, would have to be a whole second, but lemme see if I can make it less 
flaky-prone with a loop


http://gerrit.cloudera.org:8080/#/c/6856/1/src/kudu/consensus/log.cc
File src/kudu/consensus/log.cc:

Line 203:   // Tries to transition back to WORKER_STOPPED state. If successful, 
returns true.
> Nit: Add a blank line before.
Done


Line 229:   gscoped_ptr append_pool_;
> Given this design and given that the append thread is blocked on IO when ru
yea, I don't know if there's a lot of benefit in doing that... suppose it would 
be possible at some point but this seemed easier and we still get the end goal 
of no threads running for cold tablets


Line 243: .set_idle_timeout(MonoDelta::FromSeconds(0))
> Add a comment for this? I'm guessing it's because we handle idleness oursel
Done


Line 248: void Log::AppendThread::Wake() {
> DCHECK(append_pool_) maybe?
Done


Line 256:   // Stopping is a bit tricky. We have to consider the following race:
> Do we have any unit tests that can semi-reliably produce this race? A multi
hard to repro in a test without injecting various sleeps. I did manually inject 
sleeps in the right places to cover all the code paths, and we do have some 
multi-threaded tests like mt-log-test as well as any multi-threaded integration 
tests covering this.

Do you think it's worth polluting the code with MAYBE_INJECT_RANDOM_LATENCY() 
calls to trigger potential races?


Line 288: void Log::AppendThread::DoWork() {
> Maybe DCHECK that worker_state_ is WORKER_ACTIVE?
Done


Line 365: append_pool_->Wait();
> Why call Wait() prior to Shutdown()? IIRC, Shutdown() will wait for outstan
I don't think we want to cancel the pending ones, because then I think we'll 
end up orphaning entries in the blocking queue. We want the task to drain the 
queue and run the callbacks, I believe. (The previous behavior by joining is 
doing the same thing, allowing the appender thread to drain the queue)


http://gerrit.cloudera.org:8080/#/c/6856/1/src/kudu/util/blocking_queue-test.cc
File src/kudu/util/blocking_queue-test.cc:

Line 31: using std::shared_ptr;
> warning: using decl 'shared_ptr' is unused [misc-unused-using-decls]
Done


Line 64:   ASSERT_OK(test_queue.BlockingDrainTo(, MonoTime::Now() + 
MonoDelta::FromSeconds(5)));
> May be flaky if enough time passes between creation of the deadline and the
Done


http://gerrit.cloudera.org:8080/#/c/6856/1/src/kudu/util/threadpool.h
File src/kudu/util/threadpool.h:

Line 89: // timeout: How long we'll keep around an idle thread before timing it 
out.
> Change this to idle_timeout too?
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id40a4cfcec96198b537c2f50be7ff1204faf96d2
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1952 Remove round-robin for block placement

[Andrew Wong (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1952 Remove round-robin for block placement
..

KUDU-1952 Remove round-robin for block placement

This is the first of a multi-patch patchset to mitigate the
single-disk failure. Throughout the code, the term "DataDir" refers to
a data directory, which is often mounted on a distinct disk. Thus,
"disks" and "data directories" will be used interchangeably.

This patch adds a mapping from tablet to a set of disks and uses it to
replace the existing round-robin placement of blocks. Tablets are
mapped to a fixed number of disks (i.e. a DataDirGroup). New blocks
are placed randomly in directories within each tablet's DataDirGroup.

Tablet-to-group mappings are generated and stored as metadata upon
tablet creation, or upon tablet replacement during a tablet copy.
During group creation, disks are added to groups by randomly selecting
two available directories and selecting the one with fewer tablets on
it. This avoids pigeonholing new tablets to disks with relatively few
tablets, while still trending towards filling underloaded disks.

Groups are maintained when restarting the server, as they are flushed
with metadata, and are deleted upon tablet deletion.  When loading
tablet data from a previous version of Kudu, the tablet's superblock
will not have a DataDirGroup. One will be generated containing all
data directories, as the tablet's data may already be spread across
any number of disks.

As this patch only addresses block placement, it does not itself
mitigate single-disk failure. Given this, and given the tradeoff
between I/O and failure disk-failure tolerance, the default behavior
will be to spread tablet data across all disks.

Testing is done at the block manager level in block_manager-test and
log_block_manager-test, as well as in the new data_dirs-test.

A design doc can be found here:
https://docs.google.com/document/d/1zZk-vb_ETKUuePcZ9ZqoSK2oPvAAaEV1sjDXes8Pxgk/edit?usp=sharing

Change-Id: I9828147f4fa5c4d7f6ed23441dca5a116b8cb11b
---
M src/kudu/cfile/bloomfile-test-base.h
M src/kudu/cfile/cfile-test-base.h
M src/kudu/cfile/cfile-test.cc
M src/kudu/fs/CMakeLists.txt
M src/kudu/fs/block_manager-stress-test.cc
M src/kudu/fs/block_manager-test.cc
M src/kudu/fs/block_manager.h
A 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/file_block_manager.h
M src/kudu/fs/fs.proto
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/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/tablet/delta_compaction-test.cc
M src/kudu/tablet/delta_compaction.cc
M src/kudu/tablet/delta_compaction.h
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/deltafile-test.cc
M src/kudu/tablet/deltamemstore-test.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/multi_column_writer.cc
M src/kudu/tablet/multi_column_writer.h
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tablet/tablet_metadata.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_client.h
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
38 files changed, 979 insertions(+), 179 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/6636/22
-- 
To view, visit http://gerrit.cloudera.org:8080/6636
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9828147f4fa5c4d7f6ed23441dca5a116b8cb11b
Gerrit-PatchSet: 22
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-1952 Remove round-robin for block placement

[Andrew Wong (Code Review)]

Andrew Wong has posted comments on this change.

Change subject: KUDU-1952 Remove round-robin for block placement
..


Patch Set 21:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/6636/18/src/kudu/fs/block_manager-test.cc
File src/kudu/fs/block_manager-test.cc:

Line 134: if (basename == kInstanceMetadataFileName) {
> warning: parameter 'dirname' is unused [misc-unused-parameters]
Done


Line 144:   // hierarchy, ignoring '.', '..', and file 
'kInstanceMetadataFileName'.
> Nit: got some extra whitespace here.
Done


Line 325:   // Verify the results. Each path has dot, dotdot, instance file.
> Should ASSERT_OK() here.
Done


http://gerrit.cloudera.org:8080/#/c/6636/17/src/kudu/fs/data_dirs-test.cc
File src/kudu/fs/data_dirs-test.cc:

Line 96:   for (int i = 0; i < pb.uuid_indices().size(); i++) {
> Not done?
Rrg good catch, done.


http://gerrit.cloudera.org:8080/#/c/6636/18/src/kudu/fs/data_dirs-test.cc
File src/kudu/fs/data_dirs-test.cc:

PS18, Line 47: ),
> Use env_
Done


Line 72:   DataDirGroupPB pb_;
> This is a little unusual; use the Kudu Random from util/random.
More than unusual, unneeded. These tests don't require any RNG of their own, 
just that of the DataDirManager.


http://gerrit.cloudera.org:8080/#/c/6636/18/src/kudu/fs/data_dirs.cc
File src/kudu/fs/data_dirs.cc:

Line 21: #include 
> Nit: this belongs with the gflags/glog includes since boost is part of the 
Ah I see, it's a dependency. Done.


Line 540: }
> Let's make this a member of the DataDirManager and initialize it just once,
The output of std::default_random_engine() would be constant; it's the input 
r.Next() that's changing to instantiate the engine, providing different rng 
across runs.

I went with your first suggestion.

Actually that resulted in a race since GetNextDataDir only read-locks, and 
since the shuffle needs a new seed every run to select different directories 
every time, it modifies the RNG state.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9828147f4fa5c4d7f6ed23441dca5a116b8cb11b
Gerrit-PatchSet: 21
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] WIP Allow external miniclusters to use many data dirs

[Andrew Wong (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: WIP Allow external miniclusters to use many data dirs
..

WIP Allow external miniclusters to use many data dirs

In order to test different disk configurations, it is becoming
increasingly important to have end-to-end testing with nodes backed
by multiple directories.

This patch adds this functionality to the ExternalMiniCluster class,
which now supports the 'num_dirs_per_tserver' option, and the
ExternalDaemon class, which now supports a list of data dirs instead
of a single data dir.

The ExternalMiniCluster will create the ExternalDaemon with a list of
data dirs that reflects 'num_dirs_per_tserver' by using the original
data dir name as a prefix and appending an integer (e.g. '/dir_name'
with 'num_dirs_per_tserver = 2' will result in '/dir_name-0' and
'/dir_name-1').

A new test called disk-failure-itest is added that exercises this.

WIP: EIO-handling patch must be completed for further testing to make
sense.

Change-Id: Id2f5def6980ad394c8558ad97ba830f1b0257332
---
M src/kudu/integration-tests/CMakeLists.txt
A src/kudu/integration-tests/disk-failure-itest.cc
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/integration-tests/external_mini_cluster.h
M src/kudu/integration-tests/ts_itest-base.h
M src/kudu/util/env_posix.cc
M src/kudu/util/path_util.cc
M src/kudu/util/path_util.h
8 files changed, 267 insertions(+), 20 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id2f5def6980ad394c8558ad97ba830f1b0257332
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-1952 Remove round-robin for block placement

[Andrew Wong (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1952 Remove round-robin for block placement
..

KUDU-1952 Remove round-robin for block placement

This is the first of a multi-patch patchset to mitigate the
single-disk failure. Throughout the code, the term "DataDir" refers to
a data directory, which is often mounted on a distinct disk. Thus,
"disks" and "data directories" will be used interchangeably.

This patch adds a mapping from tablet to a set of disks and uses it to
replace the existing round-robin placement of blocks. Tablets are
mapped to a fixed number of disks (i.e. a DataDirGroup). New blocks
are placed randomly in directories within each tablet's DataDirGroup.

Tablet-to-group mappings are generated and stored as metadata upon
tablet creation, or upon tablet replacement during a tablet copy.
During group creation, disks are added to groups by randomly selecting
two available directories and selecting the one with fewer tablets on
it. This avoids pigeonholing new tablets to disks with relatively few
tablets, while still trending towards filling underloaded disks.

Groups are maintained when restarting the server, as they are flushed
with metadata, and are deleted upon tablet deletion.  When loading
tablet data from a previous version of Kudu, the tablet's superblock
will not have a DataDirGroup. One will be generated containing all
data directories, as the tablet's data may already be spread across
any number of disks.

As this patch only addresses block placement, it does not itself
mitigate single-disk failure. Given this, and given the tradeoff
between I/O and failure disk-failure tolerance, the default behavior
will be to spread tablet data across all disks.

Testing is done at the block manager level in block_manager-test and
log_block_manager-test, as well as in the new data_dirs-test.

A design doc can be found here:
https://docs.google.com/document/d/1zZk-vb_ETKUuePcZ9ZqoSK2oPvAAaEV1sjDXes8Pxgk/edit?usp=sharing

Change-Id: I9828147f4fa5c4d7f6ed23441dca5a116b8cb11b
---
M src/kudu/cfile/bloomfile-test-base.h
M src/kudu/cfile/cfile-test-base.h
M src/kudu/cfile/cfile-test.cc
M src/kudu/fs/CMakeLists.txt
M src/kudu/fs/block_manager-stress-test.cc
M src/kudu/fs/block_manager-test.cc
M src/kudu/fs/block_manager.h
A 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/file_block_manager.h
M src/kudu/fs/fs.proto
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/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/tablet/delta_compaction-test.cc
M src/kudu/tablet/delta_compaction.cc
M src/kudu/tablet/delta_compaction.h
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/deltafile-test.cc
M src/kudu/tablet/deltamemstore-test.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/multi_column_writer.cc
M src/kudu/tablet/multi_column_writer.h
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tablet/tablet_metadata.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_client.h
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
38 files changed, 978 insertions(+), 178 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/6636/21
-- 
To view, visit http://gerrit.cloudera.org:8080/6636
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9828147f4fa5c4d7f6ed23441dca5a116b8cb11b
Gerrit-PatchSet: 21
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [java-client] Update protoc and simplify the maven build

[Adar Dembo (Code Review)]

Adar Dembo has submitted this change and it was merged.

Change subject: [java-client] Update protoc and simplify the maven build
..


[java-client] Update protoc and simplify the maven build

This patch simplifies the Java build by using maven to
provide protoc via the Maven OS plugin and the
protocArtifact property of the Maven Protobuf plugin.

It also updates protoc to 3.3.0 and makes the required
changes due to compatibility breaks.

Change-Id: I70f7ad260777d5355497fa7e9a1047c342ff9ee9
Reviewed-on: http://gerrit.cloudera.org:8080/6846
Reviewed-by: Todd Lipcon 
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo 
---
M docs/installation.adoc
M java/README.md
M java/kudu-client/pom.xml
D 
java/kudu-client/src/main/java/com/google/protobuf/ZeroCopyLiteralByteString.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Bytes.java
M 
java/kudu-client/src/main/java/org/apache/kudu/client/ColumnRangePredicate.java
M 
java/kudu-client/src/main/java/org/apache/kudu/client/GetTableLocationsRequest.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/TestColumnRangePredicate.java
M java/pom.xml
15 files changed, 61 insertions(+), 254 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I70f7ad260777d5355497fa7e9a1047c342ff9ee9
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-586. Upgrade to protobuf 3

[Adar Dembo (Code Review)]

Adar Dembo has submitted this change and it was merged.

Change subject: KUDU-586. Upgrade to protobuf 3
..


KUDU-586. Upgrade to protobuf 3

Upgrades Kudu to use protoc 3.3.0 and
adds `syntax = "proto2”;` to the proto files.

Does not leverage the new protobuf allocation arenas,
but that should be considered in a follow up patch.

Change-Id: I30986ebe56ebb334db8f89f1652e7ef2b064925c
Reviewed-on: http://gerrit.cloudera.org:8080/6857
Reviewed-by: Adar Dembo 
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
M src/kudu/cfile/cfile.proto
M src/kudu/client/client.proto
M src/kudu/common/common.proto
M src/kudu/common/wire_protocol.proto
M src/kudu/consensus/consensus.proto
M src/kudu/consensus/log.proto
M src/kudu/consensus/metadata.proto
M src/kudu/consensus/opid.proto
M src/kudu/fs/fs.proto
M src/kudu/master/master.proto
M src/kudu/rpc/rpc_header.proto
M src/kudu/rpc/rpc_introspection.proto
M src/kudu/rpc/rtest.proto
M src/kudu/rpc/rtest_diff_package.proto
M src/kudu/security/token.proto
M src/kudu/server/server_base.proto
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/tablet.proto
M src/kudu/tserver/tablet_copy.proto
M src/kudu/tserver/tserver.proto
M src/kudu/tserver/tserver_admin.proto
M src/kudu/tserver/tserver_service.proto
M src/kudu/util/compression/compression.proto
M src/kudu/util/histogram.proto
M src/kudu/util/jsonwriter_test.proto
M src/kudu/util/maintenance_manager.proto
M src/kudu/util/pb_util.proto
M src/kudu/util/pb_util_test.proto
M src/kudu/util/proto_container_test.proto
M src/kudu/util/proto_container_test2.proto
M src/kudu/util/proto_container_test3.proto
M src/kudu/util/version_info.proto
M thirdparty/vars.sh
33 files changed, 33 insertions(+), 2 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I30986ebe56ebb334db8f89f1652e7ef2b064925c
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [java-client] Update protoc and simplify the maven build

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: [java-client] Update protoc and simplify the maven build
..


Patch Set 7: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I70f7ad260777d5355497fa7e9a1047c342ff9ee9
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs

[Hao Hao (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1875: Refuse unauthenticated connections from publicly 
routable IP addrs
..

KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs

This rejects unauthenticated connections from publicly routable IPs,
even if authentication and encryption are not configured.

An adavanced flag 'trusted_subnets' is provided to whitelist
trusted subnets. All unauthenticated or unencrypted connections
are prohibited except these from the specified subnets and local
subnets of all local network interfaces. Set the flag to '0.0.0.0/0'
can completely disable this restriction. However, if network access
is not otherwise restricted by a firewall, malicious users may be
able to gain unauthorized access.

Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
M src/kudu/util/net/socket.h
9 files changed, 342 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/6514/32
-- 
To view, visit http://gerrit.cloudera.org:8080/6514
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 32
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Harsh J 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR](branch-1.3.x) [flags] fixed typo in group flag validation logic

[Alexey Serbin (Code Review)]

Hello Adar Dembo, Kudu Jenkins,

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

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

to review the following change.

Change subject: [flags] fixed typo in group flag validation logic
..

[flags] fixed typo in group flag validation logic

This is a follow-up for e7334c2e6607ac0fa778499eda2df3f4cfcd3fe3.

This patch fixes the typo in the original commit and adds test coverage
for the case of multiple group validators in the validation pipeline.

Change-Id: Idfdc25d491941674fd7b86d8ffa33ff2e046703b
Reviewed-on: http://gerrit.cloudera.org:8080/6860
Reviewed-by: Adar Dembo 
Tested-by: Kudu Jenkins
(cherry picked from commit a0006dbaa77c276c6498dc4edc0228cae049738c)
---
M src/kudu/util/flag_validators-test.cc
M src/kudu/util/flags.cc
2 files changed, 78 insertions(+), 5 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Idfdc25d491941674fd7b86d8ffa33ff2e046703b
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR](branch-1.3.x) [util] updated output from TryRunLsof()

[Alexey Serbin (Code Review)]

Hello Adar Dembo, Kudu Jenkins,

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

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

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

Change subject: [util] updated output from TryRunLsof()
..

[util] updated output from TryRunLsof()

TryRunLsof() should not conclude it's called because of an error
binding to the specified address: in some use-cases it might not be the
case.  The output is updated to remove the statement about a failure to
bind to the specified address, leaving just the output from lsof.

This is a pure cosmetic fix to improve error message readability
in the context of KUDU-2005.

Change-Id: I1ff0d28f631c7f392420040b132f7542f08b74d6
Reviewed-on: http://gerrit.cloudera.org:8080/6852
Reviewed-by: Adar Dembo 
Tested-by: Kudu Jenkins
(cherry picked from commit ae7cfa04a64e8f9a494db36c4f20e11cea311845)
---
M src/kudu/util/net/net_util.cc
1 file changed, 3 insertions(+), 4 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1ff0d28f631c7f392420040b132f7542f08b74d6
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR](branch-1.3.x) KUDU-1941: more validation for RPC auth flags

[Alexey Serbin (Code Review)]

Hello Adar Dembo, Kudu Jenkins,

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

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

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

Change subject: KUDU-1941: more validation for RPC auth flags
..

KUDU-1941: more validation for RPC auth flags

With this patch, both master and tserver refuse to start if
authentication is 'required' but no authentication method is configured.

Prior to this patch, the inconsistency with the run-time configuration
could be detected at a later stage when a client would try to connect
to Kudu cluster.

Change-Id: I3c088fd6d7a695234e2955e09ca53626078b4e51
Reviewed-on: http://gerrit.cloudera.org:8080/6851
Reviewed-by: Adar Dembo 
Tested-by: Kudu Jenkins
(cherry picked from commit 87ddf0ae2584f2394bb26d36c01c16e6719659db)
---
M src/kudu/rpc/messenger.cc
1 file changed, 11 insertions(+), 2 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3c088fd6d7a695234e2955e09ca53626078b4e51
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [flags] fixed typo in group flag validation logic

[Alexey Serbin (Code Review)]

Alexey Serbin has submitted this change and it was merged.

Change subject: [flags] fixed typo in group flag validation logic
..


[flags] fixed typo in group flag validation logic

This is a follow-up for e7334c2e6607ac0fa778499eda2df3f4cfcd3fe3.

This patch fixes the typo in the original commit and adds test coverage
for the case of multiple group validators in the validation pipeline.

Change-Id: Idfdc25d491941674fd7b86d8ffa33ff2e046703b
Reviewed-on: http://gerrit.cloudera.org:8080/6860
Reviewed-by: Adar Dembo 
Tested-by: Kudu Jenkins
---
M src/kudu/util/flag_validators-test.cc
M src/kudu/util/flags.cc
2 files changed, 78 insertions(+), 5 deletions(-)

Approvals:
  Adar Dembo: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Idfdc25d491941674fd7b86d8ffa33ff2e046703b
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-586. Upgrade to protobuf 3

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: KUDU-586. Upgrade to protobuf 3
..


Patch Set 3: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I30986ebe56ebb334db8f89f1652e7ef2b064925c
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] [java-client] Update protoc and simplify the maven build

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: [java-client] Update protoc and simplify the maven build
..


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6846/6/java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
File java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java:

Line 264
chatted offline, but posting here too. I think this TODO is still relevant -- 
we should try to avoid the copy. But the TODO just needs to be updated for 
accuracy.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I70f7ad260777d5355497fa7e9a1047c342ff9ee9
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [java-client] Update protoc and simplify the maven build

[Grant Henke (Code Review)]

Hello Adar Dembo, Kudu Jenkins,

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

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

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

Change subject: [java-client] Update protoc and simplify the maven build
..

[java-client] Update protoc and simplify the maven build

This patch simplifies the Java build by using maven to
provide protoc via the Maven OS plugin and the
protocArtifact property of the Maven Protobuf plugin.

It also updates protoc to 3.3.0 and makes the required
changes due to compatibility breaks.

Change-Id: I70f7ad260777d5355497fa7e9a1047c342ff9ee9
---
M docs/installation.adoc
M java/README.md
M java/kudu-client/pom.xml
D 
java/kudu-client/src/main/java/com/google/protobuf/ZeroCopyLiteralByteString.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Bytes.java
M 
java/kudu-client/src/main/java/org/apache/kudu/client/ColumnRangePredicate.java
M 
java/kudu-client/src/main/java/org/apache/kudu/client/GetTableLocationsRequest.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/TestColumnRangePredicate.java
M java/pom.xml
15 files changed, 61 insertions(+), 254 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I70f7ad260777d5355497fa7e9a1047c342ff9ee9
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [java-client] Update protoc and simplify the maven build

[Grant Henke (Code Review)]

Hello Adar Dembo, Kudu Jenkins,

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

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

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

Change subject: [java-client] Update protoc and simplify the maven build
..

[java-client] Update protoc and simplify the maven build

This patch simplifies the Java build by using maven to
provide protoc via the Maven OS plugin and the
protocArtifact property of the Maven Protobuf plugin.

It also updates protoc to 3.3.0 and makes the required
changes due to compatibility breaks.

Change-Id: I70f7ad260777d5355497fa7e9a1047c342ff9ee9
---
M docs/installation.adoc
M java/README.md
M java/kudu-client/pom.xml
D 
java/kudu-client/src/main/java/com/google/protobuf/ZeroCopyLiteralByteString.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Bytes.java
M 
java/kudu-client/src/main/java/org/apache/kudu/client/ColumnRangePredicate.java
M 
java/kudu-client/src/main/java/org/apache/kudu/client/GetTableLocationsRequest.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/TestColumnRangePredicate.java
M java/pom.xml
15 files changed, 59 insertions(+), 257 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/46/6846/6
-- 
To view, visit http://gerrit.cloudera.org:8080/6846
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I70f7ad260777d5355497fa7e9a1047c342ff9ee9
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR](branch-1.3.x) KUDU-2005: actionable error messages from webserver

[Will Berkeley (Code Review)]

Will Berkeley has posted comments on this change.

Change subject: KUDU-2005: actionable error messages from webserver
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2315f7ee88c8835a36e5174cb25132967429a77
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: No

[kudu-CR](branch-1.3.x) KUDU-2005: actionable error messages from webserver

[Will Berkeley (Code Review)]

Will Berkeley has submitted this change and it was merged.

Change subject: KUDU-2005: actionable error messages from webserver
..


KUDU-2005: actionable error messages from webserver

As it turned out, squeasel outputs only errors via the log_message
callback.  Let's output them with ERROR severity into Kudu log
(they were output as INFO messages prior to this patch).

Also, if the embedded squeasel webserver fails to start, add the last
error message from it (if any) into the RuntimeError status message
returned from Webserver::Start().

Change-Id: Ia2315f7ee88c8835a36e5174cb25132967429a77
Reviewed-on: http://gerrit.cloudera.org:8080/6848
Tested-by: Kudu Jenkins
Reviewed-by: Will Berkeley 
(cherry picked from commit 83885a47a77a7206414987d07d5945525e66)
Reviewed-on: http://gerrit.cloudera.org:8080/6861
---
M src/kudu/server/webserver-test.cc
M src/kudu/server/webserver.cc
M src/kudu/server/webserver.h
3 files changed, 25 insertions(+), 6 deletions(-)

Approvals:
  Will Berkeley: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ia2315f7ee88c8835a36e5174cb25132967429a77
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] [java-client] Update protoc and simplify the maven build

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: [java-client] Update protoc and simplify the maven build
..


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/6846/5/java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
File java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java:

Line 266:   // We have UnsafeByteOperations, but that only supports an 
entire array. Here
this actually doesn't appear to be true -- UnsafeByteOperations supports 
unsafeWrap() with an offset and length. Still should be fine to do in a 
follow-up patch but at least the TODO can be made more accurate


http://gerrit.cloudera.org:8080/#/c/6846/5/java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
File java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java:

Line 326:   bytes = value;
don't we also need to check whether length == value.length?

maybe this isn't really worth it after all since this seems to only be used 
when deserializing schemas, which isn't that common an operation in the Java 
client (sorry for leading you astray)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I70f7ad260777d5355497fa7e9a1047c342ff9ee9
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR](branch-1.3.x) [util] updated output from TryRunLsof()

[Alexey Serbin (Code Review)]

Hello Adar Dembo, Kudu Jenkins,

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

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

to review the following change.

Change subject: [util] updated output from TryRunLsof()
..

[util] updated output from TryRunLsof()

TryRunLsof() should not conclude it's called because of an error
binding to the specified address: in some use-cases it might not be the
case.  The output is updated to remove the statement about a failure to
bind to the specified address, leaving just the output from lsof.

This is a pure cosmetic fix to improve error message readability
in the context of KUDU-2005.

Change-Id: I1ff0d28f631c7f392420040b132f7542f08b74d6
Reviewed-on: http://gerrit.cloudera.org:8080/6852
Reviewed-by: Adar Dembo 
Tested-by: Kudu Jenkins
(cherry picked from commit ae7cfa04a64e8f9a494db36c4f20e11cea311845)
---
M src/kudu/util/net/net_util.cc
1 file changed, 3 insertions(+), 4 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I1ff0d28f631c7f392420040b132f7542f08b74d6
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR](branch-1.3.x) KUDU-2005: actionable error messages from webserver

[Alexey Serbin (Code Review)]

Hello Will Berkeley, Kudu Jenkins,

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

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

to review the following change.

Change subject: KUDU-2005: actionable error messages from webserver
..

KUDU-2005: actionable error messages from webserver

As it turned out, squeasel outputs only errors via the log_message
callback.  Let's output them with ERROR severity into Kudu log
(they were output as INFO messages prior to this patch).

Also, if the embedded squeasel webserver fails to start, add the last
error message from it (if any) into the RuntimeError status message
returned from Webserver::Start().

Change-Id: Ia2315f7ee88c8835a36e5174cb25132967429a77
Reviewed-on: http://gerrit.cloudera.org:8080/6848
Tested-by: Kudu Jenkins
Reviewed-by: Will Berkeley 
(cherry picked from commit 83885a47a77a7206414987d07d5945525e66)
---
M src/kudu/server/webserver-test.cc
M src/kudu/server/webserver.cc
M src/kudu/server/webserver.h
3 files changed, 25 insertions(+), 6 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia2315f7ee88c8835a36e5174cb25132967429a77
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 

[kudu-CR](branch-1.3.x) KUDU-1941: more validation for RPC auth flags

[Alexey Serbin (Code Review)]

Hello Adar Dembo, Kudu Jenkins,

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

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

to review the following change.

Change subject: KUDU-1941: more validation for RPC auth flags
..

KUDU-1941: more validation for RPC auth flags

With this patch, both master and tserver refuse to start if
authentication is 'required' but no authentication method is configured.

Prior to this patch, the inconsistency with the run-time configuration
could be detected at a later stage when a client would try to connect
to Kudu cluster.

Change-Id: I3c088fd6d7a695234e2955e09ca53626078b4e51
Reviewed-on: http://gerrit.cloudera.org:8080/6851
Reviewed-by: Adar Dembo 
Tested-by: Kudu Jenkins
(cherry picked from commit 87ddf0ae2584f2394bb26d36c01c16e6719659db)
---
M src/kudu/rpc/messenger.cc
1 file changed, 11 insertions(+), 2 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I3c088fd6d7a695234e2955e09ca53626078b4e51
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [flags] fixed typo in group flag validation logic

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: [flags] fixed typo in group flag validation logic
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idfdc25d491941674fd7b86d8ffa33ff2e046703b
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] [flags] fixed typo in group flag validation logic

[Alexey Serbin (Code Review)]

Alexey Serbin has uploaded a new change for review.

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

Change subject: [flags] fixed typo in group flag validation logic
..

[flags] fixed typo in group flag validation logic

This is a follow-up for e7334c2e6607ac0fa778499eda2df3f4cfcd3fe3.

This patch fixes the typo in the original commit and adds test coverage
for the case of multiple group validators in the validation pipeline.

Change-Id: Idfdc25d491941674fd7b86d8ffa33ff2e046703b
---
M src/kudu/util/flag_validators-test.cc
M src/kudu/util/flags.cc
2 files changed, 78 insertions(+), 5 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Idfdc25d491941674fd7b86d8ffa33ff2e046703b
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 

[kudu-CR] [doc] Remove beta upgrade reference

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: [doc] Remove beta upgrade reference
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibde3132f3bffd1ca81d249fa9401d408dd47ff21
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] [doc] Remove beta upgrade reference

[Grant Henke (Code Review)]

Grant Henke has uploaded a new change for review.

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

Change subject: [doc] Remove beta upgrade reference
..

[doc] Remove beta upgrade reference

Change-Id: Ibde3132f3bffd1ca81d249fa9401d408dd47ff21
---
M docs/installation.adoc
1 file changed, 1 insertion(+), 6 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibde3132f3bffd1ca81d249fa9401d408dd47ff21
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 

[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs

[Dan Burkert (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1875: Refuse unauthenticated connections from publicly 
routable IP addrs
..

KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs

This rejects unauthenticated connections from publicly routable IPs,
even if authentication and encryption are not configured.

An adavanced flag 'trusted_subnets' is provided to whitelist
trusted subnets. All unauthenticated or unencrypted connections
are prohibited except these from the specified subnets and local
subnets of all local network interfaces. Set the flag to '0.0.0.0/0'
can completely disable this restriction. However, if network access
is not otherwise restricted by a firewall, malicious users may be
able to gain unauthorized access.

Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
M src/kudu/util/net/socket.h
9 files changed, 334 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/6514/31
-- 
To view, visit http://gerrit.cloudera.org:8080/6514
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 31
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Harsh J 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-1952 Remove round-robin for block placement

[Andrew Wong (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1952 Remove round-robin for block placement
..

KUDU-1952 Remove round-robin for block placement

This is the first of a multi-patch patchset to mitigate the
single-disk failure. Throughout the code, the term "DataDir" refers to
a data directory, which is often mounted on a distinct disk. Thus,
"disks" and "data directories" will be used interchangeably.

This patch adds a mapping from tablet to a set of disks and uses it to
replace the existing round-robin placement of blocks. Tablets are
mapped to a fixed number of disks (i.e. a DataDirGroup). New blocks
are placed randomly in directories within each tablet's DataDirGroup.

Tablet-to-group mappings are generated and stored as metadata upon
tablet creation, or upon tablet replacement during a tablet copy.
During group creation, disks are added to groups by randomly selecting
two available directories and selecting the one with fewer tablets on
it. This avoids pigeonholing new tablets to disks with relatively few
tablets, while still trending towards filling underloaded disks.

Groups are maintained when restarting the server, as they are flushed
with metadata, and are deleted upon tablet deletion.  When loading
tablet data from a previous version of Kudu, the tablet's superblock
will not have a DataDirGroup. One will be generated containing all
data directories, as the tablet's data may already be spread across
any number of disks.

As this patch only addresses block placement, it does not itself
mitigate single-disk failure. Given this, and given the tradeoff
between I/O and failure disk-failure tolerance, the default behavior
will be to spread tablet data across all disks.

Testing is done at the block manager level in block_manager-test and
log_block_manager-test, as well as in the new data_dirs-test.

A design doc can be found here:
https://docs.google.com/document/d/1zZk-vb_ETKUuePcZ9ZqoSK2oPvAAaEV1sjDXes8Pxgk/edit?usp=sharing

Change-Id: I9828147f4fa5c4d7f6ed23441dca5a116b8cb11b
---
M src/kudu/cfile/bloomfile-test-base.h
M src/kudu/cfile/cfile-test-base.h
M src/kudu/cfile/cfile-test.cc
M src/kudu/fs/CMakeLists.txt
M src/kudu/fs/block_manager-stress-test.cc
M src/kudu/fs/block_manager-test.cc
M src/kudu/fs/block_manager.h
A 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/file_block_manager.h
M src/kudu/fs/fs.proto
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/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/tablet/delta_compaction-test.cc
M src/kudu/tablet/delta_compaction.cc
M src/kudu/tablet/delta_compaction.h
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/deltafile-test.cc
M src/kudu/tablet/deltamemstore-test.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/multi_column_writer.cc
M src/kudu/tablet/multi_column_writer.h
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tablet/tablet_metadata.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_client.h
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
37 files changed, 972 insertions(+), 176 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/6636/20
-- 
To view, visit http://gerrit.cloudera.org:8080/6636
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9828147f4fa5c4d7f6ed23441dca5a116b8cb11b
Gerrit-PatchSet: 20
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] WIP Don't suicide on EIO

[Andrew Wong (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: WIP Don't suicide on EIO
..

WIP Don't suicide on EIO

Rather than suiciding when reaching an EIO, this patch adds a
mechanism that triggers error-handling in the form of a callback.
This handler is attached to the lowest non-env operations that may
result in EIO.
E.g. PosixRWFile::Write() is an env operation that may result in an
EIO. LogBlockContainer::WriteData()'s call to it must be wrapped in
the new error-handling macro KUDU_RETURN_OR_HANDLE_EIO. Thus, all
direct readers/writers of files must now implement EIO-handling code
and wrap disk IO in KUDU_RETURN_OR_HANDLE_EIO.

Also included is a new tablet data state called TABLET_DATA_CORRUPTED
in which tablet data will not be deleted until explicitly requested
or during a tablet copy.

This patch is marked WIP, as the correct behavior after a disk fails
is not yet included. For now, upon failure, the block managers will
keep track of the dead disks and will mark the appropriate tablet
peers as failed. Additionally, the error handling has been moved
between layers offline, so more cleanup is being done to align with
the above description.

Change-Id: I4c221a5ea97c7b7ec5f8d395b5527dc16e63f828
---
M src/kudu/consensus/consensus_peers.cc
M src/kudu/fs/block_manager-test.cc
M src/kudu/fs/block_manager.h
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
A src/kudu/fs/error_manager.h
M src/kudu/fs/file_block_manager.cc
M src/kudu/fs/file_block_manager.h
M src/kudu/fs/fs_manager.cc
M src/kudu/fs/fs_manager.h
M src/kudu/fs/log_block_manager.cc
M src/kudu/fs/log_block_manager.h
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tablet/tablet_metadata.h
M src/kudu/tablet/tablet_peer_mm_ops.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
M src/kudu/util/fault_injection.cc
M src/kudu/util/fault_injection.h
M src/kudu/util/status.h
27 files changed, 516 insertions(+), 60 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4c221a5ea97c7b7ec5f8d395b5527dc16e63f828
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot

[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs

[Dan Burkert (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1875: Refuse unauthenticated connections from publicly 
routable IP addrs
..

KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs

This rejects unauthenticated connections from publicly routable IPs,
even if authentication and encryption are not configured.

An adavanced flag 'trusted_subnets' is provided to whitelist
trusted subnets. All unauthenticated or unencrypted connections
are prohibited except these from the specified subnets and local
subnets of all local network interfaces. Set the flag to '0.0.0.0/0'
can completely disable this restriction. However, if network access
is not otherwise restricted by a firewall, malicious users may be
able to gain unauthorized access.

Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
M src/kudu/util/net/socket.h
9 files changed, 320 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/6514/30
-- 
To view, visit http://gerrit.cloudera.org:8080/6514
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 30
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Harsh J 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [catalog manager tsk-itest] shortened test's run-time

[Alexey Serbin (Code Review)]

Alexey Serbin has submitted this change and it was merged.

Change subject: [catalog_manager_tsk-itest] shortened test's run-time
..


[catalog_manager_tsk-itest] shortened test's run-time

The longer the test runs the more tables it creates and drops,
and that makes switching from one leader master to another longer.
The presence of some parallel activity exacerbates the problem because
re-elections happen more often.  That leads to timeouts on operations
performed by the client.

This patch shortens the test's run time to make it less flaky if running
on inferior VMs with other concurrent activity.

Change-Id: I21cca3d53d033f56f60e46fb5a3f67f5d7e44122
Reviewed-on: http://gerrit.cloudera.org:8080/6854
Tested-by: Kudu Jenkins
Reviewed-by: David Ribeiro Alves 
---
M src/kudu/integration-tests/catalog_manager_tsk-itest.cc
1 file changed, 9 insertions(+), 4 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I21cca3d53d033f56f60e46fb5a3f67f5d7e44122
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-586. Upgrade to protobuf 3

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: KUDU-586. Upgrade to protobuf 3
..


Patch Set 3: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I30986ebe56ebb334db8f89f1652e7ef2b064925c
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] KUDU-586. Upgrade to protobuf 3

[Grant Henke (Code Review)]

Hello Adar Dembo, Todd Lipcon, Kudu Jenkins,

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

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

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

Change subject: KUDU-586. Upgrade to protobuf 3
..

KUDU-586. Upgrade to protobuf 3

Upgrades Kudu to use protoc 3.3.0 and
adds `syntax = "proto2”;` to the proto files.

Does not leverage the new protobuf allocation arenas,
but that should be considered in a follow up patch.

Change-Id: I30986ebe56ebb334db8f89f1652e7ef2b064925c
---
M src/kudu/cfile/cfile.proto
M src/kudu/client/client.proto
M src/kudu/common/common.proto
M src/kudu/common/wire_protocol.proto
M src/kudu/consensus/consensus.proto
M src/kudu/consensus/log.proto
M src/kudu/consensus/metadata.proto
M src/kudu/consensus/opid.proto
M src/kudu/fs/fs.proto
M src/kudu/master/master.proto
M src/kudu/rpc/rpc_header.proto
M src/kudu/rpc/rpc_introspection.proto
M src/kudu/rpc/rtest.proto
M src/kudu/rpc/rtest_diff_package.proto
M src/kudu/security/token.proto
M src/kudu/server/server_base.proto
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/tablet.proto
M src/kudu/tserver/tablet_copy.proto
M src/kudu/tserver/tserver.proto
M src/kudu/tserver/tserver_admin.proto
M src/kudu/tserver/tserver_service.proto
M src/kudu/util/compression/compression.proto
M src/kudu/util/histogram.proto
M src/kudu/util/jsonwriter_test.proto
M src/kudu/util/maintenance_manager.proto
M src/kudu/util/pb_util.proto
M src/kudu/util/pb_util_test.proto
M src/kudu/util/proto_container_test.proto
M src/kudu/util/proto_container_test2.proto
M src/kudu/util/proto_container_test3.proto
M src/kudu/util/version_info.proto
M thirdparty/vars.sh
33 files changed, 33 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/6857/3
-- 
To view, visit http://gerrit.cloudera.org:8080/6857
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I30986ebe56ebb334db8f89f1652e7ef2b064925c
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Refactor ConsensusStatePB to hold committed and pending configs

[Will Berkeley (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Refactor ConsensusStatePB to hold committed and pending configs
..

Refactor ConsensusStatePB to hold committed and pending configs

This patch refactors ConsensusStatePB so it contains both the
committed config and, if there is a pending config, the pending
config. Previously, consumers of the consensus state would ask
for either the committed or active (= pending if there is a pending
config, else the committed config) config. With this change, the
committed and active configs are always included.

The addition of pending info to the ConsensusStatePB will be used
by ksck in a follow-up patch. Additionally, the master may use
this info in the future to be more aware of the health of tablets.

Change-Id: I4bc4bdd9752fc29a7ce2cefcdc95c4366f5353af
---
M src/kudu/consensus/consensus.h
M src/kudu/consensus/consensus.proto
M src/kudu/consensus/consensus_meta-test.cc
M src/kudu/consensus/consensus_meta.cc
M src/kudu/consensus/consensus_meta.h
M src/kudu/consensus/metadata.proto
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/consensus/raft_consensus_state.cc
M src/kudu/consensus/raft_consensus_state.h
M src/kudu/integration-tests/cluster_itest_util.cc
M src/kudu/integration-tests/cluster_itest_util.h
M src/kudu/integration-tests/delete_table-itest.cc
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/integration-tests/ts_tablet_manager-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master-path-handlers.cc
M src/kudu/master/master.proto
M src/kudu/master/sys_catalog.cc
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tserver/tablet_copy.proto
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_client.h
M src/kudu/tserver/tablet_copy_service.cc
M src/kudu/tserver/tablet_copy_source_session.cc
M src/kudu/tserver/tablet_copy_source_session.h
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/tserver-path-handlers.cc
34 files changed, 333 insertions(+), 301 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/09/6809/8
-- 
To view, visit http://gerrit.cloudera.org:8080/6809
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4bc4bdd9752fc29a7ce2cefcdc95c4366f5353af
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] KUDU-1952 Remove round-robin for block placement

[Andrew Wong (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1952 Remove round-robin for block placement
..

KUDU-1952 Remove round-robin for block placement

This is the first of a multi-patch patchset to mitigate the
single-disk failure. Throughout the code, the term "DataDir" refers to
a data directory, which is often mounted on a distinct disk. Thus,
"disks" and "data directories" will be used interchangeably.

This patch adds a mapping from tablet to a set of disks and uses it to
replace the existing round-robin placement of blocks. Tablets are
mapped to a fixed number of disks (i.e. a DataDirGroup). New blocks
are placed randomly in directories within each tablet's DataDirGroup.

Tablet-to-group mappings are generated and stored as metadata upon
tablet creation, or upon tablet replacement during a tablet copy.
During group creation, disks are added to groups by randomly selecting
two available directories and selecting the one with fewer tablets on
it. This avoids pigeonholing new tablets to disks with relatively few
tablets, while still trending towards filling underloaded disks.

Groups are maintained when restarting the server, as they are flushed
with metadata, and are deleted upon tablet deletion.  When loading
tablet data from a previous version of Kudu, the tablet's superblock
will not have a DataDirGroup. One will be generated containing all
data directories, as the tablet's data may already be spread across
any number of disks.

As this patch only addresses block placement, it does not itself
mitigate single-disk failure. Given this, and given the tradeoff
between I/O and failure disk-failure tolerance, the default behavior
will be to spread tablet data across all disks.

Testing is done at the block manager level in block_manager-test and
log_block_manager-test, as well as in the new data_dirs-test.

A design doc can be found here:
https://docs.google.com/document/d/1zZk-vb_ETKUuePcZ9ZqoSK2oPvAAaEV1sjDXes8Pxgk/edit?usp=sharing

Change-Id: I9828147f4fa5c4d7f6ed23441dca5a116b8cb11b
---
M src/kudu/cfile/bloomfile-test-base.h
M src/kudu/cfile/cfile-test-base.h
M src/kudu/cfile/cfile-test.cc
M src/kudu/fs/CMakeLists.txt
M src/kudu/fs/block_manager-stress-test.cc
M src/kudu/fs/block_manager-test.cc
M src/kudu/fs/block_manager.h
A 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/file_block_manager.h
M src/kudu/fs/fs.proto
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/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/tablet/delta_compaction-test.cc
M src/kudu/tablet/delta_compaction.cc
M src/kudu/tablet/delta_compaction.h
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/deltafile-test.cc
M src/kudu/tablet/deltamemstore-test.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/multi_column_writer.cc
M src/kudu/tablet/multi_column_writer.h
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tablet/tablet_metadata.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_client.h
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
37 files changed, 972 insertions(+), 176 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/6636/19
-- 
To view, visit http://gerrit.cloudera.org:8080/6636
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9828147f4fa5c4d7f6ed23441dca5a116b8cb11b
Gerrit-PatchSet: 19
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] WIP Don't suicide on EIO

[Andrew Wong (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: WIP Don't suicide on EIO
..

WIP Don't suicide on EIO

Rather than suiciding when reaching an EIO, this patch adds a
mechanism that triggers error-handling in the form of a callback.
This handler is attached to the lowest non-env operations that may
result in EIO.
E.g. PosixRWFile::Write() is an env operation that may result in an
EIO. LogBlockContainer::WriteData()'s call to it must be wrapped in
the new error-handling macro KUDU_RETURN_OR_HANDLE_EIO. Thus, all
direct readers/writers of files must now implement EIO-handling code
and wrap disk IO in KUDU_RETURN_OR_HANDLE_EIO.

Also included is a new tablet data state called TABLET_DATA_CORRUPTED
in which tablet data will not be deleted until explicitly requested
or during a tablet copy.

This patch is marked WIP, as the correct behavior after a disk fails
is not yet included. For now, upon failure, the block managers will
keep track of the dead disks and will mark the appropriate tablet
peers as failed. Additionally, the error handling has been moved
between layers offline, so more cleanup is being done to align with
the above description.

Change-Id: I4c221a5ea97c7b7ec5f8d395b5527dc16e63f828
---
M src/kudu/consensus/consensus_peers.cc
M src/kudu/fs/block_manager-test.cc
M src/kudu/fs/block_manager.h
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
A src/kudu/fs/error_manager.h
M src/kudu/fs/file_block_manager.cc
M src/kudu/fs/file_block_manager.h
M src/kudu/fs/fs_manager.cc
M src/kudu/fs/fs_manager.h
M src/kudu/fs/log_block_manager.cc
M src/kudu/fs/log_block_manager.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tablet/tablet_metadata.h
M src/kudu/tablet/tablet_peer_mm_ops.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
M src/kudu/util/fault_injection.cc
M src/kudu/util/fault_injection.h
M src/kudu/util/status.h
28 files changed, 519 insertions(+), 60 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4c221a5ea97c7b7ec5f8d395b5527dc16e63f828
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-2001 Add UNDO size to tablet on-disk size

[Will Berkeley (Code Review)]

Will Berkeley has posted comments on this change.

Change subject: KUDU-2001 Add UNDO size to tablet on-disk size
..


Patch Set 2:

> Build Failed
 > 
 > http://104.196.14.100/job/kudu-gerrit/7839/ : FAILURE

Known, unrelated failure.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: No

[kudu-CR] Refactor ConsensusStatePB to hold committed and pending configs

[Will Berkeley (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Refactor ConsensusStatePB to hold committed and pending configs
..

Refactor ConsensusStatePB to hold committed and pending configs

This patch refactors ConsensusStatePB so it contains both the
committed config and, if there is a pending config, the pending
config. Previously, consumers of the consensus state would ask
for either the committed or active (= pending if there is a pending
config, else the committed config) config. With this change, the
committed and active configs are always included.

The addition of pending info to the ConsensusStatePB will be used
by ksck in a follow-up patch. Additionally, the master may use
this info in the future to be more aware of the health of tablets.

Change-Id: I4bc4bdd9752fc29a7ce2cefcdc95c4366f5353af
---
M src/kudu/consensus/consensus.h
M src/kudu/consensus/consensus.proto
M src/kudu/consensus/consensus_meta-test.cc
M src/kudu/consensus/consensus_meta.cc
M src/kudu/consensus/consensus_meta.h
M src/kudu/consensus/metadata.proto
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/consensus/raft_consensus_state.cc
M src/kudu/consensus/raft_consensus_state.h
M src/kudu/integration-tests/cluster_itest_util.cc
M src/kudu/integration-tests/cluster_itest_util.h
M src/kudu/integration-tests/delete_table-itest.cc
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/integration-tests/ts_tablet_manager-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master-path-handlers.cc
M src/kudu/master/master.proto
M src/kudu/master/sys_catalog.cc
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tserver/tablet_copy.proto
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_client.h
M src/kudu/tserver/tablet_copy_service.cc
M src/kudu/tserver/tablet_copy_source_session.cc
M src/kudu/tserver/tablet_copy_source_session.h
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/tserver-path-handlers.cc
34 files changed, 331 insertions(+), 301 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4bc4bdd9752fc29a7ce2cefcdc95c4366f5353af
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] Refactor ConsensusStatePB to hold committed and pending configs

[Will Berkeley (Code Review)]

Will Berkeley has posted comments on this change.

Change subject: Refactor ConsensusStatePB to hold committed and pending configs
..


Patch Set 6:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/6809/6/src/kudu/consensus/metadata.proto
File src/kudu/consensus/metadata.proto:

PS6, Line 103: There is a corner case in Raft where a node may be elected 
leader of a
 :   // pending (uncommitted) configuration. In such a case, the 
master will
 :   // detect that the node is not a member of the committed 
configuration.
> I think you can just say: The leader may be a part of the committed or the 
Done


PS6, Line 113: 5
> 4
Done


http://gerrit.cloudera.org:8080/#/c/6809/6/src/kudu/consensus/quorum_util.cc
File src/kudu/consensus/quorum_util.cc:

PS6, Line 195:  
> nit: stray space here
Done


PS6, Line 199:   
> nit: funny extra indentation
Done


Line 219: string DiffConsensusStates(const ConsensusStatePB& old_state,
> Looks like this needs to detect differences in pending configs now too.
Done


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

PS6, Line 2235:   
> nit: indent at least an extra 4 spaces for a line wrap
Done


PS6, Line 2362: Previously, this field was omitted from the
  : // consensus state in this situation.
> I don't think this helps clarify this code, consider removing this part of 
Done


http://gerrit.cloudera.org:8080/#/c/6809/6/src/kudu/master/sys_catalog.cc
File src/kudu/master/sys_catalog.cc:

Line 134: RETURN_NOT_OK(consensus::VerifyConsensusState(cstate));
> Add something like CHECK(!cstate.has_pending_config()) since we should neve
Done


http://gerrit.cloudera.org:8080/#/c/6809/6/src/kudu/tserver/tablet_copy.proto
File src/kudu/tserver/tablet_copy.proto:

PS6, Line 116: committed 
> remove this word
Done


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

PS6, Line 951: *reported_tablet->mutable_consensus_state() =
 : consensus->ConsensusState();
> nit: This will fit on one line now
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4bc4bdd9752fc29a7ce2cefcdc95c4366f5353af
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes

[kudu-CR] log: shut down appender thread when idle

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: log: shut down appender thread when idle
..


Patch Set 1:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/6856/1/src/kudu/consensus/log-test.cc
File src/kudu/consensus/log-test.cc:

Line 1105:   ASSERT_TRUE(log_->append_thread_active_for_tests());
Could be flaky if enough time passes between Append() and now, right?


http://gerrit.cloudera.org:8080/#/c/6856/1/src/kudu/consensus/log.cc
File src/kudu/consensus/log.cc:

Line 203:   // Tries to transition back to WORKER_STOPPED state. If successful, 
returns true.
Nit: Add a blank line before.


Line 229:   gscoped_ptr append_pool_;
Given this design and given that the append thread is blocked on IO when 
running, I take it we don't want to consolidate pools across tablets?


Line 243: .set_idle_timeout(MonoDelta::FromSeconds(0))
Add a comment for this? I'm guessing it's because we handle idleness ourselves, 
so there's no reason for the threadpool to keep a thread alive if we've deemed 
it unnecessary.


Line 248: void Log::AppendThread::Wake() {
DCHECK(append_pool_) maybe?


Line 256:   // Stopping is a bit tricky. We have to consider the following race:
Do we have any unit tests that can semi-reliably produce this race? A 
multi-threaded stress test for the log?


Line 288: void Log::AppendThread::DoWork() {
Maybe DCHECK that worker_state_ is WORKER_ACTIVE?


Line 365: append_pool_->Wait();
Why call Wait() prior to Shutdown()? IIRC, Shutdown() will wait for outstanding 
tasks and cancel all the pending tasks; isn't that what we want?


http://gerrit.cloudera.org:8080/#/c/6856/1/src/kudu/util/blocking_queue-test.cc
File src/kudu/util/blocking_queue-test.cc:

Line 64:   ASSERT_OK(test_queue.BlockingDrainTo(, MonoTime::Now() + 
MonoDelta::FromSeconds(5)));
May be flaky if enough time passes between creation of the deadline and the 
call to BlockingDrainTo. Maybe make it obnoxiously higher, like 30s?


http://gerrit.cloudera.org:8080/#/c/6856/1/src/kudu/util/threadpool.h
File src/kudu/util/threadpool.h:

Line 89: // timeout: How long we'll keep around an idle thread before timing it 
out.
Change this to idle_timeout too?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id40a4cfcec96198b537c2f50be7ff1204faf96d2
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [catalog manager tsk-itest] shortened test's run-time

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: [catalog_manager_tsk-itest] shortened test's run-time
..


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I21cca3d53d033f56f60e46fb5a3f67f5d7e44122
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] KUDU-586. Upgrade to protobuf 3

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: KUDU-586. Upgrade to protobuf 3
..


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I30986ebe56ebb334db8f89f1652e7ef2b064925c
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] faststring: add shrink to fit()

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged.

Change subject: faststring: add shrink_to_fit()
..


faststring: add shrink_to_fit()

This adds a shrink_to_fit() method which reallocates the underlying buffer of a
faststring to match its current length. This is useful in the case where a
faststring acts as a long-lived buffer which occasionally gets large values,
but often contains small ones.

Change-Id: I0e437ff180fccd1957d252fb9a3551bb91ba7917
Reviewed-on: http://gerrit.cloudera.org:8080/6835
Reviewed-by: David Ribeiro Alves 
Tested-by: Kudu Jenkins
---
M src/kudu/util/CMakeLists.txt
A src/kudu/util/faststring-test.cc
M src/kudu/util/faststring.cc
M src/kudu/util/faststring.h
4 files changed, 96 insertions(+), 6 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I0e437ff180fccd1957d252fb9a3551bb91ba7917
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-586. Upgrade to protobuf 3

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: KUDU-586. Upgrade to protobuf 3
..


Patch Set 2: Code-Review+1

Will upload 3.3 later this morning

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I30986ebe56ebb334db8f89f1652e7ef2b064925c
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] KUDU-586. Upgrade to protobuf 3

[Grant Henke (Code Review)]

Grant Henke has uploaded a new patch set (#2).

Change subject: KUDU-586. Upgrade to protobuf 3
..

KUDU-586. Upgrade to protobuf 3

Upgrades Kudu to use protoc 3.3.0 and
adds `syntax = "proto2”;` to the proto files.

Does not leverage the new protobuf allocation arenas,
but that should be considered in a follow up patch.

Change-Id: I30986ebe56ebb334db8f89f1652e7ef2b064925c
---
M src/kudu/cfile/cfile.proto
M src/kudu/client/client.proto
M src/kudu/common/common.proto
M src/kudu/common/wire_protocol.proto
M src/kudu/consensus/consensus.proto
M src/kudu/consensus/log.proto
M src/kudu/consensus/metadata.proto
M src/kudu/consensus/opid.proto
M src/kudu/fs/fs.proto
M src/kudu/master/master.proto
M src/kudu/rpc/rpc_header.proto
M src/kudu/rpc/rpc_introspection.proto
M src/kudu/rpc/rtest.proto
M src/kudu/rpc/rtest_diff_package.proto
M src/kudu/security/token.proto
M src/kudu/server/server_base.proto
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/tablet.proto
M src/kudu/tserver/tablet_copy.proto
M src/kudu/tserver/tserver.proto
M src/kudu/tserver/tserver_admin.proto
M src/kudu/tserver/tserver_service.proto
M src/kudu/util/compression/compression.proto
M src/kudu/util/histogram.proto
M src/kudu/util/jsonwriter_test.proto
M src/kudu/util/maintenance_manager.proto
M src/kudu/util/pb_util.proto
M src/kudu/util/pb_util_test.proto
M src/kudu/util/proto_container_test.proto
M src/kudu/util/proto_container_test2.proto
M src/kudu/util/proto_container_test3.proto
M src/kudu/util/version_info.proto
M thirdparty/vars.sh
33 files changed, 34 insertions(+), 2 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I30986ebe56ebb334db8f89f1652e7ef2b064925c
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-1988: add support for advertised host:port info.

[Patrik Sundberg (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1988: add support for advertised host:port info.
..

KUDU-1988: add support for advertised host:port info.

Change-Id: I6735ca5630fc4c426bf72d0b21d6ef452173a890
---
M src/kudu/master/master.cc
M src/kudu/server/CMakeLists.txt
A src/kudu/server/rpc_server-test.cc
M src/kudu/server/rpc_server.cc
M src/kudu/server/rpc_server.h
M src/kudu/server/webserver-test.cc
M src/kudu/server/webserver.cc
M src/kudu/server/webserver.h
M src/kudu/server/webserver_options.cc
M src/kudu/server/webserver_options.h
M src/kudu/tserver/heartbeater.cc
11 files changed, 300 insertions(+), 4 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6735ca5630fc4c426bf72d0b21d6ef452173a890
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Patrik Sundberg 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Patrik Sundberg 
Gerrit-Reviewer: Tidy Bot

[kudu-CR] [catalog manager tsk-itest] shortened test's run-time

[Alexey Serbin (Code Review)]

Hello David Ribeiro Alves, Kudu Jenkins,

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

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

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

Change subject: [catalog_manager_tsk-itest] shortened test's run-time
..

[catalog_manager_tsk-itest] shortened test's run-time

The longer the test runs the more tables it creates and drops,
and that makes switching from one leader master to another longer.
The presence of some parallel activity exacerbates the problem because
re-elections happen more often.  That leads to timeouts on operations
performed by the client.

This patch shortens the test's run time to make it less flaky if running
on inferior VMs with other concurrent activity.

Change-Id: I21cca3d53d033f56f60e46fb5a3f67f5d7e44122
---
M src/kudu/integration-tests/catalog_manager_tsk-itest.cc
1 file changed, 9 insertions(+), 4 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I21cca3d53d033f56f60e46fb5a3f67f5d7e44122
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-1192 Periodically flush glog buffers from a thread

[William Li (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1192 Periodically flush glog buffers from a thread
..

KUDU-1192 Periodically flush glog buffers from a thread

Added a wait timeout to AsyncLogger RunThread based on FLAGS_logbufsec
When the wait times out, it will flush even if there is nothing enqueued.

Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
---
M src/kudu/util/async_logger.cc
M src/kudu/util/logging-test.cc
2 files changed, 27 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: William Li 

[kudu-CR] [catalog manager tsk-itest] shortened test's run-time

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: [catalog_manager_tsk-itest] shortened test's run-time
..


Patch Set 1:

> have you tested this on dist-test with stress? +2ing in case yes
 > but if not I'd prefer we do so as to avoid having to re-fix

I tested it with dist-test using --stress_cpu_threads=8 running 256 iterations 
in DEBUG and TSAN build configurations: 256 out of 256 passed.  Prior to this 
fix ~30 out of 256 failed in both.

Probably, I should run 1K multiples.  Let me do it just to avoid returning to 
this again.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I21cca3d53d033f56f60e46fb5a3f67f5d7e44122
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] [catalog manager tsk-itest] shortened test's run-time

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: [catalog_manager_tsk-itest] shortened test's run-time
..


Patch Set 1: Code-Review+2

have you tested this on dist-test with stress? +2ing in case yes but if not I'd 
prefer we do so as to avoid having to re-fix

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I21cca3d53d033f56f60e46fb5a3f67f5d7e44122
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] KUDU-1192 Periodically flush glog buffers from a thread

[William Li (Code Review)]

William Li has posted comments on this change.

Change subject: KUDU-1192 Periodically flush glog buffers from a thread
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6853/3/src/kudu/util/async_logger.cc
File src/kudu/util/async_logger.cc:

Line 121: active_buf_->flush = true;
> we only want to do this if we haven't flushed in the configured amount of t
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: William Li 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1192 Periodically flush glog buffers from a thread

[William Li (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1192 Periodically flush glog buffers from a thread
..

KUDU-1192 Periodically flush glog buffers from a thread

Added a wait timeout to AsyncLogger RunThread based on FLAGS_logbufsec
When wait times out, it will flush even if there is nothing enqueued.

Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
---
M src/kudu/util/async_logger.cc
M src/kudu/util/logging-test.cc
2 files changed, 27 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: William Li 

[kudu-CR] log: fix some incorrect assumptions on BlockingQueue shutdown

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged.

Change subject: log: fix some incorrect assumptions on BlockingQueue shutdown
..


log: fix some incorrect assumptions on BlockingQueue shutdown

There was an incorrect comment in the Log::AppendThread body in which it
claimed that BlockingQueue::Drain would potentially return false and
also return elements at the same time.

This was in fact not the case. This patch adds unit tests to verify
the actual behavior, and simplifies the code accordingly.

Change-Id: I43e1304ce242f2def26a29504b76bb3e74a93ca3
Reviewed-on: http://gerrit.cloudera.org:8080/6855
Reviewed-by: Adar Dembo 
Tested-by: Kudu Jenkins
---
M src/kudu/consensus/log.cc
M src/kudu/util/blocking_queue-test.cc
M src/kudu/util/blocking_queue.h
3 files changed, 35 insertions(+), 10 deletions(-)

Approvals:
  Adar Dembo: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I43e1304ce242f2def26a29504b76bb3e74a93ca3
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon