[kudu-CR] KUDU-1534 : Added software version to ListMasters RPC

[Dinesh Bhat (Code Review)]

Dinesh Bhat has posted comments on this change.

Change subject: KUDU-1534 : Added software_version to ListMasters RPC
..


Patch Set 3:

(1 comment)

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

Line 8: 
> nit: please add a blank line between the top line and the "body" of the mes
Done, thanks for catching this.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I750eeb78c989daaa6fd5d9793b1218ae5993653c
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Update contributing doc page with apache/kudu instead of apache/incubator-kudu

[Will Berkeley (Code Review)]

Will Berkeley has posted comments on this change.

Change subject: Update contributing doc page with apache/kudu instead of 
apache/incubator-kudu
..


Patch Set 3: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id9b0560f073735e55243643ddada6eac0001b9d4
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mladen Kovacevic 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mladen Kovacevic 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: No

[kudu-CR] Update contributing doc page with apache/kudu instead of apache/incubator-kudu

[Mladen Kovacevic (Code Review)]

Mladen Kovacevic has posted comments on this change.

Change subject: Update contributing doc page with apache/kudu instead of 
apache/incubator-kudu
..


Patch Set 3:

> There's a bunch of similar git clone links that use incubator-kudu
 > instead of kudu in installation.adoc. Could you fix those too?
 > Thanks!

Sure, did a quick grep, and found some changes in developing, installation and 
release_notes.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id9b0560f073735e55243643ddada6eac0001b9d4
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mladen Kovacevic 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mladen Kovacevic 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: No

[kudu-CR] Update contributing doc page with apache/kudu instead of apache/incubator-kudu

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Update contributing doc page with apache/kudu instead of 
apache/incubator-kudu
..


Patch Set 3:

Build Started http://104.196.14.100/job/kudu-gerrit/3045/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id9b0560f073735e55243643ddada6eac0001b9d4
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mladen Kovacevic 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: No

[kudu-CR] KuduSession: do not advertise thread-safety

[Alexey Serbin (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: KuduSession: do not advertise thread-safety
..

KuduSession: do not advertise thread-safety

Do not advertise thread-safety of KuduSession methods for the
Kudu C++ client, even if they are thread-safe de facto.  This is
to have one set of semantics for both C++ and Java Kudu client
libraries (corresponding methods of the Java client are not
thread-safe).  Besides, current use cases for the Kudu client
assume that operations with KuduSession object do not involve
multiple threads.

Change-Id: I6b3fcbd0d6446ccddce7a1812260c692eba18fcd
---
M docs/release_notes.adoc
M src/kudu/client/client.h
2 files changed, 3 insertions(+), 12 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6b3fcbd0d6446ccddce7a1812260c692eba18fcd
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Binglin Chang 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-1048 master should show versions of tservers, version summary

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1048 master should show versions of tservers, version 
summary
..


Patch Set 2:

Build Started http://104.196.14.100/job/kudu-gerrit/3040/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idd203209e3d99292018801b94ec2904b6634854f
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] tests: add --stress cpu threads argument

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: tests: add --stress_cpu_threads argument
..


Patch Set 1:

Build Started http://104.196.14.100/job/kudu-gerrit/3039/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9214098a9f83fad78889b2ff9621b19109f92425
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] tests: add --stress cpu threads argument

[Todd Lipcon (Code Review)]

Hello Mike Percy, Adar Dembo,

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

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

to review the following change.

Change subject: tests: add --stress_cpu_threads argument
..

tests: add --stress_cpu_threads argument

This is equivalent to running 'stress -c ' except it's built into our
test binaries. This is quite handy for reproducing race conditions,
especially on dist-test where it isn't easy to just pop open another
terminal and run 'stress' at the same time.

Change-Id: I9214098a9f83fad78889b2ff9621b19109f92425
---
M src/kudu/util/test_main.cc
1 file changed, 19 insertions(+), 0 deletions(-)


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

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

[kudu-CR] KUDU-1048 master should show versions of tservers, version summary

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: KUDU-1048 master should show versions of tservers, version 
summary
..


Patch Set 1:

(3 comments)

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

PS1, Line 71: freqs
technically these are counts not frequencies


PS1, Line 78: ++
I think a post-increment would be more standard-looking here.

Also, I wonder whether we should actually build two counts, one for 'live' and 
one for 'presumed dead' tablet servers. Currently the master never "forgets" a 
tablet server. Perhaps a map> would be helpful?

