[kudu-CR] KUDU-2612: add PartitionLock and ScopedPartitionLock
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Bankim Bhavsar, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/17097 to look at the new patch set (#10). Change subject: KUDU-2612: add PartitionLock and ScopedPartitionLock .. KUDU-2612: add PartitionLock and ScopedPartitionLock This patch introduces a coarse grained partition level lock, PartitionLock to prevent dirty writes for multi-rows transaction. A partition lock can only be held by a single transaction (or write op) at a time, the same transaction can acquire the lock for multiple times. To prevent deadlock, 'wait-die' scheme is used, which if the transaction requires a lock held by another transaction, 1) abort the transaction immediately if it has a higher txn ID than the other one. 2) Otherwise, retry the write op (or participant op) of the transaction. A ScopedPartitionLock is also introduced to automatically manage the lifecycle of a PartitionLock. Change-Id: I158115739ce3e7cfb77bbcb854e834336c1256b1 --- M src/kudu/tablet/lock_manager-test.cc M src/kudu/tablet/lock_manager.cc M src/kudu/tablet/lock_manager.h M src/kudu/tserver/tserver.proto 4 files changed, 470 insertions(+), 13 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/97/17097/10 -- To view, visit http://gerrit.cloudera.org:8080/17097 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I158115739ce3e7cfb77bbcb854e834336c1256b1 Gerrit-Change-Number: 17097 Gerrit-PatchSet: 10 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2612: add PartitionLock and ScopedPartitionLock
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/17097 )
Change subject: KUDU-2612: add PartitionLock and ScopedPartitionLock
..
Patch Set 10:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/17097/6/src/kudu/tablet/lock_manager.h
File src/kudu/tablet/lock_manager.h:
http://gerrit.cloudera.org:8080/#/c/17097/6/src/kudu/tablet/lock_manager.h@193
PS6, Line 193:
> Yeah, we updated the reference in the constructor, so if copying field-to-f
Ah makes sense. Sorry I missed that! Thanks a lot for pointing out.
http://gerrit.cloudera.org:8080/#/c/17097/9/src/kudu/tablet/lock_manager.cc
File src/kudu/tablet/lock_manager.cc:
http://gerrit.cloudera.org:8080/#/c/17097/9/src/kudu/tablet/lock_manager.cc@460
PS9, Line 460: (MonoDelta::FromMilliseconds(25
> This seems to be pretty short timeout. Why just a millisecond? Our timeou
Ack
http://gerrit.cloudera.org:8080/#/c/17097/9/src/kudu/tablet/lock_manager.cc@474
PS9, Line 474: if (!partition_lock_) {
> What if by this time the lock has been released by the other party? Will i
Done
--
To view, visit http://gerrit.cloudera.org:8080/17097
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I158115739ce3e7cfb77bbcb854e834336c1256b1
Gerrit-Change-Number: 17097
Gerrit-PatchSet: 10
Gerrit-Owner: Hao Hao
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Bankim Bhavsar
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 12 Mar 2021 21:33:57 +
Gerrit-HasComments: Yes
[kudu-CR] [test][master] KUDU-3249 Recover dead master at the same HostPort
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17126 ) Change subject: [test][master] KUDU-3249 Recover dead master at the same HostPort .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/17126 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iac65201fd39ede5e918025ca7cf6a852a92d6eec Gerrit-Change-Number: 17126 Gerrit-PatchSet: 2 Gerrit-Owner: Bankim Bhavsar Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 12 Mar 2021 19:21:54 + Gerrit-HasComments: No
[kudu-CR] [tool] Add the missing "member type" in help text of list master CLI
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/17175 )
Change subject: [tool] Add the missing "member_type" in help text of list
master CLI
..
Patch Set 1: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17175/1/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/17175/1/src/kudu/tools/kudu-tool-test.cc@3347
PS1, Line 3347: VOTER
> In what case would a single master configuration have a master that's not a
The code is a separate matter -- this is a test. If an assertion is in the
test, it's an independent check.
The point was that ASSERT_STR_CONTAINS("VOTER") matches in both "NON_VOTER" and
"VOTER" cases. If there is a simple way to make sure that's matches only the
latter -- why not to enforce it. If it's really hard, I'm fine to leave this
as is.
--
To view, visit http://gerrit.cloudera.org:8080/17175
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I00f349687866269987691f18aa85e5bf02d5df81
Gerrit-Change-Number: 17175
Gerrit-PatchSet: 1
Gerrit-Owner: Bankim Bhavsar
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Bankim Bhavsar
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 12 Mar 2021 19:18:29 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612: add PartitionLock and ScopedPartitionLock
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17097 ) Change subject: KUDU-2612: add PartitionLock and ScopedPartitionLock .. Patch Set 9: (3 comments) http://gerrit.cloudera.org:8080/#/c/17097/6/src/kudu/tablet/lock_manager.h File src/kudu/tablet/lock_manager.h: http://gerrit.cloudera.org:8080/#/c/17097/6/src/kudu/tablet/lock_manager.h@193 PS6, Line 193: > I don't think so, since we only update the reference at the constructor. Bu Yeah, we updated the reference in the constructor, so if copying field-to-field, then the Release() method is called twice: once from the destructor of the original object, and another time from the field-to-field copied object, and the latter is unexpected extra decrease of the reference count. Thank you for addressing this! http://gerrit.cloudera.org:8080/#/c/17097/9/src/kudu/tablet/lock_manager.cc File src/kudu/tablet/lock_manager.cc: http://gerrit.cloudera.org:8080/#/c/17097/9/src/kudu/tablet/lock_manager.cc@460 PS9, Line 460: (MonoDelta::FromMilliseconds(1) This seems to be pretty short timeout. Why just a millisecond? Our timeout aren't strict since this is not a real-time OS anyways. I'd give it 100ms or 250ms at least -- that way it would be much less useless CPU burn and context switching. http://gerrit.cloudera.org:8080/#/c/17097/9/src/kudu/tablet/lock_manager.cc@474 PS9, Line 474: *code = id > partition_lock_->txn_id().value() ? What if by this time the lock has been released by the other party? Will it just crash here with SIGSEGV? -- To view, visit http://gerrit.cloudera.org:8080/17097 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I158115739ce3e7cfb77bbcb854e834336c1256b1 Gerrit-Change-Number: 17097 Gerrit-PatchSet: 9 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Fri, 12 Mar 2021 19:11:21 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612: add PartitionLock and ScopedPartitionLock
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/17097 )
Change subject: KUDU-2612: add PartitionLock and ScopedPartitionLock
..
Patch Set 9:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/17097/8/src/kudu/tablet/lock_manager-test.cc
File src/kudu/tablet/lock_manager-test.cc:
http://gerrit.cloudera.org:8080/#/c/17097/8/src/kudu/tablet/lock_manager-test.cc@214
PS8, Line 214: , t
> We need to pass 't' as a copy.
Done
http://gerrit.cloudera.org:8080/#/c/17097/8/src/kudu/tablet/lock_manager.cc
File src/kudu/tablet/lock_manager.cc:
http://gerrit.cloudera.org:8080/#/c/17097/8/src/kudu/tablet/lock_manager.cc@461
PS8, Line 461: MonoDelta elapsed(MonoTime::Now() - start);
: if (elapsed > timeout) {
> nit: just store elapsed and inline MonoTime::Now()?
Done
--
To view, visit http://gerrit.cloudera.org:8080/17097
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I158115739ce3e7cfb77bbcb854e834336c1256b1
Gerrit-Change-Number: 17097
Gerrit-PatchSet: 9
Gerrit-Owner: Hao Hao
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Bankim Bhavsar
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 12 Mar 2021 18:14:19 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612: add PartitionLock and ScopedPartitionLock
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Bankim Bhavsar, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/17097 to look at the new patch set (#9). Change subject: KUDU-2612: add PartitionLock and ScopedPartitionLock .. KUDU-2612: add PartitionLock and ScopedPartitionLock This patch introduces a coarse grained partition level lock, PartitionLock to prevent dirty writes for multi-rows transaction. A partition lock can only be held by a single transaction (or write op) at a time, the same transaction can acquire the lock for multiple times. To prevent deadlock, 'wait-die' scheme is used, which if the transaction requires a lock held by another transaction, 1) abort the transaction immediately if it has a higher txn ID than the other one. 2) Otherwise, retry the write op (or participant op) of the transaction. A ScopedPartitionLock is also introduced to automatically manage the lifecycle of a PartitionLock. Change-Id: I158115739ce3e7cfb77bbcb854e834336c1256b1 --- M src/kudu/tablet/lock_manager-test.cc M src/kudu/tablet/lock_manager.cc M src/kudu/tablet/lock_manager.h M src/kudu/tserver/tserver.proto 4 files changed, 464 insertions(+), 13 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/97/17097/9 -- To view, visit http://gerrit.cloudera.org:8080/17097 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I158115739ce3e7cfb77bbcb854e834336c1256b1 Gerrit-Change-Number: 17097 Gerrit-PatchSet: 9 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [tool] Add the missing "member type" in help text of list master CLI
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/17175 ) Change subject: [tool] Add the missing "member_type" in help text of list master CLI .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/17175/1/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/17175/1/src/kudu/tools/kudu-tool-test.cc@3347 PS1, Line 3347: VOTER > If it were a non-voter, what should be the output? Is this supposed to fai In what case would a single master configuration have a master that's not a VOTER or not a LEADER? >From the code it looks pretty straightforward. https://github.com/apache/kudu/blob/master/src/kudu/master/sys_catalog.cc#L351 Or would you rather prefer we check for all possible enum values because in this case we don't really care what specific value it is as long as it is one of the valid enum values? -- To view, visit http://gerrit.cloudera.org:8080/17175 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I00f349687866269987691f18aa85e5bf02d5df81 Gerrit-Change-Number: 17175 Gerrit-PatchSet: 1 Gerrit-Owner: Bankim Bhavsar Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 12 Mar 2021 17:48:18 + Gerrit-HasComments: Yes
[kudu-CR] [master] Remove FLAGS master consensus allow status msg for failed peer
Bankim Bhavsar has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17180 Change subject: [master] Remove FLAGS_master_consensus_allow_status_msg_for_failed_peer .. [master] Remove FLAGS_master_consensus_allow_status_msg_for_failed_peer FLAGS_master_consensus_allow_status_msg_for_failed_peer goes hand-in-hand with FLAGS_master_support_change_config so removing it and using the common FLAGS_master_support_change_config instead. This reduces the need for additional validation when orchestrating the add master flow. Change-Id: I72b37ba9ff3c48bce532ad3d037d341c2c36bb9f --- M src/kudu/master/dynamic_multi_master-test.cc M src/kudu/master/master_service.cc M src/kudu/master/sys_catalog.cc 3 files changed, 10 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/80/17180/1 -- To view, visit http://gerrit.cloudera.org:8080/17180 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I72b37ba9ff3c48bce532ad3d037d341c2c36bb9f Gerrit-Change-Number: 17180 Gerrit-PatchSet: 1 Gerrit-Owner: Bankim Bhavsar
[kudu-CR] KUDU-2612: acquire and release partition lock
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/17159 )
Change subject: KUDU-2612: acquire and release partition lock
..
Patch Set 4:
(11 comments)
Addressed partial comments, will try to address the rest later.
http://gerrit.cloudera.org:8080/#/c/17159/3/src/kudu/client/batcher.cc
File src/kudu/client/batcher.cc:
http://gerrit.cloudera.org:8080/#/c/17159/3/src/kudu/client/batcher.cc@530
PS3, Line 530: //For example, Status::IllegalState()
originated from
: //TabletServerErrorPB::TXN_ILLEGAL_STATE
response and
:
> +1
No, this is not about TXN_LOCKED_RETRY. It is about error handling of
TxnOpDispatcher. IIUC, TxnOpDispatcher currently only wraps the status of the
call but not the error code. So TXN_LOCKED_ABORT is retried even it should not
be (and handled by L510) for transactional ones. But for transactional ones it
is taking care by L510.
For TXN_LOCKED_RETRY_OP we return it with ServiceUnavailable Status. so now it
is handled in L473. But added the specific checking there.
http://gerrit.cloudera.org:8080/#/c/17159/3/src/kudu/integration-tests/txn_participant-itest.cc
File src/kudu/integration-tests/txn_participant-itest.cc:
http://gerrit.cloudera.org:8080/#/c/17159/3/src/kudu/integration-tests/txn_participant-itest.cc@813
PS3, Line 813: TEST_F(TxnParticipantITest,
TestTxnSystemClientBeginTxnLockAbort) {
> Do you mind adding a scenario with calling ParticipateInTransaction() for t
Done
http://gerrit.cloudera.org:8080/#/c/17159/3/src/kudu/integration-tests/txn_participant-itest.cc@833
PS3, Line 833: s.IsAborted
> Hmm, interesting: what actor is to abort the transaction? I'm curious abou
Sure, will add a comment. Now TxnManager will just pass the error back to the
client, which will cause the op to fail. And the client need to figure out what
to do with the error (instead of TxnManager silently abort the transaction). Or
do you have other suggestions?
http://gerrit.cloudera.org:8080/#/c/17159/3/src/kudu/integration-tests/txn_participant-itest.cc@930
PS3, Line 930: FLAGS_enable_txn_partition_lock = false;
> It would be great if you could add a note explaining that this scenario bec
Done
http://gerrit.cloudera.org:8080/#/c/17159/3/src/kudu/integration-tests/txn_write_ops-itest.cc
File src/kudu/integration-tests/txn_write_ops-itest.cc:
http://gerrit.cloudera.org:8080/#/c/17159/3/src/kudu/integration-tests/txn_write_ops-itest.cc@1046
PS3, Line 1046:
> nit: for completeness sake, maybe commit `second_txn` and ensure we no long
Done
http://gerrit.cloudera.org:8080/#/c/17159/3/src/kudu/tablet/ops/participant_op.cc
File src/kudu/tablet/ops/participant_op.cc:
http://gerrit.cloudera.org:8080/#/c/17159/3/src/kudu/tablet/ops/participant_op.cc@123
PS3, Line 123: }
> Probably, we want to distinguish in the trace whether this is was a no-op o
Done
http://gerrit.cloudera.org:8080/#/c/17159/3/src/kudu/tablet/ops/write_op.cc
File src/kudu/tablet/ops/write_op.cc:
http://gerrit.cloudera.org:8080/#/c/17159/3/src/kudu/tablet/ops/write_op.cc@67
PS3, Line 67: DEFINE_int32(tablet_inject_latency_on_apply_write_op_ms, 0,
: "How much latency to inject when a write op is
applied. "
: "For testing only!");
: TAG_FLAG(tablet_inject_latency_on_apply_write_op_ms, un
> Could we just reuse --enable_txn_partition_lock? When might we want to set
Done
http://gerrit.cloudera.org:8080/#/c/17159/3/src/kudu/tablet/ops/write_op.cc@569
PS3, Line 569: }
> LOG(DFATAL) or LOG(FATAL)?
Done
http://gerrit.cloudera.org:8080/#/c/17159/3/src/kudu/tablet/tablet.h
File src/kudu/tablet/tablet.h:
http://gerrit.cloudera.org:8080/#/c/17159/3/src/kudu/tablet/tablet.h@166
PS3, Line 166: return
: // 'TXN_LOCKED_ABORT' or 'TXN_LOCKED_RETRY_OP' error
> How is it to return those if the return type of this method is Status?
Ah, sorry for not being precise. Will update the comment.
http://gerrit.cloudera.org:8080/#/c/17159/3/src/kudu/tablet/txn_participant.h
File src/kudu/tablet/txn_participant.h:
http://gerrit.cloudera.org:8080/#/c/17159/3/src/kudu/tablet/txn_participant.h@220
PS3, Line 220: partition_lock_ = std::move(partition_lock);
> nit: maybe DCHECK or CHECK that we own the lock?
Done
http://gerrit.cloudera.org:8080/#/c/17159/3/src/kudu/transactions/participant_rpc.cc
File src/kudu/transactions/participant_rpc.cc:
http://gerrit.cloudera.org:8080/#/c/17159/3/src/kudu/transactions/participant_rpc.cc@180
PS3, Line 180: case TabletServerErrorPB::TXN_ILLEGAL_STATE
> Can we also explicitly handle TXN_LOCKED_RETRY_OP?
Done
--
To view, visit http://gerrit.cloudera.org:8080/17159
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id:
[kudu-CR] KUDU-2612: acquire and release partition lock
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/17159 to look at the new patch set (#4). Change subject: KUDU-2612: acquire and release partition lock .. KUDU-2612: acquire and release partition lock This patch introduces the logic for actual acquiring and releasing the partition lock for transactions and non-transactional operations. For each transaction, we try to acquire the partition lock in the ParticipantOp prepare phase of BEGIN_TXN and release the lock when COMMIT_TXN or ABORT_TXN is applied. Moreover, we take the parition lock for non-transactional operations as well to ensure we don’t have duplicate keys. If the partition lock cannot be acquired, the transaction (or write op) will be aborted or retried. Change-Id: If26733cae16810f3b3afd1fd05dcb984e6366939 --- M src/kudu/client/batcher.cc M src/kudu/integration-tests/fuzz-itest.cc M src/kudu/integration-tests/txn_commit-itest.cc M src/kudu/integration-tests/txn_participant-itest.cc M src/kudu/integration-tests/txn_write_ops-itest.cc M src/kudu/tablet/ops/participant_op.cc M src/kudu/tablet/ops/participant_op.h M src/kudu/tablet/ops/write_op.cc M src/kudu/tablet/ops/write_op.h M src/kudu/tablet/tablet.cc M src/kudu/tablet/tablet.h M src/kudu/tablet/txn_participant-test.cc M src/kudu/tablet/txn_participant.h M src/kudu/transactions/participant_rpc.cc 14 files changed, 465 insertions(+), 14 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/59/17159/4 -- To view, visit http://gerrit.cloudera.org:8080/17159 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If26733cae16810f3b3afd1fd05dcb984e6366939 Gerrit-Change-Number: 17159 Gerrit-PatchSet: 4 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2612: add PartitionLock and ScopedPartitionLock
Andrew Wong has posted comments on this change. (
http://gerrit.cloudera.org:8080/17097 )
Change subject: KUDU-2612: add PartitionLock and ScopedPartitionLock
..
Patch Set 8:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/17097/8/src/kudu/tablet/lock_manager-test.cc
File src/kudu/tablet/lock_manager-test.cc:
http://gerrit.cloudera.org:8080/#/c/17097/8/src/kudu/tablet/lock_manager-test.cc@214
PS8, Line 214: ] {
We need to pass 't' as a copy.
http://gerrit.cloudera.org:8080/#/c/17097/8/src/kudu/tablet/lock_manager.cc
File src/kudu/tablet/lock_manager.cc:
http://gerrit.cloudera.org:8080/#/c/17097/8/src/kudu/tablet/lock_manager.cc@461
PS8, Line 461: MonoTime now(MonoTime::Now());
: MonoDelta elapsed(now - start);
nit: just store elapsed and inline MonoTime::Now()?
--
To view, visit http://gerrit.cloudera.org:8080/17097
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I158115739ce3e7cfb77bbcb854e834336c1256b1
Gerrit-Change-Number: 17097
Gerrit-PatchSet: 8
Gerrit-Owner: Hao Hao
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Bankim Bhavsar
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 12 Mar 2021 07:54:31 +
Gerrit-HasComments: Yes
