[kudu-CR] [java] KUDU-2407: Fix leaked protubuf generated class
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10057 Change subject: [java] KUDU-2407: Fix leaked protubuf generated class .. [java] KUDU-2407: Fix leaked protubuf generated class Change-Id: I5fd93e592133cb15c1b5d6390bf69051355a8907 --- M java/kudu-client/src/main/java/org/apache/kudu/client/LocatedTablet.java 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/10057/1 -- To view, visit http://gerrit.cloudera.org:8080/10057 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I5fd93e592133cb15c1b5d6390bf69051355a8907 Gerrit-Change-Number: 10057 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke
[kudu-CR] [java] KUDU-2407: Fix leaked protubuf generated class
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10057 ) Change subject: [java] KUDU-2407: Fix leaked protubuf generated class .. Patch Set 1: How do we know that it's safe to do this? Technically it's both an ABI and API compatibility break, right? -- To view, visit http://gerrit.cloudera.org:8080/10057 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5fd93e592133cb15c1b5d6390bf69051355a8907 Gerrit-Change-Number: 10057 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Fri, 13 Apr 2018 16:40:22 + Gerrit-HasComments: No
[kudu-CR] [java] KUDU-2407: Fix leaked protubuf generated class
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/10057 ) Change subject: [java] KUDU-2407: Fix leaked protubuf generated class .. Patch Set 1: It is and we don't. It was added in 1.5.0 and I can't see a reason anyone would use it. We could just mark it as private with an interface annotation if you think thats better. I think the risk of leaving it public is potential breakage in the exposed Protobuf generated Role class that we don't have control over. -- To view, visit http://gerrit.cloudera.org:8080/10057 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5fd93e592133cb15c1b5d6390bf69051355a8907 Gerrit-Change-Number: 10057 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Fri, 13 Apr 2018 16:43:12 + Gerrit-HasComments: No
[kudu-CR] [java] KUDU-2407: Fix leaked protubuf generated class
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/10057 ) Change subject: [java] KUDU-2407: Fix leaked protubuf generated class .. Patch Set 1: Note: This interface is marked @InterfaceStability.Evolving. Signifying it can break compatibility at minor release (i.e. m.x). -- To view, visit http://gerrit.cloudera.org:8080/10057 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5fd93e592133cb15c1b5d6390bf69051355a8907 Gerrit-Change-Number: 10057 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Fri, 13 Apr 2018 16:44:14 + Gerrit-HasComments: No
[kudu-CR] [java] KUDU-2407: Fix leaked protubuf generated class
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10057 ) Change subject: [java] KUDU-2407: Fix leaked protubuf generated class .. Patch Set 1: Code-Review+1 > Patch Set 1: > > It is and we don't. It was added in 1.5.0 and I can't see a reason anyone > would use it. We could just mark it as private with an interface annotation > if you think thats better. > > I think the risk of leaving it public is potential breakage in the exposed > Protobuf generated Role class that we don't have control over. Ah, I forgot that the entire Replica class is @InterfaceStability.Evolving. OK, I'm +1. Let's see what others think. -- To view, visit http://gerrit.cloudera.org:8080/10057 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5fd93e592133cb15c1b5d6390bf69051355a8907 Gerrit-Change-Number: 10057 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Fri, 13 Apr 2018 16:49:28 + Gerrit-HasComments: No
[kudu-CR] [Java] Add Yetus doclet to Gradle build
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10059 Change subject: [Java] Add Yetus doclet to Gradle build .. [Java] Add Yetus doclet to Gradle build - Adds the yetus doclet to the gradle build - Adds missing interface annotations - Fixes a couple leaked SLF4J Loggers that are also documented. Change-Id: I283a3ccb8003356c875c603c9169d68a66b45804 --- M java/build.gradle M java/gradle/compile.gradle A java/gradle/docs.gradle M java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/KuduSession.java M java/kudu-client/src/main/java/org/apache/kudu/util/DecimalUtil.java M java/kudu-hive/build.gradle 9 files changed, 72 insertions(+), 18 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/59/10059/1 -- To view, visit http://gerrit.cloudera.org:8080/10059 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I283a3ccb8003356c875c603c9169d68a66b45804 Gerrit-Change-Number: 10059 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke
[kudu-CR] [Java] Add Yetus doclet to Gradle build
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10059 ) Change subject: [Java] Add Yetus doclet to Gradle build .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/10059/1/java/gradle/compile.gradle File java/gradle/compile.gradle: http://gerrit.cloudera.org:8080/#/c/10059/1/java/gradle/compile.gradle@a28 PS1, Line 28: Is this implicitly defined in Yetus already? http://gerrit.cloudera.org:8080/#/c/10059/1/java/gradle/docs.gradle File java/gradle/docs.gradle: http://gerrit.cloudera.org:8080/#/c/10059/1/java/gradle/docs.gradle@34 PS1, Line 34: 7 Should this be 8? -- To view, visit http://gerrit.cloudera.org:8080/10059 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I283a3ccb8003356c875c603c9169d68a66b45804 Gerrit-Change-Number: 10059 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Fri, 13 Apr 2018 17:31:31 + Gerrit-HasComments: Yes
[kudu-CR] [Java] Add Yetus doclet to Gradle build
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/10059 ) Change subject: [Java] Add Yetus doclet to Gradle build .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/10059/1/java/gradle/compile.gradle File java/gradle/compile.gradle: http://gerrit.cloudera.org:8080/#/c/10059/1/java/gradle/compile.gradle@a28 PS1, Line 28: > Is this implicitly defined in Yetus already? Well Yetus does this weird thing where it filters out doclint warnings. That's why we didn't need this in the Maven build. I opened YETUS-626 to track that when removing this. http://gerrit.cloudera.org:8080/#/c/10059/1/java/gradle/docs.gradle File java/gradle/docs.gradle: http://gerrit.cloudera.org:8080/#/c/10059/1/java/gradle/docs.gradle@34 PS1, Line 34: 7 > Should this be 8? We still support Java 7 so I link to that. -- To view, visit http://gerrit.cloudera.org:8080/10059 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I283a3ccb8003356c875c603c9169d68a66b45804 Gerrit-Change-Number: 10059 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Fri, 13 Apr 2018 17:34:40 + Gerrit-HasComments: Yes
[kudu-CR] [java] KUDU-2407: Fix leaked protubuf generated class
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/10057 ) Change subject: [java] KUDU-2407: Fix leaked protubuf generated class .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10057 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5fd93e592133cb15c1b5d6390bf69051355a8907 Gerrit-Change-Number: 10057 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Fri, 13 Apr 2018 17:35:16 + Gerrit-HasComments: No
[kudu-CR] [java] KUDU-2407: Fix leaked protubuf generated class
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/10057 ) Change subject: [java] KUDU-2407: Fix leaked protubuf generated class .. Patch Set 1: Seems fine to me, this is clearly not an API meant to be publicly consumed. No point in tying ourselves up over a mistake. -- To view, visit http://gerrit.cloudera.org:8080/10057 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5fd93e592133cb15c1b5d6390bf69051355a8907 Gerrit-Change-Number: 10057 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Fri, 13 Apr 2018 17:35:40 + Gerrit-HasComments: No
[kudu-CR] [java] KUDU-2407: Fix leaked protubuf generated class
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10057 ) Change subject: [java] KUDU-2407: Fix leaked protubuf generated class .. [java] KUDU-2407: Fix leaked protubuf generated class Change-Id: I5fd93e592133cb15c1b5d6390bf69051355a8907 Reviewed-on: http://gerrit.cloudera.org:8080/10057 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo Reviewed-by: Dan Burkert --- M java/kudu-client/src/main/java/org/apache/kudu/client/LocatedTablet.java 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Kudu Jenkins: Verified Adar Dembo: Looks good to me, but someone else must approve Dan Burkert: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/10057 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I5fd93e592133cb15c1b5d6390bf69051355a8907 Gerrit-Change-Number: 10057 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [Java] Add Yetus doclet to Gradle build
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10059 ) Change subject: [Java] Add Yetus doclet to Gradle build .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10059 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I283a3ccb8003356c875c603c9169d68a66b45804 Gerrit-Change-Number: 10059 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Fri, 13 Apr 2018 17:39:47 + Gerrit-HasComments: No
[kudu-CR] [spark] KUDU-2371: Add KuduWriteOptions class and ignoreNull option
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/9834 ) Change subject: [spark] KUDU-2371: Add KuduWriteOptions class and ignoreNull option .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/9834/8/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala: http://gerrit.cloudera.org:8080/#/c/9834/8/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala@78 PS8, Line 78: || (parameters.getOrElse(OPERATION, "upsert") == "insert-ignore"); > Hi dan, i'm thinking of working around with the "insert-ignore" by doing th Strategy looks good, but I think it will be a easier to follow the logic as: Try(parameters(FAULT_TOLERANT_SCANNER).toBoolean).getOrElse(false) || Try(parameters(OPERATION) == "insert-ignore").getOrElse(false) -- To view, visit http://gerrit.cloudera.org:8080/9834 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ide908ea29f572849eca0ba850ee197c1b22a07c8 Gerrit-Change-Number: 9834 Gerrit-PatchSet: 8 Gerrit-Owner: Fengling Wang Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Fengling Wang Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Fri, 13 Apr 2018 17:48:14 + Gerrit-HasComments: Yes
[kudu-CR] [tools] minor enhancements on 'kudu cluster ksck' output
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/10054 ) Change subject: [tools] minor enhancements on 'kudu cluster ksck' output .. Patch Set 5: (1 comment) How does this behave with table and/or tablet filters? Can you check what it does make sense and add a couple tests for it? http://gerrit.cloudera.org:8080/#/c/10054/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10054/5//COMMIT_MSG@27 PS5, Line 27: Masters| 1 nit: Can we order it masters tablet servers tables tablets replicas -- To view, visit http://gerrit.cloudera.org:8080/10054 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0888ef640b215e1e0cf1f872f02cfe34f4ef5ba6 Gerrit-Change-Number: 10054 Gerrit-PatchSet: 5 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Fri, 13 Apr 2018 17:50:22 + Gerrit-HasComments: Yes
[kudu-CR] [Java] Add Yetus doclet to Gradle build
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/10059 ) Change subject: [Java] Add Yetus doclet to Gradle build .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10059 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I283a3ccb8003356c875c603c9169d68a66b45804 Gerrit-Change-Number: 10059 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Fri, 13 Apr 2018 17:50:47 + Gerrit-HasComments: No
[kudu-CR] [docs] Update kudu-spark section and add Upsert ignoreNull subsection
Dan Burkert 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 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/9849/1/docs/developing.adoc File docs/developing.adoc: http://gerrit.cloudera.org:8080/#/c/9849/1/docs/developing.adoc@98 PS1, Line 98: 1.5.0 > I'm pretty sure we stopped supporting Spark 1 for 1.6, so we should call th +1 for getting rid of Spark1 docs, since we don't ship it anymore. -- 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: 1 Gerrit-Owner: Fengling Wang Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Fri, 13 Apr 2018 17:51:52 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2191 (10/n): Hive Metastore notification log event listener
Hello Tidy Bot, Kudu Jenkins, Adar Dembo, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8313 to look at the new patch set (#9). Change subject: KUDU-2191 (10/n): Hive Metastore notification log event listener .. KUDU-2191 (10/n): Hive Metastore notification log event listener This commit adds a notification log event listener to the Hive Metastore integration. The leader master now listens for drop table and alter table events in the HMS, and applies them to the Kudu catalog as necessary. The latest handled notification log event index is recorded in the sys-catalog. Testing: this is a hard component to test, because it is necessarily tied to the catalog manager. I've added tests for specific scenarios in master_hms-itest, however I'm aware that not all codepaths in the listener are covered. master-stress-test now has the HMS integration enabled, since this was the 'missing piece' which allows the catalogs to remain (eventually) consistent in the presence of master crashes. Note on IWYU: this commit continues the trend in this patch series of ignoring files which include HMS or Thrift classes, since the upstream Jenkins instance typically has loads of false positives with these. I have manually verified against a Centos 7 machine that the commit passes IWYU (with the ignores removed). Change-Id: I32ed099c44a593ffe514152135957018f21ed775 --- M build-support/iwyu/iwyu-filter.awk M src/kudu/hms/hms_catalog.cc M src/kudu/hms/hms_catalog.h M src/kudu/integration-tests/master-stress-test.cc M src/kudu/integration-tests/master_hms-itest.cc M src/kudu/master/CMakeLists.txt M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h A src/kudu/master/hms_notification_log_listener.cc A src/kudu/master/hms_notification_log_listener.h M src/kudu/master/master.proto M src/kudu/master/sys_catalog.cc M src/kudu/master/sys_catalog.h 13 files changed, 831 insertions(+), 41 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/13/8313/9 -- To view, visit http://gerrit.cloudera.org:8080/8313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I32ed099c44a593ffe514152135957018f21ed775 Gerrit-Change-Number: 8313 Gerrit-PatchSet: 9 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot
[kudu-CR] KUDU-2191 (10/n): Hive Metastore notification log event listener
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/8313 ) Change subject: KUDU-2191 (10/n): Hive Metastore notification log event listener .. Patch Set 9: R9 includes another case that isn't handled well currently. A simple concurrent rename in both Kudu and the HMS results in two table entries in the HMS, both with the same Kudu table ID. This one's interesting because I don't think schema versions really helps here. I think we needs some logic in the notification listener to fixup (delete) the now 'orphaned' entry in the HMS. -- To view, visit http://gerrit.cloudera.org:8080/8313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I32ed099c44a593ffe514152135957018f21ed775 Gerrit-Change-Number: 8313 Gerrit-PatchSet: 9 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Fri, 13 Apr 2018 17:56:54 + Gerrit-HasComments: No
[kudu-CR] [tools] ksck improvements [5/n]: Checks for experimental, unsafe, hidden flags
Will Berkeley has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10061 Change subject: [tools] ksck improvements [5/n]: Checks for experimental, unsafe, hidden flags .. [tools] ksck improvements [5/n]: Checks for experimental, unsafe, hidden flags This adds checks to ksck that look for experimental, unsafe, and hidden flags set to non-default values on Kudu masters and tablet servers. If any are found, ksck generates a table summarizing the different flags and their values. For example: WARNING: Some masters have hidden, experimental, or unsafe flags set: Flag |Value| Tags | Master +-+--+-- codegen_dump_functions | true| runtime,experimental | localhost:7052,localhost:7053 min_compression_ratio | 0.80004 | experimental | localhost:7052,localhost:7051,localhost:7053 safe_time_max_lag_ms | 4 | experimental | localhost:7052 safe_time_max_lag_ms | 5 | experimental | localhost:7053 The table has one row for each unique (flag, value) pair, listing all daemons with --flag=value. So, in the above output, there are two rows for the flag --safe_time_max_lag_ms because it's set to two different values on two masters. This makes it easy to scan for concerning flags and their values. A new flag --check_unusual_flags controls whether flag checks are done. It default to true. Flag checks may be worth turning off if a cluster has a large amount of tablet servers with experimental flags, or if a newer kudu tool is run against a cluster without GetFlags RPC support, since in these cases the flag check output will be noisy. Change-Id: Idd6c179e5256b2f2bae2f7486c5e0365ef184706 --- M src/kudu/integration-tests/cluster_verifier.cc 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 M src/kudu/tools/ksck_remote.cc M src/kudu/tools/ksck_remote.h M src/kudu/tools/tool_action_cluster.cc 8 files changed, 323 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/61/10061/1 -- To view, visit http://gerrit.cloudera.org:8080/10061 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Idd6c179e5256b2f2bae2f7486c5e0365ef184706 Gerrit-Change-Number: 10061 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley
[kudu-CR] [Java] Add Yetus doclet to Gradle build
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10059 ) Change subject: [Java] Add Yetus doclet to Gradle build .. [Java] Add Yetus doclet to Gradle build - Adds the yetus doclet to the gradle build - Adds missing interface annotations - Fixes a couple leaked SLF4J Loggers that are also documented. Change-Id: I283a3ccb8003356c875c603c9169d68a66b45804 Reviewed-on: http://gerrit.cloudera.org:8080/10059 Reviewed-by: Adar Dembo Reviewed-by: Dan Burkert Tested-by: Kudu Jenkins --- M java/build.gradle M java/gradle/compile.gradle A java/gradle/docs.gradle M java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/KuduSession.java M java/kudu-client/src/main/java/org/apache/kudu/util/DecimalUtil.java M java/kudu-hive/build.gradle 9 files changed, 72 insertions(+), 18 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Dan Burkert: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/10059 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I283a3ccb8003356c875c603c9169d68a66b45804 Gerrit-Change-Number: 10059 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] run-test.sh: treat empty gzipped output files as text files
Hello Todd Lipcon, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/10062 to review the following change. Change subject: run-test.sh: treat empty gzipped output files as text files .. run-test.sh: treat empty gzipped output files as text files I saw a test run where pstack_watcher-test timed out and gzip didn't produce any output. Thus, we ended up with an empty pstack_watcher-test.txt.gz file which is corrupt by definition; there should at least be a gzip header. If this happens, let's treat the empty file as an empty text file so that subsequent output file post-processing (which uses commands like zcat/zgrep) doesn't fail. Importantly, the pstack_watcher-test timeout does NOT benefit from this fix. That's because pstack_watcher-test was being traced by gdb, so it couldn't deliver itself a SIGABRT to timeout after 900 seconds. Instead, ctest killed it after 930 seconds by sending a SIGKILL to run-test.sh itself, thus preventing this cleanup code from running. Change-Id: I0fbe13982412f9113b27afbc40635f8945c45f09 --- M build-support/run-test.sh 1 file changed, 9 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/62/10062/1 -- To view, visit http://gerrit.cloudera.org:8080/10062 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I0fbe13982412f9113b27afbc40635f8945c45f09 Gerrit-Change-Number: 10062 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2406: derive filesystem block size from a temp file and not a directory
Hello Todd Lipcon, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/10063 to review the following change. Change subject: KUDU-2406: derive filesystem block size from a temp file and not a directory .. KUDU-2406: derive filesystem block size from a temp file and not a directory No tests as simulating an environment where files and directories advertise different block sizes would require a lot of boilerplate code. Change-Id: Ia8da6849fae1138d5d514dacf0ba66da60857799 --- M src/kudu/fs/block_manager_util.cc 1 file changed, 19 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/63/10063/1 -- To view, visit http://gerrit.cloudera.org:8080/10063 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ia8da6849fae1138d5d514dacf0ba66da60857799 Gerrit-Change-Number: 10063 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2406: derive filesystem block size from a temp file and not a directory
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10063 ) Change subject: KUDU-2406: derive filesystem block size from a temp file and not a directory .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10063 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia8da6849fae1138d5d514dacf0ba66da60857799 Gerrit-Change-Number: 10063 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 13 Apr 2018 19:44:28 + Gerrit-HasComments: No
[kudu-CR] run-test.sh: treat empty gzipped output files as text files
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10062 ) Change subject: run-test.sh: treat empty gzipped output files as text files .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10062 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0fbe13982412f9113b27afbc40635f8945c45f09 Gerrit-Change-Number: 10062 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 13 Apr 2018 19:45:16 + Gerrit-HasComments: No
[kudu-CR] [tools] ksck improvements [5/n]: Checks for experimental, unsafe, hidden flags
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10061 ) Change subject: [tools] ksck improvements [5/n]: Checks for experimental, unsafe, hidden flags .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/10061/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10061/1//COMMIT_MSG@22 PS1, Line 22: listing all : daemons is this output going to scale to large clusters? eg if you have 200 tablet servers, it's likely that all of them have the same experimental flags set. Maybe that's fine and we'll just have a really long line in the output, but maybe we should just abbreviate to a count like "89/89 tservers"? -- To view, visit http://gerrit.cloudera.org:8080/10061 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idd6c179e5256b2f2bae2f7486c5e0365ef184706 Gerrit-Change-Number: 10061 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 13 Apr 2018 19:52:22 + Gerrit-HasComments: Yes
[kudu-CR] run-test.sh: treat empty gzipped output files as text files
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10062 ) Change subject: run-test.sh: treat empty gzipped output files as text files .. run-test.sh: treat empty gzipped output files as text files I saw a test run where pstack_watcher-test timed out and gzip didn't produce any output. Thus, we ended up with an empty pstack_watcher-test.txt.gz file which is corrupt by definition; there should at least be a gzip header. If this happens, let's treat the empty file as an empty text file so that subsequent output file post-processing (which uses commands like zcat/zgrep) doesn't fail. Importantly, the pstack_watcher-test timeout does NOT benefit from this fix. That's because pstack_watcher-test was being traced by gdb, so it couldn't deliver itself a SIGABRT to timeout after 900 seconds. Instead, ctest killed it after 930 seconds by sending a SIGKILL to run-test.sh itself, thus preventing this cleanup code from running. Change-Id: I0fbe13982412f9113b27afbc40635f8945c45f09 Reviewed-on: http://gerrit.cloudera.org:8080/10062 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon --- M build-support/run-test.sh 1 file changed, 9 insertions(+), 0 deletions(-) Approvals: Kudu Jenkins: Verified Todd Lipcon: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/10062 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I0fbe13982412f9113b27afbc40635f8945c45f09 Gerrit-Change-Number: 10062 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2406: derive filesystem block size from a temp file and not a directory
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10063 ) Change subject: KUDU-2406: derive filesystem block size from a temp file and not a directory .. KUDU-2406: derive filesystem block size from a temp file and not a directory No tests as simulating an environment where files and directories advertise different block sizes would require a lot of boilerplate code. Change-Id: Ia8da6849fae1138d5d514dacf0ba66da60857799 Reviewed-on: http://gerrit.cloudera.org:8080/10063 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon --- M src/kudu/fs/block_manager_util.cc 1 file changed, 19 insertions(+), 1 deletion(-) Approvals: Kudu Jenkins: Verified Todd Lipcon: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/10063 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ia8da6849fae1138d5d514dacf0ba66da60857799 Gerrit-Change-Number: 10063 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [tools] ksck improvements [5/n]: Checks for experimental, unsafe, hidden flags
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/10061 ) Change subject: [tools] ksck improvements [5/n]: Checks for experimental, unsafe, hidden flags .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/10061/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10061/1//COMMIT_MSG@22 PS1, Line 22: listing all : daemons > is this output going to scale to large clusters? eg if you have 200 tablet Mmm I had the same thought which inspired the flag to turn off the flags check. It's a bummer not to see which tablet servers have a flag set. Maybe, if we did something like list the first few tablet servers, and after some threshold abbreviate, e.g. my-tserver-0, my-tserver-1, my-tserver-2, and 86 other tablet servers since users running a lot of tablet servers are most likely using config management software, showing a few addresses might help them figure out why the experimental flag is set. For example, maybe they set an experimental flag on a rack and forgot to remove it from the config group for that rack, and seeing a few hostnames that have rack info encoded in them might help. Thoughts? -- To view, visit http://gerrit.cloudera.org:8080/10061 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idd6c179e5256b2f2bae2f7486c5e0365ef184706 Gerrit-Change-Number: 10061 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Fri, 13 Apr 2018 20:00:38 + Gerrit-HasComments: Yes
[kudu-CR] [spark] KUDU-2371: Add KuduWriteOptions class and ignoreNull option
Hello Will Berkeley, Dan Burkert, Kudu Jenkins, Hao Hao, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9834 to look at the new patch set (#9). Change subject: [spark] KUDU-2371: Add KuduWriteOptions class and ignoreNull option .. [spark] KUDU-2371: Add KuduWriteOptions class and ignoreNull option This patch adds the KuduWriteOptions class to allow configuration of writes to the Kudu table when writing with Spark. This allows extensibility via adding more fields in the future. The instance of this class is passed to functions (insert/delete/upsert/update) in KuduContext. KuduWriteOptions is also supported in DefaultSource APIs. Clients can set up write options in the parameters from SparkSQL. This patch also adds the ignoreNull write option so that users can upsert/update only non-Null columns and leave the rest of the columns unchanged. For example, this feature is useful when users use Spark streaming to process JSON and upsert to Kudu, because missing column values from JSON are set to NULL, resulting in some existing row values being upserted to Null, which is not desired. Change-Id: Ide908ea29f572849eca0ba850ee197c1b22a07c8 --- M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala A java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduWriteOptions.scala M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/OperationType.scala M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala 6 files changed, 237 insertions(+), 26 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/34/9834/9 -- To view, visit http://gerrit.cloudera.org:8080/9834 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ide908ea29f572849eca0ba850ee197c1b22a07c8 Gerrit-Change-Number: 9834 Gerrit-PatchSet: 9 Gerrit-Owner: Fengling Wang Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Fengling Wang Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] [spark] KUDU-2371: Add KuduWriteOptions class and ignoreNull option
Fengling Wang has posted comments on this change. ( http://gerrit.cloudera.org:8080/9834 ) Change subject: [spark] KUDU-2371: Add KuduWriteOptions class and ignoreNull option .. Patch Set 9: (1 comment) http://gerrit.cloudera.org:8080/#/c/9834/8/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala: http://gerrit.cloudera.org:8080/#/c/9834/8/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala@78 PS8, Line 78: || Try(parameters(OPERATION) == "insert-ignore").getOrElse(false) > Strategy looks good, but I think it will be a easier to follow the logic as Done. Could you tell me why writing like this? -- To view, visit http://gerrit.cloudera.org:8080/9834 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ide908ea29f572849eca0ba850ee197c1b22a07c8 Gerrit-Change-Number: 9834 Gerrit-PatchSet: 9 Gerrit-Owner: Fengling Wang Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Fengling Wang Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Fri, 13 Apr 2018 22:31:41 + Gerrit-HasComments: Yes
[kudu-CR] [docs] Add lpeers output to ntp monitoring
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10041 ) Change subject: [docs] Add lpeers output to ntp monitoring .. [docs] Add lpeers output to ntp monitoring The ntp monitoring had an example of opeers output and had a tip explaining some versions have lpeers and some others opeers. These two are in fact two separate, although similar commands, and as far as I know both of them are available in all modern versions. Added an lpeers output, but had to change the lpeers output as well to keep things consistent. I also changed the tip to reflect this. Change-Id: I5009a330cfd3e81496e95b3e3cf1fb2b7627b085 Reviewed-on: http://gerrit.cloudera.org:8080/10041 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon --- M docs/troubleshooting.adoc 1 file changed, 36 insertions(+), 24 deletions(-) Approvals: Kudu Jenkins: Verified Todd Lipcon: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/10041 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I5009a330cfd3e81496e95b3e3cf1fb2b7627b085 Gerrit-Change-Number: 10041 Gerrit-PatchSet: 2 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [docs] Add lpeers output to ntp monitoring
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10041 ) Change subject: [docs] Add lpeers output to ntp monitoring .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10041 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5009a330cfd3e81496e95b3e3cf1fb2b7627b085 Gerrit-Change-Number: 10041 Gerrit-PatchSet: 1 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 13 Apr 2018 22:55:31 + Gerrit-HasComments: No
[kudu-CR] test util: include shard number in test scratch directory name
Hello Todd Lipcon, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/10069 to review the following change. Change subject: test_util: include shard number in test scratch directory name .. test_util: include shard number in test scratch directory name Amongst other things, build-and-test.sh checks that every test cleans up its scratch directory. It only performs this check if all tests pass, because if a test fails, it'll leave behind its scratch directory for inspection. This gets a little murky when considering flaky tests, because if one fails, it is retried a few times. As such, it's important that when run-test.sh prepares to rerun a flaky test, it cleans up the scratch directory left behind by the failed run. Unfortunately, run-test.sh isn't really aware of what it needs to clean up, because each test in the test executable creates its own test-specific scratch directory within a common root. To account for this, run-test.sh figures out what to clean using the following algorithm: 1. Before the test, capture the list of all directories within TEST_TMPDIR. 2. After the test, repeat the capture. 3. Diff the two captures to extract a list of directories that exist after but not before. 4. Search for the test name as a prefix in the extracted list. While complex, the algorithm does match and delete all scratch directories that were left behind by a particular test executable, even if TEST_TMPDIR is in use by other tests running in parallel. However, sharded tests break the algorithm, because the test name now includes a shard index, and that index isn't included in the scratch directory's name. I experimented with various fixes that included giving run-test.sh more ownership over TEST_TMPDIR, but eventually settled on a smaller and more targeted fix: if the test is being sharded, include the shard index in the scratch directory name. I tested this patch by running: TEST_TMPDIR=/tmp/kudutest-1000 \ KUDU_FLAKY_TEST_ATTEMPTS=3 \ KUDU_FLAKY_TEST_LIST=<(echo raft_consensus-itest) \ ctest -j8 -R raft_consensus-itest In the background, I also induced stress on the machine. This caused RaftConsensusParamReplicationModesITest.Test_KUDU_1735/1 to flake and fail from time to time. Without the patch, the flaky test's scratch directory was always left behind in TEST_TMPDIR. With it, the directory was always gone, unless the test failed every time it was run, which is expected behavior. Change-Id: Ie5686eb7c3a011a94b6c8c4eadbedb23acbf6e60 --- M src/kudu/util/test_util.cc 1 file changed, 10 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/69/10069/1 -- To view, visit http://gerrit.cloudera.org:8080/10069 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ie5686eb7c3a011a94b6c8c4eadbedb23acbf6e60 Gerrit-Change-Number: 10069 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [docs] Add warning about unsafe-change-config
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9906 ) Change subject: [docs] Add warning about unsafe-change-config .. [docs] Add warning about unsafe-change-config Adding an explicit warning about unsafe-change-config being unsafe to avoid users skimming through the docs and deciding this is the command they need. Also pointed out that this shouldn't be done if the missing replicas are a consequence of downed tablet servers and there's a chance of recovering the tablet servers themselves. Change-Id: I266c050c237a5c2fa286208a154c2a80f906769c Reviewed-on: http://gerrit.cloudera.org:8080/9906 Reviewed-by: Will Berkeley Tested-by: Will Berkeley --- M docs/administration.adoc 1 file changed, 7 insertions(+), 4 deletions(-) Approvals: Will Berkeley: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/9906 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I266c050c237a5c2fa286208a154c2a80f906769c Gerrit-Change-Number: 9906 Gerrit-PatchSet: 4 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] test util: include shard number in test scratch directory name
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10069 ) Change subject: test_util: include shard number in test scratch directory name .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10069 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie5686eb7c3a011a94b6c8c4eadbedb23acbf6e60 Gerrit-Change-Number: 10069 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Sat, 14 Apr 2018 01:00:33 + Gerrit-HasComments: No
[kudu-CR] test util: include shard number in test scratch directory name
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10069 ) Change subject: test_util: include shard number in test scratch directory name .. test_util: include shard number in test scratch directory name Amongst other things, build-and-test.sh checks that every test cleans up its scratch directory. It only performs this check if all tests pass, because if a test fails, it'll leave behind its scratch directory for inspection. This gets a little murky when considering flaky tests, because if one fails, it is retried a few times. As such, it's important that when run-test.sh prepares to rerun a flaky test, it cleans up the scratch directory left behind by the failed run. Unfortunately, run-test.sh isn't really aware of what it needs to clean up, because each test in the test executable creates its own test-specific scratch directory within a common root. To account for this, run-test.sh figures out what to clean using the following algorithm: 1. Before the test, capture the list of all directories within TEST_TMPDIR. 2. After the test, repeat the capture. 3. Diff the two captures to extract a list of directories that exist after but not before. 4. Search for the test name as a prefix in the extracted list. While complex, the algorithm does match and delete all scratch directories that were left behind by a particular test executable, even if TEST_TMPDIR is in use by other tests running in parallel. However, sharded tests break the algorithm, because the test name now includes a shard index, and that index isn't included in the scratch directory's name. I experimented with various fixes that included giving run-test.sh more ownership over TEST_TMPDIR, but eventually settled on a smaller and more targeted fix: if the test is being sharded, include the shard index in the scratch directory name. I tested this patch by running: TEST_TMPDIR=/tmp/kudutest-1000 \ KUDU_FLAKY_TEST_ATTEMPTS=3 \ KUDU_FLAKY_TEST_LIST=<(echo raft_consensus-itest) \ ctest -j8 -R raft_consensus-itest In the background, I also induced stress on the machine. This caused RaftConsensusParamReplicationModesITest.Test_KUDU_1735/1 to flake and fail from time to time. Without the patch, the flaky test's scratch directory was always left behind in TEST_TMPDIR. With it, the directory was always gone, unless the test failed every time it was run, which is expected behavior. Change-Id: Ie5686eb7c3a011a94b6c8c4eadbedb23acbf6e60 Reviewed-on: http://gerrit.cloudera.org:8080/10069 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon --- M src/kudu/util/test_util.cc 1 file changed, 10 insertions(+), 1 deletion(-) Approvals: Kudu Jenkins: Verified Todd Lipcon: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/10069 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ie5686eb7c3a011a94b6c8c4eadbedb23acbf6e60 Gerrit-Change-Number: 10069 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon