Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/21536 )
Change subject: [Tool] Retry failed table copying tasks ...................................................................... Patch Set 9: (7 comments) http://gerrit.cloudera.org:8080/#/c/21536/9//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21536/9//COMMIT_MSG@9 PS9, Line 9: Copying a table with large data nit: Copying a large table ... or Copying a table with a lot of data ... http://gerrit.cloudera.org:8080/#/c/21536/9//COMMIT_MSG@11 PS9, Line 11: copy nit: to restart copying http://gerrit.cloudera.org:8080/#/c/21536/9/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/21536/9/src/kudu/tools/kudu-tool-test.cc@9289 PS9, Line 9289: kTableName nit: consider naming this kSrcTableName (since there is kDstTableName) http://gerrit.cloudera.org:8080/#/c/21536/9/src/kudu/tools/kudu-tool-test.cc@9306 PS9, Line 9306: ASSERT_TRUE(s.ok()); There is a dedicated macro ASSERT_OK() for this. BTW, how can this succeed when FLAGS_copy_task_fail_for_test is still set to 'true'? http://gerrit.cloudera.org:8080/#/c/21536/9/src/kudu/tools/table_scanner.cc File src/kudu/tools/table_scanner.cc: http://gerrit.cloudera.org:8080/#/c/21536/9/src/kudu/tools/table_scanner.cc@156 PS9, Line 156: nit: fix the indent http://gerrit.cloudera.org:8080/#/c/21536/9/src/kudu/tools/table_scanner.cc@159 PS9, Line 159: test nit: tests http://gerrit.cloudera.org:8080/#/c/21536/9/src/kudu/tools/table_scanner.cc@677 PS9, Line 677: retry_times_count++; I'm not sure I understand what this is for. This increments only the copy of the 'retry_time_count' that's passed by value, and this doesn't have any effect on the argument passed to CopyTaskWithRetries() at the call site. -- To view, visit http://gerrit.cloudera.org:8080/21536 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I712201178c1b34e5e14256063b66f0008fdc6007 Gerrit-Change-Number: 21536 Gerrit-PatchSet: 9 Gerrit-Owner: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Abhishek Chennaka <achenn...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <ale...@apache.org> Gerrit-Reviewer: Attila Bukor <abu...@apache.org> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <greber...@gmail.com> Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Zoltan Chovan <zcho...@cloudera.com> Gerrit-Reviewer: Zoltan Martonka <zmarto...@cloudera.com> Gerrit-Comment-Date: Sat, 27 Jul 2024 01:17:45 +0000 Gerrit-HasComments: Yes
