Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/13024 )
Change subject: [ksck] Make ksck tool more concurrent ...................................................................... Patch Set 2: (21 comments) http://gerrit.cloudera.org:8080/#/c/13024/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13024/1//COMMIT_MSG@7 PS1, Line 7: more concurrent > more concurrent Done http://gerrit.cloudera.org:8080/#/c/13024/1//COMMIT_MSG@9 PS1, Line 9: fetch > fetches Done http://gerrit.cloudera.org:8080/#/c/13024/1//COMMIT_MSG@9 PS1, Line 9: w when : fetch > when fetching Done http://gerrit.cloudera.org:8080/#/c/13024/1//COMMIT_MSG@10 PS1, Line 10: rs th > that Done http://gerrit.cloudera.org:8080/#/c/13024/1//COMMIT_MSG@11 PS1, Line 11: > What's this stand for? I____ Data Center? Internet Data Center, I've replace it with 'region' http://gerrit.cloudera.org:8080/#/c/13024/1//COMMIT_MSG@11 PS1, Line 11: fferent > the cluster Done http://gerrit.cloudera.org:8080/#/c/13024/1//COMMIT_MSG@11 PS1, Line 11: ster are > the CLI Done http://gerrit.cloudera.org:8080/#/c/13024/1//COMMIT_MSG@10 PS1, Line 10: when the CLI : and th > Remove Done http://gerrit.cloudera.org:8080/#/c/13024/1//COMMIT_MSG@12 PS1, Line 12: concurrent > concurrent Done http://gerrit.cloudera.org:8080/#/c/13024/1//COMMIT_MSG@12 PS1, Line 12: ing mas > master Done http://gerrit.cloudera.org:8080/#/c/13024/1//COMMIT_MSG@12 PS1, Line 12: nd tab > table Done http://gerrit.cloudera.org:8080/#/c/13024/1//COMMIT_MSG@12 PS1, Line 12: hen fetc > when fetching Done http://gerrit.cloudera.org:8080/#/c/13024/1/src/kudu/tools/ksck.h File src/kudu/tools/ksck.h: http://gerrit.cloudera.org:8080/#/c/13024/1/src/kudu/tools/ksck.h@423 PS1, Line 423: in > in Done http://gerrit.cloudera.org:8080/#/c/13024/1/src/kudu/tools/ksck.cc File src/kudu/tools/ksck.cc: http://gerrit.cloudera.org:8080/#/c/13024/1/src/kudu/tools/ksck.cc@60 PS1, Line 60: ); : : DEFINE_string(ksck_format, "plain_concise", > I don't understand this part. Done http://gerrit.cloudera.org:8080/#/c/13024/1/src/kudu/tools/ksck.cc@164 PS1, Line 164: } > I think returning a Status::NotFound is appropriate here, rather than a CHE Done http://gerrit.cloudera.org:8080/#/c/13024/1/src/kudu/tools/ksck.cc@337 PS1, Line 337: lliseconds(10 > At some point in the future this could be in the hundreds or thousands. I t Done http://gerrit.cloudera.org:8080/#/c/13024/1/src/kudu/tools/ksck_remote.cc File src/kudu/tools/ksck_remote.cc: http://gerrit.cloudera.org:8080/#/c/13024/1/src/kudu/tools/ksck_remote.cc@505 PS1, Line 505: int num_tables = static_cast<int>(table_names.size()); : if (num_tables == 0) { : return Status::OK(); : } > Preserve the previous behavior of returning OK if there's no tables-- it's Done http://gerrit.cloudera.org:8080/#/c/13024/1/src/kudu/tools/ksck_remote.cc@513 PS1, Line 513: lliseconds > This could be in the thousands. I think we need a reasonable default, like Done http://gerrit.cloudera.org:8080/#/c/13024/1/src/kudu/tools/ksck_remote.cc@542 PS1, Line 542: : if (bad_tables.Load() > 0) { : return Status::NetworkError( : Substitute("failed to gather info from all tables: $0 of $1 had errors", : > We need to provide at least one status from a failed OpenTable, too, so a u Done http://gerrit.cloudera.org:8080/#/c/13024/1/src/kudu/tools/ksck_remote.cc@552 PS1, Line 552: Status RemoteKsckCluster::RetrieveAllTablets() { : int num_tables = static_cast<int>(tables().size()); : if (num_tables == 0) { : > Same. This is an expected and non-error condition, so return OK. Done http://gerrit.cloudera.org:8080/#/c/13024/1/src/kudu/tools/ksck_remote.cc@560 PS1, Line 560: oncurrency > Again I think this will be too big for large clusters, creating possibly th Done -- To view, visit http://gerrit.cloudera.org:8080/13024 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2f3f0e3f5115a46dd3afc83dda75e7241618eea4 Gerrit-Change-Number: 13024 Gerrit-PatchSet: 2 Gerrit-Owner: Yingchun Lai <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley <[email protected]> Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Comment-Date: Wed, 17 Apr 2019 05:55:21 +0000 Gerrit-HasComments: Yes