The threshold for liveness is FLAGS_tserver_unresponsive_timeout_ms but that 
liveness check should probably be moved into a bool accessor in TSDescriptor 
for simplicity


PS1, Line 88: th>Count<
eg here you could have a "Count (Live)" and "Count (Dead)" or somesuch


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idd203209e3d99292018801b94ec2904b6634854f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1048 master should show versions of tservers, version summary

[Will Berkeley (Code Review)]

Will Berkeley has uploaded a new change for review.

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

Change subject: KUDU-1048 master should show versions of tservers, version 
summary
..

KUDU-1048 master should show versions of tservers, version summary

This patch adds a version summary table and a total count of
registered tablet servers to /tablet-servers.

It also fixes the display of the registration, which was printing in
red font.

Sample: 
https://raw.githubusercontent.com/wdberkeley/kudu-cr/master/KUDU-1048-after.png

Change-Id: Idd203209e3d99292018801b94ec2904b6634854f
---
M src/kudu/master/master-path-handlers.cc
1 file changed, 23 insertions(+), 6 deletions(-)


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

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

[kudu-CR] Create base class for MiniCluster and ExternalMiniCluster

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Create base class for MiniCluster and ExternalMiniCluster
..


Patch Set 5:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/3974/5/src/kudu/integration-tests/mini_cluster.h
File src/kudu/integration-tests/mini_cluster.h:

Line 91:   void ShutdownMasters();
> Not exactly related to this, but while we are here, perhaps we could just f
Our style usually tries to avoid bool arguments like this, because if you're 
looking at a call site that says 'Shutdown(true)' you've got no idea what it 
means without having to break your concentration and go find the docs for the 
Shutdown method.

see also 
https://google.github.io/styleguide/cppguide.html#Implementation_Comments which 
has:

> Consider changing the function signature to replace a bool argument with an 
> enum argument. This will make the argument values self-describing.

so maybe a Shutdown(WhatToShutdown what); with arguments like MASTERS_ONLY, 
TABLET_SERVERS_ONLY, and ALL_SERVERS or somesuch would be good


Line 157:   Status CreateClient(client::KuduClientBuilder& builder,
non-const ref arg


http://gerrit.cloudera.org:8080/#/c/3974/5/src/kudu/integration-tests/mini_cluster_base.h
File src/kudu/integration-tests/mini_cluster_base.h:

PS5, Line 18: #ifndef KUDU_INTEGRATION_TESTS_MINI_CLUSTER_BASE_H_
: #define KUDU_INTEGRATION_TESTS_MINI_CLUSTER_BASE_H_
prefer #pragma once now


Line 31: class MiniClusterBase {
some basic doc would be nice


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I62256168a9245c845e99f7253f07e69a225954bf
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Various comment / doc improvements

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged.

Change subject: Various comment / doc improvements
..


Various comment / doc improvements

* Update link to code review for supporting reinsert
* Fix comment in HybridClock header docs
* Improve comment documentation in compaction code

Change-Id: I45d45d265652b0c462aec1bedbcb4b254a2a05da
Reviewed-on: http://gerrit.cloudera.org:8080/3977
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
M src/kudu/common/row_changelist.h
M src/kudu/server/hybrid_clock.h
M src/kudu/tablet/delta_compaction.cc
3 files changed, 12 insertions(+), 9 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I45d45d265652b0c462aec1bedbcb4b254a2a05da
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-1231. Add "unlock" flag for experimental and unsafe flags

[Todd Lipcon (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1231. Add "unlock" flag for experimental and unsafe flags
..

KUDU-1231. Add "unlock" flag for experimental and unsafe flags

This adds two new flags:
  --unlock_experimental_flags
  --unlock_unsafe_flags

If a flag is tagged as 'unsafe' or 'experimental', and the user tries
to set this flag on the command line without the corresponding 'unlock'
flag being set, then the process will exit at startup with an error.

Example error output without unsafe flags unlocked:

  E0823 15:12:08.163079 28376 flags.cc:300] Flag --never_fsync is unsafe
  and unsupported.
  E0823 15:12:08.163204 28376 flags.cc:301] Use --unlock_unsafe_flags to
  allow this flag at your own risk.
  

Example error output with flag unlocked:

  W0823 15:12:47.344476 28382 flags.cc:304] Enabled unsafe flag:
  --never_fsync=true

Change-Id: Iec49e77fca604a7c5ee7501121a6263b7ee590d6
---
M docs/release_notes.adoc
M java/kudu-client/src/test/resources/flags
M python/kudu/tests/common.py
M src/kudu/client/client_samples-test.sh
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/util/flags.cc
6 files changed, 67 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iec49e77fca604a7c5ee7501121a6263b7ee590d6
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 

[kudu-CR] KUDU-1157. Don't use array reference equality for EMPTY ARRAY

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1157. Don't use array reference equality for EMPTY_ARRAY
..


Patch Set 2:

Build Started http://104.196.14.100/job/kudu-gerrit/3034/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I30163098926822aafbf23b03ba4c9e26a7c91349
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] pbc tool: fix broken build

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged.

