date:20171031

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

Change subject: KUDU-2193 (part 1): switch to a waiting mutex in TSTabletManager
..

KUDU-2193 (part 1): switch to a waiting mutex in TSTabletManager

The TSTabletManager map is sometimes held for a long time. Given that, a
sleeping mutex is more appropriate than a spinlock.

Change-Id: I763abddd74d8b1dabb618318dc84256b533077e3
Reviewed-on: http://gerrit.cloudera.org:8080/8345
Reviewed-by: Adar Dembo 
Tested-by: Kudu Jenkins
---
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
2 files changed, 25 insertions(+), 24 deletions(-)

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

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

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

[kudu-CR] KUDU-2193 (part 2): avoid holding TSTabletManager::lock for a long time

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

Change subject: KUDU-2193 (part 2): avoid holding TSTabletManager::lock_ for a 
long time
..

KUDU-2193 (part 2): avoid holding TSTabletManager::lock_ for a long time

This changes tablet report generation to only hold the TSTabletManager lock
long enough to copy a list of refs to the relevant tablets.

Even though the lock is a reader-writer lock, a long read-side critical
section can end up blocking other readers as long as any writer shows up.
I saw the following in a cluster:

- election storm ongoing
- T1 generating a tablet report (thus holding the reader lock)
  - blocks for a long time getting ConsensusState from some tablets currently
mid-fsync.
- T2 handling a DeleteTablet or CreateTablet call (waiting for writer lock)
- T3 through T20: blocking on reader lock in LookupTablet()

The effect here is that all other threads end up blocked until T1 finishes
its tablet report generation, which in this case can be tens of seconds due
to all of the fsync traffic. These blocked threads then contribute to the
ongoing election storm since they may delay timely responses to vote requests
from other tablets.

I tested this and the previous patch on a cluster that was previously
experiencing the issue. I triggered some elections by kill -STOPping some
servers and resuming them a few seconds later. Without these patches,
other servers started doing lots of context switches (due to the spinlock
used prior to this patch series) and I saw lots of blocked threads in
pstack. With the patch, things seem cleaner.

The following screenshot shows before/after voluntary_context_switch rate:

https://i.imgur.com/7zcbImw.png

(the missing data around 5pm is my restarting the servers with this patch)

Change-Id: I0f645775e8347a18af112b308dba56a3b4a2c681
Reviewed-on: http://gerrit.cloudera.org:8080/8346
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
2 files changed, 31 insertions(+), 18 deletions(-)

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

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

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

[kudu-CR] KUDU-2193 (part 2): avoid holding TSTabletManager::lock for a long time

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

Change subject: KUDU-2193 (part 2): avoid holding TSTabletManager::lock_ for a 
long time
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0f645775e8347a18af112b308dba56a3b4a2c681
Gerrit-Change-Number: 8346
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 01 Nov 2017 04:44:01 +
Gerrit-HasComments: No

[kudu-CR] [mini-cluster] allow 18-bit PID in LOOPBACK bind mode

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

Change subject: [mini-cluster] allow 18-bit PID in LOOPBACK bind mode
..

[mini-cluster] allow 18-bit PID in LOOPBACK bind mode

This changelist allows to use 18-bit PIDs for binding in LOOPBACK mode.
Prior to this patch, the minicluster could not use PIDs wider
than 16 bits.

While expanding the range of acceptable PIDs, this changelist also
decreases the maximum allowed number of 'indexed servers' from 254
to 62.  As of now, that's enough for our minicluster-based tests.

The incentive to put up this patch was change of the max PID
up to 147456 in /proc/sys/kernel/pid_max after recent restart of
one of our build servers.

Change-Id: I958729d71ed363e98bea99f3d932cc6b0546e130
Reviewed-on: http://gerrit.cloudera.org:8080/8422
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
M src/kudu/mini-cluster/mini_cluster.cc
1 file changed, 20 insertions(+), 7 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I958729d71ed363e98bea99f3d932cc6b0546e130
Gerrit-Change-Number: 8422
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] [mini-cluster] allow 18-bit PID in LOOPBACK bind mode

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

Change subject: [mini-cluster] allow 18-bit PID in LOOPBACK bind mode
..


Patch Set 4: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I958729d71ed363e98bea99f3d932cc6b0546e130
Gerrit-Change-Number: 8422
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 01 Nov 2017 04:43:03 +
Gerrit-HasComments: No

[kudu-CR] fs: store opts at construction time

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

Change subject: fs: store opts at construction time
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8437/1/src/kudu/fs/fs_manager.cc
File src/kudu/fs/fs_manager.cc:

http://gerrit.cloudera.org:8080/#/c/8437/1/src/kudu/fs/fs_manager.cc@333
PS1, Line 333:   canonicalized_data_fs_roots_, std::move(dm_opts), 
&dd_manager_));
> But it's not actually accessing dm_opts after the move, is it? The LOG_TIMI
right, but clang-tidy isn't smart enough to analyze the loop and know that it 
always only runs for one iteration. I'll see if I can come up with a 
non-looping variant for LOG_TIMING perhaps



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If64ec78f28b5c6879805a682e209d96846f03f27
Gerrit-Change-Number: 8437
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 01 Nov 2017 04:32:58 +
Gerrit-HasComments: Yes

[kudu-CR] fs: store opts at construction time

2017-10-31 Thread Andrew Wong (Code Review)

Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8437 )

Change subject: fs: store opts at construction time
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If64ec78f28b5c6879805a682e209d96846f03f27
Gerrit-Change-Number: 8437
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 01 Nov 2017 02:12:58 +
Gerrit-HasComments: No

[kudu-CR] fs: store opts at construction time

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

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

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

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

Change subject: fs: store opts at construction time
..

fs: store opts at construction time

This is easier than adding a new member for each new option.

Note: I removed the options destructors so that the compiler would
generate move constructors; see commit 312492d for more information.

Change-Id: If64ec78f28b5c6879805a682e209d96846f03f27
---
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/fs/block_manager-stress-test.cc
M src/kudu/fs/block_manager-test.cc
M src/kudu/fs/block_manager.cc
M src/kudu/fs/block_manager.h
M src/kudu/fs/data_dirs-test.cc
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
M src/kudu/fs/file_block_manager.cc
M src/kudu/fs/file_block_manager.h
M src/kudu/fs/fs_manager-test.cc
M src/kudu/fs/fs_manager.cc
M src/kudu/fs/fs_manager.h
M src/kudu/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/integration-tests/log_verifier.cc
M src/kudu/integration-tests/open-readonly-fs-itest.cc
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/master/mini_master.cc
M src/kudu/server/server_base.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/tserver/tablet_copy_client-test.cc
26 files changed, 171 insertions(+), 185 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If64ec78f28b5c6879805a682e209d96846f03f27
Gerrit-Change-Number: 8437
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] fs: store opts at construction time

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

Change subject: fs: store opts at construction time
..


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/8437/1/src/kudu/fs/fs_manager.h
File src/kudu/fs/fs_manager.h:

http://gerrit.cloudera.org:8080/#/c/8437/1/src/kudu/fs/fs_manager.h@68
PS1, Line 68:   // 'data_paths', which are both initialized to 'root'.
> nit: note that this is only used in tests?
Done


http://gerrit.cloudera.org:8080/#/c/8437/1/src/kudu/fs/fs_manager.cc
File src/kudu/fs/fs_manager.cc:

http://gerrit.cloudera.org:8080/#/c/8437/1/src/kudu/fs/fs_manager.cc@134
PS1, Line 134: wal_path
> nit: can we use "wal_root" and "data_roots" instead?
Done


