Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17475 )
Change subject: [txns] add fields for create and update times ...................................................................... Patch Set 1: Code-Review+1 (4 comments) LGTM, just few nits. http://gerrit.cloudera.org:8080/#/c/17475/1/src/kudu/tools/tool_action_txn.cc File src/kudu/tools/tool_action_txn.cc: http://gerrit.cloudera.org:8080/#/c/17475/1/src/kudu/tools/tool_action_txn.cc@118 PS1, Line 118: transition Maybe, keep this uniform and use 'transition' instead of 'update' in the other parts on the code? http://gerrit.cloudera.org:8080/#/c/17475/1/src/kudu/transactions/transactions.proto File src/kudu/transactions/transactions.proto: http://gerrit.cloudera.org:8080/#/c/17475/1/src/kudu/transactions/transactions.proto@78 PS1, Line 78: update In tools code, this is named 'transition'. Maybe, use 'transition' here as well? http://gerrit.cloudera.org:8080/#/c/17475/1/src/kudu/transactions/txn_status_manager.cc File src/kudu/transactions/txn_status_manager.cc: http://gerrit.cloudera.org:8080/#/c/17475/1/src/kudu/transactions/txn_status_manager.cc@991 PS1, Line 991: & Just curious: is there any special consideration to have this as a reference, not a value? As I understand, for x86_64 there shouldn't be much difference since it's a 64-bit number anyway. If so, why not to store the result of time() as a value? http://gerrit.cloudera.org:8080/#/c/17475/1/src/kudu/transactions/txn_status_manager.cc@1001 PS1, Line 1001: start_timestamp Does it make sense to update last_update_timestamp as well? -- To view, visit http://gerrit.cloudera.org:8080/17475 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4d4f77b4c9b7fcc3f7f749b7f79a18a24be3ce4d Gerrit-Change-Number: 17475 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, 19 May 2021 01:38:53 +0000 Gerrit-HasComments: Yes