Change subject: pbc tool: fix broken build
..


pbc tool: fix broken build

Some out-of-order patch submission caused a build breakage

Change-Id: I414a663ea559ea10904d923b1d9fa2ff1d5858de
Reviewed-on: http://gerrit.cloudera.org:8080/4103
Reviewed-by: Todd Lipcon 
Tested-by: Todd Lipcon 
---
M src/kudu/tools/tool_action_pbc.cc
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  Todd Lipcon: Looks good to me, approved; Verified



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

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

[kudu-CR] pbc tool: fix broken build

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: pbc tool: fix broken build
..


Patch Set 1: Code-Review+2 Verified+1

Locally verified this fixes the build error, will push to avoid build breakage

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I414a663ea559ea10904d923b1d9fa2ff1d5858de
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Convert pbc-dump over to the new tool infrastructure

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: Convert pbc-dump over to the new tool infrastructure
..


Patch Set 2: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7707f004ea31d1a9e7bb890611080785f667c78
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] KUDU-1231. Add "unlock" flag for experimental and unsafe flags

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1231. Add "unlock" flag for experimental and unsafe flags
..


Patch Set 2:

Build Started http://104.196.14.100/job/kudu-gerrit/3031/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec49e77fca604a7c5ee7501121a6263b7ee590d6
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] KUDU-1231. Add "unlock" flag for experimental and unsafe flags

[Todd Lipcon (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1231. Add "unlock" flag for experimental and unsafe flags
..

KUDU-1231. Add "unlock" flag for experimental and unsafe flags

This adds two new flags:
  --unlock_experimental_flags
  --unlock_unsafe_flags

If a flag is tagged as 'unsafe' or 'experimental', and the user tries
to set this flag on the command line without the corresponding 'unlock'
flag being set, then the process will exit at startup with an error.

Example error output without unsafe flags unlocked:

  E0823 15:12:08.163079 28376 flags.cc:300] Flag --never_fsync is unsafe
  and unsupported.
  E0823 15:12:08.163204 28376 flags.cc:301] Use --unlock_unsafe_flags to
  allow this flag at your own risk.
  

Example error output with flag unlocked:

  W0823 15:12:47.344476 28382 flags.cc:304] Enabled unsafe flag:
  --never_fsync=true

Change-Id: Iec49e77fca604a7c5ee7501121a6263b7ee590d6
---
M docs/release_notes.adoc
M java/kudu-client/src/test/resources/flags
M python/kudu/tests/common.py
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/util/flags.cc
5 files changed, 65 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iec49e77fca604a7c5ee7501121a6263b7ee590d6
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 

[kudu-CR] Convert pbc-dump over to the new tool infrastructure

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Convert pbc-dump over to the new tool infrastructure
..


Patch Set 2:

> Please update install_kudu.sh (on the internal CDH/cdh-package.git:kudu repo 
> and branch) before merging.

Already did.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7707f004ea31d1a9e7bb890611080785f667c78
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Convert pbc-dump over to the new tool infrastructure

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Convert pbc-dump over to the new tool infrastructure
..


Patch Set 2:

Build Started http://104.196.14.100/job/kudu-gerrit/3029/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7707f004ea31d1a9e7bb890611080785f667c78
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Convert pbc-dump over to the new tool infrastructure

[Todd Lipcon (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Convert pbc-dump over to the new tool infrastructure
..

Convert pbc-dump over to the new tool infrastructure

Change-Id: Ia7707f004ea31d1a9e7bb890611080785f667c78
---
M src/kudu/tools/CMakeLists.txt
D src/kudu/tools/pbc-dump.cc
M src/kudu/tools/tool_action.h
A src/kudu/tools/tool_action_pbc.cc
M src/kudu/tools/tool_main.cc
5 files changed, 78 insertions(+), 79 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia7707f004ea31d1a9e7bb890611080785f667c78
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] tool: improve help output

[Todd Lipcon (Code Review)]

Hello Adar Dembo, Alexey Serbin, Kudu Jenkins,

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

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

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

Change subject: tool: improve help output
..

tool: improve help output

- adds blank line in between usage summary and more detail
- better formatting for required parameters in usage
- better formatting for optional boolean parameters
- justifies the list of subcommands

Example output with subcommands:

Usage: ./build/latest/bin/kudu  []

 can be one of the following:
  fs   Operate on a local Kudu filesystem
 pbc   Operate on a PBC (protobuf container) files
  tablet   Operate on a local Kudu replica

Example output of a leaf node:

Usage: ./build/latest/bin/kudu pbc dump  [-oneline]

Dump a PBC (protobuf container) file
path (path to PBC file) type: string default: ""
-oneline (print each protobuf on a single line) type: bool default: false

Change-Id: I0b4fc2da2b03edf8ce6b9998eeabd06a4fcd216d
---
M src/kudu/tools/tool_action.cc
1 file changed, 38 insertions(+), 13 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0b4fc2da2b03edf8ce6b9998eeabd06a4fcd216d
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] tool: improve help output

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: tool: improve help output
..


Patch Set 1:

(5 comments)

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

Line 9: - adds blank line in between usage summary and more detail
> nit for consistency: adds --> add, or justify --> justifies below instead
Done


http://gerrit.cloudera.org:8080/#/c/4038/1/src/kudu/tools/tool_action.cc
File src/kudu/tools/tool_action.cc:

Line 94:   std::stringstream msg;
> nit: std::ostringstream since no input is assumed.
Done


PS1, Line 95: []
> nit: if here we are using mandatory/optional notation with <>/[] brackets, 
Just copied this from git's usage output. I think we would use: '[--foo]' if 
"--foo" was the literal string that's optional, whereas here we're saying 
'['] because  itself is a place-holder for something else


Line 96:   msg << " can be one of the following:\n";
> nit: "   ..."  i.e. add a couple of spaces for better readability 
but then it would be lined up with the subcommands. Or you think I should 
indent the subcommands even more?

Again copied from git's format:

todd@todd-ThinkPad-T540p:~/git/kudu$ git
usage: git [--version] [--help] [-C ] [-c name=value]
   [--exec-path[=]] [--html-path] [--man-path] [--info-path]
   [-p|--paginate|--no-pager] [--no-replace-objects] [--bare]
   [--git-dir=] [--work-tree=] [--namespace=]
[]

The most commonly used git commands are:
   addAdd file contents to the index
   bisect Find by binary search the change that introduced a bug
...


Line 109:   for (const auto& p : lines) {
> Nit: shouldn't it be 'l' for 'line'? Or does 'p' mean something else?
was using 'p' for 'pair'. will rename lines to line_pairs and the variable to 
'lp'


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0b4fc2da2b03edf8ce6b9998eeabd06a4fcd216d
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1534 : Added software version to ListMasters RPC

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1534 : Added software_version to ListMasters RPC
..


Patch Set 2:

Build Started http://104.196.14.100/job/kudu-gerrit/3026/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I750eeb78c989daaa6fd5d9793b1218ae5993653c
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] KUDU-1534 : Added software version to ListMasters RPC Sample output is at: https://github.com/dineshabbi/scripts/blob/master/MasterRegWebUI.pdf

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: KUDU-1534 : Added software_version to ListMasters RPC Sample 
output is at: 
https://github.com/dineshabbi/scripts/blob/master/MasterRegWebUI.pdf
..


Patch Set 1: Code-Review+1

(1 comment)

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

Line 8: Sample output is at:
nit: please add a blank line between the top line and the "body" of the message


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I750eeb78c989daaa6fd5d9793b1218ae5993653c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Convert pbc-dump over to new tool infrastructure

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Convert pbc-dump over to new tool infrastructure
..


Patch Set 1:

(9 comments)

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

Line 7: Convert pbc-dump over to new tool infrastructure
> nit: 'to the new tool infrastructure'?
Done


http://gerrit.cloudera.org:8080/#/c/4037/1/src/kudu/tools/CMakeLists.txt
File src/kudu/tools/CMakeLists.txt:

Line 90
> Note that removing the executable will cause the packaging builds to fail. 
Done


http://gerrit.cloudera.org:8080/#/c/4037/1/src/kudu/tools/tool_action.h
File src/kudu/tools/tool_action.h:

Line 253: // Returns a new "pbc" mode node.
> Well, "kudu file pbc dump" does fit "kudu   ". But yo
ok, i'll leave as is for now, and once we've migrated everything, we can do a 
quick pass for sanity before release


Line 254: std::unique_ptr BuildPbcMode();
> Nit: The comments for each of these aren't going to be useful. Let's just c
Done


http://gerrit.cloudera.org:8080/#/c/4037/1/src/kudu/tools/tool_action_pbc.cc
File src/kudu/tools/tool_action_pbc.cc:

PS1, Line 36: oneline
> This and "online" literal at line 62: do these correspond to the same param
I don't think the literal can be defined once, since here it's a token and 
below it's a string. Could use preprocessor magic to stringify the token but I 
think it makes it harder to understand vs just duplicating.


PS1, Line 45: "path"
> Does it make sense to declare a constant for the "path" option?  I see it's
added kPathArg constant.

I left the 'string path = ' since it allows a bit more flexibility in the API 
without having to change here, eg the API could decide to pass 'char*' and we 
wouldn't need to change anything. If it were perf critical I think it'd be 
worth it, but here just trying to keep it simple.


PS1, Line 62: "oneline"
> This and 'online' parameter at line 36: do these correspond to the same par
see above


PS1, Line 63: "path"
> A constant for the name of the parameter?
yea, it's a kind of quirk of the tool infrastructure. Optional arguments 
correspond to gflags, and required ones are positional.

Perhaps we should change the API to 'AddPositionalParameter'? Adar?


PS1, Line 66: a
> nit: it seems the 'a' article is not needed here.
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7707f004ea31d1a9e7bb890611080785f667c78
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1534 : Added software version to ListMasters RPC Sample output is at: https://github.com/dineshabbi/scripts/blob/master/MasterRegWebUI.pdf

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1534 : Added software_version to ListMasters RPC Sample 
output is at: 
https://github.com/dineshabbi/scripts/blob/master/MasterRegWebUI.pdf
..


Patch Set 1:

Build Started http://104.196.14.100/job/kudu-gerrit/3025/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I750eeb78c989daaa6fd5d9793b1218ae5993653c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] KUDU-1534 : Added software version to ListMasters RPC Sample output is at: https://github.com/dineshabbi/scripts/blob/master/MasterRegWebUI.pdf

[Dinesh Bhat (Code Review)]

Hello Mike Percy, Adar Dembo, Todd Lipcon,

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

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

to review the following change.

Change subject: KUDU-1534 : Added software_version to ListMasters RPC Sample 
output is at: 
https://github.com/dineshabbi/scripts/blob/master/MasterRegWebUI.pdf
..

KUDU-1534 : Added software_version to ListMasters RPC
Sample output is at:
https://github.com/dineshabbi/scripts/blob/master/MasterRegWebUI.pdf

Change-Id: I750eeb78c989daaa6fd5d9793b1218ae5993653c
---
M src/kudu/common/wire_protocol.proto
M src/kudu/master/master-path-handlers.cc
M src/kudu/master/master-test.cc
M src/kudu/master/master.cc
4 files changed, 17 insertions(+), 8 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I750eeb78c989daaa6fd5d9793b1218ae5993653c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] subprocess: allow Call() to read both stdout and stderr

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: subprocess: allow Call() to read both stdout and stderr
..


Patch Set 1: Code-Review+1

Test passes on OS X

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

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

[kudu-CR] Remove CHECK that undos always exist

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Remove CHECK that undos always exist
..


Patch Set 1:

Also, have you tested that this actually allows for downgrade? ie there's no 
other code path which will now explode if you try to compact something with no 
UNDOs?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia58a7a9e6b13c5e3257ce0de1156977319be3a89
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Remove CHECK that undos always exist

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Remove CHECK that undos always exist
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4061/1/src/kudu/tablet/compaction.cc
File src/kudu/tablet/compaction.cc:

Line 795:   out->AppendUndoDeltas(dst_row.row_index(), new_undos_head, 
_in_current_drs_);
do you need to now wrap this in an: if (new_undos_head != nullptr)?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia58a7a9e6b13c5e3257ce0de1156977319be3a89
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Forward compat for changing TSRegistrationPB

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Forward compat for changing TSRegistrationPB
..


Patch Set 3:

(1 comment)

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

Line 58: // Compares two repeated HostPortPB fields. Returns true if equal, 
false otherwise.
> I wasn't worried about non-determinism on the part of protobuf, but on the 
oh, i see, yea that makes sense


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I794517d200b1c9ad74b3897b17aa808d68d7924b
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] tool: basic integration test

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: tool: basic integration test
..


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/4058/3/build-support/dist_test.py
File build-support/dist_test.py:

Line 249: deps.extend(ldd_deps(d))
do we need to worry about de-duping deps now?


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

PS3, Line 70: expected_prefixes
don't really understand the usage here


Line 91: // Strip away everything up to the usage string to test for 
prefixes.
a little skeptical of this fancy verification vs just hard-coding a regex for 
each help output -- may be easier to maintain the test that way


Line 99: if (l.find(" " + m) != string::npos) {
this isn't checking for a prefix


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib386882c1874e987d5824cfe742cc86627cd9eaa
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Forward compat for changing TSRegistrationPB

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: Forward compat for changing TSRegistrationPB
..


Patch Set 3:

(1 comment)

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

Line 58: // Compares two repeated HostPortPB fields. Returns true if equal, 
false otherwise.
> I think pbs are deterministic in this fashion (unlike JSON).
I wasn't worried about non-determinism on the part of protobuf, but on the part 
of the sender. Suppose the sender stores the addresses in a hash-based 
container. Adding a new entry means the iteration order changes, at which point 
the iteration order of the PB repeated field here will be different too.

If the comparison done is a set comparison, it'll be protected from that.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I794517d200b1c9ad74b3897b17aa808d68d7924b
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] subprocess: allow Call() to read both stdout and stderr

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: subprocess: allow Call() to read both stdout and stderr
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4057/1/src/kudu/util/subprocess.cc
File src/kudu/util/subprocess.cc:

Line 187:   // Interrupted by a signal, do nothing.
> I think so; returning here just means we'll poll the fd again, which will i
ah, OK, I was thinking that libev was edge-triggered rather than 
level-triggered, but I remembered wrong: 
http://lists.schmorp.de/pipermail/libev/2010q4/001162.html


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

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

[kudu-CR] subprocess: allow Call() to read both stdout and stderr

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: subprocess: allow Call() to read both stdout and stderr
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4057/1/src/kudu/util/subprocess.cc
File src/kudu/util/subprocess.cc:

Line 187:   // Interrupted by a signal, do nothing.
> is this the appropriate thing instead of retrying on EINTR? (like RETRY_ON_
I think so; returning here just means we'll poll the fd again, which will 
indicate that there's more data to be read, and we'll end up right back here on 
the next iteration.


Line 487: *stdout_out = out[0];
> maybe std::move or a swap() makes sense here just cuz the output could be l
Done


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

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

[kudu-CR](branch-0.10.x) Update version to 0.10.1-SNAPSHOT on branch

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Update version to 0.10.1-SNAPSHOT on branch
..


Patch Set 1:

Build Started http://104.196.14.100/job/kudu-gerrit/3024/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8403d978605b5c5e385baa6ad6f7f4738b9c9f48
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-0.10.x
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR](branch-0.10.x) Update version to 0.10.1-SNAPSHOT on branch

[Todd Lipcon (Code Review)]

Hello Dan Burkert,

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

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

to review the following change.

Change subject: Update version to 0.10.1-SNAPSHOT on branch
..

Update version to 0.10.1-SNAPSHOT on branch

Change-Id: I8403d978605b5c5e385baa6ad6f7f4738b9c9f48
---
M java/interface-annotations/pom.xml
M java/kudu-client-tools/pom.xml
M java/kudu-client/pom.xml
M java/kudu-csd/pom.xml
M java/kudu-flume-sink/pom.xml
M java/kudu-mapreduce/pom.xml
M java/kudu-spark/pom.xml
M java/pom.xml
M version.txt
9 files changed, 10 insertions(+), 10 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I8403d978605b5c5e385baa6ad6f7f4738b9c9f48
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-0.10.x
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 

[kudu-CR](gh-pages) Update docs from tip of branch-0.10.x

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: Update docs from tip of branch-0.10.x
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4091/2/docs/release_notes.html
File docs/release_notes.html:

Line 145: 
link:https://kudu.apache.org/2016/08/23/new-range-partitioning-features.html[blog
 post].
Is this supposed to be in this raw format or the intent was to have something 
like  ?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If574b4addfe5fa37e091a8533eaf87db68214bd8
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-456 Implement AUTO FLUSH BACKGROUND flush mode

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: KUDU-456 Implement AUTO_FLUSH_BACKGROUND flush mode
..


Patch Set 21:

Build Started http://104.196.14.100/job/kudu-gerrit/3023/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I34905c30b3aad96f53cf7a1822b1cde6d25f33a8
Gerrit-PatchSet: 21
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] KUDU-456 Implement AUTO FLUSH BACKGROUND flush mode

[Alexey Serbin (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-456 Implement AUTO_FLUSH_BACKGROUND flush mode
..

KUDU-456 Implement AUTO_FLUSH_BACKGROUND flush mode

Implemented AUTO_FLUSH_BACKGROUND for the Kudu C++ client library.
In AUTO_FLUSH_BACKGROUND mode,
the KuduSession::Apply() method blocks if total amount of data
for pending operations reaches the buffer size limit.  The limit
on the buffer size can be set by the
KuduSession::SetMutationBufferSpace() method.

The background flush logic checks whether at least one of the
following two conditions is satisfied to determine whether it's time
to flush the accumulated write operations:
  * The over-the-watermark criterion: check whether the total size of the
freshly submitted (i.e. not-yet-scheduled-for-flush) write operations
is over the threshold.  The threshold can be set as a percentage
of the total buffer size using the
KuduSession::SetMutationBufferFlushWatermark() method.
  * The maximum wait time criterion: check whether the current batch
of operations has been accumulating for more than the maximum
wait time.  The maximum wait time can be specified in milliseconds
using the KuduSession::SetMutationBufferFlushInterva() method.
A KuduSession object uses RPC messenger's thread pool to monitor
batches' maximum wait time.

This change also addresses the following JIRA issue:
  KUDU-1376 KuduSession::SetMutationBufferSpace is not defined

Change-Id: I34905c30b3aad96f53cf7a1822b1cde6d25f33a8
---
M python/kudu/tests/test_client.py
M src/kudu/client/batcher.cc
M src/kudu/client/batcher.h
M src/kudu/client/client-internal.h
M src/kudu/client/client-test.cc
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/client/session-internal.cc
M src/kudu/client/session-internal.h
M src/kudu/client/write_op.cc
M src/kudu/client/write_op.h
11 files changed, 1,179 insertions(+), 174 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I34905c30b3aad96f53cf7a1822b1cde6d25f33a8
Gerrit-PatchSet: 21
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR](gh-pages) Add release announcement blog for 0.10

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Add release announcement blog for 0.10
..


Patch Set 1: Code-Review+2 Verified+1

Going to push this without review since I already sent the same text out on 
slack earlier, and would like to get this staged and ready to tweet/announce in 
the morning. Feel free to suggest changes if you find issues.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I08e9aa3e43a57e27953b867d733b8162b25ff76f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR](gh-pages) Add release announcement blog for 0.10

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged.

Change subject: Add release announcement blog for 0.10
..


Add release announcement blog for 0.10

Change-Id: I08e9aa3e43a57e27953b867d733b8162b25ff76f
Reviewed-on: http://gerrit.cloudera.org:8080/4088
Reviewed-by: Todd Lipcon 
Tested-by: Todd Lipcon 
---
A _posts/2016-08-23-apache-kudu-0-10-0-released.md
1 file changed, 30 insertions(+), 0 deletions(-)

Approvals:
  Todd Lipcon: Looks good to me, approved; Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I08e9aa3e43a57e27953b867d733b8162b25ff76f
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR](gh-pages) Update docs from tip of branch-0.10.x

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Update docs from tip of branch-0.10.x
..


Patch Set 1: Code-Review+2 Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If574b4addfe5fa37e091a8533eaf87db68214bd8
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR](gh-pages) Update docs from tip of branch-0.10.x

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged.

Change subject: Update docs from tip of branch-0.10.x
..


Update docs from tip of branch-0.10.x

Hash ffd8fa4758fd8598a8d06a79249a6ab57d72aa81

Change-Id: If574b4addfe5fa37e091a8533eaf87db68214bd8
Reviewed-on: http://gerrit.cloudera.org:8080/4091
Reviewed-by: Todd Lipcon 
Tested-by: Todd Lipcon 
---
M docs/configuration_reference.html
M docs/configuration_reference_unsupported.html
M docs/contributing.html
M docs/kudu-master_configuration_reference.html
M docs/kudu-master_configuration_reference_unsupported.html
M docs/kudu-tserver_configuration_reference.html
M docs/kudu-tserver_configuration_reference_unsupported.html
M docs/release_notes.html
M releases/0.10.0/docs/configuration_reference.html
M releases/0.10.0/docs/configuration_reference_unsupported.html
M releases/0.10.0/docs/contributing.html
M releases/0.10.0/docs/kudu-master_configuration_reference.html
M releases/0.10.0/docs/kudu-master_configuration_reference_unsupported.html
M releases/0.10.0/docs/kudu-tserver_configuration_reference.html
M releases/0.10.0/docs/kudu-tserver_configuration_reference_unsupported.html
M releases/0.10.0/docs/release_notes.html
16 files changed, 68 insertions(+), 82 deletions(-)

Approvals:
  Todd Lipcon: Looks good to me, approved; Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: If574b4addfe5fa37e091a8533eaf87db68214bd8
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR](gh-pages) Update docs from tip of branch-0.10.x

[Todd Lipcon (Code Review)]

Hello Dan Burkert, Mike Percy,

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

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

to review the following change.

Change subject: Update docs from tip of branch-0.10.x
..

Update docs from tip of branch-0.10.x

Hash ffd8fa4758fd8598a8d06a79249a6ab57d72aa81

Change-Id: If574b4addfe5fa37e091a8533eaf87db68214bd8
---
M docs/configuration_reference.html
M docs/configuration_reference_unsupported.html
M docs/contributing.html
M docs/kudu-master_configuration_reference.html
M docs/kudu-master_configuration_reference_unsupported.html
M docs/kudu-tserver_configuration_reference.html
M docs/kudu-tserver_configuration_reference_unsupported.html
M docs/release_notes.html
M releases/0.10.0/docs/configuration_reference.html
M releases/0.10.0/docs/configuration_reference_unsupported.html
M releases/0.10.0/docs/contributing.html
M releases/0.10.0/docs/kudu-master_configuration_reference.html
M releases/0.10.0/docs/kudu-master_configuration_reference_unsupported.html
M releases/0.10.0/docs/kudu-tserver_configuration_reference.html
M releases/0.10.0/docs/kudu-tserver_configuration_reference_unsupported.html
M releases/0.10.0/docs/release_notes.html
16 files changed, 68 insertions(+), 82 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: If574b4addfe5fa37e091a8533eaf87db68214bd8
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Mike Percy 

[kudu-CR](branch-0.10.x) Fix release notes formatting for multi-paragraph list items

[Todd Lipcon (Code Review)]

Todd Lipcon has uploaded a new change for review.

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

Change subject: Fix release notes formatting for multi-paragraph list items
..

Fix release notes formatting for multi-paragraph list items

If a list item spans multiple paragraphs in AsciiDoc, it needs
a '+' in the intervening blank line, and the latter paragraphs
should not be indented on their first line. The text is a bit
uglier, but it fixes the rendering.

Change-Id: Ie65570ed32acef1c3025f351f7c4b4944f443f6e
---
M docs/release_notes.adoc
1 file changed, 10 insertions(+), 10 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie65570ed32acef1c3025f351f7c4b4944f443f6e
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-0.10.x
Gerrit-Owner: Todd Lipcon 

[kudu-CR](branch-0.10.x) Fix release notes formatting for multi-paragraph list items

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged.

Change subject: Fix release notes formatting for multi-paragraph list items
..


Fix release notes formatting for multi-paragraph list items

If a list item spans multiple paragraphs in AsciiDoc, it needs
a '+' in the intervening blank line, and the latter paragraphs
should not be indented on their first line. The text is a bit
uglier, but it fixes the rendering.

Change-Id: Ie65570ed32acef1c3025f351f7c4b4944f443f6e
Reviewed-on: http://gerrit.cloudera.org:8080/4089
Reviewed-by: Todd Lipcon 
Tested-by: Todd Lipcon 
---
M docs/release_notes.adoc
1 file changed, 10 insertions(+), 10 deletions(-)

Approvals:
  Todd Lipcon: Looks good to me, approved; Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ie65570ed32acef1c3025f351f7c4b4944f443f6e
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: branch-0.10.x
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon