[kudu-CR] [tools] ksck improvements [6/n]: Refactor printing

2018-04-25 Thread Will Berkeley (Code Review)
Hello Alexey Serbin, Kudu Jenkins,

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

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

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

Change subject: [tools] ksck improvements [6/n]: Refactor printing
..

[tools] ksck improvements [6/n]: Refactor printing

This patch significantly refactors printing in ksck. It separates where
checks are done from where results are printed. This separation will
make it easier to provide machine-readable output and create tools
based on the results of ksck checks, like auto-repair.

There's also a couple of bonus changes:
- Simplifies running all ksck checks and printing the results, as done
  by the ksck tool and ClusterVerifier.
- Removes the --consensus flag. ksck is far less useful without it and
  it doesn't seem like having it brings any benefit.
- Add the changes in https://gerrit.cloudera.org/#/c/10054/, which were
  about to be committed but would've needed to be redone a bit to fit
  with this refactor.

Change-Id: Id8de619996b6cd753e6a9c01b1b60810a873e609
---
M src/kudu/integration-tests/cluster_verifier.cc
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_remote-test.cc
A src/kudu/tools/ksck_results.cc
A src/kudu/tools/ksck_results.h
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/tool_action_cluster.cc
M src/kudu/tools/tool_action_tablet.cc
11 files changed, 1,206 insertions(+), 892 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id8de619996b6cd753e6a9c01b1b60810a873e609
Gerrit-Change-Number: 10151
Gerrit-PatchSet: 9
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] KUDU-2191: Metadata Upgrade Tool

2018-04-25 Thread Hao Hao (Code Review)
Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10075 )

Change subject: KUDU-2191: Metadata Upgrade Tool
..


Patch Set 7:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/10075/6//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/10075/6//COMMIT_MSG@9
PS6, Line 9: a
> nit: an
Done


http://gerrit.cloudera.org:8080/#/c/10075/6//COMMIT_MSG@13
PS6, Line 13:
> nit: expected
Done


http://gerrit.cloudera.org:8080/#/c/10075/6/build-support/iwyu.py
File build-support/iwyu.py:

http://gerrit.cloudera.org:8080/#/c/10075/6/build-support/iwyu.py@86
PS6, Line 86:   "src/kudu/server/webserver.cc",
:   "src/kudu/util/bit-util-test.cc",
> nit: just in case if you haven't looked at that yet, Todd added a dependenc
Done


http://gerrit.cloudera.org:8080/#/c/10075/6/src/kudu/tools/tool_action_hms.cc
File src/kudu/tools/tool_action_hms.cc:

http://gerrit.cloudera.org:8080/#/c/10075/6/src/kudu/tools/tool_action_hms.cc@92
PS6, Line 92: table_name
> What if 'table_name' is already a 'new' one?  I.e., my concern is that this
I think it is hard to determine whether the tables are legacy or not by names. 
Since legacy tables can also contain '.' in the name. I think it is safe to 
assume the admin run this tool as the first thing to do when enabling hms 
integration.


http://gerrit.cloudera.org:8080/#/c/10075/6/src/kudu/tools/tool_action_hms.cc@142
PS6, Line 142: URIs
> nit: URIs
Done


http://gerrit.cloudera.org:8080/#/c/10075/6/src/kudu/tools/tool_action_hms.cc@188
PS6, Line 188:   .AddRequiredParameter({ kDefaultDatabaseArg, 
kDefaultDatabaseArgDesc })
 :   .AddOptionalParameter("hive_metastore_uris")
 :   .AddOptionalParameter("hive_metastore_sasl_enabled")
 :   .AddOptionalParameter("hive_metastore_retry_count")
 :   .AddOptionalParameter("hive_metastore_send_timeout")
 :   .AddOptionalParameter("hive_metastore_recv_timeout")
> usability nit: does it make sense to remove the 'hive_metastore_' prefix ?
All of these flags are already defined in hms_catalog.cc. Here just reuse these 
flags to avoid double flags.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1c8488e1c7b04b8bedc31c83b1496b53bae49cb3
Gerrit-Change-Number: 10075
Gerrit-PatchSet: 7
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Thu, 26 Apr 2018 05:18:35 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-2191: Metadata Upgrade Tool

2018-04-25 Thread Hao Hao (Code Review)
Hello Tidy Bot, Alexey Serbin, Dan Burkert, Kudu Jenkins,

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

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

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

Change subject: KUDU-2191: Metadata Upgrade Tool
..

KUDU-2191: Metadata Upgrade Tool

This commit introduces an upgrade tool to allows backward compatibility.
It provides support to upgrade the metadata format of existing Impala
managed/external tables in HMS entries, as well as rename the existing
tables in Kudu to adapt to the new naming rules. It adds test case
using external mini cluster to ensure the upgrade tool works as
expected.

Change-Id: I1c8488e1c7b04b8bedc31c83b1496b53bae49cb3
---
M 
java/kudu-hive/src/main/java/org/apache/kudu/hive/metastore/KuduMetastorePlugin.java
M 
java/kudu-hive/src/test/java/org/apache/kudu/hive/metastore/TestKuduMetastorePlugin.java
M src/kudu/client/client-test-util.cc
M src/kudu/client/client-test-util.h
M src/kudu/client/schema.h
M src/kudu/hms/hms_catalog-test.cc
M src/kudu/hms/hms_catalog.cc
M src/kudu/hms/hms_catalog.h
M src/kudu/hms/hms_client.cc
M src/kudu/hms/hms_client.h
M src/kudu/integration-tests/alter_table-randomized-test.cc
M src/kudu/integration-tests/master_failover-itest.cc
M src/kudu/integration-tests/master_hms-itest.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool.proto
M src/kudu/tools/tool_action.h
A src/kudu/tools/tool_action_hms.cc
M src/kudu/tools/tool_action_test.cc
M src/kudu/tools/tool_main.cc
22 files changed, 659 insertions(+), 25 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1c8488e1c7b04b8bedc31c83b1496b53bae49cb3
Gerrit-Change-Number: 10075
Gerrit-PatchSet: 7
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot


[kudu-CR] KUDU-2191: HMS Metadata Consistency Check Tool

2018-04-25 Thread Hao Hao (Code Review)
Hao Hao has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10217


Change subject: KUDU-2191: HMS Metadata Consistency Check Tool
..

KUDU-2191: HMS Metadata Consistency Check Tool

This commit introduces a metadata consistency check CLI tool for Hive
Metastore integration. It checks metadata (table ID, table name, Kudu
master addresses, and colnum name and type) between Kudu and HMS, and
report inconsistent metadata to the user if any. It adds test case
using external mini cluster to ensure the tool works as expected.

Change-Id: I3495e9c755571b85beaecca849f83457b592ee90
---
M src/kudu/hms/hms_catalog-test.cc
M src/kudu/hms/hms_catalog.cc
M src/kudu/hms/hms_catalog.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_hms.cc
5 files changed, 299 insertions(+), 31 deletions(-)



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

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


[kudu-CR] [Java] Check in the Gradle wrapper properties

2018-04-25 Thread Grant Henke (Code Review)
Grant Henke has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10215


Change subject: [Java] Check in the Gradle wrapper properties
..

[Java] Check in the Gradle wrapper properties

We were downloading the properties file, but that can result in
using and outdated Gradle wrapper distribution.

This changes .gitignore so the generated
gradle-wrapper.properties file is checked in anytime
we change the gradle verison.

I also added the silent (-s) flag to the curl command
when downloading the jar to remove some noise from
the log.

Change-Id: I3a772d5e093be29fcbabcbd8324bd6434fb33beb
---
M java/.gitignore
M java/gradle/wrapper.gradle
A java/gradle/wrapper/gradle-wrapper.properties
M java/gradlew
4 files changed, 8 insertions(+), 13 deletions(-)



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

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


[kudu-CR] [java] Upgrade to Gradle 4.7

2018-04-25 Thread Grant Henke (Code Review)
Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10206 )

Change subject: [java] Upgrade to Gradle 4.7
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10206/1/java/gradlew
File java/gradlew:

http://gerrit.cloudera.org:8080/#/c/10206/1/java/gradlew@84
PS1, Line 84:curl -o $APP_HOME/gradle/wrapper/gradle-wrapper.jar 
https://raw.githubusercontent.com/gradle/gradle/v4.7.0/gradle/wrapper/gradle-wrapper.jar
> hmm, ok. I could have sworn on my machine I had gradle output telling me I
I figured this out. It's the gradle-wrapper.properties that we are not being 
smart enough about. This is the file that defines the wrapper distribution 
version to use.

I will put up a patch to make this better.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I091244886374fef6c9a0ada97b4c3e0693848e16
Gerrit-Change-Number: 10206
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 26 Apr 2018 01:54:14 +
Gerrit-HasComments: Yes


[kudu-CR](branch-1.4.x) KUDU-2416: Fix PartialRow.setMin and add a unit test

2018-04-25 Thread Grant Henke (Code Review)
Grant Henke has removed a vote on this change.

Change subject: KUDU-2416: Fix PartialRow.setMin and add a unit test
..


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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.4.x
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I510743385cc567a8443efc8b0c9f5b641cf04c31
Gerrit-Change-Number: 10191
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins


[kudu-CR](branch-1.5.x) KUDU-2416: Fix PartialRow.setMin and add a unit test

2018-04-25 Thread Grant Henke (Code Review)
Grant Henke has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10190 )

Change subject: KUDU-2416: Fix PartialRow.setMin and add a unit test
..

KUDU-2416: Fix PartialRow.setMin and add a unit test

* Fixes INTEGER.MIN_VALUE being used for LONG colums
  that could lead to incorrect partition prunning.
* Adds a unit test for all types.

Change-Id: I510743385cc567a8443efc8b0c9f5b641cf04c31
Reviewed-on: http://gerrit.cloudera.org:8080/10190
Reviewed-by: Dan Burkert 
Tested-by: Grant Henke 
---
M java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java
2 files changed, 20 insertions(+), 1 deletion(-)

Approvals:
  Dan Burkert: Looks good to me, approved
  Grant Henke: Verified

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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.5.x
Gerrit-MessageType: merged
Gerrit-Change-Id: I510743385cc567a8443efc8b0c9f5b641cf04c31
Gerrit-Change-Number: 10190
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins


[kudu-CR](branch-1.5.x) KUDU-2416: Fix PartialRow.setMin and add a unit test

2018-04-25 Thread Grant Henke (Code Review)
Grant Henke has removed a vote on this change.

Change subject: KUDU-2416: Fix PartialRow.setMin and add a unit test
..


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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.5.x
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I510743385cc567a8443efc8b0c9f5b641cf04c31
Gerrit-Change-Number: 10190
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins


[kudu-CR](branch-1.4.x) KUDU-2416: Fix PartialRow.setMin and add a unit test

2018-04-25 Thread Grant Henke (Code Review)
Grant Henke has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10191 )

Change subject: KUDU-2416: Fix PartialRow.setMin and add a unit test
..

KUDU-2416: Fix PartialRow.setMin and add a unit test

* Fixes INTEGER.MIN_VALUE being used for LONG colums
  that could lead to incorrect partition prunning.
* Adds a unit test for all types.

Change-Id: I510743385cc567a8443efc8b0c9f5b641cf04c31
Reviewed-on: http://gerrit.cloudera.org:8080/10191
Reviewed-by: Dan Burkert 
Tested-by: Grant Henke 
---
M java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java
2 files changed, 20 insertions(+), 1 deletion(-)

Approvals:
  Dan Burkert: Looks good to me, approved
  Grant Henke: Verified

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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.4.x
Gerrit-MessageType: merged
Gerrit-Change-Id: I510743385cc567a8443efc8b0c9f5b641cf04c31
Gerrit-Change-Number: 10191
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins


[kudu-CR](branch-1.4.x) KUDU-2416: Fix PartialRow.setMin and add a unit test

2018-04-25 Thread Grant Henke (Code Review)
Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10191 )

Change subject: KUDU-2416: Fix PartialRow.setMin and add a unit test
..


Patch Set 1: Verified+1


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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.4.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I510743385cc567a8443efc8b0c9f5b641cf04c31
Gerrit-Change-Number: 10191
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 26 Apr 2018 01:43:39 +
Gerrit-HasComments: No


[kudu-CR](branch-1.5.x) KUDU-2416: Fix PartialRow.setMin and add a unit test

2018-04-25 Thread Grant Henke (Code Review)
Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10190 )

Change subject: KUDU-2416: Fix PartialRow.setMin and add a unit test
..


Patch Set 1: Verified+1


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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.5.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I510743385cc567a8443efc8b0c9f5b641cf04c31
Gerrit-Change-Number: 10190
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 26 Apr 2018 01:43:29 +
Gerrit-HasComments: No


[kudu-CR] [java] Upgrade to Gradle 4.7

2018-04-25 Thread Grant Henke (Code Review)
Grant Henke has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10206 )

Change subject: [java] Upgrade to Gradle 4.7
..

[java] Upgrade to Gradle 4.7

Our build is broken on Gradle 4.7. This patch fixes that issue
and upgrades the wrapper version to 4.7.

Change-Id: I091244886374fef6c9a0ada97b4c3e0693848e16
Reviewed-on: http://gerrit.cloudera.org:8080/10206
Reviewed-by: Todd Lipcon 
Tested-by: Grant Henke 
---
M java/gradle/dependencies.gradle
M java/gradle/docs.gradle
M java/gradlew
3 files changed, 16 insertions(+), 12 deletions(-)

Approvals:
  Todd Lipcon: Looks good to me, approved
  Grant Henke: Verified

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I091244886374fef6c9a0ada97b4c3e0693848e16
Gerrit-Change-Number: 10206
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] [java] Upgrade to Gradle 4.7

2018-04-25 Thread Grant Henke (Code Review)
Grant Henke has removed a vote on this change.

Change subject: [java] Upgrade to Gradle 4.7
..


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I091244886374fef6c9a0ada97b4c3e0693848e16
Gerrit-Change-Number: 10206
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] [java] Upgrade to Gradle 4.7

2018-04-25 Thread Grant Henke (Code Review)
Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10206 )

Change subject: [java] Upgrade to Gradle 4.7
..


Patch Set 1: Verified+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I091244886374fef6c9a0ada97b4c3e0693848e16
Gerrit-Change-Number: 10206
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 26 Apr 2018 01:41:51 +
Gerrit-HasComments: No


[kudu-CR] java: enable error-prone for java builds

2018-04-25 Thread Grant Henke (Code Review)
Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/4425 )

Change subject: java: enable error-prone for java builds
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4425/4/java/gradle/quality.gradle
File java/gradle/quality.gradle:

http://gerrit.cloudera.org:8080/#/c/4425/4/java/gradle/quality.gradle@76
PS4, Line 76: tasks.withType(JavaCompile) {
> that's sort of the design of error-prone. It's actually a subclass of the J
I like to have all the levers for control at the task level and often we use 
certain tasks for "testing" vs "packaging".

My concern is if the static code analysis complicates or breaks longer running, 
assembly only, packaging type jobs. For those we tend to use `gradle assemble` 
instead of `gradle check` or `gradle build`. If it does't break things, it 
likely makes the build longer.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iebafbb307904875490426dd699ab1f55e21d284d
Gerrit-Change-Number: 4425
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 26 Apr 2018 01:19:48 +
Gerrit-HasComments: Yes


[kudu-CR] cache: reduce contention on MemTracker::Release and Consume

2018-04-25 Thread Todd Lipcon (Code Review)
Hello David Ribeiro Alves, Kudu Jenkins, Adar Dembo,

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

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

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

Change subject: cache: reduce contention on MemTracker::Release and Consume
..

cache: reduce contention on MemTracker::Release and Consume

In an old benchmark about a year ago, this improved YCSB load throughput
from ~450k ops/sec to ~570k. For the newly-added cache-bench, this
improves performance substnatially (16x) for the scenario with high churn:

Before:
[ RUN  ] Patterns/CacheBench.RunBench/3
I0425 13:59:17.307648 95430 cache-bench.cc:176] Warming up...
I0425 13:59:18.308992 95430 cache-bench.cc:179] Running benchmark...
I0425 13:59:19.313868 95430 cache-bench.cc:187] UNIFORM ratio=3.00x 
n_unique=786432: 2.48M lookups/sec
I0425 13:59:19.313891 95430 cache-bench.cc:188] UNIFORM ratio=3.00x 
n_unique=786432: 33.3% hit rate
[   OK ] Patterns/CacheBench.RunBench/3 (2192 ms)

After:
[ RUN  ] Patterns/CacheBench.RunBench/3
I0425 13:57:40.581399 94314 cache-bench.cc:176] Warming up...
I0425 13:57:41.582798 94314 cache-bench.cc:179] Running benchmark...
I0425 13:57:42.583904 94314 cache-bench.cc:187] UNIFORM ratio=3.00x 
n_unique=786432: 39.53M lookups/sec
I0425 13:57:42.583930 94314 cache-bench.cc:188] UNIFORM ratio=3.00x 
n_unique=786432: 33.3% hit rate
[   OK ] Patterns/CacheBench.RunBench/3 (2055 ms)

Change-Id: Ic3bd24a452761b06611215f4831ef02238ba14bc
---
M src/kudu/cfile/block_cache-test.cc
M src/kudu/util/cache-test.cc
M src/kudu/util/cache.cc
3 files changed, 63 insertions(+), 4 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic3bd24a452761b06611215f4831ef02238ba14bc
Gerrit-Change-Number: 6597
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] cache: add a benchmark

2018-04-25 Thread Todd Lipcon (Code Review)
Todd Lipcon has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/6696 )

Change subject: cache: add a benchmark
..

cache: add a benchmark

In some workloads, the LRU cache is now a substantial bottleneck. This
patch adds a simple benchmark we can use to measure improvements in
cache performance under concurrency.

Change-Id: I156d170c20913a17e4f94b40879409f4798023ab
Reviewed-on: http://gerrit.cloudera.org:8080/6696
Reviewed-by: Adar Dembo 
Tested-by: Kudu Jenkins
---
M src/kudu/util/CMakeLists.txt
A src/kudu/util/cache-bench.cc
2 files changed, 192 insertions(+), 0 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I156d170c20913a17e4f94b40879409f4798023ab
Gerrit-Change-Number: 6696
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] Fast path scanning blocks of deleted rows

2018-04-25 Thread Will Berkeley (Code Review)
Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10213 )

Change subject: Fast path scanning blocks of deleted rows
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/10213/1//COMMIT_MSG@13
PS1, Line 13: move forward any
: materialize any columns.
Looks like two thoughts collided here.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9fa891c0f4e857ddd1f873ad4855154d078be6b8
Gerrit-Change-Number: 10213
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Wed, 25 Apr 2018 23:55:53 +
Gerrit-HasComments: Yes


[kudu-CR] Fast path scanning blocks of deleted rows

2018-04-25 Thread Todd Lipcon (Code Review)
Hello Will Berkeley, Mostafa Mokhtar,

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

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

to review the following change.


Change subject: Fast path scanning blocks of deleted rows
..

Fast path scanning blocks of deleted rows

This adds some very simple fast-paths in the case that an entire row
block consists of rows that were deleted.

The first is in the block materialization code: if delta application
results in all rows being deleted, we don't need to move forward any
materialize any columns.

The second is before serializing scan responses to the client. In this
case we don't need to loop over each column and read the selection
vector for each column. Instead we can just return immediately.

I tested this on a table where I'd inserted a few billion rows, deleted
them all, and then re-inserted them. Before the patch, a simple 'SELECT
* FROM t LIMIT 10' took 306sec. With the first optimization only, it
took about 10 seconds. With the second one as well, it took about 2.2
seconds.

There are probably more general optimizations that could be done for
sparsely-populated RowBlocks (eg where just a few rows are selected) but
they are much more complex, and it's relatively common for users to
delete large consecutive runs of rows. For example, users may use
'DELETE' all rows in a table or partition before re-adding them, or they
may delete all data corresponding to some prefix of the PK.

Change-Id: I9fa891c0f4e857ddd1f873ad4855154d078be6b8
---
M src/kudu/common/generic_iterators.cc
M src/kudu/tserver/tablet_service.cc
2 files changed, 11 insertions(+), 0 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9fa891c0f4e857ddd1f873ad4855154d078be6b8
Gerrit-Change-Number: 10213
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] Fix table formatting in full data dirs docs

2018-04-25 Thread Alex Rodoni (Code Review)
Alex Rodoni has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10211 )

Change subject: Fix table formatting in full data dirs docs
..


Patch Set 1:

The table looks correct now!

I cannot do +2 for Kudu, though.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I098c64c3241e5fc4184b102e10ef619feb7de30a
Gerrit-Change-Number: 10211
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 25 Apr 2018 22:47:31 +
Gerrit-HasComments: No


[kudu-CR] [java] Upgrade to Gradle 4.7

2018-04-25 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10206 )

Change subject: [java] Upgrade to Gradle 4.7
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I091244886374fef6c9a0ada97b4c3e0693848e16
Gerrit-Change-Number: 10206
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 25 Apr 2018 22:32:16 +
Gerrit-HasComments: No


[kudu-CR] java: enable error-prone for java builds

2018-04-25 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/4425 )

Change subject: java: enable error-prone for java builds
..


Patch Set 5: Code-Review+1

Looks fine, thanks


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iebafbb307904875490426dd699ab1f55e21d284d
Gerrit-Change-Number: 4425
Gerrit-PatchSet: 5
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 25 Apr 2018 22:30:18 +
Gerrit-HasComments: No


[kudu-CR] java: enable error-prone for java builds

2018-04-25 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/4425 )

Change subject: java: enable error-prone for java builds
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4425/4/java/gradle/quality.gradle
File java/gradle/quality.gradle:

http://gerrit.cloudera.org:8080/#/c/4425/4/java/gradle/quality.gradle@76
PS4, Line 76:   signature 'org.codehaus.mojo.signature:java17:1.0@signature'
> I am not a fan that this needs to run all the time when compiling as oppose
that's sort of the design of error-prone. It's actually a subclass of the Java 
compiler that hooks into the compilation process itself, rather than doing 
post-compilation bytecode analysis.

I suppose it could be an optional flag, but I kind of like that it will break 
your compile for certain classes of errors



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iebafbb307904875490426dd699ab1f55e21d284d
Gerrit-Change-Number: 4425
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 25 Apr 2018 22:29:01 +
Gerrit-HasComments: Yes


[kudu-CR] java: enable error-prone for java builds

2018-04-25 Thread Grant Henke (Code Review)
Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/4425 )

Change subject: java: enable error-prone for java builds
..


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4425/3/java/build.gradle
File java/build.gradle:

http://gerrit.cloudera.org:8080/#/c/4425/3/java/build.gradle@24
PS3, Line 24:   id 'net.ltgt.errorprone' version '0.0.13' apply false
> unfortunately I couldn't move the 'plugins' definition anywhere but here. I
This is just adding the dependencies to the build. I have been doing that in 
the /buildsrc repo to prevent this issue. I will post an update to this patch 
showing that.


http://gerrit.cloudera.org:8080/#/c/4425/4/java/gradle/quality.gradle
File java/gradle/quality.gradle:

http://gerrit.cloudera.org:8080/#/c/4425/4/java/gradle/quality.gradle@76
PS4, Line 76: tasks.withType(JavaCompile) {
I am not a fan that this needs to run all the time when compiling as opposed to 
as a separate task like findbugs, pmd, etc. I am not sure that is avoidable and 
can follow up and improve this if it is.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iebafbb307904875490426dd699ab1f55e21d284d
Gerrit-Change-Number: 4425
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 25 Apr 2018 22:27:02 +
Gerrit-HasComments: Yes


[kudu-CR] java: enable error-prone for java builds

2018-04-25 Thread Grant Henke (Code Review)
Grant Henke has uploaded a new patch set (#5) to the change originally created 
by Todd Lipcon. ( http://gerrit.cloudera.org:8080/4425 )

Change subject: java: enable error-prone for java builds
..

java: enable error-prone for java builds

error-prone (http://errorprone.info/) is a compiler extension by Google
which checks for a bunch of common Java errors.

Fixes for the exposed issues are in follow-on commits.

Change-Id: Iebafbb307904875490426dd699ab1f55e21d284d
---
M java/buildSrc/build.gradle
M java/gradle/dependencies.gradle
M java/gradle/quality.gradle
3 files changed, 19 insertions(+), 0 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iebafbb307904875490426dd699ab1f55e21d284d
Gerrit-Change-Number: 4425
Gerrit-PatchSet: 5
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] [java] Upgrade to Gradle 4.7

2018-04-25 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10206 )

Change subject: [java] Upgrade to Gradle 4.7
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10206/1/java/gradlew
File java/gradlew:

http://gerrit.cloudera.org:8080/#/c/10206/1/java/gradlew@84
PS1, Line 84:curl -o $APP_HOME/gradle/wrapper/gradle-wrapper.jar 
https://raw.githubusercontent.com/gradle/gradle/v4.7.0/gradle/wrapper/gradle-wrapper.jar
> This actual jar version isn't very important. It just contains the logic fo
hmm, ok. I could have sworn on my machine I had gradle output telling me I was 
using an older gradle compared to what the files indicated I should have been 
using (4.2 vs 4.6 or somesuch).



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I091244886374fef6c9a0ada97b4c3e0693848e16
Gerrit-Change-Number: 10206
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 25 Apr 2018 22:26:12 +
Gerrit-HasComments: Yes


[kudu-CR] java: fix remainder of error-prone issues

2018-04-25 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10202 )

Change subject: java: fix remainder of error-prone issues
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/10202/1//COMMIT_MSG@15
PS1, Line 15:   so it is probably more efficient to use plain String 
concatenation
:   instead.[1]
> There's no version of DateFormatter which takes a StringBuilder. So, I had
Makes sense, thanks.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I54f67c9718e1022d7996ab1cb827f68c1beb5d3d
Gerrit-Change-Number: 10202
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 25 Apr 2018 22:18:04 +
Gerrit-HasComments: Yes


[kudu-CR] cache: reduce contention on MemTracker::Release and Consume

2018-04-25 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/6597 )

Change subject: cache: reduce contention on MemTracker::Release and Consume
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic3bd24a452761b06611215f4831ef02238ba14bc
Gerrit-Change-Number: 6597
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 25 Apr 2018 22:17:41 +
Gerrit-HasComments: No


[kudu-CR] Fix table formatting in full data dirs docs

2018-04-25 Thread Alex Rodoni (Code Review)
Alex Rodoni has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10211 )

Change subject: Fix table formatting in full data dirs docs
..


Patch Set 1: Code-Review+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I098c64c3241e5fc4184b102e10ef619feb7de30a
Gerrit-Change-Number: 10211
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 25 Apr 2018 22:16:49 +
Gerrit-HasComments: No


[kudu-CR] [java] Upgrade to Gradle 4.7

2018-04-25 Thread Grant Henke (Code Review)
Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10206 )

Change subject: [java] Upgrade to Gradle 4.7
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10206/1/java/gradlew
File java/gradlew:

http://gerrit.cloudera.org:8080/#/c/10206/1/java/gradlew@84
PS1, Line 84:curl -o $APP_HOME/gradle/wrapper/gradle-wrapper.jar 
https://raw.githubusercontent.com/gradle/gradle/v4.7.0/gradle/wrapper/gradle-wrapper.jar
> I noticed on my machine that I had an old gradlew locally from a different
This actual jar version isn't very important. It just contains the logic for 
grabbing the actual distribution which hasn't changed.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I091244886374fef6c9a0ada97b4c3e0693848e16
Gerrit-Change-Number: 10206
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 25 Apr 2018 22:14:54 +
Gerrit-HasComments: Yes


[kudu-CR] Fix table formatting in full data dirs docs

2018-04-25 Thread Will Berkeley (Code Review)
Will Berkeley has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10211


Change subject: Fix table formatting in full data dirs docs
..

Fix table formatting in full data dirs docs

Change-Id: I098c64c3241e5fc4184b102e10ef619feb7de30a
---
M docs/administration.adoc
1 file changed, 1 insertion(+), 1 deletion(-)



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

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


[kudu-CR] [docs] Add docs on full data dirs

2018-04-25 Thread Alex Rodoni (Code Review)
Alex Rodoni has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9967 )

Change subject: [docs] Add docs on full data dirs
..


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9967/6/docs/administration.adoc
File docs/administration.adoc:

http://gerrit.cloudera.org:8080/#/c/9967/6/docs/administration.adoc@828
PS6, Line 828: ,<
Remove this.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If1081cab4c84789d29a0ccdccfd11c190b6164cf
Gerrit-Change-Number: 9967
Gerrit-PatchSet: 6
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Wed, 25 Apr 2018 22:13:36 +
Gerrit-HasComments: Yes


[kudu-CR] java: enable error-prone for java builds

2018-04-25 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/4425 )

Change subject: java: enable error-prone for java builds
..


Patch Set 3:

(3 comments)

As for error-prone vs spotbugs, I think they are likely complementary with a 
different set of checks. I personally like that error-prone produces its output 
as warnings in the normal compile output in the normal flow of development

http://gerrit.cloudera.org:8080/#/c/4425/3/java/build.gradle
File java/build.gradle:

http://gerrit.cloudera.org:8080/#/c/4425/3/java/build.gradle@24
PS3, Line 24:   id 'net.ltgt.errorprone' version '0.0.13' apply false
> I have been putting all of these static analysis checks into quality.gradle
unfortunately I couldn't move the 'plugins' definition anywhere but here. It 
has to be the first thing in the build file apparently


http://gerrit.cloudera.org:8080/#/c/4425/3/java/build.gradle@48
PS3, Line 48:   apply plugin: 'java'
> Any reason you changed this to single quotes?
mistake, reverted


http://gerrit.cloudera.org:8080/#/c/4425/3/java/build.gradle@51
PS3, Line 51:   apply from: "$rootDir/gradle/errorprone.gradle"
> Did you add this file? I have been putting all of these static analysis che
moved the contents to quality.gradle



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iebafbb307904875490426dd699ab1f55e21d284d
Gerrit-Change-Number: 4425
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 25 Apr 2018 22:06:24 +
Gerrit-HasComments: Yes


[kudu-CR] java: fix remainder of error-prone issues

2018-04-25 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10202 )

Change subject: java: fix remainder of error-prone issues
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/10202/1//COMMIT_MSG@15
PS1, Line 15:   so it is probably more efficient to use plain String 
concatenation
:   instead.[1]
> Curious why you didn't use StringBuilder here; that's the recommendation in
There's no version of DateFormatter which takes a StringBuilder. So, I had to 
use the version which returns a String, at which point there isn't really any 
advantage of creating a stringbuilder just to concat two strings.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I54f67c9718e1022d7996ab1cb827f68c1beb5d3d
Gerrit-Change-Number: 10202
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 25 Apr 2018 22:04:35 +
Gerrit-HasComments: Yes


