Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17391 )
Change subject: wip [txns][tools] show details of a transaction ...................................................................... Patch Set 1: (6 comments) http://gerrit.cloudera.org:8080/#/c/17391/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/17391/1//COMMIT_MSG@19 PS1, Line 19: txns nit: I guess this should change to 'txn' due to the recent changes in the prior patches http://gerrit.cloudera.org:8080/#/c/17391/1//COMMIT_MSG@24 PS1, Line 24: is_aborted I'm not sure this should be in the default list of columns: this might be kind of deduced from the 'commit_ts' column, and when it's necessary to confirm, users can include the 'is_aborted' column explicitly, if needed. http://gerrit.cloudera.org:8080/#/c/17391/1//COMMIT_MSG@24 PS1, Line 24: tablet_id Is there a way to output table name/UUID? I guess that's just a convenience (it's possible to find that via using other tools), but it might be handy if it's necessary to see which tables the transaction has inserted data. http://gerrit.cloudera.org:8080/#/c/17391/1//COMMIT_MSG@24 PS1, Line 24: flushed_committed_mrs What is the significance of including this column in the list of default columns to output? I guess this should not be in the list of columns shown by default, but there should be an ability to include this column. http://gerrit.cloudera.org:8080/#/c/17391/1/src/kudu/tablet/tablet_metadata.h File src/kudu/tablet/tablet_metadata.h: http://gerrit.cloudera.org:8080/#/c/17391/1/src/kudu/tablet/tablet_metadata.h@331 PS1, Line 331: Returns nit: 'return' is usually used for the return value, maybe extend this a bit to point that the metadata is output into the 'pb' output parameter? http://gerrit.cloudera.org:8080/#/c/17391/1/src/kudu/tools/tool_action_txns.cc File src/kudu/tools/tool_action_txns.cc: http://gerrit.cloudera.org:8080/#/c/17391/1/src/kudu/tools/tool_action_txns.cc@253 PS1, Line 253: vector<string> col_names = { "tablet_id", "is_aborted", "flushed_committed_mrs", : "begin_commit_op_ts (datetime)", "commit_ts (datetime)" } Instead, should there be an option to specify which columns to output? As I already mention, 'is_aborted' and 'flushed_committed_mrs' are the columns which might be omitted from the default column list, but users should be able to include those, if needed. What do you think? -- To view, visit http://gerrit.cloudera.org:8080/17391 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9cc5c23b6b46ee75e38aaffe4773881a1ece7294 Gerrit-Change-Number: 17391 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 05 May 2021 04:06:22 +0000 Gerrit-HasComments: Yes
