concurrency: implement LockTable datastructure changes to support SHARED locks #102270

arulajmani · 2023-04-25T18:33:12Z

Describe the problem

Implement LockTable datastructure changes, as described in the Lock Representation section of the [SHARED LOCKs RFC] (#101799).

Jira issue: CRDB-27376

This patch extends the `roachpb.LockAcquisition` struct to include more information -- namely the strength with which the lock is being acquired and there are any ignored sequence numbers for the transaction acquiring the lock. For now, the strength is statically initialized to `lock.Intent`. We'll generalize this as we go about supporting multiple lock strengths in the lock table. The `AcquireLock` codepath is then changed to accept a `roachpb.LockAcquisition` struct. This doesn't change any functionality yet, but will enable us to rollback savepoints on lock acquisition and support multiple lock strengths in the lock table in upcoming patches. Informs #cockroachdb#102269 Informs cockroachdb#102270 Release note: None

This patch extends the `roachpb.LockAcquisition` struct to include more information -- namely the strength with which the lock is being acquired and if there are any ignored sequence numbers for the transaction acquiring the lock. For now, the strength is statically initialized to `lock.Intent`. We'll generalize this as we go about supporting multiple lock strengths in the lock table. The `AcquireLock` codepath is then changed to accept a `roachpb.LockAcquisition` struct. This doesn't change any functionality yet, but will enable us to rollback savepoints on lock acquisition and support multiple lock strengths in the lock table in upcoming patches. Informs cockroachdb#102269 Informs cockroachdb#102270 Release note: None

101694: schemafeed: Add an optimization case where we don't wait on Peek or Pop r=Xiang-Gu a=Xiang-Gu This commit utilizes the recently introduced storage parameter "schema_locked" to achieve low-latency changefeed. Namely, we added logic to Peek/Pop to return "there is no table events" if the table schemas are locked without waiting for `highWater` to reach `atOrBefore`. This change allows us to reduce event commit-to-emit latency drastically from `changefeed.experimental_poll_interval` (which defaults to 1s) to milliseconds. Epic: CRDB-2258 Release note: None 102714: build: update nogo config to lint more generated code r=srosenberg,healthy-pod a=rickystewart To date we have used a regex like `cockroach/pkg/.*/.*\\.go` to identify first-party code, which does find our checked-in code, but does *not* match generated code, which is not staged at that path in the Bazel sandbox. Instead, these generated files are generally found in `bazel-out/.../bin`. In places where we've set `only_files`, in addition to the original regex, we add another regex to capture these generated files. Many packages are now (correctly) being flagged as missing package-level comments. In these cases I have added a very short package-level comment. Closes #102191. Epic: none Release note: None 102770: kv: extend lock acquisition struct to include more information r=nvanbenschoten a=arulajmani This patch extends the `roachpb.LockAcquisition` struct to include more information -- namely the strength with which the lock is being acquired and there are any ignored sequence numbers for the transaction acquiring the lock. For now, the strength is statically initialized to `lock.Intent`. We'll generalize this as we go about supporting multiple lock strengths in the lock table. The `AcquireLock` codepath is then changed to accept a `roachpb.LockAcquisition` struct. This doesn't change any functionality yet, but will enable us to rollback savepoints on lock acquisition and support multiple lock strengths in the lock table in upcoming patches. Informs ##102269 Informs #102270 Release note: None Co-authored-by: Xiang Gu <[email protected]> Co-authored-by: Ricky Stewart <[email protected]> Co-authored-by: Arul Ajmani <[email protected]>

The MVCC keyspace is the source of truth for replicated locks. We've previously concluded that trying to keep the in-memory state of replicated locks in sync is fraught with subtle issues. We instead do the dumb thing and forget replicated locks in a few places (see comment about mvccResolveWriteIntent in tryUpdateLockLocked). It follows that we don't need to track as much information about replicated locks (like we do for unreplicated locks). For example, we do not need to track the sequence number history for replicated locks. This patch splits out lock holder information about replicated locks and unreplicated locks into 2 different structs. As mentioned above, we no longer track seqeunce numbers for the former. Informs cockroachdb#102270 Release note: None

105383: concurrency: split out replicated/unreplicated lock holder information r=nvanbenschoten a=arulajmani The MVCC keyspace is the source of truth for replicated locks. We've previously concluded that trying to keep the in-memory state of replicated locks in sync is fraught with subtle issues. We instead do the dumb thing and forget replicated locks in a few places (see comment about mvccResolveWriteIntent in tryUpdateLockLocked). It follows that we don't need to track as much information about replicated locks (like we do for unreplicated locks). For example, we do not need to track the sequence number history for replicated locks. This patch splits out lock holder information about replicated locks and unreplicated locks into 2 different structs. As mentioned above, we no longer track seqeunce numbers for the former. Informs #102270 Release note: None 105641: kvserver: deflake TestProtectedTimestamps r=irfansharif a=arulajmani This patch fixes a few (hopefully all) issues with TestProtectedTimestamps. In particular, - The range max bytes used by the test was broken after the lower bound was bumped in a37e053. We up the value. - There was flakiness at various points in the test as a result of lease transfers. We change the test to run on a single node test cluster to get around this. Fixes: #93497 Release note: None Co-authored-by: Arul Ajmani <[email protected]>

Prior to this patch, any unreplicated lock was assumed to be held with lock strength Exclusive. This patch enables us to represent other lock strengths in the lock table, which will allow us to support shared locks in the near future. There's a small functional change in this commit. Previously, we'd track all sequence numbers an unreplicated lock is held at in the lock table. As described in the shared locks RFC, this tracking is superflous -- instead, it's sufficient to only track the lowest sequence number (that hasn't been rolled back) with which a lock is held. We do so now. Closes cockroachdb#102270 Epic: none Release note: None

Prior to this patch, any unreplicated lock was assumed to be held with lock strength Exclusive. This patch enables us to represent other lock strengths in the lock table, which will allow us to support shared locks in the near future. Closes cockroachdb#102270 Release note: None

107538: concurrency: datastructure changes to enable multiple lock strengths r=nvanbenschoten a=arulajmani Prior to this patch, any unreplicated lock was assumed to be held with lock strength Exclusive. This patch enables us to represent other lock strengths in the lock table, which will allow us to support shared locks in the near future. There's a small functional change in this commit. Previously, we'd track all sequence numbers an unreplicated lock is held at in the lock table. As described in the shared locks RFC, this tracking is superflous -- instead, it's sufficient to only track the lowest sequence number (that hasn't been rolled back) with which a lock is held. We do so now. Closes #102270 Epic: none Release note: None 107635: cluster ui release workflow lock file file r=sjbarag a=nathanstilwell This is a tiny PR to remove a `path` watched by the Github action that no longer exists (`yarn.lock` since we switched to `pnpm`). Epic: none Release note: None Co-authored-by: Arul Ajmani <[email protected]> Co-authored-by: Nathan Stilwell <[email protected]>

arulajmani added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-kv-transactions Relating to MVCC and the transactional model. T-kv KV Team A-read-committed Related to the introduction of Read Committed labels Apr 25, 2023

arulajmani mentioned this issue Apr 25, 2023

kv: implement SHARED lock strength for unreplicated locks #91545

Closed

arulajmani mentioned this issue May 4, 2023

kv: extend lock acquisition struct to include more information #102770

Merged

exalate-issue-sync bot assigned arulajmani May 8, 2023

arulajmani mentioned this issue Jun 22, 2023

concurrency: split out replicated/unreplicated lock holder information #105383

Merged

arulajmani mentioned this issue Jul 25, 2023

concurrency: datastructure changes to enable multiple lock strengths #107538

Merged

craig bot closed this as completed in f1d6a25 Jul 28, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

concurrency: implement LockTable datastructure changes to support SHARED locks #102270

concurrency: implement LockTable datastructure changes to support SHARED locks #102270

arulajmani commented Apr 25, 2023 •

edited by cockroach-jira-scripts

Loading

concurrency: implement LockTable datastructure changes to support SHARED locks #102270

concurrency: implement LockTable datastructure changes to support SHARED locks #102270

Comments

arulajmani commented Apr 25, 2023 • edited by cockroach-jira-scripts Loading

arulajmani commented Apr 25, 2023 •

edited by cockroach-jira-scripts

Loading