Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18374 )
Change subject: [tool] Add tool to copy replica from local filesystem ...................................................................... Patch Set 4: (13 comments) http://gerrit.cloudera.org:8080/#/c/18374/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/18374/3//COMMIT_MSG@7 PS3, Line 7: tool > It would be great to provide an example of the usage for the tool. Done http://gerrit.cloudera.org:8080/#/c/18374/3//COMMIT_MSG@9 PS3, Line 9: would > is Done http://gerrit.cloudera.org:8080/#/c/18374/3//COMMIT_MSG@10 PS3, Line 10: use > used Done http://gerrit.cloudera.org:8080/#/c/18374/3//COMMIT_MSG@10 PS3, Line 10: copy > copying Done http://gerrit.cloudera.org:8080/#/c/18374/3//COMMIT_MSG@11 PS3, Line 11: add > adding Done http://gerrit.cloudera.org:8080/#/c/18374/3//COMMIT_MSG@11 PS3, Line 11: disk drivers > data directories Done http://gerrit.cloudera.org:8080/#/c/18374/3//COMMIT_MSG@12 PS3, Line 12: ta from the > data directory Done http://gerrit.cloudera.org:8080/#/c/18374/3//COMMIT_MSG@12 PS3, Line 12: disk drivers > them Done http://gerrit.cloudera.org:8080/#/c/18374/3/src/kudu/tablet/tablet_replica.h File src/kudu/tablet/tablet_replica.h: http://gerrit.cloudera.org:8080/#/c/18374/3/src/kudu/tablet/tablet_replica.h@115 PS3, Line 115: Th > If this is a constructor for a limited audience, consider moving this into Done http://gerrit.cloudera.org:8080/#/c/18374/3/src/kudu/tserver/tablet_copy_client.h File src/kudu/tserver/tablet_copy_client.h: http://gerrit.cloudera.org:8080/#/c/18374/3/src/kudu/tserver/tablet_copy_client.h@316 PS3, Line 316: opy_me > nit: to be inline with tablet_copy_source_metrics_ and dst_fs_manager_, sho Done http://gerrit.cloudera.org:8080/#/c/18374/3/src/kudu/tserver/tablet_copy_client.cc File src/kudu/tserver/tablet_copy_client.cc: http://gerrit.cloudera.org:8080/#/c/18374/3/src/kudu/tserver/tablet_copy_client.cc@880 PS3, Line 880: case TabletCopyMode::REMOTE: : RETURN_NOT_OK_PREPEND(DownloadFile(data_id, writer.get()), : Substitute("Unable to download WAL segment with seq. number $0", : wal_segment_seqno)); : break; : case TabletCopyMode::LOCAL: : RETURN_NOT_OK_PREPEND(CopyFile(data_id, writer.get()), : Substitute("Unable to copy WAL segment with seq. number $0", : wal_segment_seqno)); : > nit: maybe, change this if/else to switch? Done http://gerrit.cloudera.org:8080/#/c/18374/3/src/kudu/tserver/tablet_copy_client.cc@972 PS3, Line 972: DataIdPB data_id; : data_id.set_type(DataIdPB::BLOCK); : old_block_id.CopyToPB(data_id.mutable_block_id()); : switch (mode_) { : case TabletCopyMode::REMOTE: : RETURN_NOT_OK_PREPEND(DownloadFile(data_id, block.get()), : Substitute("Unable to download block $0", old_block_id.ToString())); : > nit: maybe, change this if/else to switch? Done http://gerrit.cloudera.org:8080/#/c/18374/3/src/kudu/tserver/tablet_copy_client.cc@1067 PS3, Line 1067: segment_seqno, offset, client_maxlen, &data, > nit: maybe, move this one line below, and then there is no need to compute Done -- To view, visit http://gerrit.cloudera.org:8080/18374 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1dcafeaad900b66f297914760c54dba887874e95 Gerrit-Change-Number: 18374 Gerrit-PatchSet: 4 Gerrit-Owner: Yingchun Lai <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Comment-Date: Sat, 02 Apr 2022 07:30:23 +0000 Gerrit-HasComments: Yes
