Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/19620 )
Change subject: KUDU-3459 Download superblock piece by piece ...................................................................... Patch Set 6: (7 comments) http://gerrit.cloudera.org:8080/#/c/19620/6/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/19620/6/src/kudu/tools/kudu-tool-test.cc@9381 PS6, Line 9381: nit: remove the space. http://gerrit.cloudera.org:8080/#/c/19620/6/src/kudu/tserver/tablet_copy_client.cc File src/kudu/tserver/tablet_copy_client.cc: http://gerrit.cloudera.org:8080/#/c/19620/6/src/kudu/tserver/tablet_copy_client.cc@928 PS6, Line 928: options Missing to set is_sensitive? http://gerrit.cloudera.org:8080/#/c/19620/6/src/kudu/tserver/tablet_copy_client.cc@935 PS6, Line 935: while It is necessary to add a test to cover the case of the loop actually iterate several times. http://gerrit.cloudera.org:8080/#/c/19620/6/src/kudu/tserver/tablet_copy_service.cc File src/kudu/tserver/tablet_copy_service.cc: http://gerrit.cloudera.org:8080/#/c/19620/6/src/kudu/tserver/tablet_copy_service.cc@208 PS6, Line 208: mutable_superblock The service side is easy to know if the superblock size is larger than --rpc_max_message_size, if it is, the copy session will fail, why not copy it by pieces automatically in this case? http://gerrit.cloudera.org:8080/#/c/19620/6/src/kudu/tserver/tablet_copy_service.cc@380 PS6, Line 380: if (data_id.type() == DataIdPB::SUPER_BLOCK) { nit: It would be nice to add some comments. http://gerrit.cloudera.org:8080/#/c/19620/6/src/kudu/tserver/tablet_copy_source_session.cc File src/kudu/tserver/tablet_copy_source_session.cc: http://gerrit.cloudera.org:8080/#/c/19620/6/src/kudu/tserver/tablet_copy_source_session.cc@324 PS6, Line 324: FileExists Also remove the file when the session closed. http://gerrit.cloudera.org:8080/#/c/19620/6/src/kudu/tserver/tablet_copy_source_session.cc@324 PS6, Line 324: if (fs_manager_->env()->FileExists(sb_file)) { : RETURN_NOT_OK(fs_manager_->env()->DeleteFile(sb_file)); : } Is it possible 2 tservers copy a same tablet fron the source tserver concurrently? Delete the unique file may interrupt the other copy session. -- To view, visit http://gerrit.cloudera.org:8080/19620 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a7c212784383e247566a4701965e2897de83303 Gerrit-Change-Number: 19620 Gerrit-PatchSet: 6 Gerrit-Owner: Wang Xixu <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wang Xixu <[email protected]> Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Comment-Date: Fri, 30 Jun 2023 06:40:41 +0000 Gerrit-HasComments: Yes
