[kudu-CR] [master] add feature flag for 3-4-3 scheme
Hello Mike Percy, Dan Burkert, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9565 to look at the new patch set (#2). Change subject: [master] add feature flag for 3-4-3 scheme .. [master] add feature flag for 3-4-3 scheme This changelist addresses the case of misconfiguration of the replica management scheme between masters and tablet servers in the case when a master of version < 1.7 happen to work with tablet servers of version >= 1.7. Also, added a new run-time flag --heartbeat_incompatibility_is_fatal to control whether the replica management scheme misconfiguration should be treated as a fatal error for a tablet server. This is a follow-up for 1769eed53ee2c21a88a766cb72bf8c9ae622099d. Change-Id: I0e34a6fffc375720b26c2641e88057975d9190b9 --- M src/kudu/consensus/replica_management.proto M src/kudu/integration-tests/raft_consensus_nonvoter-itest.cc M src/kudu/master/master.proto M src/kudu/master/master_service.cc M src/kudu/tserver/heartbeater.cc 5 files changed, 111 insertions(+), 32 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/65/9565/2 -- To view, visit http://gerrit.cloudera.org:8080/9565 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0e34a6fffc375720b26c2641e88057975d9190b9 Gerrit-Change-Number: 9565 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey SerbinGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR] [master] add feature flag for 3-4-3 scheme
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9565 Change subject: [master] add feature flag for 3-4-3 scheme .. [master] add feature flag for 3-4-3 scheme This changelist addresses the case of misconfiguration of the replica management scheme between masters and tablet servers in the case when a master of version < 1.7 happen to work with tablet servers of version >= 1.7. Also, added a new run-time flag --heartbeat_incompatibility_is_fatal to control whether the replica management scheme misconfiguration should be treated as a fatal error for a tablet server. This is a follow-up for 1769eed53ee2c21a88a766cb72bf8c9ae622099d. Change-Id: I0e34a6fffc375720b26c2641e88057975d9190b9 --- M src/kudu/consensus/replica_management.proto M src/kudu/integration-tests/raft_consensus_nonvoter-itest.cc M src/kudu/master/master.proto M src/kudu/master/master_service.cc M src/kudu/tserver/heartbeater.cc 5 files changed, 103 insertions(+), 27 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/65/9565/1 -- To view, visit http://gerrit.cloudera.org:8080/9565 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I0e34a6fffc375720b26c2641e88057975d9190b9 Gerrit-Change-Number: 9565 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin
[kudu-CR] KUDU-2129 make ksck less scary when copying
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9528 ) Change subject: KUDU-2129 make ksck less scary when copying .. KUDU-2129 make ksck less scary when copying This patch changes some of the outputs for ksck to be less troubling when the cluster is recovering. Updates include: - Report tablets with replicas whose data states are TABLET_DATA_COPYING as RECOVERING instead of UNDER_REPLICATED. - Report tables with recovering tablets as RECOVERING instead of UNDER_REPLICATED. - Report replicas that aren't running as "not running" instead of "bad state". Most non-RUNNING states have a time and place during a healthy cluster's lifespan. - Don't log the error type with the errors when running the `ksck` tool. Our messages are generally good enough, and there isn't a perfect mapping to some errors. E.g. we should take the "Corruption" out of "Corruption: 1 out of 1 table(s) are bad" - Report "not healthy" instead of "bad" in logs like the one above Change-Id: Iba0c1f5b5a7083cbef99d3674dfebe1457075ce0 Reviewed-on: http://gerrit.cloudera.org:8080/9528 Tested-by: Kudu Jenkins Reviewed-by: Will Berkeley--- M src/kudu/tools/ksck-test.cc M src/kudu/tools/ksck.cc M src/kudu/tools/ksck.h M src/kudu/tools/tool_action_cluster.cc 4 files changed, 189 insertions(+), 90 deletions(-) Approvals: Kudu Jenkins: Verified Will Berkeley: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/9528 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Iba0c1f5b5a7083cbef99d3674dfebe1457075ce0 Gerrit-Change-Number: 9528 Gerrit-PatchSet: 4 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-2191 (6/n): Hive Metastore catalog manager integration
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/8312 ) Change subject: KUDU-2191 (6/n): Hive Metastore catalog manager integration .. Patch Set 9: (55 comments) http://gerrit.cloudera.org:8080/#/c/8312/8/src/kudu/hms/hms_client.cc File src/kudu/hms/hms_client.cc: http://gerrit.cloudera.org:8080/#/c/8312/8/src/kudu/hms/hms_client.cc@193 PS8, Line 193: // Also check that the HMS is configured to allow changing the type of : // columns, which is required to support dropping columns. Could you elaborate on this a bit? It's not intuitive why it needs to be legal to change a column's type in order to drop it. http://gerrit.cloudera.org:8080/#/c/8312/8/src/kudu/hms/hms_client.cc@202 PS8, Line 202: if (boost::iequals(disallow_incompatible_column_type_changes, "true")) { Hmm, is it really possible for the config's value to have a mixed case? Asking because L185 doesn't do a case-insensitive comparison when verifying that the required listeners are in hive.metastore.transactional.event.listeners. http://gerrit.cloudera.org:8080/#/c/8312/8/src/kudu/hms/hms_client.cc@218 PS8, Line 218: bool HmsClient::IsConnected() { Maybe add a bit of test coverage for this new method? http://gerrit.cloudera.org:8080/#/c/8312/8/src/kudu/hms/mini_hms.cc File src/kudu/hms/mini_hms.cc: http://gerrit.cloudera.org:8080/#/c/8312/8/src/kudu/hms/mini_hms.cc@186 PS8, Line 186: //Configures the HMS to allow altering and dropping columns. Nit: indentation is off by one character. http://gerrit.cloudera.org:8080/#/c/8312/8/src/kudu/hms/mini_hms.cc@214 PS8, Line 214: jdbc:derby:$1/metadb;create=true Why the change from 'memory' back to 'directory'? http://gerrit.cloudera.org:8080/#/c/8312/8/src/kudu/integration-tests/CMakeLists.txt File src/kudu/integration-tests/CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/8312/8/src/kudu/integration-tests/CMakeLists.txt@87 PS8, Line 87: ADD_KUDU_TEST(master_hms-itest) Could you run this new test with KUDU_MEASURE_TEST_CPU_CONSUMPTION=1 (see run-test.sh) to figure out whether to set RUN_SERIAL or to enumerate the number of PROCESSORS? http://gerrit.cloudera.org:8080/#/c/8312/9/src/kudu/integration-tests/master-stress-test.cc File src/kudu/integration-tests/master-stress-test.cc: http://gerrit.cloudera.org:8080/#/c/8312/9/src/kudu/integration-tests/master-stress-test.cc@343 PS9, Line 343: return Substitute("default.table_$0", oid_generator_.Next()); Not necessary yet? http://gerrit.cloudera.org:8080/#/c/8312/8/src/kudu/integration-tests/master_hms-itest.cc File src/kudu/integration-tests/master_hms-itest.cc: http://gerrit.cloudera.org:8080/#/c/8312/8/src/kudu/integration-tests/master_hms-itest.cc@122 PS8, Line 122: Could RETURN_NOT_OK instead. http://gerrit.cloudera.org:8080/#/c/8312/8/src/kudu/integration-tests/master_hms-itest.cc@162 PS8, Line 162: string table_name = Substitute("$0.$1", hms_database_name, hms_table_name); : : // Attempt to create the table before the database is created. : Status s = CreateTable(hms_database_name, hms_table Could be its own test (i.e. TestCreateTableWithoutDatabase) http://gerrit.cloudera.org:8080/#/c/8312/8/src/kudu/integration-tests/master_hms-itest.cc@190 PS8, Line 190: : // Shutdown the HMS and try to create a table. : ASSERT_OK(StopHms()); : : s = CreateTable(hms_database_name, "foo"); : ASSERT_TRUE(s.IsNetworkError()) << s.ToString(); : : // Start the HMS and try again. : ASSERT_OK(StartHms()); : ASSERT_OK(CreateTable(hms_database_name, "foo")); Could break out into a separate test since it only shares HMS database state with the previous test. http://gerrit.cloudera.org:8080/#/c/8312/8/src/kudu/integration-tests/master_hms-itest.cc@202 PS8, Line 202: Maybe add a test that if the HMS is down renaming fails? http://gerrit.cloudera.org:8080/#/c/8312/8/src/kudu/integration-tests/master_hms-itest.cc@207 PS8, Line 207: : // Create the database. : hive::Database db; : db.name = hms_database_name; : ASSERT_OK(hms_client_->CreateDatabase(db)); Every test does this; consider doing it in the test fixture. http://gerrit.cloudera.org:8080/#/c/8312/8/src/kudu/integration-tests/master_hms-itest.cc@243 PS8, Line 243: // TODO(HIVE-18852): match on the error message. : : // Attempt to rename the Kudu table to an invalid table name. : table_alterer.reset(client_->NewTableAlterer(table_name)); : s = table_alterer->RenameTo(Substitute("$0.☃", hms_database_name))->Alter(); : ASSERT_TRUE(s.IsIllegalState()) << s.ToString(); Would be a good negative test for creation too, right?
[kudu-CR] KUDU-2264. java: automatically attempt to re-acquire Kerberos credentials
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9050 ) Change subject: KUDU-2264. java: automatically attempt to re-acquire Kerberos credentials .. KUDU-2264. java: automatically attempt to re-acquire Kerberos credentials This fixes the Java client to notice when the Kerberos credentials it has are about to expire, and initiates a re-login when necessary. It also adds a long javadoc for AsyncKuduClient indicating the proper way to authenticate to a secured Kudu cluster for various scenarios, since this is a common question we've gotten. Change-Id: I514253e0a7f067dbc8ffe4eaf5a7a2c32900b539 Reviewed-on: http://gerrit.cloudera.org:8080/9050 Reviewed-by: Hao HaoTested-by: Kudu Jenkins Reviewed-by: Alexey Serbin --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/AuthnTokenReacquirer.java M java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.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/Negotiator.java M java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.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/BaseKuduTest.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/TestNegotiator.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool.proto M src/kudu/tools/tool_action_test.cc 14 files changed, 796 insertions(+), 102 deletions(-) Approvals: Hao Hao: Looks good to me, but someone else must approve Kudu Jenkins: Verified Alexey Serbin: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/9050 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I514253e0a7f067dbc8ffe4eaf5a7a2c32900b539 Gerrit-Change-Number: 9050 Gerrit-PatchSet: 6 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Anonymous Coward #380 Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode
David Ribeiro Alves has posted comments on this change. ( http://gerrit.cloudera.org:8080/8847 ) Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode .. Patch Set 10: lgtm, thanks waiting to get some validation from the jepsen work and then it's gtg -- To view, visit http://gerrit.cloudera.org:8080/8847 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465 Gerrit-Change-Number: 8847 Gerrit-PatchSet: 10 Gerrit-Owner: Hao HaoGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Thu, 08 Mar 2018 23:46:54 + Gerrit-HasComments: No
[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode
David Ribeiro Alves has posted comments on this change. ( http://gerrit.cloudera.org:8080/8847 ) Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode .. Patch Set 10: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/8847 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465 Gerrit-Change-Number: 8847 Gerrit-PatchSet: 10 Gerrit-Owner: Hao HaoGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Thu, 08 Mar 2018 23:45:49 + Gerrit-HasComments: No
[kudu-CR] KUDU-2093: Document AbstractKuduScannerBuilder limit has no effect
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9559 Change subject: KUDU-2093: Document AbstractKuduScannerBuilder limit has no effect .. KUDU-2093: Document AbstractKuduScannerBuilder limit has no effect Change-Id: I39cd3e61c02c357f66113f5b7207c9980e473787 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AbstractKuduScannerBuilder.java 1 file changed, 5 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/59/9559/1 -- To view, visit http://gerrit.cloudera.org:8080/9559 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I39cd3e61c02c357f66113f5b7207c9980e473787 Gerrit-Change-Number: 9559 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke
[kudu-CR] KUDU-2129 make ksck less scary when copying
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/9528 ) Change subject: KUDU-2129 make ksck less scary when copying .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/9528 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iba0c1f5b5a7083cbef99d3674dfebe1457075ce0 Gerrit-Change-Number: 9528 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew WongGerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Thu, 08 Mar 2018 22:18:59 + Gerrit-HasComments: No
[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/8847 ) Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode .. Patch Set 10: (2 comments) http://gerrit.cloudera.org:8080/#/c/8847/9/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java: http://gerrit.cloudera.org:8080/#/c/8847/9/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1085 PS9, Line 1085: tasksNum > tiny nit: java uses camel case :) Right... thanks for catching it. http://gerrit.cloudera.org:8080/#/c/8847/9/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1144 PS9, Line 1144: Waits for th > add a note indicating that any exceptions or assertion error will bubble up Done -- To view, visit http://gerrit.cloudera.org:8080/8847 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465 Gerrit-Change-Number: 8847 Gerrit-PatchSet: 10 Gerrit-Owner: Hao HaoGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Thu, 08 Mar 2018 21:50:26 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode
Hello Dan Burkert, David Ribeiro Alves, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8847 to look at the new patch set (#10). Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode .. KUDU-1704: add java client support for READ_YOUR_WRITES mode Change-Id: I6239521c022147257859e399f55c6f3f945af465 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.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/TestScannerMultiTablet.java 4 files changed, 236 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/47/8847/10 -- To view, visit http://gerrit.cloudera.org:8080/8847 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465 Gerrit-Change-Number: 8847 Gerrit-PatchSet: 10 Gerrit-Owner: Hao HaoGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode
David Ribeiro Alves has posted comments on this change. ( http://gerrit.cloudera.org:8080/8847 ) Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode .. Patch Set 9: (2 comments) http://gerrit.cloudera.org:8080/#/c/8847/9/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java: http://gerrit.cloudera.org:8080/#/c/8847/9/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1085 PS9, Line 1085: tasks_num tiny nit: java uses camel case :) http://gerrit.cloudera.org:8080/#/c/8847/9/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1144 PS9, Line 1144: future.get(); add a note indicating that any exceptions or assertion error will bubble up here. -- To view, visit http://gerrit.cloudera.org:8080/8847 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465 Gerrit-Change-Number: 8847 Gerrit-PatchSet: 9 Gerrit-Owner: Hao HaoGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Thu, 08 Mar 2018 21:18:04 + Gerrit-HasComments: Yes
[kudu-CR] [java] Ensure a license is added to the wrapper
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8333 ) Change subject: [java] Ensure a license is added to the wrapper .. [java] Ensure a license is added to the wrapper When generating a new/updated Gradle wrapper file this ensures the expected license is always added automatically. Change-Id: I590b20a963e1bb374fd9d7e290911f8fa44d4a4a Reviewed-on: http://gerrit.cloudera.org:8080/8333 Reviewed-by: Adar DemboTested-by: Kudu Jenkins --- M java/gradle/wrapper.gradle 1 file changed, 25 insertions(+), 1 deletion(-) Approvals: Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/8333 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I590b20a963e1bb374fd9d7e290911f8fa44d4a4a Gerrit-Change-Number: 8333 Gerrit-PatchSet: 4 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [java] Ensure a license is added to the wrapper
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/8333 ) Change subject: [java] Ensure a license is added to the wrapper .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/8333 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I590b20a963e1bb374fd9d7e290911f8fa44d4a4a Gerrit-Change-Number: 8333 Gerrit-PatchSet: 3 Gerrit-Owner: Grant HenkeGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Thu, 08 Mar 2018 19:39:58 + Gerrit-HasComments: No
[kudu-CR] KUDU-2191 (6/n): Hive Metastore catalog manager integration
Hello Tidy Bot, Kudu Jenkins, Adar Dembo, Hao Hao, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8312 to look at the new patch set (#9). Change subject: KUDU-2191 (6/n): Hive Metastore catalog manager integration .. KUDU-2191 (6/n): Hive Metastore catalog manager integration This commit adds integration with the Hive Metastore to CatalogManager, which can be enabled with a new flag: 'hive-metastore-addresses'. When the flag is provided, the Master will push DDL changes in its catalog to the Hive Metastore. In particular: - Create table, delete table, and alter table operations in the Kudu master will synchronously update the corresponding entry in the HMS. - New table and column names are required to be valid according to the Hive identifier rules, which are much stricter than Kudu's existing identifier rules. I think the code-paths in hms_catalog are pretty well covered by the tests added in this commit, however there is a dearth of stress and chaos tests covering the actual CatalogManager integration. In particular I've left some TODOs of roll-back code that could benefit from more coverage, as well as a TODO in master-stress-test about enabling HMS integration. Change-Id: I1b8f360408b05d6ac4f7359a8ac0dd889b196baf --- M src/kudu/hms/hms_client-test.cc M src/kudu/hms/hms_client.cc M src/kudu/hms/hms_client.h M src/kudu/hms/mini_hms.cc M src/kudu/integration-tests/CMakeLists.txt M src/kudu/integration-tests/alter_table-randomized-test.cc M src/kudu/integration-tests/master-stress-test.cc M src/kudu/integration-tests/master_failover-itest.cc A 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_catalog-test.cc A src/kudu/master/hms_catalog.cc A src/kudu/master/hms_catalog.h M src/kudu/master/master.proto M src/kudu/mini-cluster/external_mini_cluster-test.cc M src/kudu/mini-cluster/external_mini_cluster.cc M src/kudu/util/net/net_util.cc M src/kudu/util/net/net_util.h 20 files changed, 1,309 insertions(+), 42 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/12/8312/9 -- To view, visit http://gerrit.cloudera.org:8080/8312 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1b8f360408b05d6ac4f7359a8ac0dd889b196baf Gerrit-Change-Number: 8312 Gerrit-PatchSet: 9 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [java] Ensure a license is added to the wrapper
Hello Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8333 to look at the new patch set (#3). Change subject: [java] Ensure a license is added to the wrapper .. [java] Ensure a license is added to the wrapper When generating a new/updated Gradle wrapper file this ensures the expected license is always added automatically. Change-Id: I590b20a963e1bb374fd9d7e290911f8fa44d4a4a --- M java/gradle/wrapper.gradle 1 file changed, 25 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/33/8333/3 -- To view, visit http://gerrit.cloudera.org:8080/8333 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I590b20a963e1bb374fd9d7e290911f8fa44d4a4a Gerrit-Change-Number: 8333 Gerrit-PatchSet: 3 Gerrit-Owner: Grant HenkeGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [java] Ensure a licence is added to the wrapper
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/8333 ) Change subject: [java] Ensure a licence is added to the wrapper .. Patch Set 2: Code-Review+2 (1 comment) I had forgotten about this nonsense. http://gerrit.cloudera.org:8080/#/c/8333/2/java/gradle/wrapper.gradle File java/gradle/wrapper.gradle: http://gerrit.cloudera.org:8080/#/c/8333/2/java/gradle/wrapper.gradle@54 PS2, Line 54: c I'm pretty sure we discussed the correct spelling of 'license' over dinner last week... -- To view, visit http://gerrit.cloudera.org:8080/8333 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I590b20a963e1bb374fd9d7e290911f8fa44d4a4a Gerrit-Change-Number: 8333 Gerrit-PatchSet: 2 Gerrit-Owner: Grant HenkeGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Thu, 08 Mar 2018 19:05:27 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2191 (6/n): Hive Metastore catalog manager integration
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/8312 ) Change subject: KUDU-2191 (6/n): Hive Metastore catalog manager integration .. Patch Set 7: (7 comments) http://gerrit.cloudera.org:8080/#/c/8312/5/src/kudu/hms/hms_client.cc File src/kudu/hms/hms_client.cc: http://gerrit.cloudera.org:8080/#/c/8312/5/src/kudu/hms/hms_client.cc@204 PS5, Line 204: "Hive Metastore configuration is invalid: $0 must be set to false", > Add "to support dropping columns" to specify the reason. I'm not against adding that, but consider that this error will occur for every operation that hits the HMS (create table/all alter tables/drop table), not just ALTER TABLE DROP COLUMN ops. In that context I think it makes sense to just have a blanket statement 'this config is required', and not get in to too much detail. This is the same reason I didn't include the reasoning in the kudu metastore plugin check above (plus that one is way more nuanced :). http://gerrit.cloudera.org:8080/#/c/8312/5/src/kudu/integration-tests/alter_table-randomized-test.cc File src/kudu/integration-tests/alter_table-randomized-test.cc: http://gerrit.cloudera.org:8080/#/c/8312/5/src/kudu/integration-tests/alter_table-randomized-test.cc@79 PS5, Line 79: const char* kTableName = "default.test_table"; > Do we need to add a comment that when enable_hive_metastore is true, the ta We're going to have to document this new behavior very prominently, because it will surely trip up pretty much every current Kudu user that goes to turn on the HMS integration. Given that it will hopefully be documented widely in other public facing locations, I'm not sure it's worth calling out in specific unit tests. http://gerrit.cloudera.org:8080/#/c/8312/6/src/kudu/integration-tests/master_hms-itest.cc File src/kudu/integration-tests/master_hms-itest.cc: http://gerrit.cloudera.org:8080/#/c/8312/6/src/kudu/integration-tests/master_hms-itest.cc@176 PS6, Line 176: ASSERT_EQ(table->id(), hms_table.parameters[hms::HmsClient::kKuduTableIdKey]); > Assert master addresses as well? Done http://gerrit.cloudera.org:8080/#/c/8312/6/src/kudu/integration-tests/master_hms-itest.cc@236 PS6, Line 236: // Drop the HMS table entry and rename the table. > I do not quite follow why do we need to drop the hms table entry? Will the I've added the comment, and added the 'happy' path, I was so focused on the edge cases I forgot about that! http://gerrit.cloudera.org:8080/#/c/8312/6/src/kudu/integration-tests/master_hms-itest.cc@245 PS6, Line 245: ASSERT_OK(table_alterer->RenameTo(renamed_table_name)->Alter()); > Assert table id/master addresses/table schema. Done http://gerrit.cloudera.org:8080/#/c/8312/6/src/kudu/integration-tests/master_hms-itest.cc@285 PS6, Line 285: ASSERT_OK(hms_client_->AlterTable(hms_database_name, hms_table_name, hms_table)); > So alter column through HMS would not affect the table schema stored in Kud Correct. http://gerrit.cloudera.org:8080/#/c/8312/7/src/kudu/master/hms_catalog.cc File src/kudu/master/hms_catalog.cc: http://gerrit.cloudera.org:8080/#/c/8312/7/src/kudu/master/hms_catalog.cc@164 PS7, Line 164: create it in the HMS > I am not sure it is good to create a HMS entry if not present. What if the Users can't directly call this method, its called by the catalog manager, who checks that the Kudu table does exist before proceeding with an alter operation. It's important to support this usecase so that users can rename 'legacy' Kudu tables that don't have an entry in the HMS, and have that create the new entry with the corrected name. -- To view, visit http://gerrit.cloudera.org:8080/8312 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1b8f360408b05d6ac4f7359a8ac0dd889b196baf Gerrit-Change-Number: 8312 Gerrit-PatchSet: 7 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 08 Mar 2018 18:57:57 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2191 (6/n): Hive Metastore catalog manager integration
Hello Tidy Bot, Kudu Jenkins, Adar Dembo, Hao Hao, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8312 to look at the new patch set (#8). Change subject: KUDU-2191 (6/n): Hive Metastore catalog manager integration .. KUDU-2191 (6/n): Hive Metastore catalog manager integration This commit adds integration with the Hive Metastore to CatalogManager, which can be enabled with a new flag: 'hive-metastore-addresses'. When the flag is provided, the Master will push DDL changes in its catalog to the Hive Metastore. In particular: - Create table, delete table, and alter table operations in the Kudu master will synchronously update the corresponding entry in the HMS. - New table and column names are required to be valid according to the Hive identifier rules, which are much stricter than Kudu's existing identifier rules. I think the code-paths in hms_catalog are pretty well covered by the tests added in this commit, however there is a dearth of stress and chaos tests covering the actual CatalogManager integration. In particular I've left some TODOs of roll-back code that could benefit from more coverage, as well as a TODO in master-stress-test about enabling HMS integration. Change-Id: I1b8f360408b05d6ac4f7359a8ac0dd889b196baf --- M src/kudu/hms/hms_client-test.cc M src/kudu/hms/hms_client.cc M src/kudu/hms/hms_client.h M src/kudu/hms/mini_hms.cc M src/kudu/integration-tests/CMakeLists.txt M src/kudu/integration-tests/alter_table-randomized-test.cc M src/kudu/integration-tests/master-stress-test.cc M src/kudu/integration-tests/master_failover-itest.cc A 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_catalog-test.cc A src/kudu/master/hms_catalog.cc A src/kudu/master/hms_catalog.h M src/kudu/master/master.proto M src/kudu/mini-cluster/external_mini_cluster-test.cc M src/kudu/mini-cluster/external_mini_cluster.cc M src/kudu/util/net/net_util.cc M src/kudu/util/net/net_util.h 20 files changed, 1,300 insertions(+), 41 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/12/8312/8 -- To view, visit http://gerrit.cloudera.org:8080/8312 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1b8f360408b05d6ac4f7359a8ac0dd889b196baf Gerrit-Change-Number: 8312 Gerrit-PatchSet: 8 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2297 (part 6): enable periodic stack sampling by default, avoid bias
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9523 ) Change subject: KUDU-2297 (part 6): enable periodic stack sampling by default, avoid bias .. KUDU-2297 (part 6): enable periodic stack sampling by default, avoid bias This changes the timing for the collection of stack samples into the metrics log to avoid two issues: 1) Inflexible configuration: in many cases we might want to sample stacks more or less frequently than metrics. The previous implementation used the same schedule for both. 2) Sampling bias: if we collect stack samples on a fixed schedule, we risk unintended correlation with background tasks that might run on a similar fixed schedule. For example, if we collect stacks exactly once a minute, and the user has some monitoring software which polls the HTTP server once a minute for some data, we might either line up perfectly with the polling (in which case we'd overestimate its impact) or line of perfectly away from the polling (in which case we'd never see its effects). This patch changes the wakeups for stacks and metrics to be decoupled. In addition, it adds randomness to the stack sampling. The configuration now represents the mean sampling rate rather than an exact sampling rate. I manually ran a master with -diagnostics-log-stack-traces-interval-ms=2000 and verified that the stack sample times were between 0 and 4 seconds apart while the metric dumps were still exactly one minute apart. I also plan to modify some local test clusters to configure this to be relatively frequent to try to suss out any races or bugs which might occur in a real workload. Change-Id: I538eb59f95a695fa2da50f24ed82148715b15e44 Reviewed-on: http://gerrit.cloudera.org:8080/9523 Tested-by: Todd LipconReviewed-by: Adar Dembo --- M src/kudu/server/diagnostics_log.cc M src/kudu/server/diagnostics_log.h 2 files changed, 77 insertions(+), 49 deletions(-) Approvals: Todd Lipcon: Verified Adar Dembo: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/9523 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I538eb59f95a695fa2da50f24ed82148715b15e44 Gerrit-Change-Number: 9523 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2297 (part 5): dump stacks into diagnostics log on service queue overflow
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9375 ) Change subject: KUDU-2297 (part 5): dump stacks into diagnostics log on service queue overflow .. KUDU-2297 (part 5): dump stacks into diagnostics log on service queue overflow In the past we've had several bugs and performance issues which manifest themselves as service queue overflows. It would be easier to pinpoint the root cause (eg slow IO vs lock contention) if we had a process-wide stack snapshot at the time of the overflow. This patch does some plumbing to trigger such a stack trace into the diagnostics log when a service queue overflow occurs. The logging is throttled to once every 5 seconds so that we don't contribute too much to the load itself. The plumbing is a bit ugly since the diagnostics log is in the server/ module whereas the overflows surface in the rpc/ module. Having rpc/ call back into server/ would be a cyclic dependency, so instead the rpc/ module exposes a std::function<> style callback for the server to hook into. Change-Id: I62019c71121d7ca4037cab86a7c2ea048a261ad8 Reviewed-on: http://gerrit.cloudera.org:8080/9375 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo--- M src/kudu/master/master-test.cc M src/kudu/master/mini_master.h M src/kudu/rpc/service_pool.cc M src/kudu/rpc/service_pool.h M src/kudu/server/diagnostics_log.cc M src/kudu/server/diagnostics_log.h M src/kudu/server/rpc_server.cc M src/kudu/server/rpc_server.h M src/kudu/server/server_base.cc M src/kudu/server/server_base.h 10 files changed, 185 insertions(+), 19 deletions(-) Approvals: Kudu Jenkins: Verified Adar Dembo: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/9375 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I62019c71121d7ca4037cab86a7c2ea048a261ad8 Gerrit-Change-Number: 9375 Gerrit-PatchSet: 7 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2297 (part 5): dump stacks into diagnostics log on service queue overflow
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/9375 ) Change subject: KUDU-2297 (part 5): dump stacks into diagnostics log on service queue overflow .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/9375 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I62019c71121d7ca4037cab86a7c2ea048a261ad8 Gerrit-Change-Number: 9375 Gerrit-PatchSet: 6 Gerrit-Owner: Todd LipconGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 08 Mar 2018 18:50:45 + Gerrit-HasComments: No
[kudu-CR] KUDU-2297 (part 6): enable periodic stack sampling by default, avoid bias
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/9523 ) Change subject: KUDU-2297 (part 6): enable periodic stack sampling by default, avoid bias .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/9523 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I538eb59f95a695fa2da50f24ed82148715b15e44 Gerrit-Change-Number: 9523 Gerrit-PatchSet: 3 Gerrit-Owner: Todd LipconGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 08 Mar 2018 18:49:42 + Gerrit-HasComments: No
[kudu-CR] KUDU-2297 (part 6): enable periodic stack sampling by default, avoid bias
Todd Lipcon has removed a vote on this change. Change subject: KUDU-2297 (part 6): enable periodic stack sampling by default, avoid bias .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/9523 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I538eb59f95a695fa2da50f24ed82148715b15e44 Gerrit-Change-Number: 9523 Gerrit-PatchSet: 3 Gerrit-Owner: Todd LipconGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2297 (part 6): enable periodic stack sampling by default, avoid bias
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/9523 ) Change subject: KUDU-2297 (part 6): enable periodic stack sampling by default, avoid bias .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/9523 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I538eb59f95a695fa2da50f24ed82148715b15e44 Gerrit-Change-Number: 9523 Gerrit-PatchSet: 3 Gerrit-Owner: Todd LipconGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 08 Mar 2018 18:10:14 + Gerrit-HasComments: No
[kudu-CR] [java] Ensure a licence is added to the wrapper
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/8333 ) Change subject: [java] Ensure a licence is added to the wrapper .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/8333/1/java/gradlew File java/gradlew: http://gerrit.cloudera.org:8080/#/c/8333/1/java/gradlew@2 PS1, Line 2: > Nit: could you modify the writing process to omit this empty line here? Done -- To view, visit http://gerrit.cloudera.org:8080/8333 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I590b20a963e1bb374fd9d7e290911f8fa44d4a4a Gerrit-Change-Number: 8333 Gerrit-PatchSet: 1 Gerrit-Owner: Grant HenkeGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Thu, 08 Mar 2018 17:02:31 + Gerrit-HasComments: Yes