Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/19370 )
Change subject: [tools] Support to dump rowset primary key bounds and in readable format ...................................................................... Patch Set 1: (11 comments) http://gerrit.cloudera.org:8080/#/c/19370/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19370/1//COMMIT_MSG@14 PS1, Line 14: by setting flag --dump_primary_key_bounds_only. > Could you explain in which situation it is need to dump primary key bound o There may be hundreds and thousands of rows in the rowset, dump them all is heavy and hard to read. In the case we only want to know the bounds, it's helpful. http://gerrit.cloudera.org:8080/#/c/19370/1/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/19370/1/src/kudu/tools/kudu-tool-test.cc@2854 PS1, Line 2854: rowsets primary key > rowsets' primary keys Done http://gerrit.cloudera.org:8080/#/c/19370/1/src/kudu/tools/kudu-tool-test.cc@2856 PS1, Line 2856: --nodump_all_columns " : "--nodump_metadata --dump_readable_primary_key=true > nit: this looks like a strange mix of gflag notations. Could you update th Done http://gerrit.cloudera.org:8080/#/c/19370/1/src/kudu/tools/kudu-tool-test.cc@2860 PS1, Line 2860: SCOPED_TRACE(stdout); > If using SCOPED_TRACE(), then please enclose the sub-scenario in its own sc Done http://gerrit.cloudera.org:8080/#/c/19370/1/src/kudu/tools/kudu-tool-test.cc@2869 PS1, Line 2869: rowsets primary key bounds > rowsets' primary key bounds Done http://gerrit.cloudera.org:8080/#/c/19370/1/src/kudu/tools/kudu-tool-test.cc@2871 PS1, Line 2871: --nodump_all_columns " : "--nodump_metadata --dump_readable_primary_key=true " : "--dump_primary_key_bounds_only=true > nit: ditto for the gflag notations used. Done http://gerrit.cloudera.org:8080/#/c/19370/1/src/kudu/tools/tool_action_local_replica.cc File src/kudu/tools/tool_action_local_replica.cc: http://gerrit.cloudera.org:8080/#/c/19370/1/src/kudu/tools/tool_action_local_replica.cc@107 PS1, Line 107: dump_readable_primary_key > If that's about the format of the keys, maybe name this flag Done http://gerrit.cloudera.org:8080/#/c/19370/1/src/kudu/tools/tool_action_local_replica.cc@201 PS1, Line 201: when set --dump_all_columns. > when --dump_all_columns is enabled Done http://gerrit.cloudera.org:8080/#/c/19370/1/src/kudu/tools/tool_action_local_replica.cc@207 PS1, Line 207: when set --dump_all_columns. > when --dump_all_columns is enabled Done http://gerrit.cloudera.org:8080/#/c/19370/1/src/kudu/tools/tool_action_local_replica.cc@792 PS1, Line 792: string DumpRow(const Schema& key_proj, const RowBlockRow& row, faststring* key) { > "faststring* key" is not need to be posted from outside. How about defining Reuse it to reduce memory re-allocate. http://gerrit.cloudera.org:8080/#/c/19370/1/src/kudu/tools/tool_action_local_replica.cc@1033 PS1, Line 1033: if (FLAGS_dump_primary_key_bounds_only) { > Why this code outside of the loop: Line1018? Because we will only print the min & max PK, so find them in the loop, and emplace them into the list we will print later. -- To view, visit http://gerrit.cloudera.org:8080/19370 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9757104f96648c3c83b931369f0e377d8dc2079a Gerrit-Change-Number: 19370 Gerrit-PatchSet: 1 Gerrit-Owner: Yingchun Lai <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: KeDeng <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wang Xixu <[email protected]> Gerrit-Reviewer: Yifan Zhang <[email protected]> Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Reviewer: Yuqi Du <[email protected]> Gerrit-Comment-Date: Sat, 24 Dec 2022 12:18:12 +0000 Gerrit-HasComments: Yes
