[kudu-CR] KUDU-2612: allow terminal txn ops to succeed if txn hasn't started
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17233 ) Change subject: KUDU-2612: allow terminal txn ops to succeed if txn hasn't started .. Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/17233/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/17233/2//COMMIT_MSG@9 PS2, Line 9: This patch allows ABORT_TXN and FINALIZE_COMMIT ops to proceed even if : the participant hasn't yet initialized the transaction Could you add more colors on why it's desirable to allow an 'empty' participant to successfully finalize committing a transaction? Or that's not the case at all? http://gerrit.cloudera.org:8080/#/c/17233/2//COMMIT_MSG@13 PS2, Line 13: TxnOpDispatchers How does this work in case of non-empty TxnOpDispatcher when receiving FINALIZE_COMMIT (if that possible at all)? What does a client see in that case if it's in process of submitting a write operation? http://gerrit.cloudera.org:8080/#/c/17233/2/src/kudu/tablet/tablet.cc File src/kudu/tablet/tablet.cc: http://gerrit.cloudera.org:8080/#/c/17233/2/src/kudu/tablet/tablet.cc@1226 PS2, Line 1226: auto txn_rowsets = nit: it seems result isn't needed? -- To view, visit http://gerrit.cloudera.org:8080/17233 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia4c864b4f14e42008d3aa8f4454c8b2abf9bb766 Gerrit-Change-Number: 17233 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 26 Mar 2021 03:35:52 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612: acquire and release partition lock
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17159 ) Change subject: KUDU-2612: acquire and release partition lock .. Patch Set 5: (5 comments) 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: } > Done This is marked 'Done' but I don't see that a scenario with calling ParticipateInTransaction(..., ParticipantOpPB::BEGIN_TXN) for the same txn_id multiple times is added. Did I miss anything? http://gerrit.cloudera.org:8080/#/c/17159/3/src/kudu/integration-tests/txn_participant-itest.cc@930 PS3, Line 930: > Done nit: this is marked as 'Done', and I saw TODO is added in other places. Is it worth adding that comment here as well? http://gerrit.cloudera.org:8080/#/c/17159/5/src/kudu/integration-tests/txn_participant-itest.cc File src/kudu/integration-tests/txn_participant-itest.cc: http://gerrit.cloudera.org:8080/#/c/17159/5/src/kudu/integration-tests/txn_participant-itest.cc@282 PS5, Line 282: const auto& actual_txns = replicas[i]->tablet()->txn_participant()->GetTxnsForTests(); : ASSERT_EQ(txns.size(), actual_txns.size()); : ASSERT_EQ(txns, actual_txns); nit: is this change essential? If not, maybe it's better to return back to the version how it was in PS3? I guess with this version as in PS5, if the assertion ever triggers (even with ASSERT_EVENTUALLY), there isn't as much useful information anymore because the assertion on the size strikes first. http://gerrit.cloudera.org:8080/#/c/17159/5/src/kudu/tablet/ops/participant_op.cc File src/kudu/tablet/ops/participant_op.cc: http://gerrit.cloudera.org:8080/#/c/17159/5/src/kudu/tablet/ops/participant_op.cc@212 PS5, Line 212: RETURN_NOT_OK(replica->tablet()->AcquirePartitionLock(state_.get(), wait_mode)); How to release the partition lock if the following code kicks in when the driver run Status ParticipantOp::Prepare(): https://github.com/apache/kudu/blob/1eb2a2fbca329721ec7152714b7c95374754044b/src/kudu/tablet/ops/op_driver.cc#L298-L305 Basically, how to guarantee the lock is to be released in all possible outcomes in the OpDriver's code once the lock is taken? http://gerrit.cloudera.org:8080/#/c/17159/5/src/kudu/tablet/tablet.h File src/kudu/tablet/tablet.h: http://gerrit.cloudera.org:8080/#/c/17159/5/src/kudu/tablet/tablet.h@172 PS5, Line 172: // Similar to the above but for participant op. : Status AcquirePartitionLock(ParticipantOpState* op_state, : LockManager::LockWaitMode wait_mode); BTW, why do we need this method when we already have a similar one for WriteOpState? Would it be possible to acquire partition lock only when performing a write operation? If not, could you please add a blurb explaining why it's also necessary to acquire a lock while working with ParticipantOpState as well? -- 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: If26733cae16810f3b3afd1fd05dcb984e6366939 Gerrit-Change-Number: 17159 Gerrit-PatchSet: 5 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) Gerrit-Comment-Date: Fri, 26 Mar 2021 03:34:08 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2671: Adds new field to PartitionSchema.
Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/17025 ) Change subject: KUDU-2671: Adds new field to PartitionSchema. .. Patch Set 8: > Uploaded patch set 8. Ran into some issues w the decoder/encoder functions while writing test for partition pruning. This patch set updates these functions. -- To view, visit http://gerrit.cloudera.org:8080/17025 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic5d8615ab9967fdb40292b9c77eb68a19baeca1d Gerrit-Change-Number: 17025 Gerrit-PatchSet: 8 Gerrit-Owner: Mahesh Reddy Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Comment-Date: Fri, 26 Mar 2021 00:14:43 + Gerrit-HasComments: No
[kudu-CR] KUDU-2671: Adds new field to PartitionSchema.
Hello Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/17025 to look at the new patch set (#8). Change subject: KUDU-2671: Adds new field to PartitionSchema. .. KUDU-2671: Adds new field to PartitionSchema. This patch introduces a new field to PartitionSchema that combines range bounds and their respective hash bucket schemas. Any instance that assumes the same hash schemas are used for each range will need this new field. Some of the more important instances include partition pruning and many of the internal PartitionSchema functions. I moved RowOperationsPB to a separate .proto file due to some circular dependency issues between common.proto and wire_protocol.proto. Most of the proto changes in this patch revolve around this change. Change-Id: Ic5d8615ab9967fdb40292b9c77eb68a19baeca1d --- M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestOperation.java M src/kudu/client/scan_token-internal.cc M src/kudu/common/CMakeLists.txt M src/kudu/common/common.proto M src/kudu/common/partition.cc M src/kudu/common/partition.h M src/kudu/common/partition_pruner.cc M src/kudu/common/row_operations-test.cc M src/kudu/common/row_operations.h A src/kudu/common/row_operations.proto M src/kudu/common/wire_protocol.proto M src/kudu/integration-tests/alter_table-test.cc M src/kudu/master/catalog_manager.cc M src/kudu/master/master.proto M src/kudu/master/sys_catalog.cc M src/kudu/tablet/tablet_metadata.cc M src/kudu/tserver/tablet_copy_client.cc M src/kudu/tserver/tablet_server-test.cc M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/tserver.proto 21 files changed, 258 insertions(+), 85 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/25/17025/8 -- To view, visit http://gerrit.cloudera.org:8080/17025 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic5d8615ab9967fdb40292b9c77eb68a19baeca1d Gerrit-Change-Number: 17025 Gerrit-PatchSet: 8 Gerrit-Owner: Mahesh Reddy Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy
[kudu-CR] KUDU-2612: route txn op dispatching errors to write ops
Andrew Wong has removed a vote on this change. Change subject: KUDU-2612: route txn op dispatching errors to write ops .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/17217 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: Ibf85e0724ee579cb20dac642b82e3228faf90935 Gerrit-Change-Number: 17217 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2612: route txn op dispatching errors to write ops
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17217 ) Change subject: KUDU-2612: route txn op dispatching errors to write ops .. Patch Set 3: Verified+1 The test failure doesn't look related, but I'm looking into it because I haven't seen it before. -- To view, visit http://gerrit.cloudera.org:8080/17217 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibf85e0724ee579cb20dac642b82e3228faf90935 Gerrit-Change-Number: 17217 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 25 Mar 2021 21:54:58 + Gerrit-HasComments: No
[kudu-CR] KUDU-2612: route txn op dispatching errors to write ops
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17217 ) Change subject: KUDU-2612: route txn op dispatching errors to write ops .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/17217/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/17217/1//COMMIT_MSG@9 PS1, Line 9: patch routes bad statuses fr > nit: "patch makes allows ABORT_TXN" Removed/reworded entirely -- To view, visit http://gerrit.cloudera.org:8080/17217 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibf85e0724ee579cb20dac642b82e3228faf90935 Gerrit-Change-Number: 17217 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 25 Mar 2021 21:54:25 + Gerrit-HasComments: Yes
[kudu-CR] [client] add logging on slow tablet opening
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17232 ) Change subject: [client] add logging on slow tablet opening .. [client] add logging on slow tablet opening This patch adds logging on slow tablet opening with 500ms threshold. This is a follow-up to 44d687caf3633d75b0cc64f1a25f84ce43ad. Change-Id: I8d52a7adbef0b21372c5c301b1c0f79cc7ffc481 Reviewed-on: http://gerrit.cloudera.org:8080/17232 Tested-by: Alexey Serbin Reviewed-by: Bankim Bhavsar Reviewed-by: Grant Henke --- M src/kudu/client/scanner-internal.cc 1 file changed, 11 insertions(+), 10 deletions(-) Approvals: Alexey Serbin: Verified Bankim Bhavsar: Looks good to me, approved Grant Henke: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/17232 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I8d52a7adbef0b21372c5c301b1c0f79cc7ffc481 Gerrit-Change-Number: 17232 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [client] add logging on slow tablet opening
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/17232 ) Change subject: [client] add logging on slow tablet opening .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/17232 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8d52a7adbef0b21372c5c301b1c0f79cc7ffc481 Gerrit-Change-Number: 17232 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 25 Mar 2021 17:16:34 + Gerrit-HasComments: No
[kudu-CR] [client] add logging on slow tablet opening
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17232 ) Change subject: [client] add logging on slow tablet opening .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/17232/1/src/kudu/client/scanner-internal.cc File src/kudu/client/scanner-internal.cc: http://gerrit.cloudera.org:8080/#/c/17232/1/src/kudu/client/scanner-internal.cc@406 PS1, Line 406: 500 > Nit: Not this change but I wish SCOPED_LOG_SLOW_EXECUTION took MonoDelta a Yup, it's worth updating that in its own changelist, I guess. -- To view, visit http://gerrit.cloudera.org:8080/17232 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8d52a7adbef0b21372c5c301b1c0f79cc7ffc481 Gerrit-Change-Number: 17232 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 25 Mar 2021 17:14:49 + Gerrit-HasComments: Yes
[kudu-CR] [client] add logging on slow tablet opening
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/17232 ) Change subject: [client] add logging on slow tablet opening .. Patch Set 1: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/17232/1/src/kudu/client/scanner-internal.cc File src/kudu/client/scanner-internal.cc: http://gerrit.cloudera.org:8080/#/c/17232/1/src/kudu/client/scanner-internal.cc@406 PS1, Line 406: 500 Nit: Not this change but I wish SCOPED_LOG_SLOW_EXECUTION took MonoDelta as a param instead so we don't have to lookup the unit of the parameter. -- To view, visit http://gerrit.cloudera.org:8080/17232 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8d52a7adbef0b21372c5c301b1c0f79cc7ffc481 Gerrit-Change-Number: 17232 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 25 Mar 2021 16:40:55 + Gerrit-HasComments: Yes
[kudu-CR] [client] add logging on slow tablet opening
Alexey Serbin has removed a vote on this change. Change subject: [client] add logging on slow tablet opening .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/17232 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I8d52a7adbef0b21372c5c301b1c0f79cc7ffc481 Gerrit-Change-Number: 17232 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [client] add logging on slow tablet opening
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17232 ) Change subject: [client] add logging on slow tablet opening .. Patch Set 1: Verified+1 unrelated test failure in FaultFlags/DeleteTableDeletedParamTest.TestRollForwardDelete/0 -- To view, visit http://gerrit.cloudera.org:8080/17232 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8d52a7adbef0b21372c5c301b1c0f79cc7ffc481 Gerrit-Change-Number: 17232 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 25 Mar 2021 15:51:54 + Gerrit-HasComments: No
[kudu-CR] KUDU-2612: allow terminal txn ops to succeed if txn hasn't started
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/17233 to look at the new patch set (#2). Change subject: KUDU-2612: allow terminal txn ops to succeed if txn hasn't started .. KUDU-2612: allow terminal txn ops to succeed if txn hasn't started This patch allows ABORT_TXN and FINALIZE_COMMIT ops to proceed even if the participant hasn't yet initialized the transaction. This is desirable in the case where a participant has successfully registered with the TxnStatusManager, but hasn't yet replicated its BEGIN_TXN op. In these cases, the ops will unregister any pending TxnOpDispatchers, as well as mark the existence of the transaction on the participant, preventing further participant registrations for the transaction. Along the way, I hardened up an edge case for BEGIN_COMMIT so TXN_ILLEGAL_STATE is returned if the transaction doesn't exist, instead of returning a plain IllegalState, which would have resulted in the ParticipantRpc being retried. Change-Id: Ia4c864b4f14e42008d3aa8f4454c8b2abf9bb766 --- M src/kudu/integration-tests/txn_participant-itest.cc M src/kudu/tablet/tablet.cc M src/kudu/tablet/tablet.h M src/kudu/tablet/tablet_metadata.cc M src/kudu/tablet/tablet_metadata.h M src/kudu/tablet/txn_metadata.h M src/kudu/tablet/txn_participant-test.cc M src/kudu/tablet/txn_participant.h 8 files changed, 118 insertions(+), 43 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/33/17233/2 -- To view, visit http://gerrit.cloudera.org:8080/17233 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia4c864b4f14e42008d3aa8f4454c8b2abf9bb766 Gerrit-Change-Number: 17233 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2612: route txn op dispatching errors to write ops
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/17217 to look at the new patch set (#3). Change subject: KUDU-2612: route txn op dispatching errors to write ops .. KUDU-2612: route txn op dispatching errors to write ops This patch routes bad statuses from the TxnOpDispatcher to the write ops that initiated the registration, and adjusts the batcher code to handle such errors. This also enables a couple of test cases that were written with this behavior in mind; and adjusts some expected errors, addressing some TODOs that expected this behavior. A follow-up patch will make a similar change to the Java client. Change-Id: Ibf85e0724ee579cb20dac642b82e3228faf90935 --- M src/kudu/client/batcher.cc M src/kudu/integration-tests/txn_commit-itest.cc M src/kudu/integration-tests/txn_write_ops-itest.cc M src/kudu/tablet/tablet_metadata.cc M src/kudu/tablet/tablet_replica.cc M src/kudu/tablet/tablet_replica.h M src/kudu/transactions/txn_status_manager.cc M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/ts_tablet_manager.cc M src/kudu/tserver/ts_tablet_manager.h 10 files changed, 96 insertions(+), 94 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/17217/3 -- To view, visit http://gerrit.cloudera.org:8080/17217 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibf85e0724ee579cb20dac642b82e3228faf90935 Gerrit-Change-Number: 17217 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
