[kudu-CR] KUDU-1861: add range-partitions to loadgen tables
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10633 ) Change subject: KUDU-1861: add range-partitions to loadgen tables .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/10633/4/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/10633/4/src/kudu/tools/kudu-tool-test.cc@1584 PS4, Line 1584: // Since this indicator for non-sequential inserts is based on the random > How strong of a guarantee is this? Since it's a random workload, what are t Done, the new revision passed 1000/1000 times -- To view, visit http://gerrit.cloudera.org:8080/10633 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If4f552a4c73dc82f3b7934082769522557ee5013 Gerrit-Change-Number: 10633 Gerrit-PatchSet: 5 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Tue, 12 Jun 2018 06:12:51 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-1861: add range-partitions to loadgen tables
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10633 to look at the new patch set (#5). Change subject: KUDU-1861: add range-partitions to loadgen tables .. KUDU-1861: add range-partitions to loadgen tables This patch adds the ability to generate a range-partitioned table with the loadgen tool. The range partitioning schema is designed such that the non-random write workload will insert sequentially on the primary key, provided the number of threads is equal to the number of tablets. This sequential workload per tablet both reduces the number of compactions and avoids bloom filter lookups. This patch also renames --table_num_buckets to --table_num_hash_partitions, which can be combined with --table_num_range_partitions if desired. I tested this out on a singler-tserver cluster and verified via the metrics logs that the number of bloom lookups for a non-random workload where the number of insert threads and the number of tablets were equal stayed at zero. When the number of threads was not a factor of the number of buckets, the number of bloom lookups was non-zero. See TestNonRandomWorkloadLoadgen in kudu-tool-test.cc for more details. Change-Id: If4f552a4c73dc82f3b7934082769522557ee5013 --- M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_perf.cc 2 files changed, 234 insertions(+), 14 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/33/10633/5 -- To view, visit http://gerrit.cloudera.org:8080/10633 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If4f552a4c73dc82f3b7934082769522557ee5013 Gerrit-Change-Number: 10633 Gerrit-PatchSet: 5 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot
[kudu-CR] parse metrics log: update to the new format
Andrew Wong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10693 Change subject: parse_metrics_log: update to the new format .. parse_metrics_log: update to the new format The new diagnostics logs report more than just metrics, and thus, output a bit differently than they did in simpler times. This patch updates the parsing to be compatible with the new logs. Change-Id: If11b7ecc93a3f64db3b7c1994f47308b3ec44029 --- M src/kudu/scripts/parse_metrics_log.py 1 file changed, 7 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/93/10693/1 -- To view, visit http://gerrit.cloudera.org:8080/10693 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: If11b7ecc93a3f64db3b7c1994f47308b3ec44029 Gerrit-Change-Number: 10693 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong
[kudu-CR] scripts: tool to plot metrics
Andrew Wong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10695 Change subject: scripts: tool to plot metrics .. scripts: tool to plot metrics This patch adds a tool that facilitates plotting metrics via the existing metrics-log-parsing script. Change-Id: Ia39b23331bb105a0eb1782be5acd7450df43db3c --- A src/kudu/scripts/plot_metrics.py 1 file changed, 193 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/95/10695/1 -- To view, visit http://gerrit.cloudera.org:8080/10695 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ia39b23331bb105a0eb1782be5acd7450df43db3c Gerrit-Change-Number: 10695 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong
[kudu-CR] parse metrics log: minor refactoring
Andrew Wong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10694 Change subject: parse_metrics_log: minor refactoring .. parse_metrics_log: minor refactoring This patch updates the parse_metrics_log.py to be more usable by other programs. Currently, it is an application that spits out parsed metrics as a TSV. This patch updates this so the metrics can be returned as an iterable. This patch contains no functional changes. Change-Id: I8cbf1a680feb1b850a5606883659d271010a9db5 --- M src/kudu/scripts/parse_metrics_log.py 1 file changed, 78 insertions(+), 59 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/94/10694/1 -- To view, visit http://gerrit.cloudera.org:8080/10694 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I8cbf1a680feb1b850a5606883659d271010a9db5 Gerrit-Change-Number: 10694 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong
[kudu-CR] KUDU-2191: HMS Metadata Consistency Check Tool
Hello Tidy Bot, Dan Burkert, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10217 to look at the new patch set (#9). Change subject: KUDU-2191: HMS Metadata Consistency Check Tool .. KUDU-2191: HMS Metadata Consistency Check Tool This commit introduces a metadata consistency check CLI tool for Hive Metastore integration. It checks metadata (table ID, table name, Kudu master addresses, and colnum name and type) between Kudu and HMS, and report inconsistent metadata to the user if any. It adds test case using external mini cluster to ensure the tool works as expected. Change-Id: I3495e9c755571b85beaecca849f83457b592ee90 --- M src/kudu/hms/hms_catalog.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_hms.cc 3 files changed, 367 insertions(+), 40 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/10217/9 -- To view, visit http://gerrit.cloudera.org:8080/10217 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3495e9c755571b85beaecca849f83457b592ee90 Gerrit-Change-Number: 10217 Gerrit-PatchSet: 9 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot
[kudu-CR] KUDU-2191: Add a method to decide the scope of AlterTable
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/10581 ) Change subject: KUDU-2191: Add a method to decide the scope of AlterTable .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10581 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I703f12b95bb7ca2d65455f0f0602520332b3c678 Gerrit-Change-Number: 10581 Gerrit-PatchSet: 6 Gerrit-Owner: Hao Hao Gerrit-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: Mon, 11 Jun 2018 23:29:54 + Gerrit-HasComments: No
[kudu-CR] KUDU-2191: allow metadata upgrade tool to run with HMS integration
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10582 ) Change subject: KUDU-2191: allow metadata upgrade tool to run with HMS integration .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10582 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I51d4cabeb1a9defc51f4e307a116419da5588f2d Gerrit-Change-Number: 10582 Gerrit-PatchSet: 5 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 11 Jun 2018 23:21:51 + Gerrit-HasComments: No
[kudu-CR] KUDU-2191: Add a method to decide the scope of AlterTable
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10581 ) Change subject: KUDU-2191: Add a method to decide the scope of AlterTable .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10581 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I703f12b95bb7ca2d65455f0f0602520332b3c678 Gerrit-Change-Number: 10581 Gerrit-PatchSet: 6 Gerrit-Owner: Hao Hao Gerrit-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: Mon, 11 Jun 2018 23:21:15 + Gerrit-HasComments: No
[kudu-CR] KUDU-2191: Add a method to decide the scope of AlterTable
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/10581 )
Change subject: KUDU-2191: Add a method to decide the scope of AlterTable
..
Patch Set 6:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/10581/5/src/kudu/client/client.cc
File src/kudu/client/client.cc:
http://gerrit.cloudera.org:8080/#/c/10581/5/src/kudu/client/client.cc@1144
PS5, Line 1144: bool alter_external_catalogs) {
> This doesn't do the right thing if you call system(true) followed by system
Done
http://gerrit.cloudera.org:8080/#/c/10581/3/src/kudu/master/master.proto
File src/kudu/master/master.proto:
http://gerrit.cloudera.org:8080/#/c/10581/3/src/kudu/master/master.proto@568
PS3, Line 568: // Whether to apply the alteration to external catalogs, such
as the Hive Metastore,
> Sorry I'm late to this, but I don't agree with this change. 'NO_HMS' isn't
Done
--
To view, visit http://gerrit.cloudera.org:8080/10581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I703f12b95bb7ca2d65455f0f0602520332b3c678
Gerrit-Change-Number: 10581
Gerrit-PatchSet: 6
Gerrit-Owner: Hao Hao
Gerrit-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: Mon, 11 Jun 2018 22:54:09 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2191: Add a method to decide the scope of AlterTable
Hello Tidy Bot, Dan Burkert, Kudu Jenkins, Adar Dembo, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10581 to look at the new patch set (#6). Change subject: KUDU-2191: Add a method to decide the scope of AlterTable .. KUDU-2191: Add a method to decide the scope of AlterTable This commit adds a private method in KuduTableAlterer to indicate whether the table alteration should be applied to external catalogs, such as the Hive Metastore, which the Kudu master may has been configured to integrate with. This method will be useful to the HMS integration tools, such as the metadata upgrade tool which may need to alter tables in Kudu only. Change-Id: I703f12b95bb7ca2d65455f0f0602520332b3c678 --- M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/table_alterer-internal.cc M src/kudu/client/table_alterer-internal.h M src/kudu/integration-tests/master_hms-itest.cc M src/kudu/master/catalog_manager.cc M src/kudu/master/master.proto 7 files changed, 41 insertions(+), 8 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/81/10581/6 -- To view, visit http://gerrit.cloudera.org:8080/10581 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I703f12b95bb7ca2d65455f0f0602520332b3c678 Gerrit-Change-Number: 10581 Gerrit-PatchSet: 6 Gerrit-Owner: Hao Hao Gerrit-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-2191: allow metadata upgrade tool to run with HMS integration
Hello Dan Burkert, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10582 to look at the new patch set (#5). Change subject: KUDU-2191: allow metadata upgrade tool to run with HMS integration .. KUDU-2191: allow metadata upgrade tool to run with HMS integration Previously the metadata upgrade tool requires the HMS integration feature being disabled. This commit updates the tool to only alter tables in Kudu but not in the HMS during upgrade, so that the restriction no longer applies. Change-Id: I51d4cabeb1a9defc51f4e307a116419da5588f2d --- M src/kudu/client/client.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_hms.cc 3 files changed, 32 insertions(+), 10 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/82/10582/5 -- To view, visit http://gerrit.cloudera.org:8080/10582 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I51d4cabeb1a9defc51f4e307a116419da5588f2d Gerrit-Change-Number: 10582 Gerrit-PatchSet: 5 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Fix ksck checksum scan printing
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/10535 ) Change subject: Fix ksck checksum_scan printing .. Patch Set 5: (3 comments) http://gerrit.cloudera.org:8080/#/c/10535/5/src/kudu/tools/ksck.cc File src/kudu/tools/ksck.cc: http://gerrit.cloudera.org:8080/#/c/10535/5/src/kudu/tools/ksck.cc@592 PS5, Line 592: if (FLAGS_ksck_format == "plain_full" || FLAGS_ksck_format == "plain_concise") > Why is it machine-readable btw? My bad. By "this test" I meant the opposite of this test :). Machine-readable means JSON format. So the test should compare for equality against JSON_PRETTY and JSON_COMPACT. http://gerrit.cloudera.org:8080/#/c/10535/6/src/kudu/tools/ksck.cc File src/kudu/tools/ksck.cc: http://gerrit.cloudera.org:8080/#/c/10535/6/src/kudu/tools/ksck.cc@613 PS6, Line 613: I think you mean if it's not printing in a machine-readable format. http://gerrit.cloudera.org:8080/#/c/10535/6/src/kudu/tools/ksck.cc@1063 PS6, Line 1063: Also, these are the non-machine-readable format flags. -- To view, visit http://gerrit.cloudera.org:8080/10535 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I25fe6d0f14d1713b848d0a79f4d92b056924a5a5 Gerrit-Change-Number: 10535 Gerrit-PatchSet: 5 Gerrit-Owner: Fengling Wang Gerrit-Reviewer: Fengling Wang Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 11 Jun 2018 22:27:48 + Gerrit-HasComments: Yes
[kudu-CR] Fix ksck checksum scan printing
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/10535 ) Change subject: Fix ksck checksum_scan printing .. Patch Set 6: (8 comments) http://gerrit.cloudera.org:8080/#/c/10535/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10535/6//COMMIT_MSG@15 PS6, Line 15: kudu cluster ksck --checksum_scan --checksum_snapshot=false : --ksck_format=json_compact localhost:7052,localhost:7051, : localhost:7053 nit: Use \ to indicate the line breaks, so the command remains vaild and hence copy-paste-able to a shell. E.g. kudu cluster ksck --checksum_scan --checksum_snapshot=false \ --ksck_format=json_compact localhost:7052,localhost:7051,localhost:7053 http://gerrit.cloudera.org:8080/#/c/10535/6/src/kudu/tools/ksck-test.cc File src/kudu/tools/ksck-test.cc: http://gerrit.cloudera.org:8080/#/c/10535/6/src/kudu/tools/ksck-test.cc@56 PS6, Line 56: DECLARE_string(ksck_format); nit: Order alphabetically by flag name. http://gerrit.cloudera.org:8080/#/c/10535/6/src/kudu/tools/ksck-test.cc@1503 PS6, Line 1503: ASSERT_OK(r.Init()); Can you double-check this failed without this patch? I think JsonReader parses the whole json on Init() but want to be sure. http://gerrit.cloudera.org:8080/#/c/10535/6/src/kudu/tools/ksck.h File src/kudu/tools/ksck.h: http://gerrit.cloudera.org:8080/#/c/10535/6/src/kudu/tools/ksck.h@567 PS6, Line 567: (non-JSON) nit: Remove. Also, JSON is machine-readable. http://gerrit.cloudera.org:8080/#/c/10535/6/src/kudu/tools/ksck.h@567 PS6, Line 567: Check if nit: Return whether http://gerrit.cloudera.org:8080/#/c/10535/6/src/kudu/tools/ksck.h@567 PS6, Line 567: are is http://gerrit.cloudera.org:8080/#/c/10535/6/src/kudu/tools/ksck.h@568 PS6, Line 568: static bool IsMachineReadableFormat Can you make this a helper function in an anonymous namespace in ksck.cc rather that a static member function of Ksck? http://gerrit.cloudera.org:8080/#/c/10535/6/src/kudu/tools/ksck.cc File src/kudu/tools/ksck.cc: http://gerrit.cloudera.org:8080/#/c/10535/6/src/kudu/tools/ksck.cc@1063 PS6, Line 1063: FLAGS_ksck_format == "plain_full" || FLAGS_ksck_format == "plain_concise" We are usually case-insensitive about categorical flag values like this. Can you use boost::iequals to do the comparisons instead? -- To view, visit http://gerrit.cloudera.org:8080/10535 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I25fe6d0f14d1713b848d0a79f4d92b056924a5a5 Gerrit-Change-Number: 10535 Gerrit-PatchSet: 6 Gerrit-Owner: Fengling Wang Gerrit-Reviewer: Fengling Wang Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 11 Jun 2018 22:24:11 + Gerrit-HasComments: Yes
[kudu-CR] [tools] add 'kudu tablet set attributes' sub-command
Will Berkeley has uploaded a new patch set (#4) to the change originally created by Alexey Serbin. ( http://gerrit.cloudera.org:8080/10536 ) Change subject: [tools] add 'kudu tablet set_attributes' sub-command .. [tools] add 'kudu tablet set_attributes' sub-command Introduced 'kudu tablet set_attributes' sub-command for the kudu CLI tool. With this tool, it is possible to set/reset the 'promote' and 'replace' attributes in the replica's Raft configuration. This functionality might be useful for pre-KUDU-2443 clusters running the 3-4-3 replica management scheme after an attempt to move a replica of a tablet with replication factor of 1. In that case, the system would add a non-voter replica and then evict it, doing that over and over again (that's what KUDU-2443 is about). Aside from the hapless use-case described above, this functionality might be useful when removing the 'replace' attribute set on a replica by the 'kudu tablet change_config move_replica' sub-command before the move has completed. Change-Id: Ib304715100ba9062558863f140aa309fd604ace3 --- M src/kudu/tools/kudu-admin-test.cc M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_tablet.cc 3 files changed, 201 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/10536/4 -- To view, visit http://gerrit.cloudera.org:8080/10536 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib304715100ba9062558863f140aa309fd604ace3 Gerrit-Change-Number: 10536 Gerrit-PatchSet: 4 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley
[kudu-CR] [java] Add equals and hashcode to Schema
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/10574 ) Change subject: [java] Add equals and hashcode to Schema .. Patch Set 4: Related: https://issues.apache.org/jira/browse/KUDU-2471 -- To view, visit http://gerrit.cloudera.org:8080/10574 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0a6914b9b25df9e327ed6ae6bd61eac29c9c524c Gerrit-Change-Number: 10574 Gerrit-PatchSet: 4 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 11 Jun 2018 21:50:37 + Gerrit-HasComments: No
[kudu-CR] [tools] add 'kudu tablet set attributes' sub-command
Will Berkeley has posted comments on this change. (
http://gerrit.cloudera.org:8080/10536 )
Change subject: [tools] add 'kudu tablet set_attributes' sub-command
..
Patch Set 3:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/10536/3/src/kudu/tools/tool_action_tablet.cc
File src/kudu/tools/tool_action_tablet.cc:
http://gerrit.cloudera.org:8080/#/c/10536/3/src/kudu/tools/tool_action_tablet.cc@66
PS3, Line 66: using kudu::consensus::ConsensusStatePB;
> warning: using decl 'ConsensusStatePB' is unused [misc-unused-using-decls]
Done
http://gerrit.cloudera.org:8080/#/c/10536/3/src/kudu/tools/tool_action_tablet.cc@67
PS3, Line 67: using kudu::consensus::GetConsensusStateRequestPB;
> warning: using decl 'GetConsensusStateRequestPB' is unused [misc-unused-usi
Done
http://gerrit.cloudera.org:8080/#/c/10536/3/src/kudu/tools/tool_action_tablet.cc@68
PS3, Line 68: using kudu::consensus::GetConsensusStateResponsePB;
> warning: using decl 'GetConsensusStateResponsePB' is unused [misc-unused-us
Done
http://gerrit.cloudera.org:8080/#/c/10536/3/src/kudu/tools/tool_action_tablet.cc@254
PS3, Line 254: return Status::InvalidArgument("cannot set both PROMOTE and
REPLACE");
> I agree it doesn't really make sense but what's the risk if we don't do thi
You and Alexey would have a better idea than me. As I recall when I asked
Alexey about it, I think he said the replace "wins" when set on a voter, so in
that case it's equivalent to just setting replace.
I think it's a good sanity check since a user shouldn't ever intentionally set
both, regardless of how risky it is.
http://gerrit.cloudera.org:8080/#/c/10536/3/src/kudu/tools/tool_action_tablet.cc@273
PS3, Line 273: change->mutable_peer()->mutable_attrs()->set_promote(promote);
> We may likely add additional attributes in the future, so I think if you do
Done
--
To view, visit http://gerrit.cloudera.org:8080/10536
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib304715100ba9062558863f140aa309fd604ace3
Gerrit-Change-Number: 10536
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley
Gerrit-Comment-Date: Mon, 11 Jun 2018 21:48:39 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2191: Add a method to decide the scope of AlterTable
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10581 ) Change subject: KUDU-2191: Add a method to decide the scope of AlterTable .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/10581/3/src/kudu/master/master.proto File src/kudu/master/master.proto: http://gerrit.cloudera.org:8080/#/c/10581/3/src/kudu/master/master.proto@568 PS3, Line 568: // Whether to alter the table in Kudu as well as in the Hive Metastore, > Sorry I'm late to this, but I don't agree with this change. 'NO_HMS' isn't I'm OK with Dan's suggestion too. -- To view, visit http://gerrit.cloudera.org:8080/10581 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I703f12b95bb7ca2d65455f0f0602520332b3c678 Gerrit-Change-Number: 10581 Gerrit-PatchSet: 5 Gerrit-Owner: Hao Hao Gerrit-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: Mon, 11 Jun 2018 21:20:31 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2191: Add a method to decide the scope of AlterTable
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/10581 ) Change subject: KUDU-2191: Add a method to decide the scope of AlterTable .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/10581/3/src/kudu/master/master.proto File src/kudu/master/master.proto: http://gerrit.cloudera.org:8080/#/c/10581/3/src/kudu/master/master.proto@568 PS3, Line 568: // Whether to alter the table in Kudu as well as in the Hive Metastore, > Done Sorry I'm late to this, but I don't agree with this change. 'NO_HMS' isn't a system type, it's the lack of a type. 'ExternalSystem' is too general, as well. I'd prefer // Whether to apply the alteration to external catalogs, such as the Hive Metastore, // which the Kudu master has been configured to integrate with. optional bool alter_external_catalogs = 5 [default = true]; As far as enum vs bool, I don't think the enum is any more future proof than a boolean. It's impossible to know if/when we integrate with another external catalog system whether the enum will be useful vs just a boolean. I do see value in making it slightly less HMS specific, though. -- To view, visit http://gerrit.cloudera.org:8080/10581 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I703f12b95bb7ca2d65455f0f0602520332b3c678 Gerrit-Change-Number: 10581 Gerrit-PatchSet: 5 Gerrit-Owner: Hao Hao Gerrit-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: Mon, 11 Jun 2018 21:11:25 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2470: retry failed thirdparty downloads
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10684 ) Change subject: KUDU-2470: retry failed thirdparty downloads .. KUDU-2470: retry failed thirdparty downloads Currently we re-download when we fail to open thirdparty archives that fail to read. We don't currently re-attempt to download if the download itself fails. This patch adds the behavior to download failures, using curl's built-in --retry option that exponentially backs off on failure. Change-Id: Ia57dd9640d732a75659ca11763d89aefdaf16be1 Reviewed-on: http://gerrit.cloudera.org:8080/10684 Reviewed-by: Adar Dembo Tested-by: Kudu Jenkins --- M thirdparty/download-thirdparty.sh 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/10684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ia57dd9640d732a75659ca11763d89aefdaf16be1 Gerrit-Change-Number: 10684 Gerrit-PatchSet: 4 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [tools] Extra integration tests for the rebalancer
Will Berkeley has uploaded a new patch set (#10) to the change originally created by Alexey Serbin. ( http://gerrit.cloudera.org:8080/10540 ) Change subject: [tools] Extra integration tests for the rebalancer .. [tools] Extra integration tests for the rebalancer This patch adds more integration tests for the rebalancer: - Coverage for adding a new tablet server post-rebalancing and then rebalancing again. - Coverage for DDL concurrent with rebalancing. - Coverage for running two rebalancers concurrently. - Coverage for when a tablet server goes down during rebalancing. - Coverage for when a tablet server is added during rebalancing. Change-Id: I78b3dcea71ed303f6ecd199604b2385796d05da8 --- M src/kudu/tools/CMakeLists.txt M src/kudu/tools/kudu-admin-test.cc 2 files changed, 549 insertions(+), 47 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/40/10540/10 -- To view, visit http://gerrit.cloudera.org:8080/10540 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I78b3dcea71ed303f6ecd199604b2385796d05da8 Gerrit-Change-Number: 10540 Gerrit-PatchSet: 10 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley
[kudu-CR] [tools] more integration tests for rebalancer
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/10540 ) Change subject: [tools] more integration tests for rebalancer .. Patch Set 9: (1 comment) Added sharding to kudu-admin-test to speed it up when run by dist-test, in lieu of splitting it up into different files. http://gerrit.cloudera.org:8080/#/c/10540/9//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10540/9//COMMIT_MSG@11 PS9, Line 11: Added list of new tests. -- To view, visit http://gerrit.cloudera.org:8080/10540 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I78b3dcea71ed303f6ecd199604b2385796d05da8 Gerrit-Change-Number: 10540 Gerrit-PatchSet: 9 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 11 Jun 2018 20:10:17 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2191: Add a method to decide the scope of AlterTable
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/10581 )
Change subject: KUDU-2191: Add a method to decide the scope of AlterTable
..
Patch Set 5:
(2 comments)
Would like Dan to review this too.
http://gerrit.cloudera.org:8080/#/c/10581/5/src/kudu/client/client.cc
File src/kudu/client/client.cc:
http://gerrit.cloudera.org:8080/#/c/10581/5/src/kudu/client/client.cc@1144
PS5, Line 1144: if (kudu_only) {
This doesn't do the right thing if you call system(true) followed by
system(false).
It's certainly contrived, but it's easy to make it work properly, so we may as
well.
http://gerrit.cloudera.org:8080/#/c/10581/4/src/kudu/master/master.proto
File src/kudu/master/master.proto:
http://gerrit.cloudera.org:8080/#/c/10581/4/src/kudu/master/master.proto@571
PS4, Line 571: ALL_SYSTEMS = 0;
> Hmm, I used ALL_SYSTEMS because it seems SystemType is not needed to refer
I see. I thought C++11 protoc generated enum classes. Guess not.
--
To view, visit http://gerrit.cloudera.org:8080/10581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I703f12b95bb7ca2d65455f0f0602520332b3c678
Gerrit-Change-Number: 10581
Gerrit-PatchSet: 5
Gerrit-Owner: Hao Hao
Gerrit-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: Mon, 11 Jun 2018 19:56:17 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2191: use 'kudu only' in metadata upgrade tool
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10582 ) Change subject: KUDU-2191: use 'kudu_only' in metadata upgrade tool .. Patch Set 4: Code-Review+2 But I'd like Dan to take a look too. -- To view, visit http://gerrit.cloudera.org:8080/10582 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I51d4cabeb1a9defc51f4e307a116419da5588f2d Gerrit-Change-Number: 10582 Gerrit-PatchSet: 4 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 11 Jun 2018 19:52:40 + Gerrit-HasComments: No
[kudu-CR] KUDU-2191: use 'kudu only' in metadata upgrade tool
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/10582 ) Change subject: KUDU-2191: use 'kudu_only' in metadata upgrade tool .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/10582/3/src/kudu/client/client-internal.h File src/kudu/client/client-internal.h: http://gerrit.cloudera.org:8080/#/c/10582/3/src/kudu/client/client-internal.h@295 PS3, Line 295: // Helper function. When HMS integration is enabled, alter the table in Kudu but not > I'm confused; why is this function needed at all? Can't tool_action_hms cal You're right, updated the patch. Thanks! -- To view, visit http://gerrit.cloudera.org:8080/10582 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I51d4cabeb1a9defc51f4e307a116419da5588f2d Gerrit-Change-Number: 10582 Gerrit-PatchSet: 3 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 11 Jun 2018 19:42:19 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2191: Add a method to decide the scope of AlterTable
Hello Tidy Bot, Dan Burkert, Kudu Jenkins, Adar Dembo, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10581 to look at the new patch set (#5). Change subject: KUDU-2191: Add a method to decide the scope of AlterTable .. KUDU-2191: Add a method to decide the scope of AlterTable This commit adds a private method in KuduTableAlterer to indicate whether the table alteration should take place in the Hive Metastore or not when HMS integration is enabled. This method will be useful to the HMS integration tools, such as the metadata upgrade tool which may need to alter tables in Kudu only. Change-Id: I703f12b95bb7ca2d65455f0f0602520332b3c678 --- M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/table_alterer-internal.cc M src/kudu/client/table_alterer-internal.h M src/kudu/integration-tests/master_hms-itest.cc M src/kudu/master/catalog_manager.cc M src/kudu/master/master.proto 7 files changed, 48 insertions(+), 8 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/81/10581/5 -- To view, visit http://gerrit.cloudera.org:8080/10581 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I703f12b95bb7ca2d65455f0f0602520332b3c678 Gerrit-Change-Number: 10581 Gerrit-PatchSet: 5 Gerrit-Owner: Hao Hao Gerrit-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-2191: Add a method to decide the scope of AlterTable
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/10581 )
Change subject: KUDU-2191: Add a method to decide the scope of AlterTable
..
Patch Set 5:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/10581/4/src/kudu/client/client.h
File src/kudu/client/client.h:
http://gerrit.cloudera.org:8080/#/c/10581/4/src/kudu/client/client.h@45
PS4, Line 45: #include "kudu/util/kudu_export.h"
> This can't be included here; it's not part of the public client headers (an
Done
http://gerrit.cloudera.org:8080/#/c/10581/4/src/kudu/client/client.h@1194
PS4, Line 1194: KuduTableAlterer* system(bool kudu_only);
> You can't use a PB-defined enum here. That's why my original suggestion dif
Done
http://gerrit.cloudera.org:8080/#/c/10581/4/src/kudu/client/table_alterer-internal.h
File src/kudu/client/table_alterer-internal.h:
http://gerrit.cloudera.org:8080/#/c/10581/4/src/kudu/client/table_alterer-internal.h@82
PS4, Line 82: master::AlterTableRequestPB::ALL_SYSTEMS;
> Shouldn't the default be ALL_SYSTEMS?
Oops, my bad.
http://gerrit.cloudera.org:8080/#/c/10581/4/src/kudu/integration-tests/master_hms-itest.cc
File src/kudu/integration-tests/master_hms-itest.cc:
http://gerrit.cloudera.org:8080/#/c/10581/4/src/kudu/integration-tests/master_hms-itest.cc@48
PS4, Line 48: using client::KuduColumnSchema;
> You don't need to do this. See how ClientStressTest is used in FRIEND_TEST
Done
http://gerrit.cloudera.org:8080/#/c/10581/4/src/kudu/integration-tests/master_hms-itest.cc@360
PS4, Line 360: ASSERT_OK(hms_client_->GetTable("default", "a", &hms_table));
> Nit: exists
Done
http://gerrit.cloudera.org:8080/#/c/10581/4/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/10581/4/src/kudu/master/catalog_manager.cc@2158
PS4, Line 2158: req.system_to_alter() !=
master::AlterTableRequestPB::NO_HMS) {
> To make this more future-proof, change the condition to != NO_HMS. That way
Done
http://gerrit.cloudera.org:8080/#/c/10581/4/src/kudu/master/master.proto
File src/kudu/master/master.proto:
http://gerrit.cloudera.org:8080/#/c/10581/4/src/kudu/master/master.proto@570
PS4, Line 570: enum ExternalSystemType {
> Ni:t how about ExternalSystemType:?
Done
http://gerrit.cloudera.org:8080/#/c/10581/4/src/kudu/master/master.proto@571
PS4, Line 571: ALL_SYSTEMS = 0;
> Nit: just ALL is enough, given that to use the enum value you need to quali
Hmm, I used ALL_SYSTEMS because it seems SystemType is not needed to refer the
enum value. In the proto buff auto-generated code it has 'static const
SystemType ALL_SYSTEMS =
AlterTableRequestPB_SystemType_ALL_SYSTEMS;'
http://gerrit.cloudera.org:8080/#/c/10581/4/src/kudu/master/master.proto@574
PS4, Line 574: optional ExternalSystemType system_to_alter = 5 [ default =
ALL_SYSTEMS ];
> Nit: how about system_to_alter.
Done
--
To view, visit http://gerrit.cloudera.org:8080/10581
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I703f12b95bb7ca2d65455f0f0602520332b3c678
Gerrit-Change-Number: 10581
Gerrit-PatchSet: 5
Gerrit-Owner: Hao Hao
Gerrit-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: Mon, 11 Jun 2018 19:42:27 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2191: use 'kudu only' in metadata upgrade tool
Hello Dan Burkert, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10582 to look at the new patch set (#4). Change subject: KUDU-2191: use 'kudu_only' in metadata upgrade tool .. KUDU-2191: use 'kudu_only' in metadata upgrade tool Previously the metadata upgrade tool requires the HMS integration feature being disabled. This commit updates the tool to use 'kudu_only' flag when altering Kudu tables, so that the restriction no longer applies. Change-Id: I51d4cabeb1a9defc51f4e307a116419da5588f2d --- M src/kudu/client/client.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_hms.cc 3 files changed, 32 insertions(+), 10 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/82/10582/4 -- To view, visit http://gerrit.cloudera.org:8080/10582 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I51d4cabeb1a9defc51f4e307a116419da5588f2d Gerrit-Change-Number: 10582 Gerrit-PatchSet: 4 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-1038 Deleting a tablet should also delete its log recovery directory, if any
Mike Percy has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10591 ) Change subject: KUDU-1038 Deleting a tablet should also delete its log recovery directory, if any .. KUDU-1038 Deleting a tablet should also delete its log recovery directory, if any This fix ensures successful bootstrap of a failed tablet replica, in the case when its WAL segments are missing/deleted. The following scenario is tested: 1. A log segment is deleted from one of the Tablet Replicas. 2. On server restart, the replica in step 1. enters a failed state. ( Without this fix, due to the presence of log recovery directory (with deleted log segments), the replica remains in a FAILED state and fails to bootstrap ) 3. This fix deletes the log recovery directory when the tablet data is deleted, as a result of which the failed replica successfully bootstraps. Change-Id: If3f017caf41ac4e00bf89421d6e73924b9fdcbd0 Reviewed-on: http://gerrit.cloudera.org:8080/10591 Tested-by: Kudu Jenkins Reviewed-by: Mike Percy --- M src/kudu/consensus/log.cc M src/kudu/consensus/log.h M src/kudu/integration-tests/ts_recovery-itest.cc M src/kudu/tablet/tablet_bootstrap.cc M src/kudu/tserver/ts_tablet_manager.cc 5 files changed, 139 insertions(+), 38 deletions(-) Approvals: Kudu Jenkins: Verified Mike Percy: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/10591 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: If3f017caf41ac4e00bf89421d6e73924b9fdcbd0 Gerrit-Change-Number: 10591 Gerrit-PatchSet: 15 Gerrit-Owner: Anupama Gupta Gerrit-Reviewer: Anupama Gupta Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot
[kudu-CR] KUDU-1038 Deleting a tablet should also delete its log recovery directory, if any
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/10591 ) Change subject: KUDU-1038 Deleting a tablet should also delete its log recovery directory, if any .. Patch Set 14: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10591 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If3f017caf41ac4e00bf89421d6e73924b9fdcbd0 Gerrit-Change-Number: 10591 Gerrit-PatchSet: 14 Gerrit-Owner: Anupama Gupta Gerrit-Reviewer: Anupama Gupta Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Mon, 11 Jun 2018 19:28:44 + Gerrit-HasComments: No
[kudu-CR] [tools] add 'kudu tablet set attributes' sub-command
Mike Percy has posted comments on this change. (
http://gerrit.cloudera.org:8080/10536 )
Change subject: [tools] add 'kudu tablet set_attributes' sub-command
..
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/10536/3/src/kudu/tools/tool_action_tablet.cc
File src/kudu/tools/tool_action_tablet.cc:
http://gerrit.cloudera.org:8080/#/c/10536/3/src/kudu/tools/tool_action_tablet.cc@254
PS3, Line 254: return Status::InvalidArgument("cannot set both PROMOTE and
REPLACE");
I agree it doesn't really make sense but what's the risk if we don't do this
validation?
http://gerrit.cloudera.org:8080/#/c/10536/3/src/kudu/tools/tool_action_tablet.cc@273
PS3, Line 273: change->mutable_peer()->mutable_attrs()->set_promote(promote);
We may likely add additional attributes in the future, so I think if you don't
specify a flag the tool should not attempt to set it or change its value.
--
To view, visit http://gerrit.cloudera.org:8080/10536
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib304715100ba9062558863f140aa309fd604ace3
Gerrit-Change-Number: 10536
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley
Gerrit-Comment-Date: Mon, 11 Jun 2018 19:26:48 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2470: retry failed thirdparty downloads
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10684 ) Change subject: KUDU-2470: retry failed thirdparty downloads .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia57dd9640d732a75659ca11763d89aefdaf16be1 Gerrit-Change-Number: 10684 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 11 Jun 2018 19:12:58 + Gerrit-HasComments: No
[kudu-CR] Code update for KUDU-2459
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/10656 )
Change subject: Code update for KUDU-2459
..
Patch Set 4:
Reading through KUDU-2459, I agree that the approach taken here (show the table
name verbatim if it conforms to Impala's requirements, otherwise replace it
with a fixed placeholder) makes sense.
Here are some high level suggestions for improvement:
1. Decompose the table name validation logic into a separate function and add a
unit test that exercises it with both valid and invalid names. I don't think we
have a dedicated "Impala utility" module, but you can start one, perhaps
util/impala-util.{cc,h}?
2. Extend the function to handle _all_ of the Impala identifier requirements.
For example, it'd be nice to account for Impala reserved words as well.
3. Use a placeholder that is more obviously a placeholder.
--
To view, visit http://gerrit.cloudera.org:8080/10656
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If9e5242318589452a0507d3973b9be3e6a186a69
Gerrit-Change-Number: 10656
Gerrit-PatchSet: 4
Gerrit-Owner: Shriya Gupta
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Thomas Marshall
Gerrit-Comment-Date: Mon, 11 Jun 2018 19:08:26 +
Gerrit-HasComments: No
[kudu-CR] KUDU-2470: retry failed thirdparty downloads
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10684 ) Change subject: KUDU-2470: retry failed thirdparty downloads .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/10684/2/thirdparty/download-thirdparty.sh File thirdparty/download-thirdparty.sh: http://gerrit.cloudera.org:8080/#/c/10684/2/thirdparty/download-thirdparty.sh@76 PS2, Line 76: curl --retry 3 -L -O "$FULL_URL" > The curl manpage has this to say about --retry-delay: The default would be sufficient; I picked non-default here since it more closely resembles the behavior we have in our other retry code. I'll switch to the default; the 3 times is a bit arbitrary, but doesn't seem particularly unreasonable. -- To view, visit http://gerrit.cloudera.org:8080/10684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia57dd9640d732a75659ca11763d89aefdaf16be1 Gerrit-Change-Number: 10684 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 11 Jun 2018 19:00:36 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2470: retry failed thirdparty downloads
Hello Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10684 to look at the new patch set (#3). Change subject: KUDU-2470: retry failed thirdparty downloads .. KUDU-2470: retry failed thirdparty downloads Currently we re-download when we fail to open thirdparty archives that fail to read. We don't currently re-attempt to download if the download itself fails. This patch adds the behavior to download failures, using curl's built-in --retry option that exponentially backs off on failure. Change-Id: Ia57dd9640d732a75659ca11763d89aefdaf16be1 --- M thirdparty/download-thirdparty.sh 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/10684/3 -- To view, visit http://gerrit.cloudera.org:8080/10684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia57dd9640d732a75659ca11763d89aefdaf16be1 Gerrit-Change-Number: 10684 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [tools] add 'kudu tablet set attributes' sub-command
Will Berkeley has uploaded a new patch set (#3) to the change originally created by Alexey Serbin. ( http://gerrit.cloudera.org:8080/10536 ) Change subject: [tools] add 'kudu tablet set_attributes' sub-command .. [tools] add 'kudu tablet set_attributes' sub-command Introduced 'kudu tablet set_attributes' sub-command for the kudu CLI tool. With this tool, it is possible to set/reset the 'promote' and 'replace' attributes in the replica's Raft configuration. This functionality might be useful for pre-KUDU-2443 clusters running the 3-4-3 replica management scheme after an attempt to move a replica of a tablet with replication factor of 1. In that case, the system would add a non-voter replica and then evict it, doing that over and over again (that's what KUDU-2443 is about). Aside from the hapless use-case described above, this functionality might be useful when removing the 'replace' attribute set on a replica by the 'kudu tablet change_config move_replica' sub-command before the move has completed. Change-Id: Ib304715100ba9062558863f140aa309fd604ace3 --- M src/kudu/tools/kudu-admin-test.cc M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_tablet.cc 3 files changed, 175 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/10536/3 -- To view, visit http://gerrit.cloudera.org:8080/10536 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib304715100ba9062558863f140aa309fd604ace3 Gerrit-Change-Number: 10536 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-2470: retry failed thirdparty downloads
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10684 ) Change subject: KUDU-2470: retry failed thirdparty downloads .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/10684/2/thirdparty/download-thirdparty.sh File thirdparty/download-thirdparty.sh: http://gerrit.cloudera.org:8080/#/c/10684/2/thirdparty/download-thirdparty.sh@76 PS2, Line 76: curl --retry 3 --retry-delay 60 -L -O "$FULL_URL" The curl manpage has this to say about --retry-delay: Setting this delay to zero will make curl use the default backoff time. Is the default value sufficient for our purposes? And if not, why not? -- To view, visit http://gerrit.cloudera.org:8080/10684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia57dd9640d732a75659ca11763d89aefdaf16be1 Gerrit-Change-Number: 10684 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 11 Jun 2018 18:29:17 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2470: retry failed thirdparty downloads
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10684 ) Change subject: KUDU-2470: retry failed thirdparty downloads .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/10684/1/thirdparty/download-thirdparty.sh File thirdparty/download-thirdparty.sh: http://gerrit.cloudera.org:8080/#/c/10684/1/thirdparty/download-thirdparty.sh@76 PS1, Line 76: curl --retry 3 --retry-delay 60 -L -O "$FULL_URL" > Would it be sufficient to use curl's --retry option? Good call, done. -- To view, visit http://gerrit.cloudera.org:8080/10684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia57dd9640d732a75659ca11763d89aefdaf16be1 Gerrit-Change-Number: 10684 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 11 Jun 2018 18:00:53 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2470: retry failed thirdparty downloads
Hello Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10684 to look at the new patch set (#2). Change subject: KUDU-2470: retry failed thirdparty downloads .. KUDU-2470: retry failed thirdparty downloads Currently we re-download when we fail to open thirdparty archives that fail to read. We don't currently re-attempt to download if the download itself fails. This patch adds the behavior to download failures. Change-Id: Ia57dd9640d732a75659ca11763d89aefdaf16be1 --- M thirdparty/download-thirdparty.sh 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/10684/2 -- To view, visit http://gerrit.cloudera.org:8080/10684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia57dd9640d732a75659ca11763d89aefdaf16be1 Gerrit-Change-Number: 10684 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [tools] ksck: fix some output spacing and printing of GetFlags warnings
Will Berkeley has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10667 ) Change subject: [tools] ksck: fix some output spacing and printing of GetFlags warnings .. [tools] ksck: fix some output spacing and printing of GetFlags warnings The result of the call to GetFlags was not being saved, resulting in a different warning being printed than the reason GetFlags failed, when it did fail. The different warning was usually from a successful previous RPC and so was, pretty confusingly, a blank line. Change-Id: I5a7772a2d25d934ce42b27fbb81a5711cb4f12cd Reviewed-on: http://gerrit.cloudera.org:8080/10667 Tested-by: Will Berkeley Reviewed-by: Will Berkeley --- M src/kudu/tools/ksck.cc M src/kudu/tools/ksck_results.cc 2 files changed, 8 insertions(+), 3 deletions(-) Approvals: Will Berkeley: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/10667 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I5a7772a2d25d934ce42b27fbb81a5711cb4f12cd Gerrit-Change-Number: 10667 Gerrit-PatchSet: 3 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley
[kudu-CR] [tools] ksck: fix some output spacing and printing of GetFlags warnings
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/10667 ) Change subject: [tools] ksck: fix some output spacing and printing of GetFlags warnings .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10667 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5a7772a2d25d934ce42b27fbb81a5711cb4f12cd Gerrit-Change-Number: 10667 Gerrit-PatchSet: 2 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 11 Jun 2018 17:56:07 + Gerrit-HasComments: No
[kudu-CR] [tools] ksck: fix some output spacing and printing of GetFlags warnings
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/10667 ) Change subject: [tools] ksck: fix some output spacing and printing of GetFlags warnings .. Patch Set 2: Code-Review+2 Forwarding awong's +2 (only change was to commit msg) -- To view, visit http://gerrit.cloudera.org:8080/10667 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5a7772a2d25d934ce42b27fbb81a5711cb4f12cd Gerrit-Change-Number: 10667 Gerrit-PatchSet: 2 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 11 Jun 2018 17:56:34 + Gerrit-HasComments: No
[kudu-CR] [tools] ksck: fix some output spacing and printing of GetFlags warnings
Will Berkeley has removed a vote on this change. Change subject: [tools] ksck: fix some output spacing and printing of GetFlags warnings .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/10667 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I5a7772a2d25d934ce42b27fbb81a5711cb4f12cd Gerrit-Change-Number: 10667 Gerrit-PatchSet: 2 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-2470: retry failed thirdparty downloads
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10684 ) Change subject: KUDU-2470: retry failed thirdparty downloads .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/10684/1/thirdparty/download-thirdparty.sh File thirdparty/download-thirdparty.sh: http://gerrit.cloudera.org:8080/#/c/10684/1/thirdparty/download-thirdparty.sh@76 PS1, Line 76: if ! curl -L -O "$FULL_URL"; then Would it be sufficient to use curl's --retry option? -- To view, visit http://gerrit.cloudera.org:8080/10684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia57dd9640d732a75659ca11763d89aefdaf16be1 Gerrit-Change-Number: 10684 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 11 Jun 2018 17:46:17 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2470: retry failed thirdparty downloads
Andrew Wong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10684 Change subject: KUDU-2470: retry failed thirdparty downloads .. KUDU-2470: retry failed thirdparty downloads Currently we re-download when we fail to open thirdparty archives that fail to read. We don't currently re-attempt to download if the download itself fails. This patch adds the behavior to download failures. Change-Id: Ia57dd9640d732a75659ca11763d89aefdaf16be1 --- M thirdparty/download-thirdparty.sh 1 file changed, 3 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/10684/1 -- To view, visit http://gerrit.cloudera.org:8080/10684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ia57dd9640d732a75659ca11763d89aefdaf16be1 Gerrit-Change-Number: 10684 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong
[kudu-CR] [tools] ksck: fix some output spacing and printing of GetFlags warnings
Will Berkeley has posted comments on this change. (
http://gerrit.cloudera.org:8080/10667 )
Change subject: [tools] ksck: fix some output spacing and printing of GetFlags
warnings
..
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10667/1/src/kudu/tools/ksck.cc
File src/kudu/tools/ksck.cc:
http://gerrit.cloudera.org:8080/#/c/10667/1/src/kudu/tools/ksck.cc@222
PS1, Line 222: s = master->FetchUnusualFlags();
: if (!s.ok()) {
> Gotcha, thanks for explaining. Would be helpful in the commit message for t
Done
--
To view, visit http://gerrit.cloudera.org:8080/10667
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a7772a2d25d934ce42b27fbb81a5711cb4f12cd
Gerrit-Change-Number: 10667
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley
Gerrit-Comment-Date: Mon, 11 Jun 2018 17:24:02 +
Gerrit-HasComments: Yes
[kudu-CR] [tools] ksck: fix some output spacing and printing of GetFlags warnings
Hello Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10667 to look at the new patch set (#2). Change subject: [tools] ksck: fix some output spacing and printing of GetFlags warnings .. [tools] ksck: fix some output spacing and printing of GetFlags warnings The result of the call to GetFlags was not being saved, resulting in a different warning being printed than the reason GetFlags failed, when it did fail. The different warning was usually from a successful previous RPC and so was, pretty confusingly, a blank line. Change-Id: I5a7772a2d25d934ce42b27fbb81a5711cb4f12cd --- M src/kudu/tools/ksck.cc M src/kudu/tools/ksck_results.cc 2 files changed, 8 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/67/10667/2 -- To view, visit http://gerrit.cloudera.org:8080/10667 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5a7772a2d25d934ce42b27fbb81a5711cb4f12cd Gerrit-Change-Number: 10667 Gerrit-PatchSet: 2 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley
[kudu-CR] [tools] ksck: fix some output spacing and printing of GetFlags warnings
Andrew Wong has posted comments on this change. (
http://gerrit.cloudera.org:8080/10667 )
Change subject: [tools] ksck: fix some output spacing and printing of GetFlags
warnings
..
Patch Set 1: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10667/1/src/kudu/tools/ksck.cc
File src/kudu/tools/ksck.cc:
http://gerrit.cloudera.org:8080/#/c/10667/1/src/kudu/tools/ksck.cc@222
PS1, Line 222: s = master->FetchUnusualFlags();
: if (!s.ok()) {
> Before, the printing was conditioned on the success of FetchUnusualFlags bu
Gotcha, thanks for explaining. Would be helpful in the commit message for those
who don't have the immediate context.
--
To view, visit http://gerrit.cloudera.org:8080/10667
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a7772a2d25d934ce42b27fbb81a5711cb4f12cd
Gerrit-Change-Number: 10667
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley
Gerrit-Comment-Date: Mon, 11 Jun 2018 17:20:07 +
Gerrit-HasComments: Yes
[kudu-CR] [tools] ksck: fix some output spacing and printing of GetFlags warnings
Will Berkeley has posted comments on this change. (
http://gerrit.cloudera.org:8080/10667 )
Change subject: [tools] ksck: fix some output spacing and printing of GetFlags
warnings
..
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10667/1/src/kudu/tools/ksck.cc
File src/kudu/tools/ksck.cc:
http://gerrit.cloudera.org:8080/#/c/10667/1/src/kudu/tools/ksck.cc@222
PS1, Line 222: s = master->FetchUnusualFlags();
: if (!s.ok()) {
> Might be missing something, but does this change do anything? Same below.
Before, the printing was conditioned on the success of FetchUnusualFlags but
that status was not set into s, so the ToString of the Status chain on L202 was
printed instead. This was usually Status::OK() so an empty line was printed
whenever GetFlags failed.
--
To view, visit http://gerrit.cloudera.org:8080/10667
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a7772a2d25d934ce42b27fbb81a5711cb4f12cd
Gerrit-Change-Number: 10667
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley
Gerrit-Comment-Date: Mon, 11 Jun 2018 17:17:07 +
Gerrit-HasComments: Yes
