Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18813 )
Change subject: [tools] Add --fault_tolerant to 'kudu table copy/scan' ...................................................................... Patch Set 3: (7 comments) http://gerrit.cloudera.org:8080/#/c/18813/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/18813/1//COMMIT_MSG@10 PS1, Line 10: ret > drop 'are' Done http://gerrit.cloudera.org:8080/#/c/18813/1/src/kudu/tools/table_scanner.cc File src/kudu/tools/table_scanner.cc: http://gerrit.cloudera.org:8080/#/c/18813/1/src/kudu/tools/table_scanner.cc@102 PS1, Line 102: t another tablet ser > in primary key order for a single tablet ? Done http://gerrit.cloudera.org:8080/#/c/18813/1/src/kudu/tools/table_scanner.cc@103 PS1, Line 103: cans typically have lower throughput than non fault-tolerant scans, " : "but the results are returned in primary ke > I guess this part should come first in the description. The fact that the Done http://gerrit.cloudera.org:8080/#/c/18813/1/src/kudu/tools/table_scanner.cc@676 PS1, Line 676: // TODO(yingchun): push down this judgement to ScanConfiguration::SetFaultTolerant > Should we just keep this check inside TableScanner? No, I think it should be validated it in ScanConfiguration internally, then everywhere call ScanConfiguration::SetFaultTolerant can validate it. SetFaultTolerant may overwrite the read mode which is set by user explicitly, that may change what the user's intention. And the behavior of calling SetFaultTolerant first, then calling SetReadMode maybe undefined. http://gerrit.cloudera.org:8080/#/c/18813/1/src/kudu/tools/table_scanner.cc@676 PS1, Line 676: // TODO(yingchun): push down this judgement to ScanConfiguration::SetFaultTolerant > I'm not sure I understand how ScanConfiguration() could have such a respons All ScanConfiguration::Set* has a Status return value, so it is able to return error status when attempt to set invalid options. Now that it return Status, it maybe non-ok status. http://gerrit.cloudera.org:8080/#/c/18813/1/src/kudu/tools/table_scanner.cc@678 PS1, Line 678: conflicts w > conflicts Done http://gerrit.cloudera.org:8080/#/c/18813/1/src/kudu/tools/tool_action_table.cc File src/kudu/tools/tool_action_table.cc: PS1: > If adding new flags for the kudu CLI tool, please update corresponding test Done -- To view, visit http://gerrit.cloudera.org:8080/18813 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a05792db9dceb5e774ce1602158bb636bba04d0 Gerrit-Change-Number: 18813 Gerrit-PatchSet: 3 Gerrit-Owner: Yingchun Lai <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yifan Zhang <[email protected]> Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Comment-Date: Wed, 31 Aug 2022 12:56:48 +0000 Gerrit-HasComments: Yes
