You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Andrew Wong (Code Review)" <ge...@cloudera.org> on 2021/05/19 01:07:24 UTC
[kudu-CR] [txns] add fields for create and update times
Hello Alexey Serbin,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/17475
to review the following change.
Change subject: [txns] add fields for create and update times
......................................................................
[txns] add fields for create and update times
This patch adds fields to the transaction status entry for the start
time and the last update time. This is reported in both the `txn list`
and `txn show` tools.
$ ./bin/kudu txn list 0.0.0.0:8764 --included_states="*"
txn_id | user | state | commit_datetime | start_datetime | last_transition_datetime
--------+-------+-----------+-------------------------------+-------------------------------+-------------------------------
0 | awong | COMMITTED | Thu, 29 Apr 2021 18:54:56 GMT | <none> | <none>
1 | awong | COMMITTED | Thu, 29 Apr 2021 22:12:53 GMT | <none> | <none>
2 | awong | ABORTED | <none> | <none> | <none>
3 | awong | ABORTED | <none> | <none> | <none>
4 | awong | COMMITTED | Tue, 18 May 2021 07:41:34 GMT | Tue, 18 May 2021 07:41:34 GMT | Tue, 18 May 2021 07:41:34 GMT
5 | awong | ABORTED | <none> | Tue, 18 May 2021 07:42:22 GMT | Tue, 18 May 2021 07:42:56 GMT
6 | awong | COMMITTED | Tue, 18 May 2021 07:42:33 GMT | Tue, 18 May 2021 07:42:32 GMT | Tue, 18 May 2021 07:42:33 GMT
7 | awong | ABORTED | <none> | Tue, 18 May 2021 07:47:16 GMT | Tue, 18 May 2021 07:47:46 GMT
Change-Id: I4d4f77b4c9b7fcc3f7f749b7f79a18a24be3ce4d
---
M src/kudu/tools/kudu-txn-cli-test.cc
M src/kudu/tools/tool_action_txn.cc
M src/kudu/transactions/transactions.proto
M src/kudu/transactions/txn_status_manager.cc
M src/kudu/transactions/txn_status_tablet-test.cc
M src/kudu/transactions/txn_status_tablet.cc
M src/kudu/transactions/txn_status_tablet.h
7 files changed, 80 insertions(+), 35 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/75/17475/1
--
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: newchange
Gerrit-Change-Id: I4d4f77b4c9b7fcc3f7f749b7f79a18a24be3ce4d
Gerrit-Change-Number: 17475
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
[kudu-CR] [txns] add fields for create and update times
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 19 May 2021 01:38:53 +0000
Gerrit-HasComments: Yes
[kudu-CR] [txns] add fields for create and update times
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17475 )
Change subject: [txns] add fields for create and update times
......................................................................
[txns] add fields for create and update times
This patch adds fields to the transaction status entry for the start
time and the last update time. This is reported in both the `txn list`
and `txn show` tools.
$ ./bin/kudu txn list 0.0.0.0:8764 --included_states="*"
txn_id | user | state | commit_datetime | start_datetime | last_transition_datetime
--------+-------+-----------+-------------------------------+-------------------------------+-------------------------------
0 | awong | COMMITTED | Thu, 29 Apr 2021 18:54:56 GMT | <none> | <none>
1 | awong | COMMITTED | Thu, 29 Apr 2021 22:12:53 GMT | <none> | <none>
2 | awong | ABORTED | <none> | <none> | <none>
3 | awong | ABORTED | <none> | <none> | <none>
4 | awong | COMMITTED | Tue, 18 May 2021 07:41:34 GMT | Tue, 18 May 2021 07:41:34 GMT | Tue, 18 May 2021 07:41:34 GMT
5 | awong | ABORTED | <none> | Tue, 18 May 2021 07:42:22 GMT | Tue, 18 May 2021 07:42:56 GMT
6 | awong | COMMITTED | Tue, 18 May 2021 07:42:33 GMT | Tue, 18 May 2021 07:42:32 GMT | Tue, 18 May 2021 07:42:33 GMT
7 | awong | ABORTED | <none> | Tue, 18 May 2021 07:47:16 GMT | Tue, 18 May 2021 07:47:46 GMT
Change-Id: I4d4f77b4c9b7fcc3f7f749b7f79a18a24be3ce4d
Reviewed-on: http://gerrit.cloudera.org:8080/17475
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Tested-by: Kudu Jenkins
---
M src/kudu/tools/kudu-txn-cli-test.cc
M src/kudu/tools/tool_action_txn.cc
M src/kudu/transactions/transactions.proto
M src/kudu/transactions/txn_status_manager.cc
M src/kudu/transactions/txn_status_tablet-test.cc
M src/kudu/transactions/txn_status_tablet.cc
M src/kudu/transactions/txn_status_tablet.h
7 files changed, 87 insertions(+), 36 deletions(-)
Approvals:
Alexey Serbin: Looks good to me, approved
Kudu Jenkins: Verified
--
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: merged
Gerrit-Change-Id: I4d4f77b4c9b7fcc3f7f749b7f79a18a24be3ce4d
Gerrit-Change-Number: 17475
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [txns] add fields for create and update times
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17475 )
Change subject: [txns] add fields for create and update times
......................................................................
Patch Set 2:
(4 comments)
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 ot
Done
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: transi
> In tools code, this is named 'transition'. Maybe, use 'transition' here as
Done
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 referenc
Indeed, there isn't. Storing by value.
Done
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?
Done
--
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: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 19 May 2021 04:38:32 +0000
Gerrit-HasComments: Yes
[kudu-CR] [txns] add fields for create and update times
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 2: Code-Review+2
--
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: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 19 May 2021 04:47:19 +0000
Gerrit-HasComments: No
[kudu-CR] [txns] add fields for create and update times
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17475
to look at the new patch set (#2).
Change subject: [txns] add fields for create and update times
......................................................................
[txns] add fields for create and update times
This patch adds fields to the transaction status entry for the start
time and the last update time. This is reported in both the `txn list`
and `txn show` tools.
$ ./bin/kudu txn list 0.0.0.0:8764 --included_states="*"
txn_id | user | state | commit_datetime | start_datetime | last_transition_datetime
--------+-------+-----------+-------------------------------+-------------------------------+-------------------------------
0 | awong | COMMITTED | Thu, 29 Apr 2021 18:54:56 GMT | <none> | <none>
1 | awong | COMMITTED | Thu, 29 Apr 2021 22:12:53 GMT | <none> | <none>
2 | awong | ABORTED | <none> | <none> | <none>
3 | awong | ABORTED | <none> | <none> | <none>
4 | awong | COMMITTED | Tue, 18 May 2021 07:41:34 GMT | Tue, 18 May 2021 07:41:34 GMT | Tue, 18 May 2021 07:41:34 GMT
5 | awong | ABORTED | <none> | Tue, 18 May 2021 07:42:22 GMT | Tue, 18 May 2021 07:42:56 GMT
6 | awong | COMMITTED | Tue, 18 May 2021 07:42:33 GMT | Tue, 18 May 2021 07:42:32 GMT | Tue, 18 May 2021 07:42:33 GMT
7 | awong | ABORTED | <none> | Tue, 18 May 2021 07:47:16 GMT | Tue, 18 May 2021 07:47:46 GMT
Change-Id: I4d4f77b4c9b7fcc3f7f749b7f79a18a24be3ce4d
---
M src/kudu/tools/kudu-txn-cli-test.cc
M src/kudu/tools/tool_action_txn.cc
M src/kudu/transactions/transactions.proto
M src/kudu/transactions/txn_status_manager.cc
M src/kudu/transactions/txn_status_tablet-test.cc
M src/kudu/transactions/txn_status_tablet.cc
M src/kudu/transactions/txn_status_tablet.h
7 files changed, 87 insertions(+), 36 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/75/17475/2
--
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: newpatchset
Gerrit-Change-Id: I4d4f77b4c9b7fcc3f7f749b7f79a18a24be3ce4d
Gerrit-Change-Number: 17475
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)