Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/19327 )
Change subject: [Tool] Compact metadata file for old versions of Kudu ...................................................................... Patch Set 5: (15 comments) http://gerrit.cloudera.org:8080/#/c/19327/5/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: http://gerrit.cloudera.org:8080/#/c/19327/5/src/kudu/fs/log_block_manager.cc@1210 PS5, Line 1210: Status LogBlockContainer::ProcessRecord( Is it possible to reuse exist version of LogBlockContainer::ProcessRecord? http://gerrit.cloudera.org:8080/#/c/19327/5/src/kudu/fs/log_block_manager.cc@2688 PS5, Line 2688: return Status::RuntimeError("Offset file is too big, that is abnormal."); nit: Add file path in log. http://gerrit.cloudera.org:8080/#/c/19327/5/src/kudu/fs/log_block_manager.cc@2695 PS5, Line 2695: LOG(INFO) << "Read container: " << container_name << " offset: " << slice; nit: Maybe a verbos level is enough? http://gerrit.cloudera.org:8080/#/c/19327/5/src/kudu/fs/log_block_manager.cc@2697 PS5, Line 2697: return Status::RuntimeError("Offset is not a number."); nit: Add file path in log. Same to other places. http://gerrit.cloudera.org:8080/#/c/19327/5/src/kudu/fs/log_block_manager.cc@2700 PS5, Line 2700: Substitute nit: use StrCat and JoinPathSegments http://gerrit.cloudera.org:8080/#/c/19327/5/src/kudu/fs/log_block_manager.cc@2706 PS5, Line 2706: c nit: align to 'r' of "rw_opts". http://gerrit.cloudera.org:8080/#/c/19327/5/src/kudu/fs/log_block_manager.cc@2711 PS5, Line 2711: old Should be "new"? http://gerrit.cloudera.org:8080/#/c/19327/5/src/kudu/fs/log_block_manager.cc@2713 PS5, Line 2713: Substitute nit: use StrCat and JoinPathSegments http://gerrit.cloudera.org:8080/#/c/19327/5/src/kudu/fs/log_block_manager.cc@2723 PS5, Line 2723: Read Write? http://gerrit.cloudera.org:8080/#/c/19327/5/src/kudu/fs/log_block_manager.cc@2735 PS5, Line 2735: !PREDICT_TRUE(read_status.IsEndOfFile()) : && !PREDICT_TRUE(read_status.IsIncomplete()) PREDICT_FALSE(!read_status.IsEndOfFile() && !read_status.IsIncomplete()) ? http://gerrit.cloudera.org:8080/#/c/19327/5/src/kudu/fs/log_block_manager.cc@2739 PS5, Line 2739: RETURN_NOT_OK_PREPEND(compacted_pb_writer.Sync(), : "sync compacted file failed."); : RETURN_NOT_OK_PREPEND(compacted_pb_writer.Close(), : "close compacted file failed."); : RETURN_NOT_OK_PREPEND(old_metadata_pb_reader.Close(), : "close old metadata file failed."); nit: the width is less than 100 if in one line? http://gerrit.cloudera.org:8080/#/c/19327/5/src/kudu/fs/log_block_manager.cc@2747 PS5, Line 2747: b nit: alignment http://gerrit.cloudera.org:8080/#/c/19327/5/src/kudu/fs/log_block_manager.cc@2751 PS5, Line 2751: o nit: alignment http://gerrit.cloudera.org:8080/#/c/19327/5/src/kudu/fs/log_block_manager.cc@2775 PS5, Line 2775: Substitute nit: use StrCat and JoinPathSegments http://gerrit.cloudera.org:8080/#/c/19327/5/src/kudu/tools/tool_action_fs.cc File src/kudu/tools/tool_action_fs.cc: http://gerrit.cloudera.org:8080/#/c/19327/5/src/kudu/tools/tool_action_fs.cc@884 PS5, Line 884: RETURN_NOT_OK(fs_manager.Open()); Give a hint message to shutdown the server if lock instance file failed in "merge" operation. -- To view, visit http://gerrit.cloudera.org:8080/19327 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id61cb6c88d7a093a8448e2497639f5c5c094efa4 Gerrit-Change-Number: 19327 Gerrit-PatchSet: 5 Gerrit-Owner: Wang Xixu <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wang Xixu <[email protected]> Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Comment-Date: Wed, 15 Mar 2023 14:21:27 +0000 Gerrit-HasComments: Yes