[kudu-CR] cache: reduce contention on MemTracker::Release and Consume

2018-04-25 Thread Todd Lipcon (Code Review)
Hello David Ribeiro Alves, Kudu Jenkins, Adar Dembo,

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

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

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

Change subject: cache: reduce contention on MemTracker::Release and Consume
..

cache: reduce contention on MemTracker::Release and Consume

In an old benchmark about a year ago, this improved YCSB load throughput
from ~450k ops/sec to ~570k. For the newly-added cache-bench, this
improves performance substnatially (16x) for the scenario with high churn:

Before:
[ RUN  ] Patterns/CacheBench.RunBench/3
I0425 13:59:17.307648 95430 cache-bench.cc:176] Warming up...
I0425 13:59:18.308992 95430 cache-bench.cc:179] Running benchmark...
I0425 13:59:19.313868 95430 cache-bench.cc:187] UNIFORM ratio=3.00x 
n_unique=786432: 2.48M lookups/sec
I0425 13:59:19.313891 95430 cache-bench.cc:188] UNIFORM ratio=3.00x 
n_unique=786432: 33.3% hit rate
[   OK ] Patterns/CacheBench.RunBench/3 (2192 ms)

After:
[ RUN  ] Patterns/CacheBench.RunBench/3
I0425 13:57:40.581399 94314 cache-bench.cc:176] Warming up...
I0425 13:57:41.582798 94314 cache-bench.cc:179] Running benchmark...
I0425 13:57:42.583904 94314 cache-bench.cc:187] UNIFORM ratio=3.00x 
n_unique=786432: 39.53M lookups/sec
I0425 13:57:42.583930 94314 cache-bench.cc:188] UNIFORM ratio=3.00x 
n_unique=786432: 33.3% hit rate
[   OK ] Patterns/CacheBench.RunBench/3 (2055 ms)

Change-Id: Ic3bd24a452761b06611215f4831ef02238ba14bc
---
M src/kudu/cfile/block_cache-test.cc
M src/kudu/util/cache-test.cc
M src/kudu/util/cache.cc
3 files changed, 63 insertions(+), 4 deletions(-)


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

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


[kudu-CR] cache: add a benchmark

2018-04-25 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/6696 )

Change subject: cache: add a benchmark
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I156d170c20913a17e4f94b40879409f4798023ab
Gerrit-Change-Number: 6696
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 25 Apr 2018 21:56:25 +
Gerrit-HasComments: No


[kudu-CR] cache: reduce contention on MemTracker::Release and Consume

2018-04-25 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/6597 )

Change subject: cache: reduce contention on MemTracker::Release and Consume
..


Patch Set 2:

A couple tests are failing because they are making precise assertions about 
consumption. Working on figuring out a workaround.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic3bd24a452761b06611215f4831ef02238ba14bc
Gerrit-Change-Number: 6597
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 25 Apr 2018 21:55:39 +
Gerrit-HasComments: No


[kudu-CR] cache: switch to std::atomic

2018-04-25 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10209 )

Change subject: cache: switch to std::atomic
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I359f273af9ba7130d230ef639a40e12a8ac51a06
Gerrit-Change-Number: 10209
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 25 Apr 2018 21:47:54 +
Gerrit-HasComments: No


[kudu-CR] cache: reduce contention on MemTracker::Release and Consume

2018-04-25 Thread Todd Lipcon (Code Review)
Hello David Ribeiro Alves, Kudu Jenkins, Adar Dembo,

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

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

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

Change subject: cache: reduce contention on MemTracker::Release and Consume
..

cache: reduce contention on MemTracker::Release and Consume

In an old benchmark about a year ago, this improved YCSB load throughput
from ~450k ops/sec to ~570k. For the newly-added cache-bench, this
improves performance substnatially (16x) for the scenario with high churn:

Before:
[ RUN  ] Patterns/CacheBench.RunBench/3
I0425 13:59:17.307648 95430 cache-bench.cc:176] Warming up...
I0425 13:59:18.308992 95430 cache-bench.cc:179] Running benchmark...
I0425 13:59:19.313868 95430 cache-bench.cc:187] UNIFORM ratio=3.00x 
n_unique=786432: 2.48M lookups/sec
I0425 13:59:19.313891 95430 cache-bench.cc:188] UNIFORM ratio=3.00x 
n_unique=786432: 33.3% hit rate
[   OK ] Patterns/CacheBench.RunBench/3 (2192 ms)

After:
[ RUN  ] Patterns/CacheBench.RunBench/3
I0425 13:57:40.581399 94314 cache-bench.cc:176] Warming up...
I0425 13:57:41.582798 94314 cache-bench.cc:179] Running benchmark...
I0425 13:57:42.583904 94314 cache-bench.cc:187] UNIFORM ratio=3.00x 
n_unique=786432: 39.53M lookups/sec
I0425 13:57:42.583930 94314 cache-bench.cc:188] UNIFORM ratio=3.00x 
n_unique=786432: 33.3% hit rate
[   OK ] Patterns/CacheBench.RunBench/3 (2055 ms)

Change-Id: Ic3bd24a452761b06611215f4831ef02238ba14bc
---
M src/kudu/util/cache.cc
1 file changed, 40 insertions(+), 2 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic3bd24a452761b06611215f4831ef02238ba14bc
Gerrit-Change-Number: 6597
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] [docs] Add docs on full data dirs

2018-04-25 Thread Alex Rodoni (Code Review)
Alex Rodoni has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9967 )

Change subject: [docs] Add docs on full data dirs
..


Patch Set 5:

The table format needs to be fixed. It is rendering with 3 columns and 
misplacing cells.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If1081cab4c84789d29a0ccdccfd11c190b6164cf
Gerrit-Change-Number: 9967
Gerrit-PatchSet: 5
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Wed, 25 Apr 2018 21:30:43 +
Gerrit-HasComments: No


[kudu-CR] cache: switch to std::atomic

2018-04-25 Thread Todd Lipcon (Code Review)
Hello David Ribeiro Alves,

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

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

to review the following change.


Change subject: cache: switch to std::atomic
..

cache: switch to std::atomic

A previous commit added some use of std::atomic, so this patch makes the
file self-consistent.

Change-Id: I359f273af9ba7130d230ef639a40e12a8ac51a06
---
M src/kudu/util/cache.cc
1 file changed, 11 insertions(+), 12 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I359f273af9ba7130d230ef639a40e12a8ac51a06
Gerrit-Change-Number: 10209
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 


[kudu-CR] [docs] Add docs on full data dirs

2018-04-25 Thread Will Berkeley (Code Review)
Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9967 )

Change subject: [docs] Add docs on full data dirs
..


Patch Set 5: Verified+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If1081cab4c84789d29a0ccdccfd11c190b6164cf
Gerrit-Change-Number: 9967
Gerrit-PatchSet: 5
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Wed, 25 Apr 2018 21:15:25 +
Gerrit-HasComments: No


[kudu-CR] [docs] Add docs on full data dirs

2018-04-25 Thread Will Berkeley (Code Review)
Will Berkeley has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9967 )

Change subject: [docs] Add docs on full data dirs
..

[docs] Add docs on full data dirs

Change-Id: If1081cab4c84789d29a0ccdccfd11c190b6164cf
Reviewed-on: http://gerrit.cloudera.org:8080/9967
Reviewed-by: Adar Dembo 
Reviewed-by: Alexey Serbin 
Tested-by: Will Berkeley 
---
M docs/administration.adoc
1 file changed, 51 insertions(+), 0 deletions(-)

Approvals:
  Adar Dembo: Looks good to me, approved
  Alexey Serbin: Looks good to me, but someone else must approve
  Will Berkeley: Verified

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: If1081cab4c84789d29a0ccdccfd11c190b6164cf
Gerrit-Change-Number: 9967
Gerrit-PatchSet: 6
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] [docs] Add docs on full data dirs

2018-04-25 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9967 )

Change subject: [docs] Add docs on full data dirs
..


Patch Set 5: Code-Review+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If1081cab4c84789d29a0ccdccfd11c190b6164cf
Gerrit-Change-Number: 9967
Gerrit-PatchSet: 5
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Wed, 25 Apr 2018 21:07:15 +
Gerrit-HasComments: No


[kudu-CR] [docs] Add docs on full data dirs

2018-04-25 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9967 )

Change subject: [docs] Add docs on full data dirs
..


Patch Set 5: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If1081cab4c84789d29a0ccdccfd11c190b6164cf
Gerrit-Change-Number: 9967
Gerrit-PatchSet: 5
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Wed, 25 Apr 2018 21:04:29 +
Gerrit-HasComments: No


[kudu-CR] [docs] Add docs on full data dirs

2018-04-25 Thread Will Berkeley (Code Review)
Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9967 )

Change subject: [docs] Add docs on full data dirs
..


Patch Set 4:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9967/4/docs/administration.adoc@845
PS4, Line 845: If all data directories for a tablet are full, Kudu
 : will crash.
> OK. I did observe the distinction, but is it really worth pointing out? Fro
Perhaps not, but the organization is divided into pre-1.7.0 and post-1.7.0, so 
it fits as the precise description of Kudu's behavior in 1.7.0.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If1081cab4c84789d29a0ccdccfd11c190b6164cf
Gerrit-Change-Number: 9967
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Wed, 25 Apr 2018 20:46:41 +
Gerrit-HasComments: Yes


[kudu-CR] [docs] Add docs on full data dirs

2018-04-25 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9967 )

Change subject: [docs] Add docs on full data dirs
..


Patch Set 5:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/9967/4/docs/administration.adoc@845
PS4, Line 845: If all data directories for a tablet are full, Kudu
 : will crash.
> L837 says that Kudu will crash if all data directories are full. L845 says
OK. I did observe the distinction, but is it really worth pointing out? From a 
user's perspective, any bit of Kudu data is bound to a particular tablet, so if 
you've filled all data directories, haven't you also filled all data 
directories belonging to at least one tablet?


http://gerrit.cloudera.org:8080/#/c/9967/4/docs/administration.adoc@864
PS4, Line 864: Note that existing tablets will not use new data directories
> I'll file a JIRA for such a tool if you like.
I honestly don't know how useful it'd be. Tablets modified in such a way will 
have data balance issues (i.e. much more data on one set of disks than on 
another).



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If1081cab4c84789d29a0ccdccfd11c190b6164cf
Gerrit-Change-Number: 9967
Gerrit-PatchSet: 5
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Wed, 25 Apr 2018 20:44:12 +
Gerrit-HasComments: Yes


[kudu-CR] [docs] Add docs on full data dirs

2018-04-25 Thread Will Berkeley (Code Review)
Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9967 )

Change subject: [docs] Add docs on full data dirs
..


Patch Set 4:

(5 comments)

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

http://gerrit.cloudera.org:8080/#/c/9967/4/docs/administration.adoc@845
PS4, Line 845: If all data directories for a tablet are full, Kudu
 : will crash.
> Hmm, you already wrote this on L837; isn't it redundant?
L837 says that Kudu will crash if all data directories are full. L845 says Kudu 
will crash if all data directories in a tablet's data directory group are full.


http://gerrit.cloudera.org:8080/#/c/9967/4/docs/administration.adoc@846
PS4, Line 846: Periodically, Kudu will check if full data directories are still
 : full, and will resume writing to those data directories if space 
has become
 : available.
> This looks a bit confusing to me.  What happens if Kudu process is restarte
The data directories for the tablet are still full, so it will crash.


http://gerrit.cloudera.org:8080/#/c/9967/4/docs/administration.adoc@850
PS4, Line 850: it suffices
> Suffices for what?  What will happen if Kudu process is just restarted when
It suffices to restart the daemon and allow it to write to those directories, 
as you said. I will add some extra words to make this perfectly clear.


http://gerrit.cloudera.org:8080/#/c/9967/4/docs/administration.adoc@850
PS4, Line 850: due to full directories
> "because its data directories were full"
Done


http://gerrit.cloudera.org:8080/#/c/9967/4/docs/administration.adoc@864
PS4, Line 864: Note that existing tablets will not use new data directories
> BTW, this is something we could address in a CLI tool. I can imagine a tool
I'll file a JIRA for such a tool if you like.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If1081cab4c84789d29a0ccdccfd11c190b6164cf
Gerrit-Change-Number: 9967
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Wed, 25 Apr 2018 20:32:40 +
Gerrit-HasComments: Yes


[kudu-CR] cache: add a benchmark

2018-04-25 Thread Todd Lipcon (Code Review)
Hello David Ribeiro Alves, Jean-Daniel Cryans, Kudu Jenkins,

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

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

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

Change subject: cache: add a benchmark
..

cache: add a benchmark

In some workloads, the LRU cache is now a substantial bottleneck. This
patch adds a simple benchmark we can use to measure improvements in
cache performance under concurrency.

Change-Id: I156d170c20913a17e4f94b40879409f4798023ab
---
M src/kudu/util/CMakeLists.txt
A src/kudu/util/cache-bench.cc
2 files changed, 192 insertions(+), 0 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I156d170c20913a17e4f94b40879409f4798023ab
Gerrit-Change-Number: 6696
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] [tools] ksck improvements [6/n]: Refactor printing

2018-04-25 Thread Will Berkeley (Code Review)
Hello Alexey Serbin, Kudu Jenkins,

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

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

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

Change subject: [tools] ksck improvements [6/n]: Refactor printing
..

[tools] ksck improvements [6/n]: Refactor printing

This patch significantly refactors printing in ksck. It separates where
checks are done from where results are printed. This separation will
make it easier to provide machine-readable output and create tools
based on the results of ksck checks, like auto-repair.

There's also a couple of bonus changes:
- Simplifies running all ksck checks and printing the results, as done
  by the ksck tool and ClusterVerifier.
- Removes the --consensus flag. ksck is far less useful without it and
  it doesn't seem like having it brings any benefit.
- Add the changes in https://gerrit.cloudera.org/#/c/10054/, which were
  about to be committed but would've needed to be redone a bit to fit
  with this refactor.

Change-Id: Id8de619996b6cd753e6a9c01b1b60810a873e609
---
M src/kudu/integration-tests/cluster_verifier.cc
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_remote-test.cc
A src/kudu/tools/ksck_results.cc
A src/kudu/tools/ksck_results.h
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/tool_action_cluster.cc
M src/kudu/tools/tool_action_tablet.cc
11 files changed, 1,199 insertions(+), 890 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id8de619996b6cd753e6a9c01b1b60810a873e609
Gerrit-Change-Number: 10151
Gerrit-PatchSet: 8
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] cache: add a benchmark

2018-04-25 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/6696 )

Change subject: cache: add a benchmark
..


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/6696/1/src/kudu/util/cache-bench.cc
File src/kudu/util/cache-bench.cc:

http://gerrit.cloudera.org:8080/#/c/6696/1/src/kudu/util/cache-bench.cc@95
PS1, Line 95:
> nit: extra line
Done


http://gerrit.cloudera.org:8080/#/c/6696/1/src/kudu/util/cache-bench.cc@97
PS1, Line 97: TEST_P(CacheBench, RunBench) {
> would it be interesting to also measure cache churn?
do you mean the eviction rate? I think since we are doing one insert for every 
miss, and the elements are all the same size, the miss rate and eviction rate 
are the same, right?


http://gerrit.cloudera.org:8080/#/c/6696/1/src/kudu/util/cache-bench.cc@98
PS1, Line 98: 16;
> make this a flag?
Done


http://gerrit.cloudera.org:8080/#/c/6696/1/src/kudu/util/cache-bench.cc@99
PS1, Line 99:   const int kSecondsToRun = AllowSlowTests() ? 10 : 1;
> Can you then document more what this test is about?
I added a warmup phase so it should converge faster. I still made it 
configurable since most of our benchmarks are configurable and it can help to 
get better samples for 'perf record', etc



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I156d170c20913a17e4f94b40879409f4798023ab
Gerrit-Change-Number: 6696
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 25 Apr 2018 20:28:13 +
Gerrit-HasComments: Yes


[kudu-CR] [docs] Add docs on full data dirs

2018-04-25 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9967 )

Change subject: [docs] Add docs on full data dirs
..


Patch Set 4:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/9967/4/docs/administration.adoc@846
PS4, Line 846: Periodically, Kudu will check if full data directories are still
 : full, and will resume writing to those data directories if space 
has become
 : available.
This looks a bit confusing to me.  What happens if Kudu process is restarted 
and it found that all data directories are full, and then a request comes to 
write into some tablet?   Will it crash again as it's described in the prior 
sentence or it will just reject that write operation since it knows no data is 
available?


http://gerrit.cloudera.org:8080/#/c/9967/4/docs/administration.adoc@850
PS4, Line 850: it suffices
Suffices for what?  What will happen if Kudu process is just restarted when all 
the directories are full?  After reading the sentence above, I would expect 
that Kudu process would be able to start after the crash due to full 
directories and eventually, once some space is available, it will resume 
writing into those directories.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If1081cab4c84789d29a0ccdccfd11c190b6164cf
Gerrit-Change-Number: 9967
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Wed, 25 Apr 2018 20:20:50 +
Gerrit-HasComments: Yes


[kudu-CR](branch-1.7.x) KUDU-2416: Fix PartialRow.setMin and add a unit test

2018-04-25 Thread Grant Henke (Code Review)
Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10188 )

Change subject: KUDU-2416: Fix PartialRow.setMin and add a unit test
..


Patch Set 1: Verified+1


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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.7.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I510743385cc567a8443efc8b0c9f5b641cf04c31
Gerrit-Change-Number: 10188
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 25 Apr 2018 20:09:20 +
Gerrit-HasComments: No


[kudu-CR](branch-1.7.x) KUDU-2416: Fix PartialRow.setMin and add a unit test

2018-04-25 Thread Grant Henke (Code Review)
Grant Henke has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10188 )

Change subject: KUDU-2416: Fix PartialRow.setMin and add a unit test
..

KUDU-2416: Fix PartialRow.setMin and add a unit test

* Fixes a missing break after the decimal type that could
   lead to IllegalArgumentExceptions.
* Fixes INTEGER.MIN_VALUE being used for LONG colums
  that could lead to incorrect partition prunning.
* Adds a unit test for all types.

Change-Id: I510743385cc567a8443efc8b0c9f5b641cf04c31
Reviewed-on: http://gerrit.cloudera.org:8080/10188
Reviewed-by: Dan Burkert 
Tested-by: Grant Henke 
---
M java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java
2 files changed, 22 insertions(+), 1 deletion(-)

Approvals:
  Dan Burkert: Looks good to me, approved
  Grant Henke: Verified

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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.7.x
Gerrit-MessageType: merged
Gerrit-Change-Id: I510743385cc567a8443efc8b0c9f5b641cf04c31
Gerrit-Change-Number: 10188
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR](branch-1.6.x) KUDU-2416: Fix PartialRow.setMin and add a unit test

2018-04-25 Thread Grant Henke (Code Review)
Grant Henke has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10189 )

Change subject: KUDU-2416: Fix PartialRow.setMin and add a unit test
..

KUDU-2416: Fix PartialRow.setMin and add a unit test

* Fixes INTEGER.MIN_VALUE being used for LONG colums
  that could lead to incorrect partition prunning.
* Adds a unit test for all types.

Change-Id: I510743385cc567a8443efc8b0c9f5b641cf04c31
Reviewed-on: http://gerrit.cloudera.org:8080/10189
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert 
---
M java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java
2 files changed, 20 insertions(+), 1 deletion(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.6.x
Gerrit-MessageType: merged
Gerrit-Change-Id: I510743385cc567a8443efc8b0c9f5b641cf04c31
Gerrit-Change-Number: 10189
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] java: fix remainder of error-prone issues

2018-04-25 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10202 )

Change subject: java: fix remainder of error-prone issues
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/10202/1//COMMIT_MSG@15
PS1, Line 15:   so it is probably more efficient to use plain String 
concatenation
:   instead.[1]
Curious why you didn't use StringBuilder here; that's the recommendation in [1].



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I54f67c9718e1022d7996ab1cb827f68c1beb5d3d
Gerrit-Change-Number: 10202
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 25 Apr 2018 20:04:26 +
Gerrit-HasComments: Yes


[kudu-CR] [docs] Add docs on full data dirs

2018-04-25 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9967 )

Change subject: [docs] Add docs on full data dirs
..


Patch Set 4:

(3 comments)

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

http://gerrit.cloudera.org:8080/#/c/9967/4/docs/administration.adoc@845
PS4, Line 845: If all data directories for a tablet are full, Kudu
 : will crash.
Hmm, you already wrote this on L837; isn't it redundant?


http://gerrit.cloudera.org:8080/#/c/9967/4/docs/administration.adoc@850
PS4, Line 850: due to full directories
"because its data directories were full"


http://gerrit.cloudera.org:8080/#/c/9967/4/docs/administration.adoc@864
PS4, Line 864: Note that existing tablets will not use new data directories
BTW, this is something we could address in a CLI tool. I can imagine a tool 
that, for each tablet, loads the superblock, rewrites the data dir group to 
reflect all disks (or whatever), then flushes the superblock.

The opposite is not possible without losing data, but maybe this would still be 
useful.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If1081cab4c84789d29a0ccdccfd11c190b6164cf
Gerrit-Change-Number: 9967
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Wed, 25 Apr 2018 20:00:20 +
Gerrit-HasComments: Yes


[kudu-CR] java: fix various test style issues

2018-04-25 Thread Todd Lipcon (Code Review)
Hello Grant Henke,

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

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

to review the following change.


Change subject: java: fix various test style issues
..

java: fix various test style issues

error-prone flagged a number of cases where we were expecting exceptions
in tests but not calling 'fail()' in the case that the except was not
thrown. This also adds a few suppressions for cases where we are doing
odd things with catching exceptions in tests.

Change-Id: Ia9a7ce360af3f727e8702109826d7b878f98d6a5
---
M java/kudu-client/src/test/java/org/apache/kudu/client/ITClientStress.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduClient.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/TestAuthnTokenReacquire.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestBytes.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java
8 files changed, 30 insertions(+), 23 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia9a7ce360af3f727e8702109826d7b878f98d6a5
Gerrit-Change-Number: 10201
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Grant Henke 


[kudu-CR] Fix missing @Override annotations

2018-04-25 Thread Todd Lipcon (Code Review)
Hello Grant Henke,

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

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

to review the following change.


Change subject: Fix missing @Override annotations
..

Fix missing @Override annotations

This fixes a bunch of error-prone warnings. Generated the patch by
running:

./gradlew clean compileTestJava -Derrorprone-fix=MissingOverride
git diff --name-only | xargs perl -p -i  -e 's,^(\s+)(\@Override) ,\1\2\n\1,g'

(the perl script was necessary to conform to our style)

Change-Id: I7cc822d4c620b74eaaa45b0ed9546659684b3ba8
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
M java/kudu-client/src/main/java/org/apache/kudu/client/DeadlineTracker.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/LocatedTablet.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
M 
java/kudu-client/src/main/java/org/apache/kudu/client/PleaseThrottleException.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RpcProxy.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RpcTraceFrame.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Statistics.java
M java/kudu-client/src/main/java/org/apache/kudu/util/Slice.java
M 
java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduOperationsProducer.java
M 
java/kudu-mapreduce/src/main/java/org/apache/kudu/mapreduce/KuduTableMapReduceUtil.java
16 files changed, 46 insertions(+), 0 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7cc822d4c620b74eaaa45b0ed9546659684b3ba8
Gerrit-Change-Number: 10193
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Grant Henke 


[kudu-CR] java: prohibit use of a KuduTable from an unassociated KuduClient

2018-04-25 Thread Todd Lipcon (Code Review)
Hello Alexey Serbin, David Ribeiro Alves, Jean-Daniel Cryans, Kudu Jenkins, 
Anonymous Coward #314, Grant Henke,

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

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

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

Change subject: java: prohibit use of a KuduTable from an unassociated 
KuduClient
..

java: prohibit use of a KuduTable from an unassociated KuduClient

This fixes a request-tracking issue with the following code
anti-pattern in which a KuduTable associated with one client is used to
create operations applied to another client's session:

  KuduClient client1 = KuduClientBuildernewClient();
  KuduTable t = client1.openTable(...);
  KuduClient client2 = KuduClientBuildernewClient();
  KuduSession s = client2.newSession();
  s.apply(t.newUpdate());

This would cause sequence numbers to be generated out of the session's
client's RequestTracker, but then marked complete in the operation's
client's RequestTracker. This could cause any number of issues:

- a cache "hit" on the server side might cause an operation to get an
  unrelated operation's response
- a cache "miss" on the server side might result in an operation
  incorrectly being marked as already-responded and garbage collected,
  causing it to fail.

This patch adds a Preconditions check for this issue and fixes some tests
where it was triggered.

Change-Id: Ib4a977f36b9c7ba3758322a2216ce90208b5d014
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RequestTracker.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITClientStress.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestRequestTracker.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/TestScannerMultiTablet.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java
9 files changed, 106 insertions(+), 58 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib4a977f36b9c7ba3758322a2216ce90208b5d014
Gerrit-Change-Number: 7362
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Anonymous Coward #314
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] java: fix error message when attempting to import Parquet TIMESTAMP

2018-04-25 Thread Todd Lipcon (Code Review)
Hello Grant Henke,

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

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

to review the following change.


Change subject: java: fix error message when attempting to import Parquet 
TIMESTAMP
..

java: fix error message when attempting to import Parquet TIMESTAMP

The code was comparing a Parquet type object to a javax.sql Timestamp
type enum. This would never be true, so the code was dead. This fixes it
to do the appropriate comparison.

Change-Id: Ib961d5372c0d8afc436317dc7c63738860612700
---
M 
java/kudu-client-tools/src/main/java/org/apache/kudu/mapreduce/tools/ImportParquet.java
1 file changed, 5 insertions(+), 5 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib961d5372c0d8afc436317dc7c63738860612700
Gerrit-Change-Number: 10194
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Grant Henke 


[kudu-CR] java: fix error-prone DefaultCharset[1] issues

2018-04-25 Thread Todd Lipcon (Code Review)
Hello Grant Henke,

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

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

to review the following change.


Change subject: java: fix error-prone DefaultCharset[1] issues
..

java: fix error-prone DefaultCharset[1] issues

This should be a non-functional change on all systems where UTF8 is the
default character set. It may fix issues on other systems which are
exceedingly rare these days.

[1] 
https://github.com/google/error-prone/blob/master/docs/bugpattern/DefaultCharset.md

Change-Id: Ic92985b5d9466c8629475f3645c872914e3f73a1
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M 
java/kudu-client/src/main/java/org/apache/kudu/client/ColumnRangePredicate.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduPredicate.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestRowResult.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestTestUtils.java
M 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/KeyedKuduOperationsProducerTest.java
M 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/KuduSinkTest.java
M 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducerTest.java
M java/kudu-mapreduce/src/test/java/org/apache/kudu/mapreduce/TestJarFinder.java
12 files changed, 44 insertions(+), 24 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic92985b5d9466c8629475f3645c872914e3f73a1
Gerrit-Change-Number: 10197
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Grant Henke 


[kudu-CR] java: make all enum members immutable

2018-04-25 Thread Todd Lipcon (Code Review)
Hello Grant Henke,

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

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

to review the following change.


Change subject: java: make all enum members immutable
..

java: make all enum members immutable

This fixes instances of the ImmutableEnumChecker[1] error-prone check.
This should be a non-functional change.

[1] 
https://github.com/google/error-prone/blob/master/docs/bugpattern/ImmutableEnumChecker.md

Change-Id: Ia02dce78d6084e73ec4e5beb2e22aea2c206141d
---
M 
java/kudu-client-tools/src/main/java/org/apache/kudu/mapreduce/tools/IntegrationTestBigLinkedList.java
M java/kudu-client/src/main/java/org/apache/kudu/Type.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M 
java/kudu-client/src/main/java/org/apache/kudu/client/ExternalConsistencyMode.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectToCluster.java
8 files changed, 11 insertions(+), 11 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia02dce78d6084e73ec4e5beb2e22aea2c206141d
Gerrit-Change-Number: 10195
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Grant Henke 


[kudu-CR] java: fix various integer-math error-prone issues

2018-04-25 Thread Todd Lipcon (Code Review)
Hello Grant Henke,

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

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

to review the following change.


Change subject: java: fix various integer-math error-prone issues
..

java: fix various integer-math error-prone issues

- Adds extra () around various bitwise math to clarify operator
  precedence (non-functional)

- Adds an upcast to 'long' to avoid potential overflow calculating the
  size of a write request. This shouldn't overflow in practice given
  other limits on write batch sizes.

- Removed some unused methods in Slices.java

This should be non-functional.

Change-Id: I515f64ab2a594588d952f6bd7f43451150c7ca9a
---
M java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Bytes.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/util/Slice.java
M java/kudu-client/src/main/java/org/apache/kudu/util/Slices.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
6 files changed, 9 insertions(+), 32 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I515f64ab2a594588d952f6bd7f43451150c7ca9a
Gerrit-Change-Number: 10200
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Grant Henke 


[kudu-CR] java: fix a mistaken reference-equality check for BigDecimal predicates

2018-04-25 Thread Todd Lipcon (Code Review)
Hello Grant Henke,

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

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

to review the following change.


Change subject: java: fix a mistaken reference-equality check for BigDecimal 
predicates
..

java: fix a mistaken reference-equality check for BigDecimal predicates

KuduPredicate used == instead of .equals() to check whether a supplied
value was equal to the maximum value for the specified decimal type.
This could result in too-conservative pruning.

Change-Id: Ieea49dcb3c3375287eb2c5fa704c1d9baea47b93
---
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java
1 file changed, 1 insertion(+), 1 deletion(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ieea49dcb3c3375287eb2c5fa704c1d9baea47b93
Gerrit-Change-Number: 10203
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Grant Henke 


[kudu-CR] java: fix ClassCanBeStatic[1] error-prone checks

2018-04-25 Thread Todd Lipcon (Code Review)
Hello Grant Henke,

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

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

to review the following change.


Change subject: java: fix ClassCanBeStatic[1] error-prone checks
..

java: fix ClassCanBeStatic[1] error-prone checks

This should be non-functional.

[1] 
https://github.com/google/error-prone/blob/master/docs/bugpattern/ClassCanBeStatic.md

Change-Id: I58ba8cdcd2a7038518b3e7c3ef81ecec14f5c370
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java
M java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java
M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java
M java/kudu-client/src/test/java/org/apache/kudu/util/TestAsyncUtil.java
4 files changed, 5 insertions(+), 5 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I58ba8cdcd2a7038518b3e7c3ef81ecec14f5c370
Gerrit-Change-Number: 10198
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Grant Henke 


[kudu-CR] java: fix minor synchronization issues exposed by error-prone

2018-04-25 Thread Todd Lipcon (Code Review)
Hello Grant Henke,

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

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

to review the following change.


Change subject: java: fix minor synchronization issues exposed by error-prone
..

java: fix minor synchronization issues exposed by error-prone

* Unsynchronized access to 'sessions' in AsyncKuduClient.closeAllSessions().
  This isn't likely to cause problems since it only happens on 'close'
  and would only race if a new session was concurrently started.

* Add missing GuardedBy annotations in a few spots (non-functional)

* Fix synchronization in TableLocationsCache.toString() to properly
  acquire the lock. Unlikely to cause problems in practice since we only
  call toString() on this in some rare trace messages (if that).

* Fix synchronization in FakeDNS (test-only code)

Change-Id: I1c737f59928f393883d198872419e8832dfff006
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
M java/kudu-client/src/main/java/org/apache/kudu/client/TableLocationsCache.java
M java/kudu-client/src/test/java/org/apache/kudu/client/FakeDNS.java
4 files changed, 16 insertions(+), 5 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1c737f59928f393883d198872419e8832dfff006
Gerrit-Change-Number: 10199
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Grant Henke 


[kudu-CR] java: fix error-prone 'StringSplit' pattern[1]

2018-04-25 Thread Todd Lipcon (Code Review)
Hello Grant Henke,

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

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

to review the following change.


Change subject: java: fix error-prone 'StringSplit' pattern[1]
..

java: fix error-prone 'StringSplit' pattern[1]

This should be a non-functional change.

[1] 
https://github.com/google/error-prone/blob/master/docs/bugpattern/StringSplit.md

Change-Id: I5379ca32be843374fd4a9f7213d096998e436baa
---
M 
java/kudu-client-tools/src/main/java/org/apache/kudu/mapreduce/tools/IntegrationTestBigLinkedList.java
M java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectToCluster.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
4 files changed, 4 insertions(+), 4 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5379ca32be843374fd4a9f7213d096998e436baa
Gerrit-Change-Number: 10196
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Grant Henke 


[kudu-CR] java: fix remainder of error-prone issues

2018-04-25 Thread Todd Lipcon (Code Review)
Hello Grant Henke,

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

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

to review the following change.


Change subject: java: fix remainder of error-prone issues
..

java: fix remainder of error-prone issues

- Suppress a few false positives where we are doing something sketchy
  but know it to be OK

- Fix two cases where a class member shadowed a member of a superclass

- Avoid StringBuffer in timestampToString. StringBuffer is synchronized,
  so it is probably more efficient to use plain String concatenation
  instead.[1]

- Avoid reference-equality checks in Status.fromMasterErrorPB and
  Status.fromTabletServerErrorPB. Based on the call sites I believe
  these checks would never have fired, because we only call these
  methods in cases where we know there to have been an actual error.

- Fix construction of the Flume KuduOperationProducer implementation to
  properly throw exceptions by using
  clazz.getDeclaredConstructor().newInstance() instead of
  clazz.newInstance()[2]

[1] 
https://github.com/google/error-prone/blob/master/docs/bugpattern/JdkObsolete.md
[2] 
https://github.com/google/error-prone/blob/master/docs/bugpattern/ClassNewInstance.md

Change-Id: I54f67c9718e1022d7996ab1cb827f68c1beb5d3d
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M 
java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToMasterRequest.java
M 
java/kudu-client/src/main/java/org/apache/kudu/client/IsAlterTableDoneRequest.java
M 
java/kudu-client/src/main/java/org/apache/kudu/client/IsCreateTableDoneRequest.java
M java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RequestTracker.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowError.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowResult.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowResultIterator.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Status.java
M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSink.java
11 files changed, 27 insertions(+), 30 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I54f67c9718e1022d7996ab1cb827f68c1beb5d3d
Gerrit-Change-Number: 10202
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Grant Henke 


[kudu-CR] java: enable error-prone for java builds

2018-04-25 Thread Todd Lipcon (Code Review)
Hello Dan Burkert, Jean-Daniel Cryans, Kudu Jenkins, Grant Henke,

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

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

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

Change subject: java: enable error-prone for java builds
..

java: enable error-prone for java builds

error-prone (http://errorprone.info/) is a compiler extension by Google
which checks for a bunch of common Java errors.

Fixes for the exposed issues are in follow-on commits.

Change-Id: Iebafbb307904875490426dd699ab1f55e21d284d
---
M java/build.gradle
M java/gradle/dependencies.gradle
M java/gradle/quality.gradle
3 files changed, 24 insertions(+), 0 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iebafbb307904875490426dd699ab1f55e21d284d
Gerrit-Change-Number: 4425
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins


[kudu-CR](branch-1.7.x) KUDU-2416: Fix PartialRow.setMin and add a unit test

2018-04-25 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10188 )

Change subject: KUDU-2416: Fix PartialRow.setMin and add a unit test
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.7.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I510743385cc567a8443efc8b0c9f5b641cf04c31
Gerrit-Change-Number: 10188
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 25 Apr 2018 19:47:35 +
Gerrit-HasComments: No


[kudu-CR](branch-1.4.x) KUDU-2416: Fix PartialRow.setMin and add a unit test

2018-04-25 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10191 )

Change subject: KUDU-2416: Fix PartialRow.setMin and add a unit test
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.4.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I510743385cc567a8443efc8b0c9f5b641cf04c31
Gerrit-Change-Number: 10191
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 25 Apr 2018 19:47:03 +
Gerrit-HasComments: No


[kudu-CR](branch-1.5.x) KUDU-2416: Fix PartialRow.setMin and add a unit test

2018-04-25 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10190 )

Change subject: KUDU-2416: Fix PartialRow.setMin and add a unit test
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.5.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I510743385cc567a8443efc8b0c9f5b641cf04c31
Gerrit-Change-Number: 10190
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 25 Apr 2018 19:47:11 +
Gerrit-HasComments: No


[kudu-CR](branch-1.6.x) KUDU-2416: Fix PartialRow.setMin and add a unit test

2018-04-25 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10189 )

Change subject: KUDU-2416: Fix PartialRow.setMin and add a unit test
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.6.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I510743385cc567a8443efc8b0c9f5b641cf04c31
Gerrit-Change-Number: 10189
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 25 Apr 2018 19:46:56 +
Gerrit-HasComments: No


[kudu-CR] java: prohibit use of a KuduTable from an unassociated KuduClient

2018-04-25 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/7362 )

Change subject: java: prohibit use of a KuduTable from an unassociated 
KuduClient
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7362/3/java/kudu-client/src/main/java/org/apache/kudu/client/RequestTracker.java
File java/kudu-client/src/main/java/org/apache/kudu/client/RequestTracker.java:

http://gerrit.cloudera.org:8080/#/c/7362/3/java/kudu-client/src/main/java/org/apache/kudu/client/RequestTracker.java@76
PS3, Line 76: This operation is idempotent.
> Nit: with this change this statement becomes a little funky if debug is tur
oops, yea



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4a977f36b9c7ba3758322a2216ce90208b5d014
Gerrit-Change-Number: 7362
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Anonymous Coward #314
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 25 Apr 2018 19:28:54 +
Gerrit-HasComments: Yes


[kudu-CR](branch-1.4.x) KUDU-2416: Fix PartialRow.setMin and add a unit test

2018-04-25 Thread Grant Henke (Code Review)
Grant Henke has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10191


Change subject: KUDU-2416: Fix PartialRow.setMin and add a unit test
..

KUDU-2416: Fix PartialRow.setMin and add a unit test

* Fixes INTEGER.MIN_VALUE being used for LONG colums
  that could lead to incorrect partition prunning.
* Adds a unit test for all types.

Change-Id: I510743385cc567a8443efc8b0c9f5b641cf04c31
---
M java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java
2 files changed, 20 insertions(+), 1 deletion(-)



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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.4.x
Gerrit-MessageType: newchange
Gerrit-Change-Id: I510743385cc567a8443efc8b0c9f5b641cf04c31
Gerrit-Change-Number: 10191
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 


[kudu-CR](branch-1.5.x) KUDU-2416: Fix PartialRow.setMin and add a unit test

2018-04-25 Thread Grant Henke (Code Review)
Grant Henke has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10190


Change subject: KUDU-2416: Fix PartialRow.setMin and add a unit test
..

KUDU-2416: Fix PartialRow.setMin and add a unit test

* Fixes INTEGER.MIN_VALUE being used for LONG colums
  that could lead to incorrect partition prunning.
* Adds a unit test for all types.

Change-Id: I510743385cc567a8443efc8b0c9f5b641cf04c31
---
M java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java
2 files changed, 20 insertions(+), 1 deletion(-)



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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.5.x
Gerrit-MessageType: newchange
Gerrit-Change-Id: I510743385cc567a8443efc8b0c9f5b641cf04c31
Gerrit-Change-Number: 10190
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 


[kudu-CR] [docs] Update kudu-spark section and add Upsert ignoreNull subsection

2018-04-25 Thread Fengling Wang (Code Review)
Hello Will Berkeley, Alex Rodoni, Dan Burkert, Attila Bukor, Kudu Jenkins, Hao 
Hao,

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

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

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

Change subject: [docs] Update kudu-spark section and add Upsert ignoreNull 
subsection
..

[docs] Update kudu-spark section and add Upsert ignoreNull subsection

Change-Id: Ib58bca60360ccd3c3abf579ede8137a34e870b0e
---
M docs/developing.adoc
1 file changed, 65 insertions(+), 20 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib58bca60360ccd3c3abf579ede8137a34e870b0e
Gerrit-Change-Number: 9849
Gerrit-PatchSet: 4
Gerrit-Owner: Fengling Wang 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Fengling Wang 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] [docs] Update kudu-spark section and add Upsert ignoreNull subsection

2018-04-25 Thread Hao Hao (Code Review)
Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9849 )

Change subject: [docs] Update kudu-spark section and add Upsert ignoreNull 
subsection
..


Patch Set 3: Code-Review+1

(1 comment)

Looks good to me, just one minor nit.

http://gerrit.cloudera.org:8080/#/c/9849/3/docs/developing.adoc
File docs/developing.adoc:

http://gerrit.cloudera.org:8080/#/c/9849/3/docs/developing.adoc@116
PS3, Line 116: val df = spark.read.options(Map("kudu.master" -> 
"kudu.master:7051","kudu.table" -> "kudu_table")).kudu
nit: this line is too long.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib58bca60360ccd3c3abf579ede8137a34e870b0e
Gerrit-Change-Number: 9849
Gerrit-PatchSet: 3
Gerrit-Owner: Fengling Wang 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Fengling Wang 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Wed, 25 Apr 2018 18:31:06 +
Gerrit-HasComments: Yes


[kudu-CR](branch-1.6.x) KUDU-2416: Fix PartialRow.setMin and add a unit test

2018-04-25 Thread Grant Henke (Code Review)
Grant Henke has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10189


Change subject: KUDU-2416: Fix PartialRow.setMin and add a unit test
..

KUDU-2416: Fix PartialRow.setMin and add a unit test

* Fixes INTEGER.MIN_VALUE being used for LONG colums
  that could lead to incorrect partition prunning.
* Adds a unit test for all types.

Change-Id: I510743385cc567a8443efc8b0c9f5b641cf04c31
---
M java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java
2 files changed, 20 insertions(+), 1 deletion(-)



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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.6.x
Gerrit-MessageType: newchange
Gerrit-Change-Id: I510743385cc567a8443efc8b0c9f5b641cf04c31
Gerrit-Change-Number: 10189
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 


[kudu-CR] KUDU-2416: Fix PartialRow.setMin and add a unit test

2018-04-25 Thread Grant Henke (Code Review)
Grant Henke has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10185 )

Change subject: KUDU-2416: Fix PartialRow.setMin and add a unit test
..

KUDU-2416: Fix PartialRow.setMin and add a unit test

* Fixes a missing break after the decimal type that could
   lead to IllegalArgumentExceptions.
* Fixes INTEGER.MIN_VALUE being used for LONG colums
  that could lead to incorrect partition prunning.
* Adds a unit test for all types.

Change-Id: I510743385cc567a8443efc8b0c9f5b641cf04c31
Reviewed-on: http://gerrit.cloudera.org:8080/10185
Reviewed-by: Todd Lipcon 
Reviewed-by: Dan Burkert 
Tested-by: Grant Henke 
---
M java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java
2 files changed, 22 insertions(+), 1 deletion(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I510743385cc567a8443efc8b0c9f5b641cf04c31
Gerrit-Change-Number: 10185
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] KUDU-2416: Fix PartialRow.setMin and add a unit test

2018-04-25 Thread Grant Henke (Code Review)
Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10185 )

Change subject: KUDU-2416: Fix PartialRow.setMin and add a unit test
..


Patch Set 1: Verified+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I510743385cc567a8443efc8b0c9f5b641cf04c31
Gerrit-Change-Number: 10185
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 25 Apr 2018 17:52:45 +
Gerrit-HasComments: No


[kudu-CR] KUDU-2416: Fix PartialRow.setMin and add a unit test

2018-04-25 Thread Grant Henke (Code Review)
Grant Henke has removed a vote on this change.

Change subject: KUDU-2416: Fix PartialRow.setMin and add a unit test
..


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I510743385cc567a8443efc8b0c9f5b641cf04c31
Gerrit-Change-Number: 10185
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR](branch-1.7.x) KUDU-2416: Fix PartialRow.setMin and add a unit test

2018-04-25 Thread Grant Henke (Code Review)
Grant Henke has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10188


Change subject: KUDU-2416: Fix PartialRow.setMin and add a unit test
..

KUDU-2416: Fix PartialRow.setMin and add a unit test

* Fixes a missing break after the decimal type that could
   lead to IllegalArgumentExceptions.
* Fixes INTEGER.MIN_VALUE being used for LONG colums
  that could lead to incorrect partition prunning.
* Adds a unit test for all types.

Change-Id: I510743385cc567a8443efc8b0c9f5b641cf04c31
---
M java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java
2 files changed, 22 insertions(+), 1 deletion(-)



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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.7.x
Gerrit-MessageType: newchange
Gerrit-Change-Id: I510743385cc567a8443efc8b0c9f5b641cf04c31
Gerrit-Change-Number: 10188
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 


[kudu-CR] KUDU-2416: Fix PartialRow.setMin and add a unit test

2018-04-25 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10185 )

Change subject: KUDU-2416: Fix PartialRow.setMin and add a unit test
..


Patch Set 1: Code-Review+2

> Patch Set 1: Code-Review+2
>
> Looks good. Would be good to understand the impact so we know how far back to 
> backport this, etc

This bug can result in Kudu incorrectly pruning partitions in specific 
scenarios.  It requires multiple range partition columns, a partition bound 
value being less than -2^31 on any column _except_ the first.  Then, it 
requires a certain set of predicates to trigger the pruning.  Here is the 
simplest repro I've found (in Impala):

CREATE TABLE t (a BIGINT, b BIGINT, PRIMARY KEY (a, b))
PARTITION BY RANGE (a, b) (
PARTITION VALUE = (0, 0),
PARTITION VALUE = (0, -2147483649)
) STORED AS KUDU;
INSERT INTO t VALUES (0, 0), (0, -2147483649);
SELECT * FROM t WHERE a >= 0;

results in a single result: (0, 0), where it should result in both values.

I think we should backport this patch widely given the 'obviousness' of the 
fix.  I'll come up with a regression test in another patch.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I510743385cc567a8443efc8b0c9f5b641cf04c31
Gerrit-Change-Number: 10185
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 25 Apr 2018 17:31:19 +
Gerrit-HasComments: No


[kudu-CR] [docs] Update kudu-spark section and add Upsert ignoreNull subsection

2018-04-25 Thread Fengling Wang (Code Review)
Fengling Wang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9849 )

Change subject: [docs] Update kudu-spark section and add Upsert ignoreNull 
subsection
..


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/9849/2/docs/developing.adoc
File docs/developing.adoc:

http://gerrit.cloudera.org:8080/#/c/9849/2/docs/developing.adoc@93
PS2, Line 93:
> Remove trailing whitespace here and throughout.
Done


http://gerrit.cloudera.org:8080/#/c/9849/2/docs/developing.adoc@101
PS2, Line 101:
> Wrap lines in body test (not code blocks) at about 80 characters. Meaning,
Done


http://gerrit.cloudera.org:8080/#/c/9849/2/docs/developing.adoc@104
PS2, Line 104:
> 1.7.0
Done


http://gerrit.cloudera.org:8080/#/c/9849/2/docs/developing.adoc@163
PS2, Line 163: u S
> Remove.
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib58bca60360ccd3c3abf579ede8137a34e870b0e
Gerrit-Change-Number: 9849
Gerrit-PatchSet: 3
Gerrit-Owner: Fengling Wang 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Fengling Wang 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Wed, 25 Apr 2018 16:59:44 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-2416: Fix PartialRow.setMin and add a unit test

2018-04-25 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10185 )

Change subject: KUDU-2416: Fix PartialRow.setMin and add a unit test
..


Patch Set 1: Code-Review+2

Looks good. Would be good to understand the impact so we know how far back to 
backport this, etc


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I510743385cc567a8443efc8b0c9f5b641cf04c31
Gerrit-Change-Number: 10185
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 25 Apr 2018 16:41:25 +
Gerrit-HasComments: No


[kudu-CR] [docs] Update kudu-spark section and add Upsert ignoreNull subsection

2018-04-25 Thread Will Berkeley (Code Review)
Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9849 )

Change subject: [docs] Update kudu-spark section and add Upsert ignoreNull 
subsection
..


Patch Set 2:

(4 comments)

One correction and some small things and it LGTM.

http://gerrit.cloudera.org:8080/#/c/9849/2/docs/developing.adoc
File docs/developing.adoc:

http://gerrit.cloudera.org:8080/#/c/9849/2/docs/developing.adoc@93
PS2, Line 93:
Remove trailing whitespace here and throughout.


http://gerrit.cloudera.org:8080/#/c/9849/2/docs/developing.adoc@101
PS2, Line 101: to
Wrap lines in body test (not code blocks) at about 80 characters. Meaning, add 
a line break to break up a long line into two shorter ones. This won't affect 
how it's rendered on the website.


http://gerrit.cloudera.org:8080/#/c/9849/2/docs/developing.adoc@104
PS2, Line 104: 5
1.7.0


http://gerrit.cloudera.org:8080/#/c/9849/2/docs/developing.adoc@163
PS2, Line 163: the
Remove.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib58bca60360ccd3c3abf579ede8137a34e870b0e
Gerrit-Change-Number: 9849
Gerrit-PatchSet: 2
Gerrit-Owner: Fengling Wang 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Fengling Wang 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Wed, 25 Apr 2018 15:57:50 +
Gerrit-HasComments: Yes


[kudu-CR] [WIP] [tools] ksck improvements [6/n]: Refactor printing

2018-04-25 Thread Will Berkeley (Code Review)
Hello Alexey Serbin, Kudu Jenkins,

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

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

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

Change subject: [WIP] [tools] ksck improvements [6/n]: Refactor printing
..

[WIP] [tools] ksck improvements [6/n]: Refactor printing

This patch significantly refactors printing in ksck. It separates where
checks are done from where results are printed. This separation will
make it easier to provide machine-readable output and create tools
based on the results of ksck checks, like auto-repair.

There's also a couple of bonus changes:
- Simplifies running all ksck checks and printing the results, as done
  by the ksck tool and ClusterVerifier.
- Removes the --consensus flag. ksck is far less useful without it and
  it doesn't seem like having it brings any benefit.
- Add the changes in https://gerrit.cloudera.org/#/c/10054/, which were
  about to be committed but would've needed to be redone a bit to fit
  with this refactor.

Change-Id: Id8de619996b6cd753e6a9c01b1b60810a873e609
---
M src/kudu/integration-tests/cluster_verifier.cc
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_remote-test.cc
A src/kudu/tools/ksck_results.cc
A src/kudu/tools/ksck_results.h
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/tool_action_cluster.cc
M src/kudu/tools/tool_action_tablet.cc
11 files changed, 1,204 insertions(+), 887 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id8de619996b6cd753e6a9c01b1b60810a873e609
Gerrit-Change-Number: 10151
Gerrit-PatchSet: 7
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley