[kudu-CR] KUDU-1884: Set sasl protocol name for the TxnSystemClient
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17480 ) Change subject: KUDU-1884: Set sasl_protocol_name for the TxnSystemClient .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/17480/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/17480/2//COMMIT_MSG@13 PS2, Line 13: This logic matches that in server_base.cc for server : messenger configuration. > Does it make sense to add a test, at least catch regressions once the overl Probably, security-itest.cc might be even a better place for a new test scenario. -- To view, visit http://gerrit.cloudera.org:8080/17480 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic7b7c7fd154796215bd3a3ce748a2fd3155adebf Gerrit-Change-Number: 17480 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 21 May 2021 04:43:32 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-1884: Set sasl protocol name for the TxnSystemClient
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17480 ) Change subject: KUDU-1884: Set sasl_protocol_name for the TxnSystemClient .. Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/17480/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/17480/2//COMMIT_MSG@9 PS2, Line 9: witha with a http://gerrit.cloudera.org:8080/#/c/17480/2//COMMIT_MSG@10 PS2, Line 10: embeded embedded http://gerrit.cloudera.org:8080/#/c/17480/2//COMMIT_MSG@13 PS2, Line 13: This logic matches that in server_base.cc for server : messenger configuration. Does it make sense to add a test, at least catch regressions once the overlooked place for the kerberos principal customization is found? It seems a small end-to-end scenario might be added into client-test.cc or txn_write_ops-itest.cc -- To view, visit http://gerrit.cloudera.org:8080/17480 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic7b7c7fd154796215bd3a3ce748a2fd3155adebf Gerrit-Change-Number: 17480 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 21 May 2021 04:42:00 + Gerrit-HasComments: Yes
[kudu-CR](branch-1.15.x) KUDU-3223: management of per-table limit by kudu CLI
Andrew Wong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17481 Change subject: KUDU-3223: management of per-table limit by kudu CLI .. KUDU-3223: management of per-table limit by kudu CLI Use kudu CLI to set disk_size_limit and row_count_limit on table level. If '--enable_table_write_limit' is not set 'true' for kudu-master, kudu CLI fails to set the limit. The table's disk_size_limit and row_count_limit are 'N/A' when calling 'kudu table statistics'. Only when '--enable_table_write_limit' is set 'true', can the kudu CLI change the table limit. Use 'kudu table statistics' to check the updated result. Change-Id: I59bb64cc85d5fca48ae8ec980f5cfc62e4b3a1d3 Reviewed-on: http://gerrit.cloudera.org:8080/17444 Reviewed-by: Alexey Serbin Tested-by: Kudu Jenkins Reviewed-by: Andrew Wong (cherry picked from commit 323d8ea7312678b7a910e55ed4f74acaf3813ffc) --- M src/kudu/master/catalog_manager.cc M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_table.cc 3 files changed, 240 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/81/17481/1 -- To view, visit http://gerrit.cloudera.org:8080/17481 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.15.x Gerrit-MessageType: newchange Gerrit-Change-Id: I59bb64cc85d5fca48ae8ec980f5cfc62e4b3a1d3 Gerrit-Change-Number: 17481 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Hongjiang Zhang
[kudu-CR] KUDU-3223: management of per-table limit by kudu CLI
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17444 ) Change subject: KUDU-3223: management of per-table limit by kudu CLI .. KUDU-3223: management of per-table limit by kudu CLI Use kudu CLI to set disk_size_limit and row_count_limit on table level. If '--enable_table_write_limit' is not set 'true' for kudu-master, kudu CLI fails to set the limit. The table's disk_size_limit and row_count_limit are 'N/A' when calling 'kudu table statistics'. Only when '--enable_table_write_limit' is set 'true', can the kudu CLI change the table limit. Use 'kudu table statistics' to check the updated result. Change-Id: I59bb64cc85d5fca48ae8ec980f5cfc62e4b3a1d3 Reviewed-on: http://gerrit.cloudera.org:8080/17444 Reviewed-by: Alexey Serbin Tested-by: Kudu Jenkins Reviewed-by: Andrew Wong --- M src/kudu/master/catalog_manager.cc M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_table.cc 3 files changed, 240 insertions(+), 4 deletions(-) Approvals: Alexey Serbin: Looks good to me, approved Kudu Jenkins: Verified Andrew Wong: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/17444 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I59bb64cc85d5fca48ae8ec980f5cfc62e4b3a1d3 Gerrit-Change-Number: 17444 Gerrit-PatchSet: 18 Gerrit-Owner: Hongjiang Zhang Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hongjiang Zhang Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3223: management of per-table limit by kudu CLI
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17444 ) Change subject: KUDU-3223: management of per-table limit by kudu CLI .. Patch Set 17: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/17444 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I59bb64cc85d5fca48ae8ec980f5cfc62e4b3a1d3 Gerrit-Change-Number: 17444 Gerrit-PatchSet: 17 Gerrit-Owner: Hongjiang Zhang Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hongjiang Zhang Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 21 May 2021 04:33:56 + Gerrit-HasComments: No
[kudu-CR] KUDU-3223: management of per-table limit by kudu CLI
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17444 ) Change subject: KUDU-3223: management of per-table limit by kudu CLI .. Patch Set 17: Code-Review+2 LGTM! -- To view, visit http://gerrit.cloudera.org:8080/17444 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I59bb64cc85d5fca48ae8ec980f5cfc62e4b3a1d3 Gerrit-Change-Number: 17444 Gerrit-PatchSet: 17 Gerrit-Owner: Hongjiang Zhang Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hongjiang Zhang Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 21 May 2021 04:12:32 + Gerrit-HasComments: No
[kudu-CR] KUDU-3223: management of per-table limit by kudu CLI
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/17444 to look at the new patch set (#17). Change subject: KUDU-3223: management of per-table limit by kudu CLI .. KUDU-3223: management of per-table limit by kudu CLI Use kudu CLI to set disk_size_limit and row_count_limit on table level. If '--enable_table_write_limit' is not set 'true' for kudu-master, kudu CLI fails to set the limit. The table's disk_size_limit and row_count_limit are 'N/A' when calling 'kudu table statistics'. Only when '--enable_table_write_limit' is set 'true', can the kudu CLI change the table limit. Use 'kudu table statistics' to check the updated result. Change-Id: I59bb64cc85d5fca48ae8ec980f5cfc62e4b3a1d3 --- M src/kudu/master/catalog_manager.cc M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_table.cc 3 files changed, 240 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/44/17444/17 -- To view, visit http://gerrit.cloudera.org:8080/17444 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I59bb64cc85d5fca48ae8ec980f5cfc62e4b3a1d3 Gerrit-Change-Number: 17444 Gerrit-PatchSet: 17 Gerrit-Owner: Hongjiang Zhang Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hongjiang Zhang Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3223: management of per-table limit by kudu CLI
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/17444 to look at the new patch set (#16). Change subject: KUDU-3223: management of per-table limit by kudu CLI .. KUDU-3223: management of per-table limit by kudu CLI Use kudu CLI to set disk_size_limit and row_count_limit on table level. If '--enable_table_write_limit' is not set 'true' for kudu-master, kudu CLI fails to set the limit. The table's disk_size_limit and row_count_limit are 'N/A' when calling 'kudu table statistics'. Only when '--enable_table_write_limit' is set 'true', can the kudu CLI change the table limit. Use 'kudu table statistics' to check the updated result. Change-Id: I59bb64cc85d5fca48ae8ec980f5cfc62e4b3a1d3 --- M src/kudu/master/catalog_manager.cc M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_table.cc 3 files changed, 240 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/44/17444/16 -- To view, visit http://gerrit.cloudera.org:8080/17444 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I59bb64cc85d5fca48ae8ec980f5cfc62e4b3a1d3 Gerrit-Change-Number: 17444 Gerrit-PatchSet: 16 Gerrit-Owner: Hongjiang Zhang Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hongjiang Zhang Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-1884: Set sasl protocol name for the TxnSystemClient
Hello Attila Bukor, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/17480 to look at the new patch set (#2). Change subject: KUDU-1884: Set sasl_protocol_name for the TxnSystemClient .. KUDU-1884: Set sasl_protocol_name for the TxnSystemClient In clusters witha custom principal we need to set the sasl_protocol_name on the embeded client within the TxnSystemClient. This logic matches that in server_base.cc for server messenger configuration. Change-Id: Ic7b7c7fd154796215bd3a3ce748a2fd3155adebf --- M src/kudu/transactions/txn_system_client.cc 1 file changed, 8 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/80/17480/2 -- To view, visit http://gerrit.cloudera.org:8080/17480 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic7b7c7fd154796215bd3a3ce748a2fd3155adebf Gerrit-Change-Number: 17480 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-1884: Set sasl protocol name for the TxnSystemClient
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17480 Change subject: KUDU-1884: Set sasl_protocol_name for the TxnSystemClient .. KUDU-1884: Set sasl_protocol_name for the TxnSystemClient In clusters witha custom principal we need to set the sasl_protocol_name on the embeded client within the TxnSystemClient. This logic matches that in server_base.cc for server messenger configuration. Change-Id: Ic7b7c7fd154796215bd3a3ce748a2fd3155adebf --- M java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java M src/kudu/transactions/txn_system_client.cc 2 files changed, 32 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/80/17480/1 -- To view, visit http://gerrit.cloudera.org:8080/17480 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ic7b7c7fd154796215bd3a3ce748a2fd3155adebf Gerrit-Change-Number: 17480 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke
[kudu-CR] KUDU-3223: management of per-table limit by kudu CLI
Hongjiang Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/17444 ) Change subject: KUDU-3223: management of per-table limit by kudu CLI .. Patch Set 15: (5 comments) http://gerrit.cloudera.org:8080/#/c/17444/15//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/17444/15//COMMIT_MSG@11 PS15, Line 11: If the kudu-master does not set "-enable_table_write_limit=true" > How about: Ack http://gerrit.cloudera.org:8080/#/c/17444/15//COMMIT_MSG@12 PS15, Line 12: failed > nit: fails Ack http://gerrit.cloudera.org:8080/#/c/17444/15//COMMIT_MSG@12 PS15, Line 12: cli > nit: CLI Ack http://gerrit.cloudera.org:8080/#/c/17444/15//COMMIT_MSG@15 PS15, Line 15: cli > nit: CLI Ack http://gerrit.cloudera.org:8080/#/c/17444/15//COMMIT_MSG@15 PS15, Line 15: Only kudu-master sets "-enable_table_write_limit=true" > How about: Ack -- To view, visit http://gerrit.cloudera.org:8080/17444 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I59bb64cc85d5fca48ae8ec980f5cfc62e4b3a1d3 Gerrit-Change-Number: 17444 Gerrit-PatchSet: 15 Gerrit-Owner: Hongjiang Zhang Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hongjiang Zhang Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 21 May 2021 02:29:35 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3223: management of per-table limit by kudu CLI
Hongjiang Zhang has posted comments on this change. (
http://gerrit.cloudera.org:8080/17444 )
Change subject: KUDU-3223: management of per-table limit by kudu CLI
..
Patch Set 15:
(15 comments)
http://gerrit.cloudera.org:8080/#/c/17444/15/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/17444/15/src/kudu/master/catalog_manager.cc@2899
PS15, Line 2899: Alter
> nit: altering
Ack
http://gerrit.cloudera.org:8080/#/c/17444/15/src/kudu/master/catalog_manager.cc@2960
PS15, Line 2960: Modifying
> nit: Setting
Ack
http://gerrit.cloudera.org:8080/#/c/17444/15/src/kudu/master/catalog_manager.cc@2960
PS15, Line 2960: LOG(INFO)
> nit: if logging about setting a particular limit for a table disk size, why
Ack
http://gerrit.cloudera.org:8080/#/c/17444/15/src/kudu/master/catalog_manager.cc@2973
PS15, Line 2973: Modifying
> nit: Setting
Ack
http://gerrit.cloudera.org:8080/#/c/17444/15/src/kudu/master/catalog_manager.cc@2973
PS15, Line 2973: LOG(INFO)
> nit: if logging about setting a particular limit for a table row count limi
Ack
http://gerrit.cloudera.org:8080/#/c/17444/15/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/17444/15/src/kudu/tools/kudu-tool-test.cc@3975
PS15, Line 3975: KuduSchemaBuilder schema_builder;
: schema_builder.AddColumn("key")
: ->Type(client::KuduColumnSchema::INT32)
: ->NotNull()
: ->PrimaryKey();
: schema_builder.AddColumn("value")
: ->Type(client::KuduColumnSchema::INT32)
: ->NotNull();
: KuduSchema schema;
: ASSERT_OK(schema_builder.Build());
:
: // Create the table.
: TestWorkload workload(cluster_.get());
: workload.set_table_name(kTableName);
: workload.set_schema(schema);
: workload.set_num_replicas(1);
: workload.Setup();
> nit: To avoid extraneous test code, we don't need to define a schema since
Ack
http://gerrit.cloudera.org:8080/#/c/17444/15/src/kudu/tools/kudu-tool-test.cc@4043
PS15, Line 4043: KuduSchemaBuilder schema_builder;
: schema_builder.AddColumn("key")
: ->Type(client::KuduColumnSchema::INT32)
: ->NotNull()
: ->PrimaryKey();
: schema_builder.AddColumn("value")
: ->Type(client::KuduColumnSchema::INT32)
: ->NotNull();
: KuduSchema schema;
: ASSERT_OK(schema_builder.Build());
> nit: same as above, it's not necessary to define the schema for the test ta
Ack
http://gerrit.cloudera.org:8080/#/c/17444/15/src/kudu/tools/kudu-tool-test.cc@4086
PS15, Line 4086: master_addr, kTableName),
> Maybe, add the following test cases as well:
Ack
http://gerrit.cloudera.org:8080/#/c/17444/15/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:
http://gerrit.cloudera.org:8080/#/c/17444/15/src/kudu/tools/tool_action_table.cc@420
PS15, Line 420: disk_size_limit_str == "unlimited"
> nit here and below: maybe, compare this in a case-insensitive manner? Ther
Ack
http://gerrit.cloudera.org:8080/#/c/17444/15/src/kudu/tools/tool_action_table.cc@422
PS15, Line 422: safe_strtou64
> Here and below: why not to use safe_strto64() to read in a number as int64_
Ack
http://gerrit.cloudera.org:8080/#/c/17444/15/src/kudu/tools/tool_action_table.cc@426
PS15, Line 426: disk_size_limit = u_disk_size_limit
> Here and below: disk_size_limit might end up being negative if a number gre
Ack
http://gerrit.cloudera.org:8080/#/c/17444/15/src/kudu/tools/tool_action_table.cc@440
PS15, Line 440: row_count_limit_str == "unlimited"
> nit: maybe, compare this in a case-insensitive manner? There is iequals()
Ack
http://gerrit.cloudera.org:8080/#/c/17444/15/src/kudu/tools/tool_action_table.cc@1220
PS15, Line 1220: disk_size_limit
> Why not just 'disk_size' ? The upper context is already called 'set_limit'
Ack
http://gerrit.cloudera.org:8080/#/c/17444/15/src/kudu/tools/tool_action_table.cc@1227
PS15, Line 1227: row_count_limit
> Ditto: leave just 'row_count'?
Ack
http://gerrit.cloudera.org:8080/#/c/17444/15/src/kudu/tools/tool_action_table.cc@1234
PS15, Line 1234: of the table
> nit: for a table
Ack
--
To view, visit http://gerrit.cloudera.org:8080/17444
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I59bb64cc85d5fca48ae8ec980f5cfc62e4b3a1d3
Gerrit-Change-Number: 17444
Gerrit-PatchSet: 15
Gerrit-Owner: Hongjiang Zhang
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer:
[kudu-CR] [java] a property to show output while running a test
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17479 ) Change subject: [java] a property to show output while running a test .. Patch Set 2: > LGTM based on this documentation: > https://docs.gradle.org/current/dsl/org.gradle.api.tasks.testing.logging.TestLogging.html Thank you for review! -- To view, visit http://gerrit.cloudera.org:8080/17479 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I82590957004eef427a19f36a32e903012ea220d4 Gerrit-Change-Number: 17479 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 21 May 2021 01:58:42 + Gerrit-HasComments: No
[kudu-CR] [java] a property to show output while running a test
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17479 ) Change subject: [java] a property to show output while running a test .. [java] a property to show output while running a test While working on non-trivial and relatively long running test scenarios, I found it's inconvenient that the test output isn't shown while a test scenario is running. This patch introduces a new command-line property 'showTestOutput' to direct gradle outputting stdout and stderr from the JVM on the console, so it's possible to do the standard stream redirection and other stream manipulation in a command line shell. An example of running a particular test scenario with the output redirected to the console: ./gradlew -DshowTestOutput cleanTest :kudu-client:test --test ... By default the property isn't set, so this patch preserves the original behavior w.r.t. the output of running Java tests. Change-Id: I82590957004eef427a19f36a32e903012ea220d4 Reviewed-on: http://gerrit.cloudera.org:8080/17479 Tested-by: Kudu Jenkins Reviewed-by: Bankim Bhavsar Reviewed-by: Andrew Wong --- M java/gradle/tests.gradle 1 file changed, 4 insertions(+), 0 deletions(-) Approvals: Kudu Jenkins: Verified Bankim Bhavsar: Looks good to me, but someone else must approve Andrew Wong: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/17479 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I82590957004eef427a19f36a32e903012ea220d4 Gerrit-Change-Number: 17479 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [backup] KUDU-3183 Add tablePrefix option to the Kudu restore job Adding a couple of options of the Kudu Restore job:
Abhishek Chennaka has posted comments on this change. (
http://gerrit.cloudera.org:8080/17388 )
Change subject: [backup] KUDU-3183 Add tablePrefix option to the Kudu restore
job Adding a couple of options of the Kudu Restore job:
..
Patch Set 4:
(10 comments)
http://gerrit.cloudera.org:8080/#/c/17388/4//COMMIT_MSG
Commit Message:
PS4:
> Same nit feedback as those around the Options.
Will do
http://gerrit.cloudera.org:8080/#/c/17388/4//COMMIT_MSG@7
PS4, Line 7: [backup] KUDU-3183 Add tablePrefix option to the Kudu restore job
: Adding a couple of options of the Kudu Restore job:
> nit: put these on the same line
Will do
http://gerrit.cloudera.org:8080/#/c/17388/4//COMMIT_MSG@7
PS4, Line 7: [backup] KUDU-3183 Add tablePrefix option to the Kudu restore job
: Adding a couple of options of the Kudu Restore job:
> I thought the first line was a summary, and the second was the start of the
Ah thanks for pointing that out. Will update the commit message
http://gerrit.cloudera.org:8080/#/c/17388/4//COMMIT_MSG@11
PS4, Line 11: This will overwrite any existing
: database and if there is no existing database
> Kudu doesn't have a notion of a database of a standalone entity, so it's a
Right, Kudu doesn't have the notion of a database. But in here we are assuming
full table name consists of an optional database name and a table
name.(fulltablename=databasename.tablename). Maybe I can clarify that in the
commit message.
Having a new table suffix or a new database name added would create a new table
if the table is not present. In our case if we restore the table in the same
cluster then we will be having both olddb.table and newdb.table with identical
data. The backup/restore job will not affect the source tables.
http://gerrit.cloudera.org:8080/#/c/17388/4/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala
File java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala:
http://gerrit.cloudera.org:8080/#/c/17388/4/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala@57
PS4, Line 57: fullTableName.startsWith("impala::")
> nit: maybe store these in local variables; at least the ones that are reusa
Makes sense. Thanks for the review. Will update this in the next patch set
http://gerrit.cloudera.org:8080/#/c/17388/4/java/kudu-backup/src/main/scala/org/apache/kudu/backup/Options.scala
File java/kudu-backup/src/main/scala/org/apache/kudu/backup/Options.scala:
http://gerrit.cloudera.org:8080/#/c/17388/4/java/kudu-backup/src/main/scala/org/apache/kudu/backup/Options.scala@238
PS4, Line 238: "If set, replaces the existing database name and if
there is no existing database name, a new database name " +
> nit: maybe we should explicitly mention that if not set, or set to "", this
Will do
http://gerrit.cloudera.org:8080/#/c/17388/4/java/kudu-backup/src/main/scala/org/apache/kudu/backup/Options.scala@239
PS4, Line 239: For E
> nit: either "For example" or "E.g."
Noted
http://gerrit.cloudera.org:8080/#/c/17388/4/java/kudu-backup/src/main/scala/org/apache/kudu/backup/Options.scala@240
PS4, Line 240: y
> nit: add a comma
Noted
http://gerrit.cloudera.org:8080/#/c/17388/4/java/kudu-backup/src/main/scala/org/apache/kudu/backup/Options.scala@240
PS4, Line 240: be having
> nit: "will have"
Noted
http://gerrit.cloudera.org:8080/#/c/17388/4/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala
File
java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala:
http://gerrit.cloudera.org:8080/#/c/17388/4/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala@801
PS4, Line 801: val options = createRestoreOptions(Seq(tableName))
> Do we have any remaining test coverage for if the new database isn't set? P
Yea we do not. Did this based on the how the table suffix test cases are
implemented. Can reduce this down to one test case.
--
To view, visit http://gerrit.cloudera.org:8080/17388
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I65adcc1b3de0a8e1ac5b7f50a2d3a7036aa69421
Gerrit-Change-Number: 17388
Gerrit-PatchSet: 4
Gerrit-Owner: Abhishek Chennaka
Gerrit-Reviewer: Abhishek Chennaka
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 21 May 2021 00:55:25 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3223: management of per-table limit by kudu CLI
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17444 ) Change subject: KUDU-3223: management of per-table limit by kudu CLI .. Patch Set 15: (5 comments) http://gerrit.cloudera.org:8080/#/c/17444/15//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/17444/15//COMMIT_MSG@11 PS15, Line 11: If the kudu-master does not set "-enable_table_write_limit=true" How about: If '--enable_table_write_limit' is not set 'true' for kudu-master http://gerrit.cloudera.org:8080/#/c/17444/15//COMMIT_MSG@12 PS15, Line 12: cli nit: CLI http://gerrit.cloudera.org:8080/#/c/17444/15//COMMIT_MSG@12 PS15, Line 12: failed nit: fails http://gerrit.cloudera.org:8080/#/c/17444/15//COMMIT_MSG@15 PS15, Line 15: Only kudu-master sets "-enable_table_write_limit=true" How about: Only when '--enable_table_write_limit' is set 'true' http://gerrit.cloudera.org:8080/#/c/17444/15//COMMIT_MSG@15 PS15, Line 15: cli nit: CLI -- To view, visit http://gerrit.cloudera.org:8080/17444 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I59bb64cc85d5fca48ae8ec980f5cfc62e4b3a1d3 Gerrit-Change-Number: 17444 Gerrit-PatchSet: 15 Gerrit-Owner: Hongjiang Zhang Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hongjiang Zhang Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 21 May 2021 00:02:24 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3223: management of per-table limit by kudu CLI
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/17444 )
Change subject: KUDU-3223: management of per-table limit by kudu CLI
..
Patch Set 15: Code-Review+1
(14 comments)
Overall looks good! Just a request to add a few more test cases and do minor
changes in the names of CLI sub-commands.
http://gerrit.cloudera.org:8080/#/c/17444/15/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/17444/15/src/kudu/master/catalog_manager.cc@2899
PS15, Line 2899: Alter
nit: altering
http://gerrit.cloudera.org:8080/#/c/17444/15/src/kudu/master/catalog_manager.cc@2960
PS15, Line 2960: Modifying
nit: Setting
http://gerrit.cloudera.org:8080/#/c/17444/15/src/kudu/master/catalog_manager.cc@2960
PS15, Line 2960: LOG(INFO)
nit: if logging about setting a particular limit for a table disk size, why not
to log about resetting the limit to the default setting as well?
http://gerrit.cloudera.org:8080/#/c/17444/15/src/kudu/master/catalog_manager.cc@2973
PS15, Line 2973: Modifying
nit: Setting
http://gerrit.cloudera.org:8080/#/c/17444/15/src/kudu/master/catalog_manager.cc@2973
PS15, Line 2973: LOG(INFO)
nit: if logging about setting a particular limit for a table row count limit,
why not to log about resetting the limit to the default setting as well?
http://gerrit.cloudera.org:8080/#/c/17444/15/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/17444/15/src/kudu/tools/kudu-tool-test.cc@4043
PS15, Line 4043: KuduSchemaBuilder schema_builder;
: schema_builder.AddColumn("key")
: ->Type(client::KuduColumnSchema::INT32)
: ->NotNull()
: ->PrimaryKey();
: schema_builder.AddColumn("value")
: ->Type(client::KuduColumnSchema::INT32)
: ->NotNull();
: KuduSchema schema;
: ASSERT_OK(schema_builder.Build());
nit: same as above, it's not necessary to define the schema for the test table
since TestWorkload has one by default.
http://gerrit.cloudera.org:8080/#/c/17444/15/src/kudu/tools/kudu-tool-test.cc@4086
PS15, Line 4086: master_addr, kTableName),
Maybe, add the following test cases as well:
* a negative value for the limit
* the value greater than INT64_MAX
http://gerrit.cloudera.org:8080/#/c/17444/15/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:
http://gerrit.cloudera.org:8080/#/c/17444/15/src/kudu/tools/tool_action_table.cc@420
PS15, Line 420: disk_size_limit_str == "unlimited"
nit here and below: maybe, compare this in a case-insensitive manner? There is
iequals() function for that (see src/kudu/utils/string_case.h).
http://gerrit.cloudera.org:8080/#/c/17444/15/src/kudu/tools/tool_action_table.cc@422
PS15, Line 422: safe_strtou64
Here and below: why not to use safe_strto64() to read in a number as int64_t?
http://gerrit.cloudera.org:8080/#/c/17444/15/src/kudu/tools/tool_action_table.cc@426
PS15, Line 426: disk_size_limit = u_disk_size_limit
Here and below: disk_size_limit might end up being negative if a number greater
than INT64_MAX is specified. Maybe, read in the input above using
safe_strto64() instead?
http://gerrit.cloudera.org:8080/#/c/17444/15/src/kudu/tools/tool_action_table.cc@440
PS15, Line 440: row_count_limit_str == "unlimited"
nit: maybe, compare this in a case-insensitive manner? There is iequals()
function for that (see src/kudu/utils/string_case.h).
http://gerrit.cloudera.org:8080/#/c/17444/15/src/kudu/tools/tool_action_table.cc@1220
PS15, Line 1220: disk_size_limit
Why not just 'disk_size' ? The upper context is already called 'set_limit', so
duplicating 'limit' isn't necessary.
http://gerrit.cloudera.org:8080/#/c/17444/15/src/kudu/tools/tool_action_table.cc@1227
PS15, Line 1227: row_count_limit
Ditto: leave just 'row_count'?
http://gerrit.cloudera.org:8080/#/c/17444/15/src/kudu/tools/tool_action_table.cc@1234
PS15, Line 1234: of the table
nit: for a table
--
To view, visit http://gerrit.cloudera.org:8080/17444
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I59bb64cc85d5fca48ae8ec980f5cfc62e4b3a1d3
Gerrit-Change-Number: 17444
Gerrit-PatchSet: 15
Gerrit-Owner: Hongjiang Zhang
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Hongjiang Zhang
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 20 May 2021 23:37:18 +
Gerrit-HasComments: Yes
[kudu-CR] [backup] KUDU-3183 Add tablePrefix option to the Kudu restore job Adding a couple of options of the Kudu Restore job:
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17388 ) Change subject: [backup] KUDU-3183 Add tablePrefix option to the Kudu restore job Adding a couple of options of the Kudu Restore job: .. Patch Set 4: (2 comments) http://gerrit.cloudera.org:8080/#/c/17388/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/17388/4//COMMIT_MSG@7 PS4, Line 7: [backup] KUDU-3183 Add tablePrefix option to the Kudu restore job : Adding a couple of options of the Kudu Restore job: > nit: put these on the same line I thought the first line was a summary, and the second was the start of the description. We have best practices document linked from https://kudu.apache.org/docs/contributing.html#_submitting_patches : https://git-scm.com/book/en/v2/Distributed-Git-Contributing-to-a-Project#_commit_guidelines http://gerrit.cloudera.org:8080/#/c/17388/4//COMMIT_MSG@11 PS4, Line 11: This will overwrite any existing : database and if there is no existing database Kudu doesn't have a notion of a database of a standalone entity, so it's a bit strange to read 'existing database'. Also, from this description it's not quite clear what happens with the table olddb.table when --newDatabaseName=newdb: is the olddb.table will be dropped or it will stay along with the newly created newdb.table created by the restore job? -- To view, visit http://gerrit.cloudera.org:8080/17388 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I65adcc1b3de0a8e1ac5b7f50a2d3a7036aa69421 Gerrit-Change-Number: 17388 Gerrit-PatchSet: 4 Gerrit-Owner: Abhishek Chennaka Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 20 May 2021 23:02:27 + Gerrit-HasComments: Yes
[kudu-CR] [java] a property to show output while running a test
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17479 ) Change subject: [java] a property to show output while running a test .. Patch Set 2: Code-Review+2 LGTM based on this documentation: https://docs.gradle.org/current/dsl/org.gradle.api.tasks.testing.logging.TestLogging.html -- To view, visit http://gerrit.cloudera.org:8080/17479 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I82590957004eef427a19f36a32e903012ea220d4 Gerrit-Change-Number: 17479 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 20 May 2021 21:51:34 + Gerrit-HasComments: No
[kudu-CR] KUDU-3223: management of per-table limit by kudu CLI
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17444 ) Change subject: KUDU-3223: management of per-table limit by kudu CLI .. Patch Set 15: Code-Review+2 I'd actually be OK merging this as is, if there is no other feedback. Would be nice to get this into the 1.15 branch if we can. -- To view, visit http://gerrit.cloudera.org:8080/17444 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I59bb64cc85d5fca48ae8ec980f5cfc62e4b3a1d3 Gerrit-Change-Number: 17444 Gerrit-PatchSet: 15 Gerrit-Owner: Hongjiang Zhang Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hongjiang Zhang Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 20 May 2021 21:26:25 + Gerrit-HasComments: No
[kudu-CR] KUDU-3223: management of per-table limit by kudu CLI
Andrew Wong has posted comments on this change. (
http://gerrit.cloudera.org:8080/17444 )
Change subject: KUDU-3223: management of per-table limit by kudu CLI
..
Patch Set 15: Code-Review+1
(2 comments)
LGTM, minus one small testing nit.
http://gerrit.cloudera.org:8080/#/c/17444/15/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/17444/15/src/kudu/tools/kudu-tool-test.cc@3975
PS15, Line 3975: KuduSchemaBuilder schema_builder;
: schema_builder.AddColumn("key")
: ->Type(client::KuduColumnSchema::INT32)
: ->NotNull()
: ->PrimaryKey();
: schema_builder.AddColumn("value")
: ->Type(client::KuduColumnSchema::INT32)
: ->NotNull();
: KuduSchema schema;
: ASSERT_OK(schema_builder.Build());
:
: // Create the table.
: TestWorkload workload(cluster_.get());
: workload.set_table_name(kTableName);
: workload.set_schema(schema);
: workload.set_num_replicas(1);
: workload.Setup();
nit: To avoid extraneous test code, we don't need to define a schema since the
TestWorkload has one by default. Some other tests do since their test coverage
relies on having specific schemas and columns, but these new tests don't.
http://gerrit.cloudera.org:8080/#/c/17444/7/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:
http://gerrit.cloudera.org:8080/#/c/17444/7/src/kudu/tools/tool_action_table.cc@1210
PS7, Line 1210: table_creator->dimension_label(table_req.dimension_label());
: }
: return table_creator->Create();
: }
:
:
: } // anonymous
> BTW, what about using variadic arguments to read in the values? Also, it's
Nice suggestion. This approach is much more elegant indeed.
--
To view, visit http://gerrit.cloudera.org:8080/17444
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I59bb64cc85d5fca48ae8ec980f5cfc62e4b3a1d3
Gerrit-Change-Number: 17444
Gerrit-PatchSet: 15
Gerrit-Owner: Hongjiang Zhang
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Hongjiang Zhang
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 20 May 2021 21:21:02 +
Gerrit-HasComments: Yes
[kudu-CR] [backup] KUDU-3183 Add tablePrefix option to the Kudu restore job Adding a couple of options of the Kudu Restore job:
Andrew Wong has posted comments on this change. (
http://gerrit.cloudera.org:8080/17388 )
Change subject: [backup] KUDU-3183 Add tablePrefix option to the Kudu restore
job Adding a couple of options of the Kudu Restore job:
..
Patch Set 4:
(8 comments)
Thanks for updating the behavior!
http://gerrit.cloudera.org:8080/#/c/17388/4//COMMIT_MSG
Commit Message:
PS4:
Same nit feedback as those around the Options.
http://gerrit.cloudera.org:8080/#/c/17388/4//COMMIT_MSG@7
PS4, Line 7: [backup] KUDU-3183 Add tablePrefix option to the Kudu restore job
: Adding a couple of options of the Kudu Restore job:
nit: put these on the same line
http://gerrit.cloudera.org:8080/#/c/17388/4/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala
File java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala:
http://gerrit.cloudera.org:8080/#/c/17388/4/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala@57
PS4, Line 57: fullTableName.startsWith("impala::")
nit: maybe store these in local variables; at least the ones that are reusable.
Then we can simplify the logic a bit to something like
E.g.
var databaseName = ""
var impalaPrefixLen = fullTableName.startsWith("impala::") ? 8 : 0
var indexOfPeriod = fullTableName.indexOf('.') // returns -1 if doesn't it
exist; we can also use (indexOfPeriod == -1) instead of String.contains('.')
if (indexOfPeriod == -1) {
databaseName = fullTableName.substr(impalaPrefixLen, indexOfPeriod)
onlyTableName = fullTableName.substr(indexOfPeriod + 1)
} else {
onlyTableName = fullTableName.substr(impalaPrefixLen)
}
// use the above variables to build the right restore name
http://gerrit.cloudera.org:8080/#/c/17388/4/java/kudu-backup/src/main/scala/org/apache/kudu/backup/Options.scala
File java/kudu-backup/src/main/scala/org/apache/kudu/backup/Options.scala:
http://gerrit.cloudera.org:8080/#/c/17388/4/java/kudu-backup/src/main/scala/org/apache/kudu/backup/Options.scala@238
PS4, Line 238: "If set, replaces the existing database name and if
there is no existing database name, a new database name " +
nit: maybe we should explicitly mention that if not set, or set to "", this
doesn't change the database name? In case users think it removes the db name.
http://gerrit.cloudera.org:8080/#/c/17388/4/java/kudu-backup/src/main/scala/org/apache/kudu/backup/Options.scala@239
PS4, Line 239: For E
nit: either "For example" or "E.g."
http://gerrit.cloudera.org:8080/#/c/17388/4/java/kudu-backup/src/main/scala/org/apache/kudu/backup/Options.scala@240
PS4, Line 240: y
nit: add a comma
http://gerrit.cloudera.org:8080/#/c/17388/4/java/kudu-backup/src/main/scala/org/apache/kudu/backup/Options.scala@240
PS4, Line 240: be having
nit: "will have"
http://gerrit.cloudera.org:8080/#/c/17388/4/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala
File
java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala:
http://gerrit.cloudera.org:8080/#/c/17388/4/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala@801
PS4, Line 801: val options = createRestoreOptions(Seq(tableName))
Do we have any remaining test coverage for if the new database isn't set?
Perhaps it's worth leaving the default empty and adding an explicit test for
both adding and changing the database name?
--
To view, visit http://gerrit.cloudera.org:8080/17388
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I65adcc1b3de0a8e1ac5b7f50a2d3a7036aa69421
Gerrit-Change-Number: 17388
Gerrit-PatchSet: 4
Gerrit-Owner: Abhishek Chennaka
Gerrit-Reviewer: Abhishek Chennaka
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 20 May 2021 21:00:22 +
Gerrit-HasComments: Yes
[kudu-CR] [java] a property to show output while running a test
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/17479 ) Change subject: [java] a property to show output while running a test .. Patch Set 2: Code-Review+1 This looks very useful. +1 because I'm not familiar with gradle and I'm not sure if there is a better way... -- To view, visit http://gerrit.cloudera.org:8080/17479 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I82590957004eef427a19f36a32e903012ea220d4 Gerrit-Change-Number: 17479 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 20 May 2021 20:09:18 + Gerrit-HasComments: No
[kudu-CR] [java] a property to show output while running a test
Hello Kudu Jenkins, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/17479 to look at the new patch set (#2). Change subject: [java] a property to show output while running a test .. [java] a property to show output while running a test While working on non-trivial and relatively long running test scenarios, I found it's inconvenient that the test output isn't shown while a test scenario is running. This patch introduces a new command-line property 'showTestOutput' to direct gradle outputting stdout and stderr from the JVM on the console, so it's possible to do the standard stream redirection and other stream manipulation in a command line shell. An example of running a particular test scenario with the output redirected to the console: ./gradlew -DshowTestOutput cleanTest :kudu-client:test --test ... By default the property isn't set, so this patch preserves the original behavior w.r.t. the output of running Java tests. Change-Id: I82590957004eef427a19f36a32e903012ea220d4 --- M java/gradle/tests.gradle 1 file changed, 4 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/79/17479/2 -- To view, visit http://gerrit.cloudera.org:8080/17479 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I82590957004eef427a19f36a32e903012ea220d4 Gerrit-Change-Number: 17479 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [java] a property to show output while running a test
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17479 Change subject: [java] a property to show output while running a test .. [java] a property to show output while running a test While working on non-trivial and relatively long running test scenarios, I found it's inconvenient that the test output isn't shown while a test scenario is running. This patch introduces a new command-line property 'testShowOutput' to direct gradle outputting stdout and stderr from the JVM on the console, so it's possible to do the standard stream redirection and other stream manipulation in a command line shell. An example of running a particular test scenario with the output redirected to the console: ./gradlew -DtestShowOutput cleanTest :kudu-client:test --test ... By default the property isn't set, so this patch preserves the original behavior w.r.t. the output of running Java tests. Change-Id: I82590957004eef427a19f36a32e903012ea220d4 --- M java/gradle/tests.gradle 1 file changed, 4 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/79/17479/1 -- To view, visit http://gerrit.cloudera.org:8080/17479 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I82590957004eef427a19f36a32e903012ea220d4 Gerrit-Change-Number: 17479 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin
