Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15323 )
Change subject: ksck: display quiecing-related info ...................................................................... Patch Set 1: (5 comments) The test failures seem relevant to these changes. http://gerrit.cloudera.org:8080/#/c/15323/1/src/kudu/integration-tests/tablet_server_quiescing-itest.cc File src/kudu/integration-tests/tablet_server_quiescing-itest.cc: http://gerrit.cloudera.org:8080/#/c/15323/1/src/kudu/integration-tests/tablet_server_quiescing-itest.cc@398 PS1, Line 398: " Quiescing | Tablet leader count | Active scanner count\n" : "-----------+---------------------+----------------------\n" : " true | 1 | 0"); : ASSERT_TRUE(ts->server()->quiescing()); : : // Same with ksck. : ASSERT_OK(RunKuduTool({ "cluster", "ksck", master_addr }, &stdout)); : ASSERT_STR_MATCHES(stdout, : ".* Quiescing | Tablet Leaders | Active Scanners\n" : ".* ----------+----------------+----------------\n" : ".* true | 1 | 0"); nit: maybe, make the names of the corresponding columns match for both 'tserver quiesce' and 'cluster ksck'? http://gerrit.cloudera.org:8080/#/c/15323/1/src/kudu/integration-tests/tablet_server_quiescing-itest.cc@433 PS1, Line 433: --noquiescing_info Instead, maybe output the quiescing information only if --quiescing_info flag is specified? http://gerrit.cloudera.org:8080/#/c/15323/1/src/kudu/tools/ksck_remote.cc File src/kudu/tools/ksck_remote.cc: http://gerrit.cloudera.org:8080/#/c/15323/1/src/kudu/tools/ksck_remote.cc@81 PS1, Line 81: true Do we really want to include the information into ksck by default? Given the presence of a dedicated sub-command for quiescing and thinking about backwards compatibility, I would expect it is set to 'false' by default, no? http://gerrit.cloudera.org:8080/#/c/15323/1/src/kudu/tools/ksck_remote.cc@82 PS1, Line 82: to check What does 'check' means here? Simply 'display'? http://gerrit.cloudera.org:8080/#/c/15323/1/src/kudu/tserver/tablet_service.cc File src/kudu/tserver/tablet_service.cc: http://gerrit.cloudera.org:8080/#/c/15323/1/src/kudu/tserver/tablet_service.cc@1020 PS1, Line 1020: default: What about other already supported features? -- To view, visit http://gerrit.cloudera.org:8080/15323 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibdc650eb3ee30e8993330f2cbd389076ea2bad49 Gerrit-Change-Number: 15323 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Sun, 01 Mar 2020 06:52:38 +0000 Gerrit-HasComments: Yes
