[kudu-CR] [tools] ksck improvements [6/n]: Refactor printing
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 BerkeleyGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-2191: Metadata Upgrade Tool
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 HaoGerrit-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
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 HaoGerrit-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
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
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
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 HenkeGerrit-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
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 HenkeGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins
[kudu-CR](branch-1.5.x) KUDU-2416: Fix PartialRow.setMin and add a unit test
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 BurkertTested-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
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 HenkeGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins
[kudu-CR](branch-1.4.x) KUDU-2416: Fix PartialRow.setMin and add a unit test
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 BurkertTested-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
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 HenkeGerrit-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
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 HenkeGerrit-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
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 LipconTested-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
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 HenkeGerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [java] Upgrade to Gradle 4.7
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 HenkeGerrit-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
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 LipconGerrit-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
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 LipconGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] cache: add a benchmark
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 DemboTested-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
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 LipconGerrit-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
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 LipconGerrit-Reviewer: Mostafa Mokhtar Gerrit-Reviewer: Will Berkeley
[kudu-CR] Fix table formatting in full data dirs docs
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 BerkeleyGerrit-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
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 HenkeGerrit-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
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 LipconGerrit-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
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 LipconGerrit-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
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 LipconGerrit-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
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 LipconGerrit-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
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 HenkeGerrit-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
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 LipconGerrit-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
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 LipconGerrit-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
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 BerkeleyGerrit-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
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 HenkeGerrit-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
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
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 BerkeleyGerrit-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
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 LipconGerrit-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
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 LipconGerrit-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
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 LipconGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] cache: add a benchmark
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 LipconGerrit-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
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 LipconGerrit-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
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 LipconGerrit-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
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 LipconGerrit-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
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 BerkeleyGerrit-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
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 LipconGerrit-Reviewer: David Ribeiro Alves
[kudu-CR] [docs] Add docs on full data dirs
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 BerkeleyGerrit-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
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 DemboReviewed-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
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 BerkeleyGerrit-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
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 BerkeleyGerrit-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
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 BerkeleyGerrit-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
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 BerkeleyGerrit-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
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 BerkeleyGerrit-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
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 LipconGerrit-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
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 BerkeleyGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley
[kudu-CR] cache: add a benchmark
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 LipconGerrit-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
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 BerkeleyGerrit-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
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 HenkeGerrit-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
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 BurkertTested-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
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
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 LipconGerrit-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
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 BerkeleyGerrit-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
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 LipconGerrit-Reviewer: Grant Henke
[kudu-CR] Fix missing @Override annotations
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 LipconGerrit-Reviewer: Grant Henke
[kudu-CR] java: prohibit use of a KuduTable from an unassociated KuduClient
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 LipconGerrit-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
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 LipconGerrit-Reviewer: Grant Henke
[kudu-CR] java: fix error-prone DefaultCharset[1] issues
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 LipconGerrit-Reviewer: Grant Henke
[kudu-CR] java: make all enum members immutable
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 LipconGerrit-Reviewer: Grant Henke
[kudu-CR] java: fix various integer-math error-prone issues
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 LipconGerrit-Reviewer: Grant Henke
[kudu-CR] java: fix a mistaken reference-equality check for BigDecimal predicates
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 LipconGerrit-Reviewer: Grant Henke
[kudu-CR] java: fix ClassCanBeStatic[1] error-prone checks
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 LipconGerrit-Reviewer: Grant Henke
[kudu-CR] java: fix minor synchronization issues exposed by error-prone
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 LipconGerrit-Reviewer: Grant Henke
[kudu-CR] java: fix error-prone 'StringSplit' pattern[1]
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 LipconGerrit-Reviewer: Grant Henke
[kudu-CR] java: fix remainder of error-prone issues
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 LipconGerrit-Reviewer: Grant Henke
[kudu-CR] java: enable error-prone for java builds
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 LipconGerrit-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
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 HenkeGerrit-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
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 HenkeGerrit-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
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 HenkeGerrit-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
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 HenkeGerrit-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
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 LipconGerrit-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
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
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
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 WangGerrit-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
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 WangGerrit-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
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
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 LipconReviewed-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
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 HenkeGerrit-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
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 HenkeGerrit-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
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
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 HenkeGerrit-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
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 WangGerrit-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
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 HenkeGerrit-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
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 WangGerrit-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
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 BerkeleyGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley