Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/19620 )
Change subject: KUDU-3459 Download superblock piece by piece ...................................................................... Patch Set 4: (15 comments) http://gerrit.cloudera.org:8080/#/c/19620/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19620/2//COMMIT_MSG@16 PS2, Line 16: wnload superblock piece by piece. A new > 1. If the superblock is larger than --rpc_max_message_size, the copy proced 1. You mean if found the error, transfer into using tablet_copy_download_superblock_in_batch? 2.This problem has been solved. Please see src/kudu/tserver/tablet_copy_source_session.cc#DetermineReadLength() http://gerrit.cloudera.org:8080/#/c/19620/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19620/3//COMMIT_MSG@10 PS3, Line 10: it may over the : size of the rpc maximum message size defined as: : --rpc_max_message_size. > Describe what happend then. Done http://gerrit.cloudera.org:8080/#/c/19620/3/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/19620/3/src/kudu/tools/kudu-tool-test.cc@9158 PS3, Line 9158: TestDownloadSuperblockInBatch > It seems this test only checked the function of 'local_replica copy_from_re It has been checked. Please see line 9176. http://gerrit.cloudera.org:8080/#/c/19620/3/src/kudu/tools/kudu-tool-test.cc@9164 PS3, Line 9164: tablet_ids_to_copy > Check it's not empty. Done http://gerrit.cloudera.org:8080/#/c/19620/3/src/kudu/tools/kudu-tool-test.cc@9177 PS3, Line 9177: r > nit: remove the space Done http://gerrit.cloudera.org:8080/#/c/19620/3/src/kudu/tools/tool_action_local_replica.cc File src/kudu/tools/tool_action_local_replica.cc: http://gerrit.cloudera.org:8080/#/c/19620/3/src/kudu/tools/tool_action_local_replica.cc@1370 PS3, Line 1370: .AddOptionalParameter("tablet_copy_download_threads_nums_per_session") : .AddOptionalParameter("num_threads") : .AddOptionalParameter("tablet_copy_download_superblock_in_batch") > Could you please reorder them in lexicographic order? Done http://gerrit.cloudera.org:8080/#/c/19620/3/src/kudu/tserver/tablet_copy_client.cc File src/kudu/tserver/tablet_copy_client.cc: http://gerrit.cloudera.org:8080/#/c/19620/3/src/kudu/tserver/tablet_copy_client.cc@325 PS3, Line 325: if (!superblock_path.empty() && : dst_fs_manager_->env()->FileExists(superblock_path)) { > When the code execute here, this is always true, right? No. Superblock maybe download failed. Then the file of superblock_path will not exist. http://gerrit.cloudera.org:8080/#/c/19620/3/src/kudu/tserver/tablet_copy_client.cc@328 PS3, Line 328: o > nit: Use Substitute instead to keep the style. Done http://gerrit.cloudera.org:8080/#/c/19620/3/src/kudu/tserver/tablet_copy_client.cc@334 PS3, Line 334: path, > Is there any tests verify that encryption is enabled? Superblock is encrypted default. Please see src/kudu/tablet/tablet_metadata.cc#ReadSuperBlockFromDisk(). http://gerrit.cloudera.org:8080/#/c/19620/3/src/kudu/tserver/tablet_copy_client.cc@917 PS3, Line 917: > Move it closer to the place where use it. Done http://gerrit.cloudera.org:8080/#/c/19620/3/src/kudu/tserver/tablet_copy_client.cc@928 PS3, Line 928: > Add a MakeScopedCleanup to delete the file if download failed. I have added a MakeScopedCleanup to delete the file, please see line 324. http://gerrit.cloudera.org:8080/#/c/19620/3/src/kudu/tserver/tablet_copy_client.cc@929 PS3, Line 929: ons.is_sensit > Set RWFileOptions.is_sensitive = true, tablet metadata is sensitive. Done http://gerrit.cloudera.org:8080/#/c/19620/3/src/kudu/tserver/tablet_copy_client.cc@944 PS3, Line 944: > nit: error Done http://gerrit.cloudera.org:8080/#/c/19620/3/src/kudu/tserver/tablet_copy_client.cc@950 PS3, Line 950: // Write the data. > Is there any tests you want to inject this flag? No. I will remove this. http://gerrit.cloudera.org:8080/#/c/19620/3/src/kudu/tserver/tablet_copy_source_session.cc File src/kudu/tserver/tablet_copy_source_session.cc: http://gerrit.cloudera.org:8080/#/c/19620/3/src/kudu/tserver/tablet_copy_source_session.cc@322 PS3, Line 322: string path = fs_manager_->GetTabletMetadataPath(tablet_id_); > Reading superblock from disk could not resolve the case of schema change, b But how to read tablet_superblock_ piece by piece? -- 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: 4 Gerrit-Owner: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Yingchun Lai <laiyingc...@apache.org> Gerrit-Comment-Date: Tue, 27 Jun 2023 08:30:41 +0000 Gerrit-HasComments: Yes