http://gerrit.cloudera.org:8080/#/c/8437/1/src/kudu/fs/fs_manager.cc@320
PS1, Line 320:   if (!opts_.read_only) {
> nit: !read_only()?
Eh, there are quite a few references to opts_.read_only in fs_manager.cc, and I 
think that's OK; read_only() is largely for external callers.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If64ec78f28b5c6879805a682e209d96846f03f27
Gerrit-Change-Number: 8437
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 01 Nov 2017 01:44:52 +
Gerrit-HasComments: Yes

[kudu-CR] fs: store opts at construction time

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

Change subject: fs: store opts at construction time
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8437/1/src/kudu/fs/fs_manager.cc
File src/kudu/fs/fs_manager.cc:

http://gerrit.cloudera.org:8080/#/c/8437/1/src/kudu/fs/fs_manager.cc@333
PS1, Line 333:   canonicalized_data_fs_roots_, std::move(dm_opts), 
&dd_manager_));
> hrm, this seems like it's due to the implementation of LOG_TIMING using a f
But it's not actually accessing dm_opts after the move, is it? The LOG_TIMING 
loop is exited on its second iteration, right?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If64ec78f28b5c6879805a682e209d96846f03f27
Gerrit-Change-Number: 8437
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 01 Nov 2017 01:27:51 +
Gerrit-HasComments: Yes

[kudu-CR](branch-1.2.x) [client-test] Reduce flakyness of TestWriteWithDeadTabletServer

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

Change subject: [client-test] Reduce flakyness of TestWriteWithDeadTabletServer
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Ice347a30c71b1cb3937cd1b30b0744fcd2dae0cf
Gerrit-Change-Number: 8429
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Wed, 01 Nov 2017 01:26:46 +
Gerrit-HasComments: No

[kudu-CR] fs: store opts at construction time

2017-10-31 Thread Andrew Wong (Code Review)

Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8437 )

Change subject: fs: store opts at construction time
..


Patch Set 1:

(3 comments)

Some nits

http://gerrit.cloudera.org:8080/#/c/8437/1/src/kudu/fs/fs_manager.h
File src/kudu/fs/fs_manager.h:

http://gerrit.cloudera.org:8080/#/c/8437/1/src/kudu/fs/fs_manager.h@68
PS1, Line 68:   // 'data_paths', which are both initialized to 'root'.
nit: note that this is only used in tests?


http://gerrit.cloudera.org:8080/#/c/8437/1/src/kudu/fs/fs_manager.cc
File src/kudu/fs/fs_manager.cc:

http://gerrit.cloudera.org:8080/#/c/8437/1/src/kudu/fs/fs_manager.cc@134
PS1, Line 134: wal_path
nit: can we use "wal_root" and "data_roots" instead?
"path" seems particularly overloaded since it can generally refer to any 
arbitrary file/directory. "wal_path" and "data_path" could then potentially be 
misconstrued as a path pointing at a WAL file or some data file, whatever that 
might mean.


http://gerrit.cloudera.org:8080/#/c/8437/1/src/kudu/fs/fs_manager.cc@320
PS1, Line 320:   if (!opts_.read_only) {
nit: !read_only()?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If64ec78f28b5c6879805a682e209d96846f03f27
Gerrit-Change-Number: 8437
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 01 Nov 2017 01:25:05 +
Gerrit-HasComments: Yes

[kudu-CR] fs: store opts at construction time

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

Change subject: fs: store opts at construction time
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8437/1/src/kudu/fs/fs_manager.cc
File src/kudu/fs/fs_manager.cc:

http://gerrit.cloudera.org:8080/#/c/8437/1/src/kudu/fs/fs_manager.cc@333
PS1, Line 333:   canonicalized_data_fs_roots_, std::move(dm_opts), 
&dd_manager_));
> warning: 'dm_opts' used after it was moved [misc-use-after-move]
hrm, this seems like it's due to the implementation of LOG_TIMING using a for 
loop. Maybe we can use some other trick for the macro



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If64ec78f28b5c6879805a682e209d96846f03f27
Gerrit-Change-Number: 8437
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 01 Nov 2017 01:23:14 +
Gerrit-HasComments: Yes

[kudu-CR] fs: store opts at construction time

Hello Andrew Wong,

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

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

to review the following change.


Change subject: fs: store opts at construction time
..

