[kudu-CR] WIP [catalog manager] added CreateTable DoS scenario
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15912 ) Change subject: WIP [catalog_manager] added CreateTable DoS scenario .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/15912/1//COMMIT_MSG Commit Message: PS1: A JIRA item describing the underlying issue: https://issues.apache.org/jira/browse/KUDU-3124 -- To view, visit http://gerrit.cloudera.org:8080/15912 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ida42c59b2227ea9a71ee24cc8f8a50bdcb9648fb Gerrit-Change-Number: 15912 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 14 Apr 2021 04:48:33 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2671: Adds new field to PartitionSchema.
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17025 ) 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 Reviewed-on: http://gerrit.cloudera.org:8080/17025 Tested-by: Kudu Jenkins Reviewed-by: Andrew Wong --- 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-test.cc 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 22 files changed, 453 insertions(+), 90 deletions(-) Approvals: Kudu Jenkins: Verified Andrew Wong: Looks good to me, approved -- 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: merged Gerrit-Change-Id: Ic5d8615ab9967fdb40292b9c77eb68a19baeca1d Gerrit-Change-Number: 17025 Gerrit-PatchSet: 14 Gerrit-Owner: Mahesh Reddy Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2612: acquire and release partition lock
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17159 ) Change subject: KUDU-2612: acquire and release partition lock .. KUDU-2612: acquire and release partition lock This patch plumbs partition locks into write ops for transactional and non-transactional operations. Upon attempting to write, we try to acquire the partition lock in the WriteOp prepare phase, and upon successfully applying the op, we transfer ownership of the lock to the Txn that the write was a part of (or just release the partition lock if the write was non-transactional). Txns release the lock when FINALIZE_COMMIT or ABORT_TXN is applied. We take the partition lock for non-transactional write ops as well to ensure we don’t duplicate keys (e.g. if a transaction inserts key=1 and then a non-transactional write inserts key=1 before the transaction commits). If the partition lock cannot be acquired, the write op is aborted or retried, based on the wait-die mechanics already in the lock manager. A flag is also introduced to disable this locking for tests that currently expect support for concurrent transactions. Change-Id: If26733cae16810f3b3afd1fd05dcb984e6366939 Reviewed-on: http://gerrit.cloudera.org:8080/17159 Tested-by: Andrew Wong Reviewed-by: Alexey Serbin --- 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/lock_manager.h M src/kudu/tablet/ops/participant_op.cc 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/tablet_bootstrap.cc M src/kudu/tablet/txn_participant-test.cc M src/kudu/tablet/txn_participant.cc M src/kudu/tablet/txn_participant.h M src/kudu/transactions/participant_rpc.cc 16 files changed, 600 insertions(+), 16 deletions(-) Approvals: Andrew Wong: Verified Alexey Serbin: Looks good to me, approved -- 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: merged Gerrit-Change-Id: If26733cae16810f3b3afd1fd05dcb984e6366939 Gerrit-Change-Number: 17159 Gerrit-PatchSet: 12 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] [txns][ksck] KUDU-3258: add txn system table to ksck output
Andrew Wong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17315 Change subject: [txns][ksck] KUDU-3258: add txn system table to ksck output .. [txns][ksck] KUDU-3258: add txn system table to ksck output This patch adds the transaction system table to the ksck output in its own section for system tables. Here's a sample snippet of an output that has the system table: Summary by system table Name | RF | Status | Total Tablets | Healthy | Recovering | Under-replicated | Unavailable ---++--+---+-++--+- kudu_system.kudu_transactions | 3 | UNDER_REPLICATED | 1 | 0 | 0 | 1| 0 Summary by table Name | RF | Status| Total Tablets | Healthy | Recovering | Under-replicated | Unavailable ---++-+---+-++--+- default.loadgen_auto_05cf5be513ea4a84a052e8044f641c1a | 1 | UNAVAILABLE | 8 | 6 | 0 | 0| 2 default.loadgen_auto_0c7ea48d5f6948408694b176f70e69ec | 1 | UNAVAILABLE | 8 | 5 | 0 | 0| 3 default.loadgen_auto_241be343981c46d081ab2b3d2e3b6e6a | 1 | UNAVAILABLE | 8 | 5 | 0 | 0| 3 default.loadgen_auto_385476d5d3b6493f8cbf659c8a4cf7cc | 1 | UNAVAILABLE | 8 | 6 | 0 | 0| 2 default.loadgen_auto_430e280e8aa7450591da67ae15ff0f37 | 1 | UNAVAILABLE | 8 | 6 | 0 | 0| 2 The section can be included/excluded via the --sections flag of ksck. Change-Id: I8162f6eb046d98791c6bdeb5c15a0af72487300d --- M src/kudu/integration-tests/txn_status_table-itest.cc M src/kudu/rebalance/cluster_status.h M src/kudu/tools/ksck-test.cc M src/kudu/tools/ksck.cc M src/kudu/tools/ksck.h M src/kudu/tools/ksck_remote.cc M src/kudu/tools/ksck_results.cc M src/kudu/tools/ksck_results.h M src/kudu/tools/tool.proto M src/kudu/tools/tool_action_cluster.cc 10 files changed, 205 insertions(+), 76 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/17315/1 -- To view, visit http://gerrit.cloudera.org:8080/17315 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I8162f6eb046d98791c6bdeb5c15a0af72487300d Gerrit-Change-Number: 17315 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong
[kudu-CR] KUDU-2671: Adds new field to PartitionSchema.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17025 ) Change subject: KUDU-2671: Adds new field to PartitionSchema. .. Patch Set 13: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/17025/11/src/kudu/common/partition-test.cc File src/kudu/common/partition-test.cc: http://gerrit.cloudera.org:8080/#/c/17025/11/src/kudu/common/partition-test.cc@1355 PS11, Line 1355: EXPECT_EQ(0, ranges_with_hash_schemas[2].hash_schemas.size > That's essentially what CheckSerializationFunctions() does. It's just that Thanks, I appreciate the extra test coverage! -- 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: 13 Gerrit-Owner: Mahesh Reddy Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 14 Apr 2021 01:45:24 + 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 11: Code-Review+2 -- 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: 11 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: Wed, 14 Apr 2021 01:32:40 + Gerrit-HasComments: No
[kudu-CR] KUDU-2671: Adds new field to PartitionSchema.
Hello Tidy Bot, 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 (#13). 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-test.cc 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 22 files changed, 453 insertions(+), 90 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/25/17025/13 -- 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: 13 Gerrit-Owner: Mahesh Reddy Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2612: acquire and release partition lock
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17159 ) Change subject: KUDU-2612: acquire and release partition lock .. Patch Set 11: Verified+1 Test failure is unrelated: MiniRangerTest.TestGrantPrivilege Seems to be an issue with starting up the mini cluster. -- 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: 11 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: Wed, 14 Apr 2021 00:19:59 + Gerrit-HasComments: No
[kudu-CR] KUDU-2612: acquire and release partition lock
Andrew Wong has removed a vote on this change. Change subject: KUDU-2612: acquire and release partition lock .. Removed Verified-1 by Kudu Jenkins (120) -- 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: deleteVote Gerrit-Change-Id: If26733cae16810f3b3afd1fd05dcb984e6366939 Gerrit-Change-Number: 17159 Gerrit-PatchSet: 11 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: acquire and release partition lock
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17159 ) Change subject: KUDU-2612: acquire and release partition lock .. Patch Set 11: (1 comment) http://gerrit.cloudera.org:8080/#/c/17159/9/src/kudu/tablet/ops/write_op.cc File src/kudu/tablet/ops/write_op.cc: http://gerrit.cloudera.org:8080/#/c/17159/9/src/kudu/tablet/ops/write_op.cc@574 PS9, Line 574: CHECK(!s.ok()) << s.ToString(); : completio > I guess DCHECK() wouldn't trigger in case of s.ok() because there is LOG(DF Ah that's true. I'll change this to a CHECK to be a bit more conservative, and given it's easily verifiable today from this code snippet at least. -- 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: 11 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: Tue, 13 Apr 2021 23:28:10 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612: acquire and release partition lock
Andrew Wong has uploaded a new patch set (#11) to the change originally created by Hao Hao. ( http://gerrit.cloudera.org:8080/17159 ) Change subject: KUDU-2612: acquire and release partition lock .. KUDU-2612: acquire and release partition lock This patch plumbs partition locks into write ops for transactional and non-transactional operations. Upon attempting to write, we try to acquire the partition lock in the WriteOp prepare phase, and upon successfully applying the op, we transfer ownership of the lock to the Txn that the write was a part of (or just release the partition lock if the write was non-transactional). Txns release the lock when FINALIZE_COMMIT or ABORT_TXN is applied. We take the partition lock for non-transactional write ops as well to ensure we don’t duplicate keys (e.g. if a transaction inserts key=1 and then a non-transactional write inserts key=1 before the transaction commits). If the partition lock cannot be acquired, the write op is aborted or retried, based on the wait-die mechanics already in the lock manager. A flag is also introduced to disable this locking for tests that currently expect support for concurrent transactions. 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/lock_manager.h M src/kudu/tablet/ops/participant_op.cc 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/tablet_bootstrap.cc M src/kudu/tablet/txn_participant-test.cc M src/kudu/tablet/txn_participant.cc M src/kudu/tablet/txn_participant.h M src/kudu/transactions/participant_rpc.cc 16 files changed, 600 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/59/17159/11 -- 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: 11 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: 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 10: Code-Review+2 (2 comments) http://gerrit.cloudera.org:8080/#/c/17159/9/src/kudu/tablet/ops/write_op.cc File src/kudu/tablet/ops/write_op.cc: http://gerrit.cloudera.org:8080/#/c/17159/9/src/kudu/tablet/ops/write_op.cc@574 PS9, Line 574: DCHECK(!s.ok()) << s.ToString(); : completio > Done as a DCHECK I guess DCHECK() wouldn't trigger in case of s.ok() because there is LOG(DFATAL) a couple of lines above, but if you are sure there aren't different code paths in release vs debug mode, it's totally fine with me. http://gerrit.cloudera.org:8080/#/c/17159/9/src/kudu/tablet/txn_participant.h File src/kudu/tablet/txn_participant.h: http://gerrit.cloudera.org:8080/#/c/17159/9/src/kudu/tablet/txn_participant.h@115 PS9, Line 115: It is thus expected that repeat callers are taking : // the same lock > Done Thank you! -- 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: 10 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: Tue, 13 Apr 2021 23:20:00 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612: acquire and release partition lock
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17159 ) Change subject: KUDU-2612: acquire and release partition lock .. Patch Set 9: (8 comments) http://gerrit.cloudera.org:8080/#/c/17159/9/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/9/src/kudu/integration-tests/txn_write_ops-itest.cc@352 PS9, Line 352: error.IsTimedOut() > nit: could you add a comment explaining why both IsAborted() and IsTimedOut Done http://gerrit.cloudera.org:8080/#/c/17159/9/src/kudu/tablet/ops/write_op.h File src/kudu/tablet/ops/write_op.h: http://gerrit.cloudera.org:8080/#/c/17159/9/src/kudu/tablet/ops/write_op.h@179 PS9, Line 179: locks > nit: lock? Done http://gerrit.cloudera.org:8080/#/c/17159/9/src/kudu/tablet/ops/write_op.cc File src/kudu/tablet/ops/write_op.cc: http://gerrit.cloudera.org:8080/#/c/17159/9/src/kudu/tablet/ops/write_op.cc@217 PS9, Line 217: rows > nit: row Done http://gerrit.cloudera.org:8080/#/c/17159/9/src/kudu/tablet/ops/write_op.cc@574 PS9, Line 574: completion_callback()->set_error(s, code); : return s; > nit: does it make sense to add CHECK(!s.ok()) before setting the error code Done as a DCHECK http://gerrit.cloudera.org:8080/#/c/17159/9/src/kudu/tablet/txn_participant-test.cc File src/kudu/tablet/txn_participant-test.cc: http://gerrit.cloudera.org:8080/#/c/17159/9/src/kudu/tablet/txn_participant-test.cc@525 PS9, Line 525: Status s = Write(0); : ASSERT_TRUE(s.IsAborted()) << s.ToString(); : ASSERT_STR_CONTAINS(s.ToString(), "partition lock that is held by another"); : : s = Write(0, kTxnTwo); : ASSERT_TRUE(s.IsAborted()) << s.ToString(); : ASSERT_STR_CONTAINS(s.ToString(), "partition lock that is held by another"); > nit: is it possible to turn this into a functor and reuse it in the code of Done http://gerrit.cloudera.org:8080/#/c/17159/9/src/kudu/tablet/txn_participant.h File src/kudu/tablet/txn_participant.h: http://gerrit.cloudera.org:8080/#/c/17159/9/src/kudu/tablet/txn_participant.h@115 PS9, Line 115: It is thus expected that repeat callers are taking : // the same lock > nit: is it possible to check for this invariant in the implementation of th Done http://gerrit.cloudera.org:8080/#/c/17159/9/src/kudu/tablet/txn_participant.h@117 PS9, Line 117: AcquirePartitionLock > nit: would AdoptPartitionLock() be a better name for this method? Done http://gerrit.cloudera.org:8080/#/c/17159/9/src/kudu/tablet/txn_participant.cc File src/kudu/tablet/txn_participant.cc: http://gerrit.cloudera.org:8080/#/c/17159/9/src/kudu/tablet/txn_participant.cc@78 PS9, Line 78: TabletServerErrorPB::Code code = tserver::TabletServerErrorPB::UNKNOWN_ERROR; > nit: maybe, move this under the 'if ()' clause below? 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: If26733cae16810f3b3afd1fd05dcb984e6366939 Gerrit-Change-Number: 17159 Gerrit-PatchSet: 9 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: Tue, 13 Apr 2021 23:03:28 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612: acquire and release partition lock
Andrew Wong has uploaded a new patch set (#10) to the change originally created by Hao Hao. ( http://gerrit.cloudera.org:8080/17159 ) Change subject: KUDU-2612: acquire and release partition lock .. KUDU-2612: acquire and release partition lock This patch plumbs partition locks into write ops for transactional and non-transactional operations. Upon attempting to write, we try to acquire the partition lock in the WriteOp prepare phase, and upon successfully applying the op, we transfer ownership of the lock to the Txn that the write was a part of (or just release the partition lock if the write was non-transactional). Txns release the lock when FINALIZE_COMMIT or ABORT_TXN is applied. We take the partition lock for non-transactional write ops as well to ensure we don’t duplicate keys (e.g. if a transaction inserts key=1 and then a non-transactional write inserts key=1 before the transaction commits). If the partition lock cannot be acquired, the write op is aborted or retried, based on the wait-die mechanics already in the lock manager. A flag is also introduced to disable this locking for tests that currently expect support for concurrent transactions. 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/lock_manager.h M src/kudu/tablet/ops/participant_op.cc 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/tablet_bootstrap.cc M src/kudu/tablet/txn_participant-test.cc M src/kudu/tablet/txn_participant.cc M src/kudu/tablet/txn_participant.h M src/kudu/transactions/participant_rpc.cc 16 files changed, 600 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/59/17159/10 -- 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: 10 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-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 12: (1 comment) http://gerrit.cloudera.org:8080/#/c/17025/11/src/kudu/common/partition-test.cc File src/kudu/common/partition-test.cc: http://gerrit.cloudera.org:8080/#/c/17025/11/src/kudu/common/partition-test.cc@1355 PS11, Line 1355: CheckSerializationFunctions(pb, partition_schema, schema); > Yeah, maybe not. I'm thinking something more along the lines of: That's essentially what CheckSerializationFunctions() does. It's just that the new fields of PartitionSchemaPB are not populated in other tests so the new changes to the encoder/decoder function will not be tested in those tests. But I can still go ahead and call this from other tests. -- 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: 12 Gerrit-Owner: Mahesh Reddy Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Tue, 13 Apr 2021 18:13:52 + Gerrit-HasComments: Yes
