[kudu-CR] [WIP] updated to use READ YOUR WRITES scan mode for Jepsen tests

[David Ribeiro Alves (Code Review)]

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

Change subject: [WIP] updated to use READ_YOUR_WRITES scan mode for Jepsen tests
..


Patch Set 1:

(5 comments)

we'll also need to test RYR, which I'm not sure the register test can do, since 
it only writes a single row.

cockroachdb has a good set of tests that we might want to look at: 
https://github.com/jepsen-io/jepsen/tree/master/cockroachdb

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

http://gerrit.cloudera.org:8080/#/c/9526/1//COMMIT_MSG@9
PS1, Line 9: This patches updated the Jespen tests to use READ_YOUR_WRITES scan 
mode.
we're supposed to have both eventually right?


http://gerrit.cloudera.org:8080/#/c/9526/1//COMMIT_MSG@13
PS1, Line 13: I ran the Jepesen test with this change and it passed.
the test only failed very occasionally, we'd need quite a few runs before we're 
confident the problem doens't reappear


http://gerrit.cloudera.org:8080/#/c/9526/1//COMMIT_MSG@14
PS1, Line 14: 
https://drive.google.com/file/d/1jW1CeGjB9AdnJZ2XbhJrf0IyOwXnKC_p/view?usp=sharing
thanks for sharing this.


http://gerrit.cloudera.org:8080/#/c/9526/1/java/kudu-jepsen/src/main/clojure/jepsen/kudu/register.clj
File java/kudu-jepsen/src/main/clojure/jepsen/kudu/register.clj:

http://gerrit.cloudera.org:8080/#/c/9526/1/java/kudu-jepsen/src/main/clojure/jepsen/kudu/register.clj@36
PS1, Line 36:   [table-created? kclient ktable]
:   (reify jepsen.client/Client
: (setup! [_ test _]
:   "Create the client and the test table. Use the same Kudu 
client instance "
:   "across all test actors to achieve timestamp propagation 
for all "
:   "operations."
:   (let [kclient (kc/sync-client (:master-addresses test))
: table-name (:table-name test)
: ktable (locking table-created?
:  (when (compare-and-set! table-created? false 
true)
:(kc/create-table
: kclient
: table-name
: kt/kv-table-schema
:  (let [ranges (:table-ranges test)
:rep-factor (:num-replicas test)]
:(if (nil? ranges)
:  (kt/kv-table-options-hash rep-factor 
(count (:tservers test)))
:  (kt/kv-table-options-range 
rep-factor ranges)
:  (kc/open-table kclient table-name))]
: (client table-created? kclient ktable)))
this is just a revert of the changes we made to have 1 client per table, vs one 
per thread. eventually we'll want to be able to choose so that we test both 
modes.


http://gerrit.cloudera.org:8080/#/c/9526/1/java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj
File java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj:

http://gerrit.cloudera.org:8080/#/c/9526/1/java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj@100
PS1, Line 100: READ_YOUR_WRITES
this will have to be a parameter



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
Gerrit-Change-Number: 9526
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 07 Mar 2018 07:26:15 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2297 (part 5): dump stacks into diagnostics log on service queue overflow

[Todd Lipcon (Code Review)]

Hello Tidy Bot, Mike Percy, Kudu Jenkins, Adar Dembo,

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

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

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

Change subject: KUDU-2297 (part 5): dump stacks into diagnostics log on service 
queue overflow
..

KUDU-2297 (part 5): dump stacks into diagnostics log on service queue overflow

In the past we've had several bugs and performance issues which manifest
themselves as service queue overflows. It would be easier to pinpoint
the root cause (eg slow IO vs lock contention) if we had a process-wide
stack snapshot at the time of the overflow.

This patch does some plumbing to trigger such a stack trace into the
diagnostics log when a service queue overflow occurs. The logging is
throttled to once every 5 seconds so that we don't contribute too much
to the load itself.

The plumbing is a bit ugly since the diagnostics log is in the server/
module whereas the overflows surface in the rpc/ module. Having rpc/
call back into server/ would be a cyclic dependency, so instead the rpc/
module exposes a std::function<> style callback for the server to hook
into.

Change-Id: I62019c71121d7ca4037cab86a7c2ea048a261ad8
---
M src/kudu/master/master-test.cc
M src/kudu/master/mini_master.h
M src/kudu/rpc/service_pool.cc
M src/kudu/rpc/service_pool.h
M src/kudu/server/diagnostics_log.cc
M src/kudu/server/diagnostics_log.h
M src/kudu/server/rpc_server.cc
M src/kudu/server/rpc_server.h
M src/kudu/server/server_base.cc
M src/kudu/server/server_base.h
10 files changed, 179 insertions(+), 19 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I62019c71121d7ca4037cab86a7c2ea048a261ad8
Gerrit-Change-Number: 9375
Gerrit-PatchSet: 5
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot

[kudu-CR] WIP KUDU-2129 make ksck less scary when copying

[Andrew Wong (Code Review)]

Andrew Wong has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9528


Change subject: WIP KUDU-2129 make ksck less scary when copying
..

WIP KUDU-2129 make ksck less scary when copying

WIP: There should be a nicer way to test this without the long lines.
Also there are a few changes here, so open to feedback about which
should stay/go, if there are more, etc. Also pondering whether we should
special-case bootstrapping tablets as well.

This patch changes some of the outputs for ksck to be less troubling
when the cluster is recovering. Updates include:
- Report tablets with replicas whose data states are TABLET_DATA_COPYING
  as RECOVERING instead of UNDER_REPLICATED.
- Report tables with recovering tablets as RECOVERING instead of
  UNDER_REPLICATED.
- Report replicas that aren't running as "not running" instead of "bad
  state". Most non-RUNNING states have a time and place during a healthy
  cluster's lifespan.
- Don't log the Status error when using the tool, just the message (e.g.
  to avoid messages like "Corruption: 1 out of 1 table(s) are bad").
- Report "non-HEALTHY" instead of "bad" in the above log.

Change-Id: Iba0c1f5b5a7083cbef99d3674dfebe1457075ce0
---
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/tool_action_cluster.cc
4 files changed, 94 insertions(+), 46 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iba0c1f5b5a7083cbef99d3674dfebe1457075ce0
Gerrit-Change-Number: 9528
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong 

[kudu-CR] [tools] KUDU-2331: Allow move tool to work when uninvolved tserver is down

[Will Berkeley (Code Review)]

Hello Kudu Jenkins, Adar Dembo,

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

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

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

Change subject: [tools] KUDU-2331: Allow move tool to work when uninvolved 
tserver is down
..

[tools] KUDU-2331: Allow move tool to work when uninvolved tserver is down

The move tool requires a clean ksck for the tablet it's acting on
for safety reasons, but the check for this failed if a tablet server
was down, even if the tablet server didn't host a replica for the
tablet and wasn't the destination server. This fixes the move
command so a down tserver uninvolved in the move won't prevent the
move.

Change-Id: Id1fc60233f4f50f478da7ccb104e37df3067400c
---
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/tool_action_tablet.cc
2 files changed, 78 insertions(+), 1 deletion(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id1fc60233f4f50f478da7ccb104e37df3067400c
Gerrit-Change-Number: 9510
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] [WIP] updated to use READ YOUR WRITES scan mode for Jepsen tests

[Hao Hao (Code Review)]

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


Change subject: [WIP] updated to use READ_YOUR_WRITES scan mode for Jepsen tests
..

[WIP] updated to use READ_YOUR_WRITES scan mode for Jepsen tests

This patches updated the Jespen tests to use READ_YOUR_WRITES scan mode.
Also, in this change every Jepsen worker will operate in a Kudu client,
instead of sharing one Kudu client for all workers.

I ran the Jepesen test with this change and it passed. The log is
available: 
https://drive.google.com/file/d/1jW1CeGjB9AdnJZ2XbhJrf0IyOwXnKC_p/view?usp=sharing

Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
---
M java/kudu-jepsen/src/main/clojure/jepsen/kudu/register.clj
M java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj
2 files changed, 14 insertions(+), 13 deletions(-)



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

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

[kudu-CR] docs: update docs for update dirs tool

[Andrew Wong (Code Review)]

Andrew Wong has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9110 )

Change subject: docs: update docs for update_dirs tool
..

docs: update docs for update_dirs tool

The `kudu fs update_dirs` tool now supports the removal of directories.
Some minor updates around the metadata directory are also included.

A rendered version can be found here:
https://github.com/andrwng/kudu/blob/docs_update_dirs/docs/administration.adoc#change_dir_config

Change-Id: Ic3c139326aee35f72495a1cb1aaf8df1d58776cb
Reviewed-on: http://gerrit.cloudera.org:8080/9110
Tested-by: Kudu Jenkins
Reviewed-by: Andrew Wong 
---
M docs/administration.adoc
M docs/configuration.adoc
2 files changed, 70 insertions(+), 78 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Andrew Wong: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic3c139326aee35f72495a1cb1aaf8df1d58776cb
Gerrit-Change-Number: 9110
Gerrit-PatchSet: 7
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] docs: update docs for update dirs tool

[Andrew Wong (Code Review)]

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

Change subject: docs: update docs for update_dirs tool
..


Patch Set 6:

Rolling forward Adar's +2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic3c139326aee35f72495a1cb1aaf8df1d58776cb
Gerrit-Change-Number: 9110
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 07 Mar 2018 02:36:31 +
Gerrit-HasComments: No

[kudu-CR] docs: update docs for update dirs tool

[Andrew Wong (Code Review)]

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

Change subject: docs: update docs for update_dirs tool
..


Patch Set 6: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic3c139326aee35f72495a1cb1aaf8df1d58776cb
Gerrit-Change-Number: 9110
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 07 Mar 2018 02:36:21 +
Gerrit-HasComments: No

[kudu-CR] [tools] KUDU-2331: Allow move tool to work when uninvolved tserver is down

[Will Berkeley (Code Review)]

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

Change subject: [tools] KUDU-2331: Allow move tool to work when uninvolved 
tserver is down
..


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/9510/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/9510/2//COMMIT_MSG@7
PS2, Line 7: [tools] KUDU-2331: Allow move tool to work when uninvolved tserver 
is down
> Could you add a regression test for this?
Done


http://gerrit.cloudera.org:8080/#/c/9510/2/src/kudu/tools/tool_action_tablet.cc
File src/kudu/tools/tool_action_tablet.cc:

http://gerrit.cloudera.org:8080/#/c/9510/2/src/kudu/tools/tool_action_tablet.cc@150
PS2, Line 150:   // The return Status is ignored since a tserver that is not 
the destination
> Would it be worth logging as a warning? Or would that be distracting and no
Distracting and not useful. Plus it'll get displayed a bunch of times since 
ksck is run multiple time for one move.


http://gerrit.cloudera.org:8080/#/c/9510/2/src/kudu/tools/tool_action_tablet.cc@154
PS2, Line 154:   ksck.FetchInfoFromTabletServers();
> Wrapping this in ignore_result() is an idiomatic way to express that we don
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id1fc60233f4f50f478da7ccb104e37df3067400c
Gerrit-Change-Number: 9510
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Wed, 07 Mar 2018 01:53:42 +
Gerrit-HasComments: Yes

[kudu-CR] rowset metadata: cache min/max encoded keys

[Todd Lipcon (Code Review)]

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

Change subject: rowset_metadata: cache min/max encoded keys
..


Patch Set 2:

I think we could pretty easily benchmark this by loading a few hundred GB using 
YCSB and then measure startup time of the tserver after a drop_caches. I 
usually see many tens of seconds where stacks indicate it's opening cfile 
footers at startup in that setting.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I37d6f7160e3a7188753684e063963110f70e9b8d
Gerrit-Change-Number: 9372
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 07 Mar 2018 01:47:33 +
Gerrit-HasComments: No

[kudu-CR] build: don't name unsharded tests with a shard suffix

[Todd Lipcon (Code Review)]

Hello Adar Dembo,

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

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

to review the following change.


Change subject: build: don't name unsharded tests with a shard suffix
..

build: don't name unsharded tests with a shard suffix

Change-Id: I33611f9766381d367c7a5ac7b09a00f01c48fe74
---
M CMakeLists.txt
M build-support/run-test.sh
2 files changed, 12 insertions(+), 3 deletions(-)



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

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

[kudu-CR] docs: update docs for update dirs tool

[Andrew Wong (Code Review)]

Hello Alex Rodoni, Kudu Jenkins, Adar Dembo,

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

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

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

Change subject: docs: update docs for update_dirs tool
..

docs: update docs for update_dirs tool

The `kudu fs update_dirs` tool now supports the removal of directories.
Some minor updates around the metadata directory are also included.

A rendered version can be found here:
https://github.com/andrwng/kudu/blob/docs_update_dirs/docs/administration.adoc#change_dir_config

Change-Id: Ic3c139326aee35f72495a1cb1aaf8df1d58776cb
---
M docs/administration.adoc
M docs/configuration.adoc
2 files changed, 70 insertions(+), 78 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic3c139326aee35f72495a1cb1aaf8df1d58776cb
Gerrit-Change-Number: 9110
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] docs: update docs for update dirs tool

[Andrew Wong (Code Review)]

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

Change subject: docs: update docs for update_dirs tool
..


Patch Set 5:

rebase


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic3c139326aee35f72495a1cb1aaf8df1d58776cb
Gerrit-Change-Number: 9110
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 07 Mar 2018 01:31:40 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2297 (part 6): enable periodic stack sampling by default, avoid bias

[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/9523

to review the following change.


Change subject: KUDU-2297 (part 6): enable periodic stack sampling by default, 
avoid bias
..

KUDU-2297 (part 6): enable periodic stack sampling by default, avoid bias

This changes the timing for the collection of stack samples into the
metrics log to avoid two issues:

1) Inflexible configuration: in many cases we might want to sample
stacks more or less frequently than metrics. The previous implementation
used the same schedule for both.

2) Sampling bias: if we collect stack samples on a fixed schedule, we
risk unintended correlation with background tasks that might run on a
similar fixed schedule. For example, if we collect stacks exactly once a
minute, and the user has some monitoring software which polls the HTTP
server once a minute for some data, we might either line up perfectly
with the polling (in which case we'd overestimate its impact) or line of
perfectly away from the polling (in which case we'd never see its
effects).

This patch changes the wakeups for stacks and metrics to be decoupled.
In addition, it adds randomness to the stack sampling. The configuration
now represents the mean sampling rate rather than an exact sampling
rate.

I manually ran a master with -diagnostics-log-stack-traces-interval-ms=2000
and verified that the stack sample times were between 0 and 4 seconds apart
while the metric dumps were still exactly one minute apart. I also plan
to modify some local test clusters to configure this to be relatively
frequent to try to suss out any races or bugs which might occur in a
real workload.

Change-Id: I538eb59f95a695fa2da50f24ed82148715b15e44
---
M src/kudu/server/diagnostics_log.cc
M src/kudu/server/diagnostics_log.h
2 files changed, 77 insertions(+), 49 deletions(-)



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

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

[kudu-CR] KUDU-2297 (part 5): dump stacks into diagnostics log on service queue overflow

[Todd Lipcon (Code Review)]

Hello Tidy Bot, Mike Percy, Kudu Jenkins, Adar Dembo,

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

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

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

Change subject: KUDU-2297 (part 5): dump stacks into diagnostics log on service 
queue overflow
..

KUDU-2297 (part 5): dump stacks into diagnostics log on service queue overflow

In the past we've had several bugs and performance issues which manifest
themselves as service queue overflows. It would be easier to pinpoint
the root cause (eg slow IO vs lock contention) if we had a process-wide
stack snapshot at the time of the overflow.

This patch does some plumbing to trigger such a stack trace into the
diagnostics log when a service queue overflow occurs. The logging is
throttled to once every 5 seconds so that we don't contribute too much
to the load itself.

The plumbing is a bit ugly since the diagnostics log is in the server/
module whereas the overflows surface in the rpc/ module. Having rpc/
call back into server/ would be a cyclic dependency, so instead the rpc/
module exposes a std::function<> style callback for the server to hook
into.

Change-Id: I62019c71121d7ca4037cab86a7c2ea048a261ad8
---
M src/kudu/master/master-test.cc
M src/kudu/master/mini_master.h
M src/kudu/rpc/service_pool.cc
M src/kudu/rpc/service_pool.h
M src/kudu/server/diagnostics_log.cc
M src/kudu/server/diagnostics_log.h
M src/kudu/server/rpc_server.cc
M src/kudu/server/rpc_server.h
M src/kudu/server/server_base.cc
M src/kudu/server/server_base.h
10 files changed, 170 insertions(+), 18 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I62019c71121d7ca4037cab86a7c2ea048a261ad8
Gerrit-Change-Number: 9375
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot

[kudu-CR] env: generalize resource limits and add RLIMIT NPROC support

[David Ribeiro Alves (Code Review)]

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

Change subject: env: generalize resource limits and add RLIMIT_NPROC support
..


Patch Set 1:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/9521/1/src/kudu/util/env-test.cc@675
PS1, Line 675: KUDU-1798.
hum was the number of the jira wrong?


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

http://gerrit.cloudera.org:8080/#/c/9521/1/src/kudu/util/env.h@319
PS1, Line 319: RUNNING_THREADS
hum, technically RLIMIT_NPROC is the limit of the total number of processes 
that can run for a user, not the total number of threads a process can run. yes 
that translates to almost the same thing when kudu is run with its own user, 
but wondering if conflating the two isn't confusing...



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I567f581a6f8a85ac1f08878b61ac316cc2da36a0
Gerrit-Change-Number: 9521
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 07 Mar 2018 01:25:44 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1913: cap number of threads on server-wide pools

[David Ribeiro Alves (Code Review)]

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

Change subject: KUDU-1913: cap number of threads on server-wide pools
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9522/1/src/kudu/kserver/kserver.cc
File src/kudu/kserver/kserver.cc:

http://gerrit.cloudera.org:8080/#/c/9522/1/src/kudu/kserver/kserver.cc@39
PS1, Line 39: server_thread_pool_thread_limit
lgtm, only minor nit is the name of the flag server_thread_pool_thread_limit, 
how about server_thread_pool_max_count or server_thread_pool_max_thread_count ?
don't feel super strongly about it so fine by folks like this name.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I194907a7f8a483c9cba71eba8caed6bc6090f618
Gerrit-Change-Number: 9522
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 07 Mar 2018 01:15:46 +
Gerrit-HasComments: Yes

[kudu-CR] Fix python build script print newlines

[Dan Burkert (Code Review)]

Dan Burkert has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9520 )

Change subject: Fix python build script print newlines
..

Fix python build script print newlines

This is a fixup for 112869cf22c47543.

Change-Id: I36fb45f3045dea3d16689db730d2372d4cf4a936
Reviewed-on: http://gerrit.cloudera.org:8080/9520
Reviewed-by: Adar Dembo 
Tested-by: Kudu Jenkins
---
M build-support/build_source_release.py
M build-support/kudu_util.py
M build-support/push_to_asf.py
3 files changed, 7 insertions(+), 3 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I36fb45f3045dea3d16689db730d2372d4cf4a936
Gerrit-Change-Number: 9520
Gerrit-PatchSet: 4
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] env: generalize resource limits and add RLIMIT NPROC support

[Adar Dembo (Code Review)]

Hello David Ribeiro Alves, Todd Lipcon,

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

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

to review the following change.


Change subject: env: generalize resource limits and add RLIMIT_NPROC support
..

env: generalize resource limits and add RLIMIT_NPROC support

A follow-on patch will use this to cap the max number of threads in some
process-wide thread pools (see KUDU-1913).

Change-Id: I567f581a6f8a85ac1f08878b61ac316cc2da36a0
---
M src/kudu/fs/block_manager.cc
M src/kudu/rpc/rpc-test.cc
M src/kudu/util/env-test.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
5 files changed, 100 insertions(+), 32 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I567f581a6f8a85ac1f08878b61ac316cc2da36a0
Gerrit-Change-Number: 9521
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-1913: cap number of threads on server-wide pools

[Adar Dembo (Code Review)]

Hello David Ribeiro Alves, Todd Lipcon,

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

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

to review the following change.


Change subject: KUDU-1913: cap number of threads on server-wide pools
..

KUDU-1913: cap number of threads on server-wide pools

The last remaining piece of work is to do away with the unbounded number of
threads that may be started in the Raft and Prepare server-wide threadpools.
These caps make it easier for admins to reason about appropriate values for
the configuration of the Kudu processes RLIMIT_NPROC resource.

KUDU-1913 proposed a cap of "number of cores + number of disks", but a
lively Slack discussion yielded a better solution: set the cap at some
percentage of the process' RLIMIT_NPROC value. Given that the rest of Kudu
generally uses a constant number of threads, this should prevent spikes from
ever exceeding the RLIMIT_NPROC and crashing the server due to an election
storm. This patch implements a cap of 10% per pool and also provides a new
gflag as an "escape hatch" (in case we were horribly wrong).

Note: it's still possible for a massive number of "hot" replicas to exceed
RLIMIT_NPROC by virtue of each replica's log append thread, but the server
is more likely to run out of memory for MemRowSets before that happens.

Change-Id: I194907a7f8a483c9cba71eba8caed6bc6090f618
---
M src/kudu/kserver/kserver.cc
1 file changed, 55 insertions(+), 11 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I194907a7f8a483c9cba71eba8caed6bc6090f618
Gerrit-Change-Number: 9522
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2297 (part 5): dump stacks into diagnostics log on service queue overflow

[Todd Lipcon (Code Review)]

Hello Tidy Bot, Mike Percy, Kudu Jenkins,

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

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

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

Change subject: KUDU-2297 (part 5): dump stacks into diagnostics log on service 
queue overflow
..

KUDU-2297 (part 5): dump stacks into diagnostics log on service queue overflow

In the past we've had several bugs and performance issues which manifest
themselves as service queue overflows. It would be easier to pinpoint
the root cause (eg slow IO vs lock contention) if we had a process-wide
stack snapshot at the time of the overflow.

This patch does some plumbing to trigger such a stack trace into the
diagnostics log when a service queue overflow occurs. The logging is
throttled to once every 5 seconds so that we don't contribute too much
to the load itself.

The plumbing is a bit ugly since the diagnostics log is in the server/
module whereas the overflows surface in the rpc/ module. Having rpc/
call back into server/ would be a cyclic dependency, so instead the rpc/
module exposes a std::function<> style callback for the server to hook
into.

Change-Id: I62019c71121d7ca4037cab86a7c2ea048a261ad8
---
M src/kudu/master/master-test.cc
M src/kudu/master/mini_master.h
M src/kudu/rpc/service_pool.cc
M src/kudu/rpc/service_pool.h
M src/kudu/server/diagnostics_log.cc
M src/kudu/server/diagnostics_log.h
M src/kudu/server/rpc_server.cc
M src/kudu/server/rpc_server.h
M src/kudu/server/server_base.cc
M src/kudu/server/server_base.h
10 files changed, 169 insertions(+), 18 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I62019c71121d7ca4037cab86a7c2ea048a261ad8
Gerrit-Change-Number: 9375
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot

[kudu-CR] KUDU-2297 (part 5): dump stacks into diagnostics log on service queue overflow

[Todd Lipcon (Code Review)]

Hello Tidy Bot, Mike Percy, Kudu Jenkins,

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

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

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

Change subject: KUDU-2297 (part 5): dump stacks into diagnostics log on service 
queue overflow
..

KUDU-2297 (part 5): dump stacks into diagnostics log on service queue overflow

In the past we've had several bugs and performance issues which manifest
themselves as service queue overflows. It would be easier to pinpoint
the root cause (eg slow IO vs lock contention) if we had a process-wide
stack snapshot at the time of the overflow.

This patch does some plumbing to trigger such a stack trace into the
diagnostics log when a service queue overflow occurs. The logging is
throttled to once every 5 seconds so that we don't contribute too much
to the load itself.

The plumbing is a bit ugly since the diagnostics log is in the server/
module whereas the overflows surface in the rpc/ module. Having rpc/
call back into server/ would be a cyclic dependency, so instead the rpc/
module exposes a std::function<> style callback for the server to hook
into.

Change-Id: I62019c71121d7ca4037cab86a7c2ea048a261ad8
---
M src/kudu/master/master-test.cc
M src/kudu/master/mini_master.h
M src/kudu/rpc/service_pool.cc
M src/kudu/rpc/service_pool.h
M src/kudu/server/diagnostics_log.cc
M src/kudu/server/diagnostics_log.h
M src/kudu/server/rpc_server.cc
M src/kudu/server/rpc_server.h
M src/kudu/server/server_base.cc
M src/kudu/server/server_base.h
10 files changed, 166 insertions(+), 13 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I62019c71121d7ca4037cab86a7c2ea048a261ad8
Gerrit-Change-Number: 9375
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot

[kudu-CR] docs: update docs for update dirs tool

[Andrew Wong (Code Review)]

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

Change subject: docs: update docs for update_dirs tool
..


Patch Set 5: Verified+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic3c139326aee35f72495a1cb1aaf8df1d58776cb
Gerrit-Change-Number: 9110
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 07 Mar 2018 00:13:06 +
Gerrit-HasComments: No

[kudu-CR] docs: update docs for update dirs tool

[Andrew Wong (Code Review)]

Andrew Wong has removed a vote on this change.

Change subject: docs: update docs for update_dirs tool
..


Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/9110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ic3c139326aee35f72495a1cb1aaf8df1d58776cb
Gerrit-Change-Number: 9110
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] Fix python build script print newlines

[Adar Dembo (Code Review)]

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

Change subject: Fix python build script print newlines
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I36fb45f3045dea3d16689db730d2372d4cf4a936
Gerrit-Change-Number: 9520
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 07 Mar 2018 00:06:42 +
Gerrit-HasComments: No

[kudu-CR] Fix python build script print newlines

[Dan Burkert (Code Review)]

Hello Kudu Jenkins, Adar Dembo, Todd Lipcon,

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

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

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

Change subject: Fix python build script print newlines
..

Fix python build script print newlines

This is a fixup for 112869cf22c47543.

Change-Id: I36fb45f3045dea3d16689db730d2372d4cf4a936
---
M build-support/build_source_release.py
M build-support/kudu_util.py
M build-support/push_to_asf.py
3 files changed, 7 insertions(+), 3 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I36fb45f3045dea3d16689db730d2372d4cf4a936
Gerrit-Change-Number: 9520
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Fix python build script print newlines

[Adar Dembo (Code Review)]

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

Change subject: Fix python build script print newlines
..


Patch Set 2: Code-Review+2

Can't say I understand, but if it fixes the problem, Ship It!


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I36fb45f3045dea3d16689db730d2372d4cf4a936
Gerrit-Change-Number: 9520
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 06 Mar 2018 23:52:22 +
Gerrit-HasComments: No

[kudu-CR] docs: update docs for update dirs tool

[Adar Dembo (Code Review)]

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

Change subject: docs: update docs for update_dirs tool
..


Patch Set 5: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9110/4/docs/administration.adoc
File docs/administration.adoc:

http://gerrit.cloudera.org:8080/#/c/9110/4/docs/administration.adoc@a721
PS4, Line 721:
 :
 :
 :
 :
 :
> I'll note that that's actually possible, provided they specify the fs_metad
Ah, right.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic3c139326aee35f72495a1cb1aaf8df1d58776cb
Gerrit-Change-Number: 9110
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 06 Mar 2018 23:51:32 +
Gerrit-HasComments: Yes

[kudu-CR] Fix python build script print newlines

[Dan Burkert (Code Review)]

Hello Kudu Jenkins, Adar Dembo, Todd Lipcon,

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

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

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

Change subject: Fix python build script print newlines
..

Fix python build script print newlines

This is a fixup for 112869cf22c47543.

Change-Id: I36fb45f3045dea3d16689db730d2372d4cf4a936
---
M build-support/build_source_release.py
M build-support/kudu_util.py
M build-support/push_to_asf.py
3 files changed, 3 insertions(+), 3 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I36fb45f3045dea3d16689db730d2372d4cf4a936
Gerrit-Change-Number: 9520
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Fix python build script print newlines

[Dan Burkert (Code Review)]

Hello Adar Dembo, Todd Lipcon,

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

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

to review the following change.


Change subject: Fix python build script print newlines
..

Fix python build script print newlines

This is a fixup for 112869cf22c47543.

Change-Id: I36fb45f3045dea3d16689db730d2372d4cf4a936
---
M build-support/build_source_release.py
M build-support/kudu_util.py
M build-support/push_to_asf.py
3 files changed, 3 insertions(+), 3 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I36fb45f3045dea3d16689db730d2372d4cf4a936
Gerrit-Change-Number: 9520
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] docs: update docs for update dirs tool

[Andrew Wong (Code Review)]

Hello Alex Rodoni, Kudu Jenkins, Adar Dembo,

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

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

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

Change subject: docs: update docs for update_dirs tool
..

docs: update docs for update_dirs tool

The `kudu fs update_dirs` tool now supports the removal of directories.
Some minor updates around the metadata directory are also included.

A rendered version can be found here:
https://github.com/andrwng/kudu/blob/docs_update_dirs/docs/administration.adoc#change_dir_config

Change-Id: Ic3c139326aee35f72495a1cb1aaf8df1d58776cb
---
M docs/administration.adoc
M docs/configuration.adoc
2 files changed, 70 insertions(+), 78 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic3c139326aee35f72495a1cb1aaf8df1d58776cb
Gerrit-Change-Number: 9110
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] docs: update docs for update dirs tool

[Andrew Wong (Code Review)]

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

Change subject: docs: update docs for update_dirs tool
..


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9110/4/docs/administration.adoc
File docs/administration.adoc:

http://gerrit.cloudera.org:8080/#/c/9110/4/docs/administration.adoc@a721
PS4, Line 721:
 :
 :
 :
 :
 : 
> This is kinda still relevant in that clusters created before 1.7 will store
I'll note that that's actually possible, provided they specify the 
fs_metadata_dirs flag while running the tool.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic3c139326aee35f72495a1cb1aaf8df1d58776cb
Gerrit-Change-Number: 9110
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 06 Mar 2018 23:28:52 +
Gerrit-HasComments: Yes

[kudu-CR] decimal util: do not produce warnings when building with gcc

[Adar Dembo (Code Review)]

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

Change subject: decimal_util: do not produce warnings when building with gcc
..

decimal_util: do not produce warnings when building with gcc

When I build with gcc5, I get the following warning:

  ../../src/kudu/util/decimal_util.cc:31:45: warning: ‘no_sanitize’ attribute 
directive ignored [-Wattributes]
   int128_t MaxUnscaledDecimal(int8_t precision) {

This is because gcc doesn't recognize the "no_sanitize" attribute, so let's
deal with this as we've dealt with other sanitizer suppressions: compile it
conditionally using ASAN support as a proxy.

Change-Id: Iac9b145a914d551e3a18d62a0984aad34b95f4dd
Reviewed-on: http://gerrit.cloudera.org:8080/9499
Tested-by: Kudu Jenkins
Reviewed-by: Grant Henke 
---
M src/kudu/gutil/port.h
M src/kudu/util/decimal_util.cc
2 files changed, 17 insertions(+), 2 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Grant Henke: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Iac9b145a914d551e3a18d62a0984aad34b95f4dd
Gerrit-Change-Number: 9499
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] docs: update docs for update dirs tool

[Adar Dembo (Code Review)]

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

Change subject: docs: update docs for update_dirs tool
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9110/4/docs/administration.adoc
File docs/administration.adoc:

http://gerrit.cloudera.org:8080/#/c/9110/4/docs/administration.adoc@a721
PS4, Line 721:
 :
 :
 :
 :
 :
This is kinda still relevant in that clusters created before 1.7 will store 
metadata in their first data dir, such that it's not possible to remove that 
data dir.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic3c139326aee35f72495a1cb1aaf8df1d58776cb
Gerrit-Change-Number: 9110
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 06 Mar 2018 23:06:11 +
Gerrit-HasComments: Yes

[kudu-CR] docs: update docs for update dirs tool

[Andrew Wong (Code Review)]

Hello Alex Rodoni, Kudu Jenkins, Adar Dembo,

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

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

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

Change subject: docs: update docs for update_dirs tool
..

docs: update docs for update_dirs tool

The `kudu fs update_dirs` tool now supports the removal of directories.
Some minor updates around the metadata directory are also included.

A rendered version can be found here:
https://github.com/andrwng/kudu/blob/docs_update_dirs/docs/administration.adoc#change_dir_config

Change-Id: Ic3c139326aee35f72495a1cb1aaf8df1d58776cb
---
M docs/administration.adoc
M docs/configuration.adoc
2 files changed, 64 insertions(+), 78 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic3c139326aee35f72495a1cb1aaf8df1d58776cb
Gerrit-Change-Number: 9110
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] build: stop generating md5 file

[Todd Lipcon (Code Review)]

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

Change subject: build: stop generating md5 file
..

build: stop generating md5 file

New guidance from the ASF is to not generate md5 files when producing source
releases. Instead we just generate the sha checksum.

Change-Id: I6b0359d63b660b285c0853cd9b88654a400f14c2
Reviewed-on: http://gerrit.cloudera.org:8080/9504
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert 
---
M build-support/build_source_release.py
1 file changed, 11 insertions(+), 14 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I6b0359d63b660b285c0853cd9b88654a400f14c2
Gerrit-Change-Number: 9504
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] build: adjust the number of processors for various tests

[Todd Lipcon (Code Review)]

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

Change subject: build: adjust the number of processors for various tests
..

build: adjust the number of processors for various tests

Many of our tests are multithreaded and can utilize many cores worth of
CPU during execution. Other tests are somewhat timing-sensitive and
might time out if run in coordination with a stress test, particularly
on a small machine which might only have four CPUs total.

This patch adjusts the core count property for a bunch of tests based on
some empirical analysis using a new 'perf-stat' wrapper capability
provided in run-test.sh. The wrapper produces periodic samples of the
CPU usage for each test. Measurements are written into log files which
can then be analyzed with a command like the following:

for x in build/latest/test-logs/*.perf.txt ; do
  echo -n $x
  grep task-clock $x | sort -nk2 | tail -1
done | sort -nk3

This resulted in a list of tests along with their peak CPU usage (from
their busiest 1000ms period). I used this as a rough guide to find tests
with inappropriate settings in CMakeLists.txt. The net effect of this
change is that 'ctest -j' might run slightly longer, but that we'll have
fewer flaky tests in such a run configuration because we don't have
stress tests perturbing timing of unrelated non-stress tests.

Change-Id: I922031af69178b98459d0d59287442b692425afa
Reviewed-on: http://gerrit.cloudera.org:8080/9503
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo 
---
M CMakeLists.txt
M build-support/run-test.sh
M src/kudu/cfile/CMakeLists.txt
M src/kudu/client/CMakeLists.txt
M src/kudu/clock/CMakeLists.txt
M src/kudu/consensus/CMakeLists.txt
M src/kudu/integration-tests/CMakeLists.txt
M src/kudu/mini-cluster/CMakeLists.txt
M src/kudu/rpc/CMakeLists.txt
M src/kudu/security/CMakeLists.txt
M src/kudu/tablet/CMakeLists.txt
M src/kudu/tools/CMakeLists.txt
M src/kudu/tserver/CMakeLists.txt
M src/kudu/util/CMakeLists.txt
14 files changed, 80 insertions(+), 48 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I922031af69178b98459d0d59287442b692425afa
Gerrit-Change-Number: 9503
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [docs] Improvements to multi-master migration doc

[Will Berkeley (Code Review)]

Will Berkeley has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9466 )

Change subject: [docs] Improvements to multi-master migration doc
..

[docs] Improvements to multi-master migration doc

- Add extra reminder to run as the Kudu user.

- Note that the copy_from_remote command requires authenticating
  to the remote service as the Kudu user.

- Note that the workflow can be used to migrate 2->3 masters by
  making straightforward adjustments to the procedure.

- Move steps for verifying the migration was successful to a new
  section so they are more noticeable.

Change-Id: I77ef796f8b35729871ef8ddf2b635989278c2ebc
Reviewed-on: http://gerrit.cloudera.org:8080/9466
Reviewed-by: Adar Dembo 
Tested-by: Will Berkeley 
---
M docs/administration.adoc
1 file changed, 46 insertions(+), 38 deletions(-)

Approvals:
  Adar Dembo: Looks good to me, approved
  Will Berkeley: Verified

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I77ef796f8b35729871ef8ddf2b635989278c2ebc
Gerrit-Change-Number: 9466
Gerrit-PatchSet: 5
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] [docs] Improvements to multi-master migration doc

[Will Berkeley (Code Review)]

Will Berkeley has removed Kudu Jenkins from this change.  ( 
http://gerrit.cloudera.org:8080/9466 )

Change subject: [docs] Improvements to multi-master migration doc
..


Removed reviewer Kudu Jenkins.
--
To view, visit http://gerrit.cloudera.org:8080/9466
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: I77ef796f8b35729871ef8ddf2b635989278c2ebc
Gerrit-Change-Number: 9466
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] [docs] Improvements to multi-master migration doc

[Will Berkeley (Code Review)]

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

Change subject: [docs] Improvements to multi-master migration doc
..


Patch Set 4: Verified+1

Docs-only change.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77ef796f8b35729871ef8ddf2b635989278c2ebc
Gerrit-Change-Number: 9466
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Tue, 06 Mar 2018 22:58:05 +
Gerrit-HasComments: No

[kudu-CR] [docs] Improvements to multi-master migration doc

[Adar Dembo (Code Review)]

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

Change subject: [docs] Improvements to multi-master migration doc
..


Patch Set 4: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77ef796f8b35729871ef8ddf2b635989278c2ebc
Gerrit-Change-Number: 9466
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Tue, 06 Mar 2018 22:56:09 +
Gerrit-HasComments: No

[kudu-CR] build: adjust the number of processors for various tests

[Adar Dembo (Code Review)]

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

Change subject: build: adjust the number of processors for various tests
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I922031af69178b98459d0d59287442b692425afa
Gerrit-Change-Number: 9503
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 06 Mar 2018 22:54:53 +
Gerrit-HasComments: No

[kudu-CR] [docs] Improvements to multi-master migration doc

[Will Berkeley (Code Review)]

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

Change subject: [docs] Improvements to multi-master migration doc
..


Patch Set 3:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/9466/3/docs/administration.adoc
File docs/administration.adoc:

http://gerrit.cloudera.org:8080/#/c/9466/3/docs/administration.adoc@631
PS3, Line 631: Congratulations, the masters have now been removed! To verify 
that all masters are working properly,
> Could you reformat this the same way you did the migration doc?
Done


http://gerrit.cloudera.org:8080/#/c/9466/3/docs/administration.adoc@654
PS3, Line 654: $ kudu cluster ksck 
master-01.example.com,master-02.example.com,master-03.example.com
> This (and the other ksck commands) should get a "sudo -u kudu".
Done


http://gerrit.cloudera.org:8080/#/c/9466/3/docs/administration.adoc@709
PS3, Line 709: === Changing Directory Configurations
> Could you bug Andrew to update this section? KUDU-2202 has been fixed and e
Done


http://gerrit.cloudera.org:8080/#/c/9466/3/docs/administration.adoc@754
PS3, Line 754: $ kudu fs update_dirs --fs_wal_dir=/wals 
--fs_data_dirs=/data/1,/data/2,/data/3
> And "sudo -u kudu" in these commands too.
Done


http://gerrit.cloudera.org:8080/#/c/9466/3/docs/administration.adoc@813
PS3, Line 813: // TODO(awong): revise this when KUDU-616 is complete.
> Could you bug Andrew about this too?
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77ef796f8b35729871ef8ddf2b635989278c2ebc
Gerrit-Change-Number: 9466
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Tue, 06 Mar 2018 22:48:23 +
Gerrit-HasComments: Yes

[kudu-CR] [docs] Improvements to multi-master migration doc

[Will Berkeley (Code Review)]

Hello Alex Rodoni, Kudu Jenkins, Andrew Wong, Adar Dembo,

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

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

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

Change subject: [docs] Improvements to multi-master migration doc
..

[docs] Improvements to multi-master migration doc

- Add extra reminder to run as the Kudu user.

- Note that the copy_from_remote command requires authenticating
  to the remote service as the Kudu user.

- Note that the workflow can be used to migrate 2->3 masters by
  making straightforward adjustments to the procedure.

- Move steps for verifying the migration was successful to a new
  section so they are more noticeable.

Change-Id: I77ef796f8b35729871ef8ddf2b635989278c2ebc
---
M docs/administration.adoc
1 file changed, 46 insertions(+), 38 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I77ef796f8b35729871ef8ddf2b635989278c2ebc
Gerrit-Change-Number: 9466
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] docs: add warning to wait between FS rebuilds

[Todd Lipcon (Code Review)]

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

Change subject: docs: add warning to wait between FS rebuilds
..


Patch Set 4: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5659c2ba05a0a6b9905213c5df6ba0dcb371f312
Gerrit-Change-Number: 9279
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 06 Mar 2018 22:44:04 +
Gerrit-HasComments: No

[kudu-CR] docs: add warning to wait between FS rebuilds

[Todd Lipcon (Code Review)]

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

Change subject: docs: add warning to wait between FS rebuilds
..

docs: add warning to wait between FS rebuilds

Customers who are rebuilding multiple Kudu nodes may permanently delete
all healthy replicas for a tablet. This won't prevent data loss in the
event that a only single copy remains and is deleted, but waiting should
help prevent such situations.

Change-Id: I5659c2ba05a0a6b9905213c5df6ba0dcb371f312
Reviewed-on: http://gerrit.cloudera.org:8080/9279
Reviewed-by: Alex Rodoni 
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
M docs/administration.adoc
1 file changed, 5 insertions(+), 0 deletions(-)

Approvals:
  Alex Rodoni: Looks good to me, but someone else must approve
  Kudu Jenkins: Verified
  Todd Lipcon: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I5659c2ba05a0a6b9905213c5df6ba0dcb371f312
Gerrit-Change-Number: 9279
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode

[David Ribeiro Alves (Code Review)]

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

Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode
..


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java:

http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1150
PS7, Line 1150: // Fail the main thread if ever encounter AssertionError.
  : if (assertionError != null) {
  :   fail("the test should not throw AssertionError: " + 
assertionError);
  : }
> Since the JUnit only captures assertion errors in the main thread of the te
right this kind of mess is why futures are much better (and the impl in line 
918 seems to have a bug btw).
you could not catch the exception in the thread and just have
ExecutorService threadPool = Executors.newFixedThreadPool(3);
Future future = threadPool.submit(new MyTest());

when you do future.get() if an exception occurred it'll propagate to the caller 
thread, no need to catch any exception inside runnable



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465
Gerrit-Change-Number: 8847
Gerrit-PatchSet: 7
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 06 Mar 2018 22:43:55 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2309: /masters can show the wrong list of masters

[Will Berkeley (Code Review)]

Will Berkeley has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9378 )

Change subject: KUDU-2309: /masters can show the wrong list of masters
..

KUDU-2309: /masters can show the wrong list of masters

The ListMasters RPC (on which the /masters page is based) returned
masters based on the -master_addresses flag. It should return masters
based on the consensus configuration. Since we verify the two sets of
addresses are the same at startup, this usually makes no difference.
However, if a master is replaced by a server with a new UUID,
previously we would report the new UUID, even though it may not be
part of the previously established quorum. This patch adds an
additional check that the UUIDs match, and returns an error along with
the registration information if there is a mismatch.

Screenshot for down master: 
https://github.com/wdberkeley/kudu/blob/kudu2309screenshots/www/mastersonedown.png
Screenshot for UUID mismatch: 
https://github.com/wdberkeley/kudu/blob/kudu2309screenshots/www/mastersmismatch.png

Change-Id: I603ebc22e998bac9bd00edc939577ae339587f26
Reviewed-on: http://gerrit.cloudera.org:8080/9378
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
M src/kudu/master/master.cc
M src/kudu/master/master_path_handlers.cc
M src/kudu/master/sys_catalog.h
M www/masters.mustache
4 files changed, 53 insertions(+), 27 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I603ebc22e998bac9bd00edc939577ae339587f26
Gerrit-Change-Number: 9378
Gerrit-PatchSet: 6
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] KUDU-2309: /masters can show the wrong list of masters

[Todd Lipcon (Code Review)]

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

Change subject: KUDU-2309: /masters can show the wrong list of masters
..


Patch Set 5: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I603ebc22e998bac9bd00edc939577ae339587f26
Gerrit-Change-Number: 9378
Gerrit-PatchSet: 5
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Tue, 06 Mar 2018 22:32:14 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2230: java: sync client exception stack traces should make sense

[Alexey Serbin (Code Review)]

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

Change subject: KUDU-2230: java: sync client exception stack traces should make 
sense
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9489/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.java@92
PS1, Line 92:   StackTraceElement[] stack = new Exception().getStackTrace();
> I was thinking about saving the original but I think if we print it out in
Thank you for the explanation.

Yep, I think that adding the original stack trace into the debug or trace level 
message would make sense.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I874d47b5239bcc8493c15ef763ecfe0fd878d795
Gerrit-Change-Number: 9489
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 06 Mar 2018 22:27:06 +
Gerrit-HasComments: Yes

[kudu-CR] [webui] Add warning for even number of masters

[Will Berkeley (Code Review)]

Will Berkeley has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9513 )

Change subject: [webui] Add warning for even number of masters
..

[webui] Add warning for even number of masters

Preview at 
https://github.com/wdberkeley/kudu/blob/warnevenmasters_screenshots/www/evenmasterwarning.png

Change-Id: I9838ca99423722492bc2d4f27eb5fd8f5cbb199c
Reviewed-on: http://gerrit.cloudera.org:8080/9513
Reviewed-by: Todd Lipcon 
Tested-by: Kudu Jenkins
---
M src/kudu/master/master_options.cc
M src/kudu/master/master_path_handlers.cc
M www/masters.mustache
3 files changed, 9 insertions(+), 0 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I9838ca99423722492bc2d4f27eb5fd8f5cbb199c
Gerrit-Change-Number: 9513
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] build: adjust the number of processors for various tests

[Todd Lipcon (Code Review)]

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

Change subject: build: adjust the number of processors for various tests
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9503/2/build-support/run-test.sh
File build-support/run-test.sh:

http://gerrit.cloudera.org:8080/#/c/9503/2/build-support/run-test.sh@173
PS2, Line 173: && [ "$KUDU_MEASURE_TEST_CPU_CONSUMPTION" -ne 0 ]
> Fine by me, I would just like some consistency between the various "If FOO
ok I fixed KUDU_REPORT_TEST_RESULTS



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I922031af69178b98459d0d59287442b692425afa
Gerrit-Change-Number: 9503
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 06 Mar 2018 21:59:42 +
Gerrit-HasComments: Yes

[kudu-CR] build: adjust the number of processors for various tests

[Todd Lipcon (Code Review)]

Hello Kudu Jenkins, Adar Dembo,

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

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

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

Change subject: build: adjust the number of processors for various tests
..

build: adjust the number of processors for various tests

Many of our tests are multithreaded and can utilize many cores worth of
CPU during execution. Other tests are somewhat timing-sensitive and
might time out if run in coordination with a stress test, particularly
on a small machine which might only have four CPUs total.

This patch adjusts the core count property for a bunch of tests based on
some empirical analysis using a new 'perf-stat' wrapper capability
provided in run-test.sh. The wrapper produces periodic samples of the
CPU usage for each test. Measurements are written into log files which
can then be analyzed with a command like the following:

for x in build/latest/test-logs/*.perf.txt ; do
  echo -n $x
  grep task-clock $x | sort -nk2 | tail -1
done | sort -nk3

This resulted in a list of tests along with their peak CPU usage (from
their busiest 1000ms period). I used this as a rough guide to find tests
with inappropriate settings in CMakeLists.txt. The net effect of this
change is that 'ctest -j' might run slightly longer, but that we'll have
fewer flaky tests in such a run configuration because we don't have
stress tests perturbing timing of unrelated non-stress tests.

Change-Id: I922031af69178b98459d0d59287442b692425afa
---
M CMakeLists.txt
M build-support/run-test.sh
M src/kudu/cfile/CMakeLists.txt
M src/kudu/client/CMakeLists.txt
M src/kudu/clock/CMakeLists.txt
M src/kudu/consensus/CMakeLists.txt
M src/kudu/integration-tests/CMakeLists.txt
M src/kudu/mini-cluster/CMakeLists.txt
M src/kudu/rpc/CMakeLists.txt
M src/kudu/security/CMakeLists.txt
M src/kudu/tablet/CMakeLists.txt
M src/kudu/tools/CMakeLists.txt
M src/kudu/tserver/CMakeLists.txt
M src/kudu/util/CMakeLists.txt
14 files changed, 80 insertions(+), 48 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I922031af69178b98459d0d59287442b692425afa
Gerrit-Change-Number: 9503
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2230: java: sync client exception stack traces should make sense

[Todd Lipcon (Code Review)]

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

Change subject: KUDU-2230: java: sync client exception stack traces should make 
sense
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9489/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.java@92
PS1, Line 92:   StackTraceElement[] stack = new Exception().getStackTrace();
> Would it make sense to safe the original stack into some member of KuduExce
I was thinking about saving the original but I think if we print it out in logs 
it defeats the purpose of removing confusion here. If we just save it in a 
member and don't print it anywhere it doesn't have much use... so I elected to 
just ditch it.

One option I considered was to log it at TRACE level or something so that we 
can get to it if we need to in a debug situation. Do you think that solves the 
use case you had in mind?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I874d47b5239bcc8493c15ef763ecfe0fd878d795
Gerrit-Change-Number: 9489
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 06 Mar 2018 21:34:22 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2230: java: sync client exception stack traces should make sense

[Alexey Serbin (Code Review)]

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

Change subject: KUDU-2230: java: sync client exception stack traces should make 
sense
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9489/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.java@92
PS1, Line 92:   StackTraceElement[] stack = new Exception().getStackTrace();
Would it make sense to safe the original stack into some member of 
KuduException and then print it out along with the new stack in the overridden 
KuduException.printStackStrace() method?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I874d47b5239bcc8493c15ef763ecfe0fd878d795
Gerrit-Change-Number: 9489
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 06 Mar 2018 21:30:08 +
Gerrit-HasComments: Yes

[kudu-CR] [webui] Add warning for even number of masters

[Todd Lipcon (Code Review)]

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

Change subject: [webui] Add warning for even number of masters
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9838ca99423722492bc2d4f27eb5fd8f5cbb199c
Gerrit-Change-Number: 9513
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 06 Mar 2018 20:52:03 +
Gerrit-HasComments: No

[kudu-CR] tablet: remove an ancient TODO about being able to configure storage attributes

[Todd Lipcon (Code Review)]

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

Change subject: tablet: remove an ancient TODO about being able to configure 
storage attributes
..


Patch Set 1: Verified+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b799c4096fa4deba82fcd2d3a63f83ce8677eaa
Gerrit-Change-Number: 9512
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 06 Mar 2018 20:42:09 +
Gerrit-HasComments: No

[kudu-CR] tablet: remove an ancient TODO about being able to configure storage attributes

[Todd Lipcon (Code Review)]

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

Change subject: tablet: remove an ancient TODO about being able to configure 
storage attributes
..

tablet: remove an ancient TODO about being able to configure storage attributes

This TODO was implemented years ago but we forgot to remove it.

Change-Id: I0b799c4096fa4deba82fcd2d3a63f83ce8677eaa
Reviewed-on: http://gerrit.cloudera.org:8080/9512
Reviewed-by: Dan Burkert 
Tested-by: Todd Lipcon 
---
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/multi_column_writer.cc
2 files changed, 0 insertions(+), 10 deletions(-)

Approvals:
  Dan Burkert: Looks good to me, approved
  Todd Lipcon: Verified

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I0b799c4096fa4deba82fcd2d3a63f83ce8677eaa
Gerrit-Change-Number: 9512
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] tablet: remove an ancient TODO about being able to configure storage attributes

[Todd Lipcon (Code Review)]

Todd Lipcon has removed a vote on this change.

Change subject: tablet: remove an ancient TODO about being able to configure 
storage attributes
..


Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/9512
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I0b799c4096fa4deba82fcd2d3a63f83ce8677eaa
Gerrit-Change-Number: 9512
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [webui] Add warning for even number of masters

[Will Berkeley (Code Review)]

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


Change subject: [webui] Add warning for even number of masters
..

[webui] Add warning for even number of masters

Preview at 
https://github.com/wdberkeley/kudu/blob/warnevenmasters_screenshots/www/evenmasterwarning.png

Change-Id: I9838ca99423722492bc2d4f27eb5fd8f5cbb199c
---
M src/kudu/master/master_options.cc
M src/kudu/master/master_path_handlers.cc
M www/masters.mustache
3 files changed, 9 insertions(+), 0 deletions(-)



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

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

[kudu-CR] Make metrics name matching case-insensitive

[Will Berkeley (Code Review)]

Will Berkeley has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9462 )

Change subject: Make metrics name matching case-insensitive
..

Make metrics name matching case-insensitive

Some metrics have mixed-case name, for example

handler_latency_kudu_tserver_TabletCopyService_BeginTabletCopySession

Previously, filtered metrics like /metrics?metrics=copy would omit
this metric since "Copy" doesn't match "copy". This fixes it so
the above metric would be returned.

Change-Id: I49d76f969873c532e7cd297bee6fde13c98c68e7
Reviewed-on: http://gerrit.cloudera.org:8080/9462
Reviewed-by: Adar Dembo 
Tested-by: Kudu Jenkins
---
M src/kudu/util/metrics-test.cc
M src/kudu/util/metrics.cc
2 files changed, 18 insertions(+), 2 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I49d76f969873c532e7cd297bee6fde13c98c68e7
Gerrit-Change-Number: 9462
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] KUDU-721: [Python] Add DECIMAL column type support

[David Ribeiro Alves (Code Review)]

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

Change subject: KUDU-721: [Python] Add DECIMAL column type support
..


Patch Set 4: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8e0855100ab1ea891f990931ec94d0b98c0dece1
Gerrit-Change-Number: 9496
Gerrit-PatchSet: 4
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jordan Birdsell 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Wes McKinney 
Gerrit-Reviewer: a...@phdata.io
Gerrit-Comment-Date: Tue, 06 Mar 2018 20:18:37 +
Gerrit-HasComments: No

[kudu-CR] KUDU-721: [Python] Add DECIMAL column type support

[David Ribeiro Alves (Code Review)]

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

Change subject: KUDU-721: [Python] Add DECIMAL column type support
..


Patch Set 4:

leaving open in case wes/cpcloud want to take a look


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8e0855100ab1ea891f990931ec94d0b98c0dece1
Gerrit-Change-Number: 9496
Gerrit-PatchSet: 4
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jordan Birdsell 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Wes McKinney 
Gerrit-Reviewer: a...@phdata.io
Gerrit-Comment-Date: Tue, 06 Mar 2018 20:18:59 +
Gerrit-HasComments: No

[kudu-CR] tablet: remove an ancient TODO about being able to configure storage attributes

[Dan Burkert (Code Review)]

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

Change subject: tablet: remove an ancient TODO about being able to configure 
storage attributes
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b799c4096fa4deba82fcd2d3a63f83ce8677eaa
Gerrit-Change-Number: 9512
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 06 Mar 2018 19:58:52 +
Gerrit-HasComments: No

[kudu-CR] tablet: remove an ancient TODO about being able to configure storage attributes

[Todd Lipcon (Code Review)]

Hello Dan Burkert,

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

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

to review the following change.


Change subject: tablet: remove an ancient TODO about being able to configure 
storage attributes
..

tablet: remove an ancient TODO about being able to configure storage attributes

This TODO was implemented years ago but we forgot to remove it.

Change-Id: I0b799c4096fa4deba82fcd2d3a63f83ce8677eaa
---
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/multi_column_writer.cc
2 files changed, 0 insertions(+), 10 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0b799c4096fa4deba82fcd2d3a63f83ce8677eaa
Gerrit-Change-Number: 9512
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 

[kudu-CR] [tools] KUDU-2331: Allow move tool to work when uninvolved tserver is down

[Adar Dembo (Code Review)]

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

Change subject: [tools] KUDU-2331: Allow move tool to work when uninvolved 
tserver is down
..


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/9510/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/9510/2//COMMIT_MSG@7
PS2, Line 7: [tools] KUDU-2331: Allow move tool to work when uninvolved tserver 
is down
Could you add a regression test for this?


http://gerrit.cloudera.org:8080/#/c/9510/2/src/kudu/tools/tool_action_tablet.cc
File src/kudu/tools/tool_action_tablet.cc:

http://gerrit.cloudera.org:8080/#/c/9510/2/src/kudu/tools/tool_action_tablet.cc@150
PS2, Line 150:   // The return Status is ignored since a tserver that is not 
the destination
Would it be worth logging as a warning? Or would that be distracting and not 
useful?


http://gerrit.cloudera.org:8080/#/c/9510/2/src/kudu/tools/tool_action_tablet.cc@154
PS2, Line 154:   ksck.FetchInfoFromTabletServers();
Wrapping this in ignore_result() is an idiomatic way to express that we don't 
care about the result.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id1fc60233f4f50f478da7ccb104e37df3067400c
Gerrit-Change-Number: 9510
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 06 Mar 2018 19:52:26 +
Gerrit-HasComments: Yes

[kudu-CR] build: adjust the number of processors for various tests

[Adar Dembo (Code Review)]

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

Change subject: build: adjust the number of processors for various tests
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9503/2/build-support/run-test.sh
File build-support/run-test.sh:

http://gerrit.cloudera.org:8080/#/c/9503/2/build-support/run-test.sh@173
PS2, Line 173: && [ "$KUDU_MEASURE_TEST_CPU_CONSUMPTION" -ne 0 ]
> I would argue that KUDU_REPORT_TEST_RESULTS is mis-documented. Setting it t
Fine by me, I would just like some consistency between the various "If FOO is 
non-zero, ...".


http://gerrit.cloudera.org:8080/#/c/9503/2/src/kudu/mini-cluster/CMakeLists.txt
File src/kudu/mini-cluster/CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/9503/2/src/kudu/mini-cluster/CMakeLists.txt@52
PS2, Line 52: ADD_KUDU_TEST(external_mini_cluster-test RESOURCE_LOCK 
"master-rpc-ports" PROCESSORS 3)
> You're going to barf a bit in your mouth, but it appears that starting an H
Nice.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I922031af69178b98459d0d59287442b692425afa
Gerrit-Change-Number: 9503
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 06 Mar 2018 19:50:24 +
Gerrit-HasComments: Yes

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

[Adar Dembo (Code Review)]

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

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


Patch Set 4:

(10 comments)

Just responding to your responses, will defer a rereview until the tests pass 
(it's been long enough since my first review that I've forgotten all of the new 
code).

http://gerrit.cloudera.org:8080/#/c/8312/2/src/kudu/integration-tests/CMakeLists.txt
File src/kudu/integration-tests/CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/8312/2/src/kudu/integration-tests/CMakeLists.txt@79
PS2, Line 79: ADD_KUDU_TEST(fuzz-itest RUN_SERIAL true)
> Nit: sort order.
I think you missed this one.


http://gerrit.cloudera.org:8080/#/c/8312/2/src/kudu/integration-tests/master_hms-itest.cc
File src/kudu/integration-tests/master_hms-itest.cc:

http://gerrit.cloudera.org:8080/#/c/8312/2/src/kudu/integration-tests/master_hms-itest.cc@75
PS2, Line 75: op());
> Maybe use ExternalMiniClusterITestBase?
Done


http://gerrit.cloudera.org:8080/#/c/8312/2/src/kudu/integration-tests/master_hms-itest.cc@79
PS2, Line 79:   Status StopHms() {
> Can this be omitted?
Done


http://gerrit.cloudera.org:8080/#/c/8312/2/src/kudu/integration-tests/master_hms-itest.cc@112
PS2, Line 112: b.AddColumn("decimal32_val")->Type(KuduColumnSchema::DECIMAL)
> ???
The table_name() function expects a cref string, so std::move(table_name) has 
no effect.


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

http://gerrit.cloudera.org:8080/#/c/8312/2/src/kudu/master/catalog_manager.cc@1066
PS2, Line 1066: return s;
> The order of operations in CatalogManager::Shutdown is tricky and has bitte
I think you missed this one.


http://gerrit.cloudera.org:8080/#/c/8312/2/src/kudu/master/catalog_manager.cc@1598
PS2, Line 1598:   }
> Why is it OK if this succeeds but step 3 fails? I can take my answer in the
Hmm, the new behavior treats HMS table entry dropping as fatal, so this new 
comment is no longer correct.


http://gerrit.cloudera.org:8080/#/c/8312/2/src/kudu/master/catalog_manager.cc@2092
PS2, Line 2092:   case AlterTableRequestPB::DROP_RANGE_PARTITION: {
> Shouldn't some aspect of this be conditioned on one (or several) of has_foo
I think you missed this.


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

http://gerrit.cloudera.org:8080/#/c/8312/4/src/kudu/master/catalog_manager.cc@1561
PS4, Line 1561:   // TODO(dan): figure out how to test this.
You can inject faults into SysCatalogTable::SyncWrite. Maybe that will help?


http://gerrit.cloudera.org:8080/#/c/8312/4/src/kudu/master/catalog_manager.cc@2253
PS4, Line 2253:   // TODO(dan): figure out how to test this.
See an earlier comment about injecting failures into syscatalog writes.


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

http://gerrit.cloudera.org:8080/#/c/8312/3/src/kudu/master/hms_catalog.h@29
PS3, Line 29: #include "kudu/util/net/net_util.h"
> optional seems to require the include in order to compile.
Not using optional anymore?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1b8f360408b05d6ac4f7359a8ac0dd889b196baf
Gerrit-Change-Number: 8312
Gerrit-PatchSet: 4
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 06 Mar 2018 19:48:33 +
Gerrit-HasComments: Yes

[kudu-CR] [tools] KUDU-2331: Allow move tool to work when uninvolved tserver is down

[Will Berkeley (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: [tools] KUDU-2331: Allow move tool to work when uninvolved 
tserver is down
..

[tools] KUDU-2331: Allow move tool to work when uninvolved tserver is down

The move tool requires a clean ksck for the tablet it's acting on
for safety reasons, but the check for this failed if a tablet server
was down, even if the tablet server didn't host a replica for the
tablet and wasn't the destination server. This fixes the move
command so a down tserver uninvolved in the move won't prevent the
move.

Change-Id: Id1fc60233f4f50f478da7ccb104e37df3067400c
---
M src/kudu/tools/tool_action_tablet.cc
1 file changed, 5 insertions(+), 1 deletion(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id1fc60233f4f50f478da7ccb104e37df3067400c
Gerrit-Change-Number: 9510
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [tools] Allow move tool to work when uninvolved tserver is down

[Will Berkeley (Code Review)]

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


Change subject: [tools] Allow move tool to work when uninvolved tserver is down
..

[tools] Allow move tool to work when uninvolved tserver is down

The move tool requires a clean ksck for the tablet it's acting on
for safety reasons, but the check for this failed if a tablet server
was down, even if the tablet server didn't host a replica for the
tablet and wasn't the destination server. This fixes the move
command so a down tserver uninvolved in the move won't prevent the
move.

Change-Id: Id1fc60233f4f50f478da7ccb104e37df3067400c
---
M src/kudu/tools/tool_action_tablet.cc
1 file changed, 5 insertions(+), 1 deletion(-)



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

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

[kudu-CR] build: adjust the number of processors for various tests

[Todd Lipcon (Code Review)]

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

Change subject: build: adjust the number of processors for various tests
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9503/2/build-support/run-test.sh
File build-support/run-test.sh:

http://gerrit.cloudera.org:8080/#/c/9503/2/build-support/run-test.sh@173
PS2, Line 173: && [ "$KUDU_MEASURE_TEST_CPU_CONSUMPTION" -ne 0 ]
> This isn't really necessary; see KUDU_REPORT_TEST_RESULTS.
I would argue that KUDU_REPORT_TEST_RESULTS is mis-documented. Setting it to 
'0' will result in reporting test results, because it's non-empty. Perhaps we 
should fix that to be more like this?


http://gerrit.cloudera.org:8080/#/c/9503/2/src/kudu/mini-cluster/CMakeLists.txt
File src/kudu/mini-cluster/CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/9503/2/src/kudu/mini-cluster/CMakeLists.txt@52
PS2, Line 52: ADD_KUDU_TEST(external_mini_cluster-test RESOURCE_LOCK 
"master-rpc-ports" PROCESSORS 3)
> This is surprising; external_mini_cluster-test basically just restarts a cl
You're going to barf a bit in your mouth, but it appears that starting an HMS 
monopolizes 3 cores for several seconds.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I922031af69178b98459d0d59287442b692425afa
Gerrit-Change-Number: 9503
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 06 Mar 2018 19:33:48 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2312: Scan predicate application ordering is non-deterministic

[Dan Burkert (Code Review)]

Dan Burkert has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9440 )

Change subject: KUDU-2312: Scan predicate application ordering is 
non-deterministic
..

KUDU-2312: Scan predicate application ordering is non-deterministic

This changes the scan predicate evaluation ordering so that it primarily
orders by selectivity (as before), but breaks ties by column index.

Change-Id: I99b2cabecd8626cad7e11fbdd492af7276e08348
Reviewed-on: http://gerrit.cloudera.org:8080/9440
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
M docs/release_notes.adoc
M src/kudu/common/generic_iterators-test.cc
M src/kudu/common/generic_iterators.cc
M src/kudu/common/generic_iterators.h
M src/kudu/tablet/cfile_set.h
5 files changed, 117 insertions(+), 20 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I99b2cabecd8626cad7e11fbdd492af7276e08348
Gerrit-Change-Number: 9440
Gerrit-PatchSet: 5
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Make metrics name matching case-insensitive

[Adar Dembo (Code Review)]

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

Change subject: Make metrics name matching case-insensitive
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I49d76f969873c532e7cd297bee6fde13c98c68e7
Gerrit-Change-Number: 9462
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Tue, 06 Mar 2018 19:10:16 +
Gerrit-HasComments: No

[kudu-CR] [docs] Improvements to multi-master migration doc

[Adar Dembo (Code Review)]

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

Change subject: [docs] Improvements to multi-master migration doc
..


Patch Set 3:

(5 comments)

In the vein of "no good deed goes unpunished"...

http://gerrit.cloudera.org:8080/#/c/9466/3/docs/administration.adoc
File docs/administration.adoc:

http://gerrit.cloudera.org:8080/#/c/9466/3/docs/administration.adoc@631
PS3, Line 631: Congratulations, the masters have now been removed! To verify 
that all masters are working properly,
Could you reformat this the same way you did the migration doc?


http://gerrit.cloudera.org:8080/#/c/9466/3/docs/administration.adoc@654
PS3, Line 654: $ kudu cluster ksck 
master-01.example.com,master-02.example.com,master-03.example.com
This (and the other ksck commands) should get a "sudo -u kudu".


http://gerrit.cloudera.org:8080/#/c/9466/3/docs/administration.adoc@709
PS3, Line 709: === Changing Directory Configurations
Could you bug Andrew to update this section? KUDU-2202 has been fixed and even 
though KUDU-2117 is still open, the new --fs_metadata_dir flag means the "add" 
section should be updated.


http://gerrit.cloudera.org:8080/#/c/9466/3/docs/administration.adoc@754
PS3, Line 754: $ kudu fs update_dirs --fs_wal_dir=/wals 
--fs_data_dirs=/data/1,/data/2,/data/3
And "sudo -u kudu" in these commands too.


http://gerrit.cloudera.org:8080/#/c/9466/3/docs/administration.adoc@813
PS3, Line 813: // TODO(awong): revise this when KUDU-616 is complete.
Could you bug Andrew about this too?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77ef796f8b35729871ef8ddf2b635989278c2ebc
Gerrit-Change-Number: 9466
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Tue, 06 Mar 2018 19:09:20 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-721: [Python] Add DECIMAL column type support

[Grant Henke (Code Review)]

Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9496 )

Change subject: KUDU-721: [Python] Add DECIMAL column type support
..


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/9496/2/python/kudu/libkudu_client.pxd
File python/kudu/libkudu_client.pxd:

http://gerrit.cloudera.org:8080/#/c/9496/2/python/kudu/libkudu_client.pxd@65
PS2, Line 65: cdef extern from "kudu/util/int128.h" namespace "kudu" nogil:
: ctypedef int int128_t
> that's fair regarding the typedef, regarding the nogil: the gil or "global
Done


http://gerrit.cloudera.org:8080/#/c/9496/3/python/kudu/tests/test_scanner.py
File python/kudu/tests/test_scanner.py:

http://gerrit.cloudera.org:8080/#/c/9496/3/python/kudu/tests/test_scanner.py@263
PS3, Line 263: def test_decimal_pred(self):
 : # Test a decimal predicate
 : # Does a row check count only
> why is decimal row check count only?
Done


http://gerrit.cloudera.org:8080/#/c/9496/3/python/kudu/util.py
File python/kudu/util.py:

http://gerrit.cloudera.org:8080/#/c/9496/3/python/kudu/util.py@105
PS3, Line 105:
> I think the style is to have two lines between methods. can you add one her
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8e0855100ab1ea891f990931ec94d0b98c0dece1
Gerrit-Change-Number: 9496
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jordan Birdsell 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Wes McKinney 
Gerrit-Reviewer: a...@phdata.io
Gerrit-Comment-Date: Tue, 06 Mar 2018 19:03:16 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-721: [Python] Add DECIMAL column type support

[Grant Henke (Code Review)]

Hello a...@phdata.io, David Ribeiro Alves, Wes McKinney, Kudu Jenkins, Jordan 
Birdsell, Adar Dembo,

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

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

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

Change subject: KUDU-721: [Python] Add DECIMAL column type support
..

KUDU-721: [Python] Add DECIMAL column type support

This patch adds basic support to the Python client to
create, read, and write tables with DECIMAL columns.

Change-Id: I8e0855100ab1ea891f990931ec94d0b98c0dece1
---
M python/kudu/__init__.py
M python/kudu/client.pyx
M python/kudu/libkudu_client.pxd
M python/kudu/schema.pxd
M python/kudu/schema.pyx
M python/kudu/tests/test_scanner.py
M python/kudu/tests/test_scantoken.py
M python/kudu/tests/test_schema.py
A python/kudu/tests/test_util.py
M python/kudu/tests/util.py
M python/kudu/util.py
11 files changed, 368 insertions(+), 11 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8e0855100ab1ea891f990931ec94d0b98c0dece1
Gerrit-Change-Number: 9496
Gerrit-PatchSet: 4
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jordan Birdsell 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Wes McKinney 
Gerrit-Reviewer: a...@phdata.io

[kudu-CR] build: adjust the number of processors for various tests

[Adar Dembo (Code Review)]

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

Change subject: build: adjust the number of processors for various tests
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9503/2/build-support/run-test.sh
File build-support/run-test.sh:

http://gerrit.cloudera.org:8080/#/c/9503/2/build-support/run-test.sh@173
PS2, Line 173: && [ "$KUDU_MEASURE_TEST_CPU_CONSUMPTION" -ne 0 ]
This isn't really necessary; see KUDU_REPORT_TEST_RESULTS.


http://gerrit.cloudera.org:8080/#/c/9503/2/src/kudu/mini-cluster/CMakeLists.txt
File src/kudu/mini-cluster/CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/9503/2/src/kudu/mini-cluster/CMakeLists.txt@52
PS2, Line 52: ADD_KUDU_TEST(external_mini_cluster-test RESOURCE_LOCK 
"master-rpc-ports" PROCESSORS 3)
This is surprising; external_mini_cluster-test basically just restarts a 
cluster a few times. How did its perf measurements yield a consumption of three 
cores?

Note: I haven't looked at the other PROCESSORS values; just considering this as 
a "spot check" of sorts.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I922031af69178b98459d0d59287442b692425afa
Gerrit-Change-Number: 9503
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 06 Mar 2018 18:57:40 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2309: /masters can show the wrong list of masters

[Sailesh Mukil (Code Review)]

Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9378 )

Change subject: KUDU-2309: /masters can show the wrong list of masters
..


Patch Set 4: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9378/4/www/masters.mustache
File www/masters.mustache:

http://gerrit.cloudera.org:8080/#/c/9378/4/www/masters.mustache@40
PS4, Line 40:   {{^has_no_errors}}
> The '?' operator isn't working when I test it, unfortunately. Also, it appe
Interesting. That's fine by me then!



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I603ebc22e998bac9bd00edc939577ae339587f26
Gerrit-Change-Number: 9378
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Tue, 06 Mar 2018 18:51:02 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2312: Scan predicate application ordering is non-deterministic

[Todd Lipcon (Code Review)]

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

Change subject: KUDU-2312: Scan predicate application ordering is 
non-deterministic
..


Patch Set 4: Code-Review+2

> No, we put the predicates into an hash map, and at that point they lose all 
> ordering (ScanSpec::predicates_).  The /scans page is also populated from 
> this unorderd map, so it doesn't reflect the true ordering either.  I don't 
> think it's wise to guarantee an ordering externally.  Ideally we'd be using 
> our own internal stats to do the predicate evaluation ordering on a per-cfile 
> basis, so there wouldn't even be a consistent global ordering.

OK. Well, you've convinced me that this patch isn't actively harmful. Given we 
don't have any internal cardinality stats, though, I think our next step should 
be to allow Impala to pass through its own ordering explicitly when it _does_ 
have stats. We should see a good speed boost from that until we get to 
eventually computing our own cardinality estimates per-DRS or per-tablet.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b2cabecd8626cad7e11fbdd492af7276e08348
Gerrit-Change-Number: 9440
Gerrit-PatchSet: 4
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 06 Mar 2018 18:50:46 +
Gerrit-HasComments: No

[kudu-CR] KUDU-721: [Python] Add DECIMAL column type support

[David Ribeiro Alves (Code Review)]

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

Change subject: KUDU-721: [Python] Add DECIMAL column type support
..


Patch Set 2:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/9496/2/python/kudu/client.pyx
File python/kudu/client.pyx:

http://gerrit.cloudera.org:8080/#/c/9496/2/python/kudu/client.pyx@1324
PS2, Line 1324: cdef inline get_scale(self, int i):
> The default is 0 for scale because that's the common default for most datab
k, thanks for the explanation


http://gerrit.cloudera.org:8080/#/c/9496/2/python/kudu/libkudu_client.pxd
File python/kudu/libkudu_client.pxd:

http://gerrit.cloudera.org:8080/#/c/9496/2/python/kudu/libkudu_client.pxd@65
PS2, Line 65: cdef extern from "kudu/util/int128.h" namespace "kudu" nogil:
: ctypedef int int128_t
> int128_t isn't actually a c++ type. We define it in int128.h based on signe
that's fair regarding the typedef, regarding the nogil: the gil or "global 
interpreter lock" is python's master lock that is held at all times python code 
is being executed (cpython is single threaded). When you step out of python 
though, like when you're calling C++/C code, you can choose to drop it (and get 
some parallelism that way, that's what numpy and other libraries do).

Dropping the gil only makes sense if you're actually executing something 
though, here it seems like you're just using a typedef so not executing 
anything, dropping the lock should have no effect.


http://gerrit.cloudera.org:8080/#/c/9496/3/python/kudu/tests/test_scanner.py
File python/kudu/tests/test_scanner.py:

http://gerrit.cloudera.org:8080/#/c/9496/3/python/kudu/tests/test_scanner.py@263
PS3, Line 263: def test_decimal_pred(self):
 : # Test a decimal predicate
 : # Does a row check count only
why is decimal row check count only?


http://gerrit.cloudera.org:8080/#/c/9496/3/python/kudu/util.py
File python/kudu/util.py:

http://gerrit.cloudera.org:8080/#/c/9496/3/python/kudu/util.py@105
PS3, Line 105:
I think the style is to have two lines between methods. can you add one here 
too (since you added one above?)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8e0855100ab1ea891f990931ec94d0b98c0dece1
Gerrit-Change-Number: 9496
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jordan Birdsell 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Wes McKinney 
Gerrit-Reviewer: a...@phdata.io
Gerrit-Comment-Date: Tue, 06 Mar 2018 18:50:18 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2309: /masters can show the wrong list of masters

[Will Berkeley (Code Review)]

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

Change subject: KUDU-2309: /masters can show the wrong list of masters
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9378/4/www/masters.mustache
File www/masters.mustache:

http://gerrit.cloudera.org:8080/#/c/9378/4/www/masters.mustache@40
PS4, Line 40:   {{^has_no_errors}}
> I have 2 suggestions below:
The '?' operator isn't working when I test it, unfortunately. Also, it appears 
the Impala mustache library is older at 80cbe5 than ours at 87a592e.

This SO question has some alternative methods:

https://stackoverflow.com/questions/11653764

but I like the current workaround. It's a little goofy because of the extra 
variable and double-negation, but it does exactly what is desired without using 
implementation-specific or unreliable features.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I603ebc22e998bac9bd00edc939577ae339587f26
Gerrit-Change-Number: 9378
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Tue, 06 Mar 2018 18:45:43 +
Gerrit-HasComments: Yes

[kudu-CR] build: adjust the number of processors for various tests

[Todd Lipcon (Code Review)]

Hello Kudu Jenkins, Adar Dembo,

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

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

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

Change subject: build: adjust the number of processors for various tests
..

build: adjust the number of processors for various tests

Many of our tests are multithreaded and can utilize many cores worth of
CPU during execution. Other tests are somewhat timing-sensitive and
might time out if run in coordination with a stress test, particularly
on a small machine which might only have four CPUs total.

This patch adjusts the core count property for a bunch of tests based on
some empirical analysis using a new 'perf-stat' wrapper capability
provided in run-test.sh. The wrapper produces periodic samples of the
CPU usage for each test. Measurements are written into log files which
can then be analyzed with a command like the following:

for x in build/latest/test-logs/*.perf.txt ; do
  echo -n $x
  grep task-clock $x | sort -nk2 | tail -1
done | sort -nk3

This resulted in a list of tests along with their peak CPU usage (from
their busiest 1000ms period). I used this as a rough guide to find tests
with inappropriate settings in CMakeLists.txt. The net effect of this
change is that 'ctest -j' might run slightly longer, but that we'll have
fewer flaky tests in such a run configuration because we don't have
stress tests perturbing timing of unrelated non-stress tests.

Change-Id: I922031af69178b98459d0d59287442b692425afa
---
M CMakeLists.txt
M build-support/run-test.sh
M src/kudu/cfile/CMakeLists.txt
M src/kudu/client/CMakeLists.txt
M src/kudu/clock/CMakeLists.txt
M src/kudu/consensus/CMakeLists.txt
M src/kudu/integration-tests/CMakeLists.txt
M src/kudu/mini-cluster/CMakeLists.txt
M src/kudu/rpc/CMakeLists.txt
M src/kudu/security/CMakeLists.txt
M src/kudu/tablet/CMakeLists.txt
M src/kudu/tools/CMakeLists.txt
M src/kudu/tserver/CMakeLists.txt
M src/kudu/util/CMakeLists.txt
14 files changed, 79 insertions(+), 47 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I922031af69178b98459d0d59287442b692425afa
Gerrit-Change-Number: 9503
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] dist test: fix the ability to pass flags to the 'run' option

[Todd Lipcon (Code Review)]

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

Change subject: dist_test: fix the ability to pass flags to the 'run' option
..

dist_test: fix the ability to pass flags to the 'run' option

The python 'argparse' module ends up considering the special '--' flag
an actual flag in the "remainder". So, for a typical command line like:

  dist_test.py run -- --stress_cpu_threads 3

... it was ending up passing too many '--' through to the underlying
binary, which caused the arguments after it to be ignored.

This simply removes the extra '--' when it is detected.

Change-Id: Id191a6f35520678ce48e78be0507d9f4607a2ea8
Reviewed-on: http://gerrit.cloudera.org:8080/9502
Reviewed-by: Adar Dembo 
Tested-by: Kudu Jenkins
---
M build-support/dist_test.py
1 file changed, 2 insertions(+), 0 deletions(-)

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

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

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

[kudu-CR] KUDU-2309: /masters can show the wrong list of masters

[Sailesh Mukil (Code Review)]

Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9378 )

Change subject: KUDU-2309: /masters can show the wrong list of masters
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9378/4/www/masters.mustache
File www/masters.mustache:

http://gerrit.cloudera.org:8080/#/c/9378/4/www/masters.mustache@40
PS4, Line 40:   {{^has_no_errors}}
> In that case, the {{#errors}} section would need to surround the whole Errors
  

  UUID
  Error


{{#errors}}
  
{{uuid}}
{{error}}
  
{{/errors}}

  
  {{/errors}}

I'm not sure if Impala and Kudu use the same version of mustache, but it's 
worth a try.



A final suggestion is to try this, but it may or may not be what you want. We 
could display the static table headers unconditionally, and only fill in the 
body through the mustache list:


  Errors
  

  UUID
  Error


{{#errors}}
  
{{uuid}}
{{error}}
  
{{/errors}}
{{^errors}}
  
N/A
N/A
  
{{/errors}}

  

If we don't want to display even the table header when there are no errors, 
then this doesn't apply.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I603ebc22e998bac9bd00edc939577ae339587f26
Gerrit-Change-Number: 9378
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Tue, 06 Mar 2018 18:05:46 +
Gerrit-HasComments: Yes

[kudu-CR] build: adjust the number of processors for various tests

[Todd Lipcon (Code Review)]

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

Change subject: build: adjust the number of processors for various tests
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9503/1/src/kudu/util/test_main.cc@78
PS1, Line 78:   Stopwatch sw(Stopwatch::ALL_THREADS);
> yea, I looked at that, but then the average across the entire test lifetime
actually I am now looking at the results of the 'perf stat' experiment (I had 
it run overnight after I left work) and it seems that it reveals a lot more 
tests that use more processors than the method in this patch. So I think it's 
probably worth being accurate here. Makes sense since most of our heavier tests 
are based on external minicluster and a lot of the CPU is consumed by the 
tservers, not the client which runs in the test case itself. I'll revert this 
junk and add an env-var-based method to the test wrapper.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I922031af69178b98459d0d59287442b692425afa
Gerrit-Change-Number: 9503
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 06 Mar 2018 18:03:52 +
Gerrit-HasComments: Yes

[kudu-CR] build: adjust the number of processors for various tests

[Todd Lipcon (Code Review)]

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

Change subject: build: adjust the number of processors for various tests
..


Patch Set 1:

(5 comments)

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

http://gerrit.cloudera.org:8080/#/c/9503/1//COMMIT_MSG@27
PS1, Line 27: I used this as a rough guide to find tests
: with inappropriate settings in CMakeLists.txt.
> What's the net effect of these changes? What can I expect if I run "ctest -
Done


http://gerrit.cloudera.org:8080/#/c/9503/1/CMakeLists.txt
File CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/9503/1/CMakeLists.txt@699
PS1, Line 699: #   a heavy consumer of CPU. The PROCESSORS flag allows 
ctest to ensure
> So the idea is that any test NOT tagged with PROCESSORS or RUN_SERIAL is a
assumption is it uses 1 core. will comment


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

http://gerrit.cloudera.org:8080/#/c/9503/1/src/kudu/util/test_main.cc@77
PS1, Line 77:   const auto kInterval = MonoDelta::FromMilliseconds(500);
> How did you arrive at this particular interval value?
no particular reason... figured that a too-short value might over-estimate CPU 
consumption. If tests use a short burst of lots of CPU at startup or something 
it's not likely to cause other tests to fail. More concerned with if a test 
actually monopolizes cores for a reasonable amount of time


http://gerrit.cloudera.org:8080/#/c/9503/1/src/kudu/util/test_main.cc@78
PS1, Line 78:   Stopwatch sw(Stopwatch::ALL_THREADS);
> What about the answers in https://stackoverflow.com/questions/12871090/how-
yea, I looked at that, but then the average across the entire test lifetime is 
not great.

I also measured things by editing run-test.sh locally to wrap the test run in 
'perf stat -e task-clock -I1000 -o ${LOGFILE}.stat' and ran all the tests. That 
actually measures subprocesses as well, but has the disadvantage it can't be 
used in dist-test (since perf is not available inside our docker containers). 
If you think that way is better I can revert this change and instead add it as 
an environment variable option in run_test.sh?


http://gerrit.cloudera.org:8080/#/c/9503/1/src/kudu/util/test_main.cc@104
PS1, Line 104:
> Nit: revert this?
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I922031af69178b98459d0d59287442b692425afa
Gerrit-Change-Number: 9503
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 06 Mar 2018 17:53:03 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2309: /masters can show the wrong list of masters

[Will Berkeley (Code Review)]

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

Change subject: KUDU-2309: /masters can show the wrong list of masters
..


Patch Set 4:

(4 comments)

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

http://gerrit.cloudera.org:8080/#/c/9378/3/src/kudu/master/master.cc@331
PS3, Line 331: LOG(WARNING) << s.ToString();
> Just another thought: maybe, it's worth at least add the UUID of the peer f
Done


http://gerrit.cloudera.org:8080/#/c/9378/3/www/masters.mustache
File www/masters.mustache:

http://gerrit.cloudera.org:8080/#/c/9378/3/www/masters.mustache@40
PS3, Line 40:   {{^has_no_errors}}
> Does it make sense to show this if there are no errors?
Done


http://gerrit.cloudera.org:8080/#/c/9378/4/www/masters.mustache
File www/masters.mustache:

http://gerrit.cloudera.org:8080/#/c/9378/4/www/masters.mustache@40
PS4, Line 40:   {{^has_no_errors}}
> mustache doesn't have a way to condition based on the length of the 'errors
Yep. You can say "if the array is empty, do this" and "if the array is 
nonempty, do this for each element", but you can't say "if the array is 
nonempty, do this *once*", AFAIK.


http://gerrit.cloudera.org:8080/#/c/9378/4/www/masters.mustache@40
PS4, Line 40:   {{^has_no_errors}}
> "An inverted section begins with a caret (hat) and ends with a slash. That
In that case, the {{#errors}} section would need to surround the whole  
element in order to have it be suppressed when there are no errors-- but then 
there'd be one table per error.

I spent a bit of time trying things to make this exact situation work while 
doing previous mustache patches for Kudu, and I think this is the simplest way 
to do it. I'd be happy if someone had a better way, though.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I603ebc22e998bac9bd00edc939577ae339587f26
Gerrit-Change-Number: 9378
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Tue, 06 Mar 2018 17:06:26 +
Gerrit-HasComments: Yes

[kudu-CR] Make metrics name matching case-insensitive

[Will Berkeley (Code Review)]

Hello Kudu Jenkins, Andrew Wong, Adar Dembo,

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

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

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

Change subject: Make metrics name matching case-insensitive
..

Make metrics name matching case-insensitive

Some metrics have mixed-case name, for example

handler_latency_kudu_tserver_TabletCopyService_BeginTabletCopySession

Previously, filtered metrics like /metrics?metrics=copy would omit
this metric since "Copy" doesn't match "copy". This fixes it so
the above metric would be returned.

Change-Id: I49d76f969873c532e7cd297bee6fde13c98c68e7
---
M src/kudu/util/metrics-test.cc
M src/kudu/util/metrics.cc
2 files changed, 18 insertions(+), 2 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I49d76f969873c532e7cd297bee6fde13c98c68e7
Gerrit-Change-Number: 9462
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] [docs] Improvements to multi-master migration doc

[Will Berkeley (Code Review)]

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

Change subject: [docs] Improvements to multi-master migration doc
..


Patch Set 2:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/9466/2/docs/administration.adoc
File docs/administration.adoc:

http://gerrit.cloudera.org:8080/#/c/9466/2/docs/administration.adoc@297
PS2, Line 297: , which should be run as the Kudu UNIX user, typically `kudu`
> Given the addition of "sudo -u kudu" throughout (and the prior existence of
Done


http://gerrit.cloudera.org:8080/#/c/9466/2/docs/administration.adoc@354
PS2, Line 354: . If your Kudu cluster is secure, in addition to running as the 
Kudu UNIX user, you must
> Hmm, breaking it out like this will make it look like a separate step. What
Done


http://gerrit.cloudera.org:8080/#/c/9466/2/docs/administration.adoc@354
PS2, Line 354: . If your Kudu cluster is secure, in addition to running as the 
Kudu UNIX user, you must
> I agree, I think pushing this into a WARNING would read better. I'm not sur
I think how exactly to authenticate is out of scope and might depend on the 
user's environment. I don't want to start writing a Kerberos manual.


http://gerrit.cloudera.org:8080/#/c/9466/2/docs/administration.adoc@475
PS2, Line 475: $ kudu fs dump uuid --fs_wal_dir= 
[--fs_data_dirs=] 2>/dev/null
> Could you add "sudo -u kudu" to these steps too?
Done


http://gerrit.cloudera.org:8080/#/c/9466/2/docs/administration.adoc@534
PS2, Line 534: . Copy the master data to the replacement master with the 
following command:
> Need the authentication warning here too.
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77ef796f8b35729871ef8ddf2b635989278c2ebc
Gerrit-Change-Number: 9466
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Tue, 06 Mar 2018 16:52:57 +
Gerrit-HasComments: Yes

[kudu-CR] [docs] Improvements to multi-master migration doc

[Will Berkeley (Code Review)]

Hello Alex Rodoni, Kudu Jenkins, Andrew Wong, Adar Dembo,

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

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

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

Change subject: [docs] Improvements to multi-master migration doc
..

[docs] Improvements to multi-master migration doc

- Add extra reminder to run as the Kudu user.

- Note that the copy_from_remote command requires authenticating
  to the remote service as the Kudu user.

- Note that the workflow can be used to migrate 2->3 masters by
  making straightforward adjustments to the procedure.

- Move steps for verifying the migration was successful to a new
  section so they are more noticeable.

Change-Id: I77ef796f8b35729871ef8ddf2b635989278c2ebc
---
M docs/administration.adoc
1 file changed, 38 insertions(+), 31 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I77ef796f8b35729871ef8ddf2b635989278c2ebc
Gerrit-Change-Number: 9466
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley