kv: bump timestamp cache during replicated {shared,exclusive} lock resolution #111536
Assignees
Labels
A-kv-transactions
Relating to MVCC and the transactional model.
A-read-committed
Related to the introduction of Read Committed
C-bug
Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
T-kv
KV Team
Comments
arulajmani
added
C-bug
arulajmani
added a commit
to arulajmani/cockroach
that referenced
this issue
Sep 30, 2023
This patch teaches ResolveIntent and ResolveIntentRange requests to
bump the timestamp cache if any replicated shared/exclusive locks were
resolved by them (if the transaction that held the lock was committed).
In all other cases (only unreplicated locks, no shared or exclusive
locks, or aborted lock holder transaction) the timestamp cache is not
bumped.
The handling of ResolveIntentRange requests deserves some words -- for
these, we choose to bump the timestamp cache over the entire keyspan
they operated over if there's a single replicated {shared, exclusive}
lock. This means we're losing fidelity over specific keys that had point
locks on them; we choose this approach instead of trying to plumb high
fidelity information back up.
Lastly, it's worth noting that `EndTxn` requests also resolve local
locks. As such, any replicated {shared, exclusive} locks resolved by a
EndTxn request also need to be handled in similar fashion. This patch
does not do that -- we leave that to an subsequent patch, at which point
the linked issue can be closed.
Informs cockroachdb#111536
Release note: None
nvanbenschoten
pushed a commit
to arulajmani/cockroach
that referenced
this issue
Oct 3, 2023
This patch teaches ResolveIntent and ResolveIntentRange requests to
bump the timestamp cache if any replicated shared/exclusive locks were
resolved by them (if the transaction that held the lock was committed).
In all other cases (only unreplicated locks, no shared or exclusive
locks, or aborted lock holder transaction) the timestamp cache is not
bumped.
The handling of ResolveIntentRange requests deserves some words -- for
these, we choose to bump the timestamp cache over the entire keyspan
they operated over if there's a single replicated {shared, exclusive}
lock. This means we're losing fidelity over specific keys that had point
locks on them; we choose this approach instead of trying to plumb high
fidelity information back up for simplicity and to avoid cases where the
response message to a ResolveIntentRange request is significantly larger
than the request itself. The downside of this is that we bump the timestamp
cache over keys that may not have been locked. In practice, we don't think
this will be any more impactful than the other downsides of ranged intent
resolution, like grabbing a write latch across keys that weren't locked.
Lastly, it's worth noting that `EndTxn` requests also resolve local
locks. As such, any replicated {shared, exclusive} locks resolved by a
EndTxn request also need to be handled in similar fashion. This patch
does not do that -- we leave that to an subsequent patch, at which point
the linked issue can be closed.
Informs cockroachdb#111536
Release note: None
nvanbenschoten
added a commit
to nvanbenschoten/cockroach
that referenced
this issue
Oct 3, 2023
Fixes cockroachdb#111536. This commit is the second half of cockroachdb#111546. Whereas that commit taught intent/lock resolution to bump the timestamp cache when releasing replicated locks for committed transactions asynchronously through ResolveIntent and ResolveIntentRange requests, this commit teaches intent/lock resolution to bump the timestamp cache when releasing replicated locks for committed transactions synchronously through EndTxn requests. The interface changes here look slightly different than the ones in that commit. Instead of attaching the commit timestamp to the response proto, we attach the spans over which replicated locks were released. This is because the commit timestamp was already present on the response, but the request may have resolved multiple lock spans. Release note: None
nvanbenschoten
pushed a commit
to arulajmani/cockroach
that referenced
this issue
Oct 4, 2023
This patch teaches ResolveIntent and ResolveIntentRange requests to
bump the timestamp cache if any replicated shared/exclusive locks were
resolved by them (if the transaction that held the lock was committed).
In all other cases (only unreplicated locks, no shared or exclusive
locks, or aborted lock holder transaction) the timestamp cache is not
bumped.
The handling of ResolveIntentRange requests deserves some words -- for
these, we choose to bump the timestamp cache over the entire keyspan
they operated over if there's a single replicated {shared, exclusive}
lock. This means we're losing fidelity over specific keys that had point
locks on them; we choose this approach instead of trying to plumb high
fidelity information back up for simplicity and to avoid cases where the
response message to a ResolveIntentRange request is significantly larger
than the request itself. The downside of this is that we bump the timestamp
cache over keys that may not have been locked. In practice, we don't think
this will be any more impactful than the other downsides of ranged intent
resolution, like grabbing a write latch across keys that weren't locked.
Lastly, it's worth noting that `EndTxn` requests also resolve local
locks. As such, any replicated {shared, exclusive} locks resolved by a
EndTxn request also need to be handled in similar fashion. This patch
does not do that -- we leave that to an subsequent patch, at which point
the linked issue can be closed.
Informs cockroachdb#111536
Release note: None
nvanbenschoten
pushed a commit
to arulajmani/cockroach
that referenced
this issue
Oct 4, 2023
This patch teaches ResolveIntent and ResolveIntentRange requests to
bump the timestamp cache if any replicated shared/exclusive locks were
resolved by them (if the transaction that held the lock was committed).
In all other cases (only unreplicated locks, no shared or exclusive
locks, or aborted lock holder transaction) the timestamp cache is not
bumped.
The handling of ResolveIntentRange requests deserves some words -- for
these, we choose to bump the timestamp cache over the entire keyspan
they operated over if there's a single replicated {shared, exclusive}
lock. This means we're losing fidelity over specific keys that had point
locks on them; we choose this approach instead of trying to plumb high
fidelity information back up for simplicity and to avoid cases where the
response message to a ResolveIntentRange request is significantly larger
than the request itself. The downside of this is that we bump the timestamp
cache over keys that may not have been locked. In practice, we don't think
this will be any more impactful than the other downsides of ranged intent
resolution, like grabbing a write latch across keys that weren't locked.
Lastly, it's worth noting that `EndTxn` requests also resolve local
locks. As such, any replicated {shared, exclusive} locks resolved by a
EndTxn request also need to be handled in similar fashion. This patch
does not do that -- we leave that to an subsequent patch, at which point
the linked issue can be closed.
Informs cockroachdb#111536
Release note: None
craig bot
pushed a commit
that referenced
this issue
Oct 5, 2023
111318: plpgsql: add support for FETCH and MOVE statements r=DrewKimball a=DrewKimball #### plpgsql: implement builtin function for FETCH statements This patch adds the undocumentd `crdb_internal.plpgsql_fetch` builtin function, which seeks a cursor by the specified number of rows in the specified direction, and then returns the row at the ending location (if any). It reuses the same logic as the SQL FETCH and MOVE statements. Note that it differs from the SQL behavior for the `ALL` variants, since it only returns one row. This is consistent with PLpgSQL cursor behavior. The builtin function has 4 parameters: the name of the cursor, the seek direction (an integer representing `tree.FetchType`), the seek count (0 if not applicable), and a tuple containing typed NULL values that represents the expected return types for the columns. This type argument is similar to the one for `crdb_internal.assignment_cast`, with one addition: the result columns are padded with NULLs or truncated as necessary to fit the number of expected types. When the actual types returned by the cursor must be coerced to the expected types, an explicit cast is used, but width truncation is disallowed. This is in line with PG behavior, which allows casting a String to an Int, but does not allow casting a string like 'abc' to a Char. Informs #109709 Release note: None #### plpgsql: add support for FETCH and MOVE statements This patch adds support for the PLpgSQL FETCH and MOVE statements, which seek and return rows from a cursor. This is handled by a builtin function, `crdb_internal.plpgsql_fetch`, which calls into the same logic that implements SQL FETCH and MOVE. Since it is possible to call `FETCH` with `INTO` variables of different types, the `crdb_internal.plpgsql_fetch` builtin takes an argument that supplies the expected column types as a tuple of typed NULL values like this: `(NULL::INT, NULL::BOOL)`. The actual types supplied by the cursor are coerced into the expected types. Note that the current implementation does not support using dynamic expressions in the FETCH/MOVE direction; only constant integer values. Dynamic direction counts like `FORWARD x` are not allowed in SQL syntax, but are allowed by PLpgSQL. Informs #109709 Release note (sql change): Added support for PLpgSQL FETCH and MOVE statements. Similar to SQL FETCH/MOVE statements, commands that would seek the cursor backward will fail. In addition, expressions other than constant integers are not yet supported for the `count` option. 111546: kv: bump timestamp cache when resolving replicated locks r=nvanbenschoten a=arulajmani This patch teaches ResolveIntent and ResolveIntentRange requests to bump the timestamp cache if any replicated shared/exclusive locks were resolved by them (if the transaction that held the lock was committed). In all other cases (only unreplicated locks, no shared or exclusive locks, or aborted lock holder transaction) the timestamp cache is not bumped. The handling of ResolveIntentRange requests deserves some words -- for these, we choose to bump the timestamp cache over the entire keyspan they operated over if there's a single replicated {shared, exclusive} lock. This means we're losing fidelity over specific keys that had point locks on them; we choose this approach instead of trying to plumb high fidelity information back up. Lastly, it's worth noting that `EndTxn` requests also resolve local locks. As such, any replicated {shared, exclusive} locks resolved by a EndTxn request also need to be handled in similar fashion. This patch does not do that -- we leave that to an subsequent patch, at which point the linked issue can be closed. Informs #111536 Release note: None Co-authored-by: Drew Kimball <[email protected]> Co-authored-by: Arul Ajmani <[email protected]>
nvanbenschoten
added a commit
to nvanbenschoten/cockroach
that referenced
this issue
Oct 5, 2023
Fixes cockroachdb#111536. This commit is the second half of cockroachdb#111546. Whereas that commit taught intent/lock resolution to bump the timestamp cache when releasing replicated locks for committed transactions asynchronously through ResolveIntent and ResolveIntentRange requests, this commit teaches intent/lock resolution to bump the timestamp cache when releasing replicated locks for committed transactions synchronously through EndTxn requests. The interface changes here look slightly different than the ones in that commit. Instead of attaching the commit timestamp to the response proto, we attach the spans over which replicated locks were released. This is because the commit timestamp was already present on the response, but the request may have resolved multiple lock spans. Release note: None
craig bot
pushed a commit
that referenced
this issue
Oct 6, 2023
111670: kv: bump timestamp cache when releasing replicated locks synchronously r=miraradeva a=nvanbenschoten Fixes #111536. This commit is the second half of #111546. Whereas that commit taught intent/lock resolution to bump the timestamp cache when releasing replicated locks for committed transactions asynchronously through ResolveIntent and ResolveIntentRange requests, this commit teaches intent/lock resolution to bump the timestamp cache when releasing replicated locks for committed transactions synchronously through EndTxn requests. The interface changes here look slightly different than the ones in that commit. Instead of attaching the commit timestamp to the response proto, we attach the spans over which replicated locks were released. This is because the commit timestamp was already present on the response, but the request may have resolved multiple lock spans. Release note: None 111872: ui: extract txn details api into its own file r=xinhaoz a=xinhaoz This commit simply moves txn details functions out of `txnDetailsApi` and into its own file. No functional changes are made. New filesto organize shared functions: - txnInsightsUtils.ts This patch also moves contention related functions from `txnInsightsApi` to `contentionApi`. Epic: none Release note: None Co-authored-by: Nathan VanBenschoten <[email protected]> Co-authored-by: Xin Hao Zhang <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the problem
Unlike intents, where provisional values prevent other writers from writing underneath the intent once a transaction commits, replicated locks of other lock strengths (read: shared, exclusive) leave no trace behind once resolved. However, to ensure replicated locks continue to provide protection from other writers up till the transaction's commit timestamp[1], we must ensure resolving a replicated lock bumps the timestamp cache on the key the lock was protecting.
[1] This hazard is only ever meaningful in the case of read-committed transaction's, which can tolerate write skew, and thus don't have to refresh their reads before committing. Serializable transactions have to refresh their reads which ends up bumping the timestamp cache.
cc @nvanbenschoten
Jira issue: CRDB-31954
The text was updated successfully, but these errors were encountered: