Skip to content

Commit

Permalink
[BACKPORT pg15-cherrypicks][#22519] YSQL: Substitute YBSetRowLockPoli…
Browse files Browse the repository at this point in the history
…cy function with YBGetDocDBWaitPolicy

Summary:
Original commit: 7f59bd8 / D38003
Auxiliary diff to fix `TODO` section to simplify the `YBSetRowLockPolicy` function usage by substituting it with the `YBGetDocDBWaitPolicy` function.

The purpose of this diff is to reduce changes in main diff for the #22519 issue (https://phorge.dev.yugabyte.com/D37821)
Jira: DB-11445

Merge:
  src/postgres/src/backend/executor/nodeYbSeqscan.c - trivial conflict between YB pg15 71dc8d0 and YB master 7f59bd8 in the YbSeqNext function

Test Plan: Jenkins

Reviewers: jason, tfoucher

Reviewed By: jason

Subscribers: yql

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D38071
  • Loading branch information
d-uspenskiy committed Sep 17, 2024
1 parent 88e0e85 commit 0f0c36f
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 31 deletions.
15 changes: 3 additions & 12 deletions src/postgres/src/backend/access/yb_access/yb_scan.c
Original file line number Diff line number Diff line change
Expand Up @@ -3757,15 +3757,6 @@ YbFetchHeapTuple(Relation relation, Datum ybctid, HeapTuple* tuple)
return has_data;
}

// TODO: Substitute the YBSetRowLockPolicy with this function
static int
YBCGetRowLockPolicy(LockWaitPolicy pg_wait_policy)
{
int docdb_wait_policy;
YBSetRowLockPolicy(&docdb_wait_policy, pg_wait_policy);
return docdb_wait_policy;
}

/*
* The return value of this function depends on whether we are batching or not.
* Currently, batching is enabled if the GUC yb_explicit_row_locking_batch_size > 1
Expand All @@ -3779,7 +3770,7 @@ YBCLockTuple(
Relation relation, Datum ybctid, RowMarkType mode,
LockWaitPolicy pg_wait_policy, EState* estate)
{
int docdb_wait_policy = YBCGetRowLockPolicy(pg_wait_policy);
const int docdb_wait_policy = YBGetDocDBWaitPolicy(pg_wait_policy);
const YBCPgExplicitRowLockParams lock_params = {
.rowmark = mode,
.pg_wait_policy = pg_wait_policy,
Expand All @@ -3788,8 +3779,8 @@ YBCLockTuple(
const Oid relfile_oid = YbGetRelfileNodeId(relation);
const Oid db_oid = YBCGetDatabaseOid(relation);

if (yb_explicit_row_locking_batch_size > 1
&& lock_params.pg_wait_policy != LockWaitSkip)
if (yb_explicit_row_locking_batch_size > 1 &&
lock_params.pg_wait_policy != LockWaitSkip)
{
// TODO: Error message requires conversion
HandleYBStatus(YBCAddExplicitRowLockIntent(
Expand Down
4 changes: 2 additions & 2 deletions src/postgres/src/backend/executor/nodeIndexscan.c
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,8 @@ IndexNext(IndexScanState *node)
if (erm->markType != ROW_MARK_REFERENCE && erm->markType != ROW_MARK_COPY) {
scandesc->yb_exec_params->rowmark = erm->markType;
scandesc->yb_exec_params->pg_wait_policy = erm->waitPolicy;
YBSetRowLockPolicy(&scandesc->yb_exec_params->docdb_wait_policy,
erm->waitPolicy);
scandesc->yb_exec_params->docdb_wait_policy =
YBGetDocDBWaitPolicy(erm->waitPolicy);
}
}
}
Expand Down
4 changes: 1 addition & 3 deletions src/postgres/src/backend/executor/nodeYbSeqscan.c
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,7 @@ YbSeqNext(YbSeqScanState *node)

ybScan->exec_params->rowmark = erm->markType;
ybScan->exec_params->pg_wait_policy = erm->waitPolicy;
YBSetRowLockPolicy(
&ybScan->exec_params->docdb_wait_policy,
erm->waitPolicy);
ybScan->exec_params->docdb_wait_policy = YBGetDocDBWaitPolicy(erm->waitPolicy);
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/postgres/src/backend/executor/ybc_fdw.c
Original file line number Diff line number Diff line change
Expand Up @@ -370,8 +370,8 @@ ybcBeginForeignScan(ForeignScanState *node, int eflags)
{
ybc_state->exec_params->rowmark = erm->markType;
ybc_state->exec_params->pg_wait_policy = erm->waitPolicy;
YBSetRowLockPolicy(&ybc_state->exec_params->docdb_wait_policy,
erm->waitPolicy);
ybc_state->exec_params->docdb_wait_policy =
YBGetDocDBWaitPolicy(erm->waitPolicy);
}
}
}
Expand Down
17 changes: 8 additions & 9 deletions src/postgres/src/backend/utils/misc/pg_yb_utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -4669,8 +4669,10 @@ uint64_t YbGetSharedCatalogVersion()
return version;
}

void YBSetRowLockPolicy(int *docdb_wait_policy, LockWaitPolicy pg_wait_policy)
LockWaitPolicy YBGetDocDBWaitPolicy(LockWaitPolicy pg_wait_policy)
{
LockWaitPolicy result = pg_wait_policy;

if (XactIsoLevel == XACT_REPEATABLE_READ && pg_wait_policy == LockWaitError)
{
/* The user requested NOWAIT, which isn't allowed in RR. */
Expand All @@ -4689,14 +4691,10 @@ void YBSetRowLockPolicy(int *docdb_wait_policy, LockWaitPolicy pg_wait_policy)
"(GH issue #11761)",
pg_wait_policy == LockWaitSkip ? "SKIP LOCKED" : "NO WAIT");

*docdb_wait_policy = LockWaitBlock;
}
else
{
*docdb_wait_policy = pg_wait_policy;
result = LockWaitBlock;
}

if (*docdb_wait_policy == LockWaitBlock && !YBIsWaitQueueEnabled())
if (result == LockWaitBlock && !YBIsWaitQueueEnabled())
{
/*
* If wait-queues are not enabled, we default to the "Fail-on-Conflict" policy which is
Expand All @@ -4705,9 +4703,10 @@ void YBSetRowLockPolicy(int *docdb_wait_policy, LockWaitPolicy pg_wait_policy)
* semantics but to Fail-on-Conflict semantics).
*/
elog(DEBUG1, "Falling back to LockWaitError since wait-queues are not enabled");
*docdb_wait_policy = LockWaitError;
result = LockWaitError;
}
elog(DEBUG2, "docdb_wait_policy=%d pg_wait_policy=%d", *docdb_wait_policy, pg_wait_policy);
elog(DEBUG2, "docdb_wait_policy=%d pg_wait_policy=%d", result, pg_wait_policy);
return result;
}

uint32_t YbGetNumberOfDatabases()
Expand Down
8 changes: 5 additions & 3 deletions src/postgres/src/include/pg_yb_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -1012,19 +1012,21 @@ bool YbCatalogVersionTableInPerdbMode();
* This function maps the user intended row-level lock policy i.e., "pg_wait_policy" of
* type enum LockWaitPolicy to the "docdb_wait_policy" of type enum WaitPolicy as defined in
* common.proto.
* Note: enum WaitPolicy values are equal to enum LockWaitPolicy.
* That is why function maps enum LockWaitPolicy into enum LockWaitPolicy.
*
* The semantics of the WaitPolicy enum differ slightly from those of the traditional LockWaitPolicy
* in Postgres, as explained in common.proto. This is for historical reasons. WaitPolicy in
* common.proto was created as a copy of LockWaitPolicy to be passed to the Tserver to help in
* appropriate conflict-resolution steps for the different row-level lock policies.
*
* In isolation level SERIALIZABLE, this function sets docdb_wait_policy to WAIT_BLOCK as
* In isolation level SERIALIZABLE, this function returns WAIT_BLOCK as
* this is the only policy currently supported for SERIALIZABLE.
*
* However, if wait queues aren't enabled in the following cases:
* * Isolation level SERIALIZABLE
* * The user requested LockWaitBlock in another isolation level
* this function sets docdb_wait_policy to WAIT_ERROR (which actually uses the "Fail on Conflict"
* this function returns WAIT_ERROR (which actually uses the "Fail on Conflict"
* conflict management policy instead of "no wait" semantics, as explained in "enum WaitPolicy" in
* common.proto).
*
Expand All @@ -1034,7 +1036,7 @@ bool YbCatalogVersionTableInPerdbMode();
* 2. In isolation level REPEATABLE READ for a pg_wait_policy of LockWaitError because NOWAIT
* is not supported.
*/
void YBSetRowLockPolicy(int *docdb_wait_policy, LockWaitPolicy pg_wait_policy);
LockWaitPolicy YBGetDocDBWaitPolicy(LockWaitPolicy pg_wait_policy);

const char *yb_fetch_current_transaction_priority(void);

Expand Down

0 comments on commit 0f0c36f

Please sign in to comment.