Skip to content

Commit

Permalink
[#24778] DocDB: Set field pg_txn_start_us for restarted txns
Browse files Browse the repository at this point in the history
Summary:
Commit 269a518 / D39339 rightly changed the way we set field `pg_txn_start_us` for docdb transactions, by setting it during the first call to `DoBeginTransactionIfNecessary` instead of setting it on each `Perform` call.

We seem to have missed setting the field when a new transaction is created through `PgClientSession::RestartTransaction`. This path is triggered by transactions that encounter read restart error, and that undergo retries triggered by the query layer.

`pg_txn_start_us` field is used by the wait-queue to resume waiters in order of earliest transactions first i.e for a set of resolved waiters, the resumption is in the order - earliest to the latest txn start time. The wait-queue has a DCHECK ensuring that docdb distributed txns have a non-zero txn start time, which was not being set for txns restarted due to read restarts, and hence the failures of test `TestPgTransparentRestarts#testUpdateLong`.

This diff addresses the issue by explicitly copying `pg_txn_start_us` in `YBTransaction::Impl::FillRestartedTransaction`, essentially restoring the behavior of preserving priority of waiter resumption for read-restarted txns.
Jira: DB-13873

Test Plan:
Jenkins

./yb_build.sh fastdebug --java-test TestPgTransparentRestarts#testUpdateLong -n 10 --tp 1

Reviewers: sergei, pjain, patnaik.balivada

Reviewed By: sergei

Subscribers: ybase

Differential Revision: https://phorge.dev.yugabyte.com/D39713
  • Loading branch information
basavaraj29 committed Nov 5, 2024
1 parent e9038f8 commit 20da8d5
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 1 deletion.
1 change: 1 addition & 0 deletions src/yb/client/transaction.cc
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,7 @@ class YBTransaction::Impl final : public internal::TxnBatcherIf {
} else {
other->metadata_.start_time = other->read_point_.Now();
}
other->metadata_.pg_txn_start_us = metadata_.pg_txn_start_us;
state_.store(TransactionState::kAborted, std::memory_order_release);
}
DoAbort(TransactionRpcDeadline(), transaction);
Expand Down
2 changes: 1 addition & 1 deletion src/yb/docdb/wait_queue.cc
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ struct WaiterData : public std::enable_shared_from_this<WaiterData> {
finished_waiting_latency_(*finished_waiting_latency),
unlocked_(locks->Unlock()),
deadline_(deadline) {
DCHECK(txn_start_us || id.IsNil());
LOG_IF_WITH_PREFIX(DFATAL, !txn_start_us && !id.IsNil()) << "Expected non-zero txn_start_us";
VLOG_WITH_PREFIX(4) << "Constructed waiter";
}

Expand Down

0 comments on commit 20da8d5

Please sign in to comment.