storage: add replicated locks to MVCCStats #109645
Assignees
Labels
A-kv-transactions
Relating to MVCC and the transactional model.
A-read-committed
Related to the introduction of Read Committed
C-enhancement
Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
T-kv
KV Team
Comments
nvanbenschoten
added
C-enhancement
nvanbenschoten
added a commit
to nvanbenschoten/cockroach
that referenced
this issue
Sep 11, 2023
Fixes cockroachdb#109646. Informs cockroachdb#100193. This commit adds and implements two new MVCC functions: `MVCCCheckAcquireLock` and `MVCCAcquireLock`. The former scans the replicated lock table to determine whether a lock acquisition is permitted. It will be used by unreplicated lock acquisition. The latter does the same, but then also writes the lock to the replicated lock table if permitted. It will be used by replicated lock acquisition. MVCCStats handling is left as a TODO for after cockroachdb#109645 is addressed. ---- The two functions are build using a new abstraction, the `lockTableKeyScanner`. The lockTableKeyScanner uses a LockTableIterator to scan a single key in the replicated lock table. It searches for locks on the key that conflict with a (transaction, lock strength) pair and for locks that the transaction has already acquired on the key. The purpose of a lockTableKeyScanner is to determine whether a transaction can acquire a lock on a key or perform an MVCC mutation on a key, and if so, what lock table keys the transaction should write to perform the operation. It is used by this commit to implement the two new MVCC functions. In a future commit, it will also start to be used by `mvccPutInternal`, the kernel of all MVCC mutations. Release note: None
nvanbenschoten
added a commit
to nvanbenschoten/cockroach
that referenced
this issue
Sep 13, 2023
Fixes cockroachdb#109646. Informs cockroachdb#100193. This commit adds and implements two new MVCC functions: `MVCCCheckAcquireLock` and `MVCCAcquireLock`. The former scans the replicated lock table to determine whether a lock acquisition is permitted. It will be used by unreplicated lock acquisition. The latter does the same, but then also writes the lock to the replicated lock table if permitted. It will be used by replicated lock acquisition. MVCCStats handling is left as a TODO for after cockroachdb#109645 is addressed. ---- The two functions are build using a new abstraction, the `lockTableKeyScanner`. The lockTableKeyScanner uses a LockTableIterator to scan a single key in the replicated lock table. It searches for locks on the key that conflict with a (transaction, lock strength) pair and for locks that the transaction has already acquired on the key. The purpose of a lockTableKeyScanner is to determine whether a transaction can acquire a lock on a key or perform an MVCC mutation on a key, and if so, what lock table keys the transaction should write to perform the operation. It is used by this commit to implement the two new MVCC functions. In a future commit, it will also start to be used by `mvccPutInternal`, the kernel of all MVCC mutations. Release note: None
nvanbenschoten
added a commit
to nvanbenschoten/cockroach
that referenced
this issue
Sep 14, 2023
Fixes cockroachdb#109646. Informs cockroachdb#100193. This commit adds and implements two new MVCC functions: `MVCCCheckForAcquireLock` and `MVCCAcquireLock`. The former scans the replicated lock table to determine whether a lock acquisition is permitted. It will be used by unreplicated lock acquisition. The latter does the same, but then also writes the lock to the replicated lock table if permitted. It will be used by replicated lock acquisition. MVCCStats handling is left as a TODO for after cockroachdb#109645 is addressed. ---- The two functions are built using a new abstraction, the `lockTableKeyScanner`. The lockTableKeyScanner uses a LockTableIterator to scan a single key in the replicated lock table. It searches for locks on the key that conflict with a (transaction, lock strength) pair and for locks that the transaction has already acquired on the key. The purpose of a lockTableKeyScanner is to determine whether a transaction can acquire a lock on a key or perform an MVCC mutation on a key, and if so, what lock table keys the transaction should write to perform the operation. It is used by this commit to implement the two new MVCC functions. In a future commit, it will also start to be used by `mvccPutInternal`, the kernel of all MVCC mutations. Release note: None
nvanbenschoten
added a commit
to nvanbenschoten/cockroach
that referenced
this issue
Sep 14, 2023
Fixes cockroachdb#109646. Informs cockroachdb#100193. This commit adds and implements two new MVCC functions: `MVCCCheckForAcquireLock` and `MVCCAcquireLock`. The former scans the replicated lock table to determine whether a lock acquisition is permitted. It will be used by unreplicated lock acquisition. The latter does the same, but then also writes the lock to the replicated lock table if permitted. It will be used by replicated lock acquisition. MVCCStats handling is left as a TODO for after cockroachdb#109645 is addressed. ---- The two functions are built using a new abstraction, the `lockTableKeyScanner`. The lockTableKeyScanner uses a LockTableIterator to scan a single key in the replicated lock table. It searches for locks on the key that conflict with a (transaction, lock strength) pair and for locks that the transaction has already acquired on the key. The purpose of a lockTableKeyScanner is to determine whether a transaction can acquire a lock on a key or perform an MVCC mutation on a key, and if so, what lock table keys the transaction should write to perform the operation. It is used by this commit to implement the two new MVCC functions. In a future commit, it will also start to be used by `mvccPutInternal`, the kernel of all MVCC mutations. Release note: None
nvanbenschoten
added a commit
to nvanbenschoten/cockroach
that referenced
this issue
Sep 14, 2023
Fixes cockroachdb#109646. Informs cockroachdb#100193. This commit adds and implements two new MVCC functions: `MVCCCheckForAcquireLock` and `MVCCAcquireLock`. The former scans the replicated lock table to determine whether a lock acquisition is permitted. It will be used by unreplicated lock acquisition. The latter does the same, but then also writes the lock to the replicated lock table if permitted. It will be used by replicated lock acquisition. MVCCStats handling is left as a TODO for after cockroachdb#109645 is addressed. ---- The two functions are built using a new abstraction, the `lockTableKeyScanner`. The lockTableKeyScanner uses a LockTableIterator to scan a single key in the replicated lock table. It searches for locks on the key that conflict with a (transaction, lock strength) pair and for locks that the transaction has already acquired on the key. The purpose of a lockTableKeyScanner is to determine whether a transaction can acquire a lock on a key or perform an MVCC mutation on a key, and if so, what lock table keys the transaction should write to perform the operation. It is used by this commit to implement the two new MVCC functions. In a future commit, it will also start to be used by `mvccPutInternal`, the kernel of all MVCC mutations. Release note: None
craig bot
pushed a commit
that referenced
this issue
Sep 14, 2023
110323: storage: add MVCCCheckForAcquireLock and MVCCAcquireLock functions r=nvanbenschoten a=nvanbenschoten Fixes #109646. Informs #100193. This PR adds and implements two new MVCC functions: `MVCCCheckForAcquireLock` and `MVCCAcquireLock`. The former scans the replicated lock table to determine whether a lock acquisition is permitted. It will be used by unreplicated lock acquisition. The latter does the same, but then also writes the lock to the replicated lock table if permitted. It will be used by replicated lock acquisition. MVCCStats handling is left as a TODO for after #109645 is addressed. ---- The two functions are built using a new abstraction, the `lockTableKeyScanner`. The `lockTableKeyScanner` uses a `LockTableIterator` to scan a single key in the replicated lock table. It searches for locks on the key that conflict with a (transaction, strength) pair and for locks that the transaction has already acquired on the key. The purpose of a `lockTableKeyScanner` is to determine whether a transaction can acquire a lock on a key or perform an MVCC mutation on a key, and if so, what lock table keys the transaction should write to perform the operation. It is used by this commit to implement the two new MVCC functions. In a future commit, it will also start to be used by `mvccPutInternal`, the kernel of all MVCC mutations. Release note: None 110652: kvserver: get Stats and GCHint under same RLock r=erikgrinaker a=pavelkalinnikov Before this commit, `makeMVCCGCQueueScore` unlocked `Replica.mu` between reading `ReplicaState.Stats` and `ReplicaState.GCHint`. The GC scoring function uses both fields, we would like them to be in sync. The unlock makes it possible for `Stats` and `GCHint` to correspond to different states of the `Replica`. This commit moves both reads under the same `RLock`/`RUnlock`. Epic: none Release note: none Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com> Co-authored-by: Pavel Kalinnikov <pavel@cockroachlabs.com>
nvanbenschoten
added a commit
to nvanbenschoten/cockroach
that referenced
this issue
Sep 14, 2023
…unt,Age} Informs cockroachdb#109645. Simple rename of the fields. The new names incorporate the increased role that these fields will play with replicated locks. Release note: None
|
@erikgrinaker points out that one of these commits should have a release note which explains the new/renamed fields. |
craig bot
pushed a commit
that referenced
this issue
Sep 19, 2023
110590: storage: rename MVCCStats.{SeparatedIntentCount,IntentAge} to Lock{Count,Age} r=nvanbenschoten a=nvanbenschoten
Informs #109645.
Simple rename of the fields. The new names incorporate the increased role that these fields will play with replicated locks.
Release note: None
110900: sql: fix builtin format_type for array types r=fqazi a=fqazi
Previously, the format_type builtin did not correctly include length information for the contents of an array. So, if we formatted an array of VARCHAR(32) the length would not be included, which leads to an incompatibility with Postgres. This patch passes the typemod information down, formatting the array's contents, which leads to the correct behaviour.
Fixes: #110539
Release note (bug fix): format_type builtin did not honour typemod information for array types, leading to incorrect output.
Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com>
Co-authored-by: Faizan Qazi <faizan@cockroachlabs.com>
craig bot
pushed a commit
that referenced
this issue
Sep 25, 2023
…111232 110967: asim: enable random zone config event generation r=kvoli a=wenyihu6 Previously, zone config event generation used hardcoded span configurations. This limits our ability to test the allocator more thoroughly. To improve this, this patch enables random span configs to be generated and applied as part of the simulation. These configurations are generated by randomly selecting the primary region, region V.S. zone survival goal, and leaseholder preference. ``` The following command is now supported: "rand_events" [cycle_via_random_survival_goals] ``` Part of: #106192 Release Note: none Epic: none 111192: bulk: allow caller-configurable priority in SSTBatcher r=adityamaru a=stevendanna This adds the ability for some callers to use a higher admission priority in SSTBatcher. This is helpful for streaming where we want to run at a priority that isn't subject to the elastic admission regime. Epic: none Release note: None 111206: kv: fix (store|node) not found err checking r=erikgrinaker a=kvoli `StoreNotFoundError` and `NodeNotFoundError` errors were moved to the `kvpb` pkg in #110374. As part of the move, `crdb_internal` functions which checked if the error were `DescNotFoundError` were also updated so that node/store not found errors would be recognized e.g. ``` errors.Is(kvpb.NewNodeNotFoundError(nodeID), &kvpb.DescNotFoundError{}) ``` This didn't work, because the error doesn't match the reference error variable being given. It does match the type. Update these error assertions to use `HasType` instead. Resolves: #111084 Epic: none Release note: None 111214: release: fix roachtest artifacts name r=srosenberg a=rail This fixes the roachtest artifacts directory name. Epic: none Release note: None 111217: cloud/azure: Fix azure schemes r=benbardin a=benbardin Part of: https://cockroachlabs.atlassian.net/browse/CRDB-31120 Release note (bug fix): Fixes azure schemes in storage, kms and external conns. 111223: storage: populate timestamp field in lock table values r=nvanbenschoten a=nvanbenschoten Informs #109645. This commit starts writing the `Timestamp` field in lock table `MVCCMetadata` values for shared and exclusive locks. This mirrors the behavior of intent locks. This is not strictly needed, as the timestamp is always equal to `Txn.WriteTimestamp`, but it is cheap to do and helps unify some stats logic, which uses this field to compute "lock age". Maybe we'll get rid of this for all lock strengths one day... Release note: None 111230: authors: add xhesikam to authors r=xhesikam a=xhesikam Release note: None Epic: None 111231: backupccl: add missing ctx cancel check r=msbutler a=adityamaru In #111159 we deduced from the stacks a situation in which the goroutine draining `spanCh` had exited due to a context cancelation but the writer was not listening for a ctx cancelation. This manifests as a stuck restore when using the non-default make simple import spans implementation. Fixes: #111159 Release note: None 111232: kv: deflake TestLeaseExpirationBasedDrainTransferWithExtension r=nvanbenschoten a=nvanbenschoten Informs #110715, which will be fixed by a non-clean backport (see 42e45b4) of this commit. This commit deflakes `TestLeaseExpirationBasedDrainTransferWithExtension` by disabling the node suspect timer in leaseTransferTest tests. These tests manually control the clock and have a habit of inducing destabilizing clock jumps. In this case, if n2 looked at liveness immediately after one of these manual clock jumps, it would mark n1 as suspect and refuse to transfer it the lease for the 30 second `server.time_after_store_suspect`, which is longer than the 5 second `server.shutdown.lease_transfer_wait`. This would cause the test to fail. Before this patch, the test would fail under stress race in about 8 minutes. Since the patch, it hasn't failed in over 30 minutes. Release note: None Co-authored-by: wenyihu6 <wenyi.hu@cockroachlabs.com> Co-authored-by: Steven Danna <danna@cockroachlabs.com> Co-authored-by: Austen McClernon <austen@cockroachlabs.com> Co-authored-by: Rail Aliiev <rail@iqchoice.com> Co-authored-by: Ben Bardin <bardin@cockroachlabs.com> Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com> Co-authored-by: xhesikam <xhesika@cockroachlabs.com> Co-authored-by: adityamaru <adityamaru@gmail.com>
nvanbenschoten
added a commit
to nvanbenschoten/cockroach
that referenced
this issue
Sep 26, 2023
Fixes cockroachdb#109645. Informs cockroachdb#100193. This commit adds `MVCCStats` for replicated locks. To do so, it first adds a new field to the stats struct, `LockBytes`. `LockBytes` is the encoded size of replicated locks with shared or exclusive strengths, which are stored in the lock table keyspace. The field includes the size of the locks' keys and their values. For historical reasons, the field excludes the size of intent metadata key-values, even though they are also stored in the lock table keyspace. Intent metadata keys are tracked under KeyBytes and their values are tracked under ValBytes. This is not to be confused with the provisional versioned values protected by the intents, which are tracked by the IntentBytes field (and also by KeyBytes and ValBytes). Hence the vague "without their meta keys" comment above. The patch then begins accounting for the contributions of replicated locks to `LockBytes`, `LockCount`, and `LockAge`, of which the second two fields already exist. This accounting is straightforward. The less straightforward part of the patch is MVCC stats computation. Scanning the lock table requires the use of an EngineIterator. To this point, all stats computation has taken place on an MVCCIterator. The patch addresses this by directly scanning the lock table with an EngineIterator (wrapped in a LockTableIterator) during stats computation. Release note: None
craig bot
pushed a commit
that referenced
this issue
Sep 28, 2023
111293: storage: add replicated locks to MVCCStats r=nvanbenschoten a=nvanbenschoten Fixes #109645. Informs #100193. This commit adds `MVCCStats` for replicated locks. To do so, it first adds a new field to the stats struct, `LockBytes`. `LockBytes` is the encoded size of replicated locks with shared or exclusive strengths, which are stored in the lock table keyspace. The field includes the size of the locks' keys and their values. For historical reasons, the field excludes the size of intent metadata key-values, even though they are also stored in the lock table keyspace. Intent metadata keys are tracked under KeyBytes and their values are tracked under ValBytes. This is not to be confused with the provisional versioned values protected by the intents, which are tracked by the IntentBytes field (and also by KeyBytes and ValBytes). Hence the vague "without their meta keys" comment above. The patch then begins accounting for the contributions of replicated locks to `LockBytes`, `LockCount`, and `LockAge`, of which the second two fields already exist. This accounting is straightforward. The less straightforward part of the patch is MVCC stats computation. Scanning the lock table requires the use of an EngineIterator. To this point, all stats computation has taken place on an MVCCIterator. The patch addresses this by directly scanning the lock table with an EngineIterator (wrapped in a LockTableIterator) during stats computation. Release note: None 111354: sql: fix bug when dropping enum value when used in ARRAY column r=Xiang-Gu a=Xiang-Gu Previously, if an enum value had been used at least twice in an ARRAY column in a table, attempts to drop that enum value resulted in a wrong error message. This PR fixes that. Epic: None Fix #110827 Release note: None 111428: kvcoord: add shared and replicated lock tests to TestTxnCoordSenderRetries r=nvanbenschoten a=arulajmani First commit from #111357. See individual commits for details. Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com> Co-authored-by: Xiang Gu <xiang@cockroachlabs.com> Co-authored-by: Arul Ajmani <arulajmani@gmail.com>
THardy98
pushed a commit
to THardy98/cockroach
that referenced
this issue
Oct 6, 2023
Fixes cockroachdb#109645. Informs cockroachdb#100193. This commit adds `MVCCStats` for replicated locks. To do so, it first adds a new field to the stats struct, `LockBytes`. `LockBytes` is the encoded size of replicated locks with shared or exclusive strengths, which are stored in the lock table keyspace. The field includes the size of the locks' keys and their values. For historical reasons, the field excludes the size of intent metadata key-values, even though they are also stored in the lock table keyspace. Intent metadata keys are tracked under KeyBytes and their values are tracked under ValBytes. This is not to be confused with the provisional versioned values protected by the intents, which are tracked by the IntentBytes field (and also by KeyBytes and ValBytes). Hence the vague "without their meta keys" comment above. The patch then begins accounting for the contributions of replicated locks to `LockBytes`, `LockCount`, and `LockAge`, of which the second two fields already exist. This accounting is straightforward. The less straightforward part of the patch is MVCC stats computation. Scanning the lock table requires the use of an EngineIterator. To this point, all stats computation has taken place on an MVCCIterator. The patch addresses this by directly scanning the lock table with an EngineIterator (wrapped in a LockTableIterator) during stats computation. Release note: None
aliher1911
pushed a commit
to aliher1911/cockroach
that referenced
this issue
Oct 9, 2023
Fixes cockroachdb#109645. Informs cockroachdb#100193. This commit adds `MVCCStats` for replicated locks. To do so, it first adds a new field to the stats struct, `LockBytes`. `LockBytes` is the encoded size of replicated locks with shared or exclusive strengths, which are stored in the lock table keyspace. The field includes the size of the locks' keys and their values. For historical reasons, the field excludes the size of intent metadata key-values, even though they are also stored in the lock table keyspace. Intent metadata keys are tracked under KeyBytes and their values are tracked under ValBytes. This is not to be confused with the provisional versioned values protected by the intents, which are tracked by the IntentBytes field (and also by KeyBytes and ValBytes). Hence the vague "without their meta keys" comment above. The patch then begins accounting for the contributions of replicated locks to `LockBytes`, `LockCount`, and `LockAge`, of which the second two fields already exist. This accounting is straightforward. The less straightforward part of the patch is MVCC stats computation. Scanning the lock table requires the use of an EngineIterator. To this point, all stats computation has taken place on an MVCCIterator. The patch addresses this by directly scanning the lock table with an EngineIterator (wrapped in a LockTableIterator) during stats computation. Release note: None
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
As part of #100193, we'll want to start tracking durable locks in the MVCCStats.
In the Replicated Locks: Implementation Outline doc, we decided to extend and rename the existing
IntentBytes,IntentCount, andIntentAgefields for this purpose. We can then keepSeparatedIntentCount(renamed toIntentCount) as an intent-specific metric.Jira issue: CRDB-31029
Epic CRDB-26544
The text was updated successfully, but these errors were encountered: