[kudu-CR] KUDU-687: use client in ksck for master operations
Todd Lipcon has posted comments on this change. Change subject: KUDU-687: use client in ksck for master operations .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4147 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icbd6b28ea1b2c88e02cd451095e4dec94b0ebfc3 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar DemboGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-687: use client in ksck for master operations
Todd Lipcon has submitted this change and it was merged. Change subject: KUDU-687: use client in ksck for master operations .. KUDU-687: use client in ksck for master operations This patch modifies ksck to use the client for all master operations, restricting direct access only for tserver operations. In doing so, ksck can now work with multi-master clusters, retrying operations when the leader master dies. Interesting things of note: - I went back and forth on what the semantics of KsckMaster::Connect() ought to be. At first I thought we should ping every master, but in the end I settled on building the client, which just verifies the existence of a leader master. My justification: today's ksck isn't really concerned with master health; the master merely provides information that is consumed during the real checks: of table and tablet integrity. - I relented on commit cf009d4 and restored a MiniCluster method to find the leader master. It's got the same signature as the ExternalMiniCluster variant so that Mike's common cluster interface (a work in progress) can expose it. Change-Id: Icbd6b28ea1b2c88e02cd451095e4dec94b0ebfc3 Reviewed-on: http://gerrit.cloudera.org:8080/4147 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon--- M src/kudu/client/schema.h M src/kudu/integration-tests/cluster_verifier.cc M src/kudu/integration-tests/mini_cluster.cc M src/kudu/integration-tests/mini_cluster.h M src/kudu/tools/CMakeLists.txt M src/kudu/tools/ksck-test.cc M src/kudu/tools/ksck.h M src/kudu/tools/ksck_remote-test.cc M src/kudu/tools/ksck_remote.cc M src/kudu/tools/ksck_remote.h M src/kudu/tools/tool_action_cluster.cc 11 files changed, 163 insertions(+), 166 deletions(-) Approvals: Todd Lipcon: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4147 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Icbd6b28ea1b2c88e02cd451095e4dec94b0ebfc3 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-687: use client in ksck for master operations
Adar Dembo has posted comments on this change. Change subject: KUDU-687: use client in ksck for master operations .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/4147/1/src/kudu/tools/ksck_remote.cc File src/kudu/tools/ksck_remote.cc: Line 284: for (auto s : servers) { > ah. I think 'const auto*' is nice, then. Makes it clearer it's a pointer. OK, added * where appropriate. -- To view, visit http://gerrit.cloudera.org:8080/4147 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icbd6b28ea1b2c88e02cd451095e4dec94b0ebfc3 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar DemboGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-687: use client in ksck for master operations
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4147 to look at the new patch set (#4). Change subject: KUDU-687: use client in ksck for master operations .. KUDU-687: use client in ksck for master operations This patch modifies ksck to use the client for all master operations, restricting direct access only for tserver operations. In doing so, ksck can now work with multi-master clusters, retrying operations when the leader master dies. Interesting things of note: - I went back and forth on what the semantics of KsckMaster::Connect() ought to be. At first I thought we should ping every master, but in the end I settled on building the client, which just verifies the existence of a leader master. My justification: today's ksck isn't really concerned with master health; the master merely provides information that is consumed during the real checks: of table and tablet integrity. - I relented on commit cf009d4 and restored a MiniCluster method to find the leader master. It's got the same signature as the ExternalMiniCluster variant so that Mike's common cluster interface (a work in progress) can expose it. Change-Id: Icbd6b28ea1b2c88e02cd451095e4dec94b0ebfc3 --- M src/kudu/client/schema.h M src/kudu/integration-tests/cluster_verifier.cc M src/kudu/integration-tests/mini_cluster.cc M src/kudu/integration-tests/mini_cluster.h M src/kudu/tools/CMakeLists.txt M src/kudu/tools/ksck-test.cc M src/kudu/tools/ksck.h M src/kudu/tools/ksck_remote-test.cc M src/kudu/tools/ksck_remote.cc M src/kudu/tools/ksck_remote.h M src/kudu/tools/tool_action_cluster.cc 11 files changed, 163 insertions(+), 166 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/47/4147/4 -- To view, visit http://gerrit.cloudera.org:8080/4147 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Icbd6b28ea1b2c88e02cd451095e4dec94b0ebfc3 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar DemboGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-687: use client in ksck for master operations
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4147 to look at the new patch set (#3). Change subject: KUDU-687: use client in ksck for master operations .. KUDU-687: use client in ksck for master operations This patch modifies ksck to use the client for all master operations, restricting direct access only for tserver operations. In doing so, ksck can now work with multi-master clusters, retrying operations when the leader master dies. Interesting things of note: - I went back and forth on what the semantics of KsckMaster::Connect() ought to be. At first I thought we should ping every master, but in the end I settled on building the client, which just verifies the existence of a leader master. My justification: today's ksck isn't really concerned with master health; the master merely provides information that is consumed during the real checks: of table and tablet integrity. - I relented on commit cf009d4 and restored a MiniCluster method to find the leader master. It's got the same signature as the ExternalMiniCluster variant so that Mike's common cluster interface (a work in progress) can expose it. Change-Id: Icbd6b28ea1b2c88e02cd451095e4dec94b0ebfc3 --- M src/kudu/client/schema.h M src/kudu/integration-tests/cluster_verifier.cc M src/kudu/integration-tests/mini_cluster.cc M src/kudu/integration-tests/mini_cluster.h M src/kudu/tools/CMakeLists.txt M src/kudu/tools/ksck-test.cc M src/kudu/tools/ksck.h M src/kudu/tools/ksck_remote-test.cc M src/kudu/tools/ksck_remote.cc M src/kudu/tools/ksck_remote.h M src/kudu/tools/tool_action_cluster.cc 11 files changed, 161 insertions(+), 164 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/47/4147/3 -- To view, visit http://gerrit.cloudera.org:8080/4147 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Icbd6b28ea1b2c88e02cd451095e4dec94b0ebfc3 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar DemboGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-687: use client in ksck for master operations
Kudu Jenkins has posted comments on this change. Change subject: KUDU-687: use client in ksck for master operations .. Patch Set 3: Build Started http://104.196.14.100/job/kudu-gerrit/3124/ -- To view, visit http://gerrit.cloudera.org:8080/4147 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icbd6b28ea1b2c88e02cd451095e4dec94b0ebfc3 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar DemboGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-687: use client in ksck for master operations
Kudu Jenkins has posted comments on this change. Change subject: KUDU-687: use client in ksck for master operations .. Patch Set 2: Build Started http://104.196.14.100/job/kudu-gerrit/3120/ -- To view, visit http://gerrit.cloudera.org:8080/4147 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icbd6b28ea1b2c88e02cd451095e4dec94b0ebfc3 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar DemboGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-687: use client in ksck for master operations
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4147 to look at the new patch set (#2). Change subject: KUDU-687: use client in ksck for master operations .. KUDU-687: use client in ksck for master operations This patch modifies ksck to use the client for all master operations, restricting direct access only for tserver operations. In doing so, ksck can now work with multi-master clusters, retrying operations when the leader master dies. Interesting things of note: - For 'kudu cluster ksck', I opted to make the master addresses a variadic (i.e. space-delimited) list, but I can also see the case for making it a comma-separated list, to be more consistent with --master_addresses. - I went back and forth on what the semantics of KsckMaster::Connect() ought to be. At first I thought we should ping every master, but in the end I settled on building the client, which just verifies the existence of a leader master. My justification: today's ksck isn't really concerned with master health; the master merely provides information that is consumed during the real checks: of table and tablet integrity. - I relented on commit cf009d4 and restored a MiniCluster method to find the leader master. It's got the same signature as the ExternalMiniCluster variant so that Mike's common cluster interface (a work in progress) can expose it. Change-Id: Icbd6b28ea1b2c88e02cd451095e4dec94b0ebfc3 --- M src/kudu/client/schema.h M src/kudu/integration-tests/cluster_verifier.cc M src/kudu/integration-tests/mini_cluster.cc M src/kudu/integration-tests/mini_cluster.h M src/kudu/tools/CMakeLists.txt M src/kudu/tools/ksck-test.cc M src/kudu/tools/ksck.h M src/kudu/tools/ksck_remote-test.cc M src/kudu/tools/ksck_remote.cc M src/kudu/tools/ksck_remote.h M src/kudu/tools/tool_action_cluster.cc 11 files changed, 161 insertions(+), 164 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/47/4147/2 -- To view, visit http://gerrit.cloudera.org:8080/4147 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Icbd6b28ea1b2c88e02cd451095e4dec94b0ebfc3 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar DemboGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-687: use client in ksck for master operations
Todd Lipcon has posted comments on this change. Change subject: KUDU-687: use client in ksck for master operations .. Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/4147/1//COMMIT_MSG Commit Message: Line 17: comma-separated list, to be more consistent with --master_addresses. yea, I think I prefer comma-separated since it's the same way we specify it everywhere else (and in other tools) http://gerrit.cloudera.org:8080/#/c/4147/1/src/kudu/integration-tests/mini_cluster.cc File src/kudu/integration-tests/mini_cluster.cc: PS1, Line 294: * style http://gerrit.cloudera.org:8080/#/c/4147/1/src/kudu/tools/ksck_remote.cc File src/kudu/tools/ksck_remote.cc: Line 284: for (auto s : servers) { const auto& Line 324: for (auto t : tokens) { const auto& Line 328: for (const auto r : t->tablet().replicas()) { auto& -- To view, visit http://gerrit.cloudera.org:8080/4147 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icbd6b28ea1b2c88e02cd451095e4dec94b0ebfc3 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar DemboGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-687: use client in ksck for master operations
Hello Dan Burkert, Mike Percy, Todd Lipcon, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/4147 to review the following change. Change subject: KUDU-687: use client in ksck for master operations .. KUDU-687: use client in ksck for master operations This patch modifies ksck to use the client for all master operations, restricting direct access only for tserver operations. In doing so, ksck can now work with multi-master clusters, retrying operations when the leader master dies. Interesting things of note: - For 'kudu cluster ksck', I opted to make the master addresses a variadic (i.e. space-delimited) list, but I can also see the case for making it a comma-separated list, to be more consistent with --master_addresses. - I went back and forth on what the semantics of KsckMaster::Connect() ought to be. At first I thought we should ping every master, but in the end I settled on building the client, which just verifies the existence of a leader master. My justification: today's ksck isn't really concerned with master health; the master merely provides information that is consumed during the real checks: of table and tablet integrity. - I relented on commit cf009d4 and restored a MiniCluster method to find the leader master. It's got the same signature as the ExternalMiniCluster variant so that Mike's common cluster interface (a work in progress) can expose it. Change-Id: Icbd6b28ea1b2c88e02cd451095e4dec94b0ebfc3 --- M src/kudu/client/schema.h M src/kudu/integration-tests/cluster_verifier.cc M src/kudu/integration-tests/mini_cluster.cc M src/kudu/integration-tests/mini_cluster.h M src/kudu/tools/CMakeLists.txt M src/kudu/tools/ksck-test.cc M src/kudu/tools/ksck.h M src/kudu/tools/ksck_remote-test.cc M src/kudu/tools/ksck_remote.cc M src/kudu/tools/ksck_remote.h M src/kudu/tools/tool_action_cluster.cc 11 files changed, 157 insertions(+), 156 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/47/4147/1 -- To view, visit http://gerrit.cloudera.org:8080/4147 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Icbd6b28ea1b2c88e02cd451095e4dec94b0ebfc3 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar DemboGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon