Skip to content

Commit 44cb504

Browse files
committed
[#28761] DocDB: Prevent requesting new txn when releasing object locks on finish if txn doesn't exist
Summary: `NextObjectLockingTxnMeta` returns transaction if one exists, else creates a new one and waits in-line. This is used for acquiring/releasing object locks when a distributed docdb transaction doesn't exist. It could happen that we receive redundant finish transaction calls, and try releasing object locks when both docdb transaction and `next_plain_` don't exist. In that case, the release would request a new transaction, which isn't necessary. The above causes some tests to fail because of the clean shutdown semantics enforced in [[ https://phorge.dev.yugabyte.com/D46293 | commit ]]. This diff fixes the issue by not requesting a new transaction for release/finish calls. Jira: DB-18473 Test Plan: Jenkins Enabled object locking and ran the following ``` PgAutoAnalyzeTest_CheckTableMutationsCount PgCatalogPerfTest.CacheRefreshRPCCountWithoutPartitionTables ``` Also triggered a jenkins run with table locks enabled and the changes in this revision, some failures in https://phorge.dev.yugabyte.com/D44416 are resolved. Reviewers: rthallam, amitanand Reviewed By: amitanand Subscribers: yql, ybase Differential Revision: https://phorge.dev.yugabyte.com/D47342
1 parent 0728269 commit 44cb504

File tree

1 file changed

+24
-8
lines changed

1 file changed

+24
-8
lines changed

src/yb/tserver/pg_client_session.cc

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1173,6 +1173,18 @@ class TransactionProvider {
11731173
}
11741174
}
11751175

1176+
std::optional<TransactionMetadata> NextTxnMetaForPlainIfExists(CoarseTimePoint deadline) {
1177+
if (!next_plain_) {
1178+
return std::nullopt;
1179+
}
1180+
auto res = NextTxnMetaForPlain(deadline, true /* is_for_release */);
1181+
if (!res.ok()) {
1182+
LOG(DFATAL) << "Unexpected error while fetching existing plain re-usable txn";
1183+
return std::nullopt;
1184+
}
1185+
return *res;
1186+
}
1187+
11761188
Result<TransactionMetadata> NextTxnMetaForPlain(
11771189
CoarseTimePoint deadline, bool is_for_release = false) {
11781190
client::internal::InFlightOpsGroupsWithMetadata ops_info;
@@ -3608,10 +3620,16 @@ class PgClientSession::Impl {
36083620
plain_session_has_exclusive_object_locks_.store(false);
36093621
DEBUG_ONLY_TEST_SYNC_POINT("PlainTxnStateReset");
36103622
}
3611-
auto txn_id = txn
3612-
? txn->id()
3613-
: VERIFY_RESULT(NextObjectLockingTxnMeta(deadline, is_final_release)).transaction_id;
3614-
return DoReleaseObjectLocks(txn_id, subtxn_id, deadline, has_exclusive_locks);
3623+
if (txn) {
3624+
return DoReleaseObjectLocks(txn->id(), subtxn_id, deadline, has_exclusive_locks);
3625+
}
3626+
// It could happen that transaction_provider_.next_plain_ is null when txn finish
3627+
// calls are redundant. If so, treat it as a no-op instead of setting next_plain_.
3628+
auto opt_txn_meta = transaction_provider_.NextTxnMetaForPlainIfExists(deadline);
3629+
return opt_txn_meta
3630+
? DoReleaseObjectLocks(
3631+
opt_txn_meta->transaction_id, subtxn_id, deadline, has_exclusive_locks)
3632+
: Status::OK();
36153633
}
36163634

36173635
Status DoReleaseObjectLocks(
@@ -3649,10 +3667,8 @@ class PgClientSession::Impl {
36493667

36503668
[[nodiscard]] bool IsObjectLockingEnabled() const { return ts_lock_manager() != nullptr; }
36513669

3652-
Result<TransactionMetadata> NextObjectLockingTxnMeta(
3653-
CoarseTimePoint deadline, bool is_final_release = false) {
3654-
auto txn_meta = VERIFY_RESULT(
3655-
transaction_provider_.NextTxnMetaForPlain(deadline, is_final_release));
3670+
Result<TransactionMetadata> NextObjectLockingTxnMeta(CoarseTimePoint deadline) {
3671+
auto txn_meta = VERIFY_RESULT(transaction_provider_.NextTxnMetaForPlain(deadline));
36563672
RegisterLockOwner(txn_meta.transaction_id, txn_meta.status_tablet);
36573673
return txn_meta;
36583674
}

0 commit comments

Comments
 (0)