fs: store opts at construction time

This is easier than adding a new member for each new option.

Note: I removed the options destructors so that the compiler would
generate move constructors; see commit 312492d for more information.

Change-Id: If64ec78f28b5c6879805a682e209d96846f03f27
---
M src/kudu/fs/block_manager-stress-test.cc
M src/kudu/fs/block_manager-test.cc
M src/kudu/fs/block_manager.cc
M src/kudu/fs/block_manager.h
M src/kudu/fs/data_dirs-test.cc
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
M src/kudu/fs/file_block_manager.cc
M src/kudu/fs/file_block_manager.h
M src/kudu/fs/fs_manager-test.cc
M src/kudu/fs/fs_manager.cc
M src/kudu/fs/fs_manager.h
M src/kudu/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/server/server_base.cc
M src/kudu/tools/tool_action_fs.cc
17 files changed, 136 insertions(+), 152 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: If64ec78f28b5c6879805a682e209d96846f03f27
Gerrit-Change-Number: 8437
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Andrew Wong

[kudu-CR] mvcc: allow tablet shutdown without completing txs

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

Change subject: mvcc: allow tablet shutdown without completing txs
..

Patch Set 21:

(13 comments)

http://gerrit.cloudera.org:8080/#/c/7439/21/src/kudu/integration-tests/stop_tablet-itest.cc
File src/kudu/integration-tests/stop_tablet-itest.cc:

http://gerrit.cloudera.org:8080/#/c/7439/21/src/kudu/integration-tests/stop_tablet-itest.cc@108
PS21, Line 108: // Insert data until the tablet becomes visible to the
server.
you don't need to insert data to get it to be visible - just creating the table
should be enough to create the tablet, right?

http://gerrit.cloudera.org:8080/#/c/7439/21/src/kudu/integration-tests/stop_tablet-itest.cc@202
PS21, Line 202: // Now that the leader has been stopped, wait longer and
ensure that no
: // more writes get through.
: SleepFor(MonoDelta::FromSeconds(5));
: ASSERT_EQ(num_rows_after_stop, writes.rows_inserted());
this is odd. Wouldn't you expect it to re-elect a new leader? Or is this
because you're reaching into the server and calling 'Stop()' but not actually
shutting down the tablet, so the leader keeps sending heartbeats? Worth a
comment.

http://gerrit.cloudera.org:8080/#/c/7439/21/src/kudu/tablet/mvcc.h
File src/kudu/tablet/mvcc.h:

http://gerrit.cloudera.org:8080/#/c/7439/21/src/kudu/tablet/mvcc.h@276
PS21, Line 276: should
nit: s/should/will/

http://gerrit.cloudera.org:8080/#/c/7439/21/src/kudu/tablet/mvcc.h@280
PS21, Line 280: inline
> Is "inline" useful here? I thought methods defined in their header declarat
yea, not necessary

http://gerrit.cloudera.org:8080/#/c/7439/21/src/kudu/tablet/mvcc.h@280
PS21, Line 280: is_closed
> nit: Perhaps is_open() would be more intuitive than is_closed()? This is be
I could go either way on this. I think it's natural to talk about whether
something is or is not closed (I don't interpret "closed" as a negative
adjective in the same way as "disabled")

http://gerrit.cloudera.org:8080/#/c/7439/21/src/kudu/tablet/mvcc.h@281
PS21, Line 281: .load()
> Can we get away with std::memory_order_relaxed?
I actually looked at the assembly generated by the default memory order, and
load() just turns into a normal load instruction with nothing expensive about
it, so I think this is fine as is.

http://gerrit.cloudera.org:8080/#/c/7439/21/src/kudu/tablet/mvcc.h@382
PS21, Line 382: closed_
> perhaps open_ and default to true?
I kinda disagree on this one since we often have this "shutdown_" type member
variable.

http://gerrit.cloudera.org:8080/#/c/7439/21/src/kudu/tablet/tablet.h
File src/kudu/tablet/tablet.h:

http://gerrit.cloudera.org:8080/#/c/7439/21/src/kudu/tablet/tablet.h@121
PS21, Line 121: Unlike Shutdown(), this is not a lifecycle change, but rather a
functional
: // one. Calls to this are orthogonal to calls to Shutdown().
> It's not clear when one vs. the other should be called. Can you clarify her
yea, agree this can still be clarified better whether it's required or
optional, should be called before/after Shutdown(), etc.

http://gerrit.cloudera.org:8080/#/c/7439/21/src/kudu/tablet/tablet.h@175
PS21, Line 175: error the tablet
nit: missing "if"

http://gerrit.cloudera.org:8080/#/c/7439/21/src/kudu/tablet/tablet.h@176
PS21, Line 176: It is critical for
: // transactional integrity that errors here Stop() the tablet.
this bit of the comment is confusing in this context.

General rule of thumb for header comments is that you should try to focus on
what external callers need to know about the function, and be clear which
things are "instructions" for the caller vs "FYI" for the caller, vs
implementation details (which probably should go in the .cc instead).

In this particular example, the caller might be confused by the phrasing --
does this mean that, if this returns an error, the caller is responsible for
calling Stop()? Or that if this returns an error, the tablet is guaranteed to
have already stopped itself?

Structuring it as a positive statement on a post-condition may help make it
clearer: "If this returns an error, the Tablet will be in stopped state." (and
then if you feel it's critical to explain why this is critical for maintaining
correctness, do so in the implementation?)

http://gerrit.cloudera.org:8080/#/c/7439/21/src/kudu/tablet/tablet.h@183
PS21, Line 183: // Returns an error the tablet has been stopped or if there
was an error
: // (e.g. a disk failure) Applying the operations. It is
critical for
: // transactional integrity that errors here Stop() the tablet.
same

http://gerrit.cloudera.org:8080/#/c/7439/21/src/kudu/tablet/tablet.h@480
PS21, Line 480: // Returns an error the tablet has been stopped or if there
was an error
: // (e.g. a disk failure) when checking the keys. It

[kudu-CR] KUDU-1809: Add batch size configuration for ScanToken API

2017-10-31 Thread Hao Hao (Code Review)

Hao Hao has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/8435


Change subject: KUDU-1809: Add batch size configuration for ScanToken API
..

KUDU-1809: Add batch size configuration for ScanToken API

Both C++/Java client implementation of ScanToken API ignore the scan
batch size configuration option. This patch adds this option into
ScanToken protobuf message. It also adds the logic to set this field
through KuduScanToken and then properly takes effect on the deserialized
scanners.

Change-Id: Ie147ba4aa8e457c7063f7bb5b51b4e7e0bd1fe73
---
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
M src/kudu/client/client.proto
M src/kudu/client/scan_token-internal.cc
M src/kudu/client/scan_token-test.cc
5 files changed, 24 insertions(+), 2 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie147ba4aa8e457c7063f7bb5b51b4e7e0bd1fe73
Gerrit-Change-Number: 8435
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao

[kudu-CR] KUDU-2191 (2/n): Hive Metastore client

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

Change subject: KUDU-2191 (2/n): Hive Metastore client
..


Patch Set 36:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7053/30/src/kudu/hms/hms_client.h
File src/kudu/hms/hms_client.h:

http://gerrit.cloudera.org:8080/#/c/7053/30/src/kudu/hms/hms_client.h@85
PS30, Line 85: in the HMS.
 :   //
> Not in my opinion.  What advantage would that have?  This method will only
I'm generally against bool parameters for readability. I liked this C++ tip: 
https://abseil.io/tips/94


http://gerrit.cloudera.org:8080/#/c/7053/30/src/kudu/hms/hms_client.h@121
PS30, Line 121:   // Retrieves all tables in an HMS database.
> They can be very large for HDFS tables with a lot of partitions.  There's c
I guess there is no way to limit the request by a certain size? eg we might 
want to get 1000 events normally, but if we cross some threshold like 10MB then 
stop at that point?

I'm just not sure how we'll appropriately decide on a value for 'max_events' 
that is both high-throughput and memory-safe. 1000 events is probably fine most 
of the time, but if we get 1000x 10MB events in some catch-up scenario then all 
of a sudden we have memory issues to worry about.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-Change-Number: 7053
Gerrit-PatchSet: 36
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 31 Oct 2017 23:36:07 +
Gerrit-HasComments: Yes

[kudu-CR] mvcc: allow tablet shutdown without completing txs

2017-10-31 Thread Mike Percy (Code Review)

Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/7439 )

Change subject: mvcc: allow tablet shutdown without completing txs
..


Patch Set 21:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/7439/21/src/kudu/tablet/mvcc.h
File src/kudu/tablet/mvcc.h:

http://gerrit.cloudera.org:8080/#/c/7439/21/src/kudu/tablet/mvcc.h@277
PS21, Line 277: SetClosed
nit: Close() ?


http://gerrit.cloudera.org:8080/#/c/7439/21/src/kudu/tablet/mvcc.h@280
PS21, Line 280: is_closed
nit: Perhaps is_open() would be more intuitive than is_closed()? This is 
because closed is a negative and checking for is not closed is a bit of a 
boolean trap: https://ariya.io/2011/08/hall-of-api-shame-boolean-trap


http://gerrit.cloudera.org:8080/#/c/7439/21/src/kudu/tablet/mvcc.h@280
PS21, Line 280: inline
Is "inline" useful here? I thought methods defined in their header declarations 
were generally inlined anyway.


http://gerrit.cloudera.org:8080/#/c/7439/21/src/kudu/tablet/mvcc.h@281
PS21, Line 281: .load()
Can we get away with std::memory_order_relaxed?


http://gerrit.cloudera.org:8080/#/c/7439/21/src/kudu/tablet/mvcc.h@316
PS21, Line 316: CheckNotClosed
nit: CheckOpen() seems more intuitive to me per the above


http://gerrit.cloudera.org:8080/#/c/7439/21/src/kudu/tablet/mvcc.h@382
PS21, Line 382: closed_
perhaps open_ and default to true?


http://gerrit.cloudera.org:8080/#/c/7439/21/src/kudu/tablet/mvcc.cc
File src/kudu/tablet/mvcc.cc:

http://gerrit.cloudera.org:8080/#/c/7439/21/src/kudu/tablet/mvcc.cc@98
PS21, Line 98: closed_.load()
nit: is_closed()


http://gerrit.cloudera.org:8080/#/c/7439/21/src/kudu/tablet/mvcc.cc@280
PS21, Line 280: did
did not


http://gerrit.cloudera.org:8080/#/c/7439/21/src/kudu/tablet/tablet.h
File src/kudu/tablet/tablet.h:

http://gerrit.cloudera.org:8080/#/c/7439/21/src/kudu/tablet/tablet.h@121
PS21, Line 121: Unlike Shutdown(), this is not a lifecycle change, but rather a 
functional
  :   // one. Calls to this are orthogonal to calls to Shutdown().
It's not clear when one vs. the other should be called. Can you clarify here? 
Also, is it ever possible to go from Stopped() back to running? If not, this 
seems like a lifecycle change to me, despite the header comment here.


http://gerrit.cloudera.org:8080/#/c/7439/21/src/kudu/tablet/tablet_replica.cc
File src/kudu/tablet/tablet_replica.cc:

http://gerrit.cloudera.org:8080/#/c/7439/21/src/kudu/tablet/tablet_replica.cc@293
PS21, Line 293:  tablet_->Shutdown();
We call Tablet::Shutdown() here. So why not just have Tablet::Shutdown() call 
Tablet::Stop()?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I983620f27e7226806a2cca253db7619731914d42
Gerrit-Change-Number: 7439
Gerrit-PatchSet: 21
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 31 Oct 2017 23:00:20 +
Gerrit-HasComments: Yes

[kudu-CR] cfile set: make FindRow more robust to errors

Adar Dembo has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/8423 )

Change subject: cfile_set: make FindRow more robust to errors
..

cfile_set: make FindRow more robust to errors

Previously, FindRow() would not handle certain error cases well. I.e.
when seeking on a key, it wouldn't consider all errors before returning
the seek results.

For example, currently, FindRow() scans over CFiles and if 1) it hits a
Status::NotFound() error, or 2) the key search returned without being
an exact match, it returns success, returning the row is not present.

However, 2) does not take into account the OK-ness of the returned
scans, and thus may return healthily saying it couldn't find the row,
even though this may not be the case. This allows the currently-running
operation to continue under the potentially incorrect assumption that
the key is not present, when it should actually return with the
appropriate error.

In the case of DuplicatingRowSet::MutateRow(), this is particularly
nasty: it expects operations to be mirrored to two separate rowsets, and
the above scenario may present itself as a fatal failure to do so.

In practice, this hasn't been an issue because we might only expect
various fs-level errors (e.g. corruption, disk errors) here, which we
already don't handle gracefully. However, this needs to change to be
robust to disk failures.

Similarly, FindRow() will now return early if it hits a disk failure in
the bloom look-up.

A small test is added to diskrowset-test to ensure errors are returned
appropriately.

Change-Id: I41b56cbbf1b3b00fdb4dbf1ec08f12ced97088e7
Reviewed-on: http://gerrit.cloudera.org:8080/8423
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo 
---
M src/kudu/tablet/cfile_set.cc
M src/kudu/tablet/diskrowset-test.cc
2 files changed, 37 insertions(+), 1 deletion(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I41b56cbbf1b3b00fdb4dbf1ec08f12ced97088e7
Gerrit-Change-Number: 8423
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] cfile set: make FindRow more robust to errors

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

Change subject: cfile_set: make FindRow more robust to errors
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I41b56cbbf1b3b00fdb4dbf1ec08f12ced97088e7
Gerrit-Change-Number: 8423
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 31 Oct 2017 22:43:17 +
Gerrit-HasComments: No

[kudu-CR] Add log parser script

2017-10-31 Thread Mike Percy (Code Review)

Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8229 )

Change subject: Add log parser script
..


Patch Set 1:

(3 comments)

Thanks for taking a look at this, Andrew!

http://gerrit.cloudera.org:8080/#/c/8229/1/src/kudu/scripts/kudu-log-parser.pl
File src/kudu/scripts/kudu-log-parser.pl:

http://gerrit.cloudera.org:8080/#/c/8229/1/src/kudu/scripts/kudu-log-parser.pl@5
PS1, Line 5: # Pass it a list of logs from a whole cluster.
> For the perl-uninitiated, maybe a sample shell line?
There is a usage message that gets printed if the script is run without 
arguments. I'll note that here.


http://gerrit.cloudera.org:8080/#/c/8229/1/src/kudu/scripts/kudu-log-parser.pl@39
PS1, Line 39: (?:leader )?((?:pre-)?
> I think this matches "leader pre-election", "leader election", and "pre-ele
Yes. Unfortunately, both "leader election" and "pre-election" occur:

I0926 11:18:02.670809 23512 raft_consensus.cc:412] T 
ad3b9d7c7706426a8d881bac74a4f7a6 P a283d70a0bfb4ef8a324e4f0893bfa2d [term 42314 
FOLLOWER]: Starting pre-election (detected failure of leader 
b80b06235d304dad93ada855a442e051)
I0926 11:18:02.750224  7163 raft_consensus.cc:412] T 
1f96edff8a964101bfe31a68f3c325ef P a283d70a0bfb4ef8a324e4f0893bfa2d [term 23380 
FOLLOWER]: Starting leader election (detected failure of leader 
b80b06235d304dad93ada855a442e051)

Ah, interesting catch on the forced leader election. I'll add that.


http://gerrit.cloudera.org:8080/#/c/8229/1/src/kudu/scripts/kudu-log-parser.pl@236
PS1, Line 236: entry
> nit: I'm having some trouble parsing out what an "entry" is vs what a "reco
Sure, will do.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3bd8bb5d9a3c598ade7bc1bbc8f5a9e24ca618af
Gerrit-Change-Number: 8229
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Tue, 31 Oct 2017 18:29:49 +
Gerrit-HasComments: Yes

[kudu-CR](branch-1.2.x) [client-test] Reduce flakyness of TestWriteWithDeadTabletServer

Hello Kudu Jenkins, Todd Lipcon,

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

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

to review the following change.


Change subject: [client-test] Reduce flakyness of TestWriteWithDeadTabletServer
..

[client-test] Reduce flakyness of TestWriteWithDeadTabletServer

This test fails often due to an unexpected reason for the write
timeout. This is related to KUDU-1466.

I just commented out the assertion and left a TODO to reenable
once we address the jira. Also added some info to the jira
itself.

Change-Id: Ice347a30c71b1cb3937cd1b30b0744fcd2dae0cf
Reviewed-on: http://gerrit.cloudera.org:8080/6198
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
(cherry picked from commit 02ce2a8a23fb127697c10dffea84dcfa2f2b3933)
---
M src/kudu/client/client-test.cc
1 file changed, 4 insertions(+), 1 deletion(-)



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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.2.x
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ice347a30c71b1cb3937cd1b30b0744fcd2dae0cf
Gerrit-Change-Number: 8429
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] Add log parser script

2017-10-31 Thread Andrew Wong (Code Review)

Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8229 )

Change subject: Add log parser script
..


Patch Set 1:

(3 comments)

Overall looks pretty good.

http://gerrit.cloudera.org:8080/#/c/8229/1/src/kudu/scripts/kudu-log-parser.pl
File src/kudu/scripts/kudu-log-parser.pl:

http://gerrit.cloudera.org:8080/#/c/8229/1/src/kudu/scripts/kudu-log-parser.pl@5
PS1, Line 5: # Pass it a list of logs from a whole cluster.
For the perl-uninitiated, maybe a sample shell line?


http://gerrit.cloudera.org:8080/#/c/8229/1/src/kudu/scripts/kudu-log-parser.pl@39
PS1, Line 39: (?:leader )?((?:pre-)?
I think this matches "leader pre-election", "leader election", and 
"pre-election". Can all of these happen? Would "((?:leader |pre-)election)" be 
good here?

Also would "forced leader election" be useful here (looking at 
raft_consensus.cc:367)?


http://gerrit.cloudera.org:8080/#/c/8229/1/src/kudu/scripts/kudu-log-parser.pl@236
PS1, Line 236: entry
nit: I'm having some trouble parsing out what an "entry" is vs what a "record" 
is vs what a "rec" is. Are there less similar names we could use here?

I think "entry" is synonymous with "record", so maybe pick one and use it 
throughout?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3bd8bb5d9a3c598ade7bc1bbc8f5a9e24ca618af
Gerrit-Change-Number: 8229
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Comment-Date: Tue, 31 Oct 2017 16:21:28 +
Gerrit-HasComments: Yes

[kudu-CR] Update auth token validity seconds description

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

Change subject: Update auth_token_validity_seconds description
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I050e39d0377049cdaac0afb5085a8a9a19d620a5
Gerrit-Change-Number: 8413
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 31 Oct 2017 16:02:50 +
Gerrit-HasComments: No

[kudu-CR] Update auth token validity seconds description

Alexey Serbin has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/8413 )

Change subject: Update auth_token_validity_seconds description
..

Update auth_token_validity_seconds description

The caveats are no longer relevant since KUDU-2013 landed.

Change-Id: I050e39d0377049cdaac0afb5085a8a9a19d620a5
Reviewed-on: http://gerrit.cloudera.org:8080/8413
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin 
---
M src/kudu/master/master.cc
1 file changed, 2 insertions(+), 3 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I050e39d0377049cdaac0afb5085a8a9a19d620a5
Gerrit-Change-Number: 8413
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [mini-cluster] allow 18-bit PID in LOOPBACK bind mode