Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kv: implement SHARED lock strength for unreplicated locks #91545

Closed
arulajmani opened this issue Nov 8, 2022 · 2 comments · Fixed by #109638
Closed

kv: implement SHARED lock strength for unreplicated locks #91545

arulajmani opened this issue Nov 8, 2022 · 2 comments · Fixed by #109638
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) P-1 Issues/test failures with a fix SLA of 1 month T-kv KV Team

Comments

@arulajmani
Copy link
Collaborator

arulajmani commented Nov 8, 2022

The lockTable currently only supports the exclusive lock strength, but was designed to support varying degrees of lock strengths. Specifically, it was designed to support both SHARED and UPGRADE (tracked separately in #49684) locking strength as well.

// +-----------+-----------+-----------+-----------+-----------+
// | | None | Shared | Upgrade | Exclusive |
// +-----------+-----------+-----------+-----------+-----------+
// | None | | | | X^† |
// +-----------+-----------+-----------+-----------+-----------+
// | Shared | | | X | X |
// +-----------+-----------+-----------+-----------+-----------+
// | Upgrade | | X | X | X |
// +-----------+-----------+-----------+-----------+-----------+
// | Exclusive | X^† | X | X | X |
// +-----------+-----------+-----------+-----------+-----------+

Additional context

Today SQL accepts syntax of the form SELECT ... FOR SHARE but does not lock any of the rows returned. It should be easy enough to correctly use the SHARED locking strength once the lockTable supports it.

Jira issue: CRDB-21312

Epic CRDB-26544

@arulajmani arulajmani added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) A-kv-transactions Relating to MVCC and the transactional model. labels Nov 8, 2022
@arulajmani arulajmani self-assigned this Nov 8, 2022
@arulajmani arulajmani added the T-kv KV Team label Nov 8, 2022
@exalate-issue-sync exalate-issue-sync bot removed the T-kv KV Team label Dec 20, 2022
@exalate-issue-sync exalate-issue-sync bot added the T-kv KV Team label Jan 4, 2023
arulajmani added a commit to arulajmani/cockroach that referenced this issue Jan 26, 2023
This patch is mostly a mechanical change, that renames what was
previously called `lock.Strength` to `lock.LockMode`.

Previously, `lock.Strength` was used both by users of the KV layer to
indicate desired locking modes and by the `concurrency` package
internally to do conflict resolution. With this change, `lock.LockMode`
assumes the former responsbility.

This patch prepares us to redefine `lock.Strength` as an internal
concept to the `concurrency` package in an upcoming commit. We'll
primarilay use it to define conflict resolution rules by attaching
information (read: timestamps) in addition to the desired
 `lock.LockMode`.

References: cockroachdb#91545

Release note: None
arulajmani added a commit to arulajmani/cockroach that referenced this issue Feb 10, 2023
This patch introduces the concept of a `lock.Mode`, which ties together
`lock.Strength` with other auxiliary information (read: timestamps) to
resolve conflicts between 2 lock holders.

Previously, `lock.Strength` was used both by users of the KV layer to
indicate desired locking intention and by the `concurrency` package
internally to do conflict resolution. `lock.Strength` continues to
do the former. With this change, `lock.Mode` assumes the responsibility
for the latter.

Given this motivation, this patch expresses lock compatiility/conflict
as pure functions of 2 `lock.Modes`.

This patch also introduces a new lock Strength, called Intent.
Conceptually, these are stronger than `Exclusive` locks and map on-to
the existing concept of Intents. The difference lies in their
interaction with non-locking reads. Non-locking reads conflict with
intents, whereas `Exclusive` locks do not. This is a forward looking
change with non-blocking reads in mind.

Note that non-locking reads not blocking on `Exclusive` locks changes
existing behavior. As such, we introduce a new cluster setting
(`kv.lock.exclusive_locks_block_non_locking_reads`) which, when set,
reverts to the old behavior. By default, this setting is set to true,
which means we're not changing any default behavior with this patch
just yet.

References cockroachdb#91545

Release note: None
arulajmani added a commit to arulajmani/cockroach that referenced this issue Feb 16, 2023
This patch introduces the concept of a `lock.Mode`, which ties together
`lock.Strength` with other auxiliary information (read: timestamps) to
resolve conflicts between 2 lock holders.

Previously, `lock.Strength` was used both by users of the KV layer to
indicate desired locking intention and by the `concurrency` package
internally to do conflict resolution. `lock.Strength` continues to
do the former. With this change, `lock.Mode` assumes the responsibility
for the latter.

Given this motivation, this patch expresses lock compatiility/conflict
as pure functions of 2 `lock.Modes`.

This patch also introduces a new lock Strength, called Intent.
Conceptually, these are stronger than `Exclusive` locks and map on-to
the existing concept of Intents. The difference lies in their
interaction with non-locking reads. Non-locking reads conflict with
intents, whereas `Exclusive` locks do not. This is a forward looking
change with non-blocking reads in mind.

Note that non-locking reads not blocking on `Exclusive` locks changes
existing behavior. As such, we introduce a new cluster setting
(`kv.lock.exclusive_locks_block_non_locking_reads`) which, when set,
reverts to the old behavior. By default, this setting is set to true,
which means we're not changing any default behavior with this patch
just yet.

References cockroachdb#91545

Release note: None
craig bot pushed a commit that referenced this issue Feb 17, 2023
96026: concurrency: introduce lock.Mode concept  r=nvanbenschoten a=arulajmani

This patch introduces the concept of a `lock.Mode`, which ties together
`lock.Strength` with other auxiliary information (read: timestamps) to
resolve conflicts between 2 lock holders.

Previously, `lock.Strength` was used both by users of the KV layer to
indicate desired locking intention and by the `concurrency` package
internally to do conflict resolution. `lock.Strength` continues to
do the former. With this change, `lock.Mode` assumes the responsibility
for the latter.

Given this motivation, this patch expresses lock compatiility/conflict
as pure functions of 2 `lock.Modes`.

This patch also introduces a new lock Strength, called Intent.
Conceptually, these are stronger than `Exclusive` locks and map on-to
the existing concept of Intents. The difference lies in their
interaction with non-locking reads. Non-locking reads conflict with
intents, whereas `Exclusive` locks do not. This is a forward looking
change with non-blocking reads in mind.

Note that non-locking reads not blocking on `Exclusive` locks changes
existing behavior. As such, we introduce a new cluster setting
(`kv.lock.exclusive_locks_block_non_locking_reads`) which, when set,
reverts to the old behavior. By default, this setting is set to true,
which means we're not changing any default behavior with this patch
just yet.

Informs #91545

Release note: None

Co-authored-by: Arul Ajmani <[email protected]>
@nvanbenschoten nvanbenschoten added the A-read-committed Related to the introduction of Read Committed label Mar 30, 2023
arulajmani added a commit to arulajmani/cockroach that referenced this issue Apr 18, 2023
SQL databases support multiple forms of row-level locks, which are locks
held by transactions on individual rows. The most common forms of these
locks are shared locks and exclusive locks. Shared locks are to reader
locks as exclusive locks are to writer locks in a reader-writer mutex.

This RFC proposes introducing SHARED key-level locks in the key-value
layer and exposing them through the `SELECT FOR SHARE` SQL statement.
This will enable the use of SHARED locks externally through
`SELECT FOR SHARE` statements. It will also enable their use internally
for operations such as foreign key checks under weaker isolation levels
(see: Read Committed).

Informs: cockroachdb#91545

Release note: None
arulajmani added a commit to arulajmani/cockroach that referenced this issue Apr 25, 2023
SQL databases support multiple forms of row-level locks, which are locks
held by transactions on individual rows. The most common forms of these
locks are shared locks and exclusive locks. Shared locks are to reader
locks as exclusive locks are to writer locks in a reader-writer mutex.

This RFC proposes introducing SHARED key-level locks in the key-value
layer and exposing them through the `SELECT FOR SHARE` SQL statement.
This will enable the use of SHARED locks externally through
`SELECT FOR SHARE` statements. It will also enable their use internally
for operations such as foreign key checks under weaker isolation levels
(see: Read Committed).

Informs: cockroachdb#91545

Release note: None
arulajmani added a commit to arulajmani/cockroach that referenced this issue May 4, 2023
When the lock table hears about lock acquisition at a newer epoch, it
no longer needs to track lock history from prior epochs. Prior to this
patch, however, we would only throw away lock history from prior epochs
for the durability with which we were acquiring the lock. This meant
that we could be needlessly tracking lock history for the other
durability from a prior epoch.

This patch changes this behavior -- we now get rid of all prior lock
acquisition history from the previous epoch for both replicated and
unreplicated locks. This change enables us to stop storing lock
histories based on durability, which is what is proposed in the shared
locks RFC.

With this behavior change, I had to rework some of our existing tests
that asserted the previous behavior. Some of the changes were
non-trivial, but I tried to make sure we continued to test what the
original test was intending to.

While here, we also add an assertion to ensure the epoch of a lock
holder never regresses. This codifies the requirement around epoch
monotonicity from a comment on the `lockHolderInfo` struct.

Informs cockroachdb#102269
Informs cockroachdb#91545

Release note: None
arulajmani added a commit to arulajmani/cockroach that referenced this issue Aug 19, 2023
Locks on a single key are stored in the `lockState` struct. Prior to
this patch, the lock table only expected a single transaction to hold
a lock on a given key at any point in time. This restriction needs to
be lifted for shared locks, whose semantics allow multiple transactions
to hold locks on a single key.

This patch changes the `lockState` datastructure so that it can be
generalized in the future. We don't actually allow multiple transactions
to acquire locks on a single key just yet -- that'll come in a subsequent
patch.

Informs cockroachdb#91545

Release note: None
arulajmani added a commit to arulajmani/cockroach that referenced this issue Aug 21, 2023
Locks on a single key are stored in the `lockState` struct. Prior to
this patch, the lock table only expected a single transaction to hold
a lock on a given key at any point in time. This restriction needs to
be lifted for shared locks, whose semantics allow multiple transactions
to hold locks on a single key.

This patch changes the `lockState` datastructure so that it can be
generalized in the future. We don't actually allow multiple transactions
to acquire locks on a single key just yet -- that'll come in a subsequent
patch.

Informs cockroachdb#91545

Release note: None
craig bot pushed a commit that referenced this issue Aug 21, 2023
…109142 #109152 #109156 #109157 #109161 #109165 #109166 #109172

107957: asim: convert randomized testing to data-driven r=kvoli a=wenyihu6

**asim: remove extra parsing for []float64, float64, time.Duration**

In cockroachdb/datadriven#45, we upstreamed the
scanning implementation in `datadriven` library. We can now handle parsing of
[]float64, float64, and time.Duration without additional handling.

Release Note: none
Epic: none

---

**asim: enable user-defined repliFactor, placement in rand range_gen**

This patch introduces two additional options for randomized range generations,
letting users define  replication factor and placement type. Although some
aspects of ranges configs are randomly generated (ranges and keyspace), these
two configurations are not randomized. Once set by the user, the configuration
will persist across iterations.

Release Note: none
Part Of: #106311

---

**asim: convert randomized testing to data-driven**
Previously, the randomized testing framework depends on default settings
hardcoded in the tests, requiring users to change code-configured
parameters to change the settings. This patch converts the framework to a
data-driven approach, enabling more dynamic user inputs, more testing examples,
and greater visibility into what each iteration is testing.

TestRandomized is a randomized data-driven testing framework that validates
allocators by creating randomized configurations. It is designed for
regression and exploratory testing.

**There are three modes for every aspect of randomized generation.**
- Static Mode:
1. If randomization options are disabled (e.g. no rand_ranges command is
used), the system uses the default configurations (defined in
default_settings.go) with no randomization.
- Randomized: two scenarios occur:
2. Use default settings for randomized generation (e.g.rand_ranges)
3. Use settings specified with commands (e.g.rand_ranges
range_gen_type=zipf)

**The following commands are provided:**
```
1. "rand_cluster" [cluster_gen_type=(single_region|multi_region|any_region)]
	e.g. rand_cluster cluster_gen_type=(multi_region)
	- rand_cluster: randomly picks a predefined cluster configuration
   according to the specified type.
	- cluster_gen_type (default value is multi_region) is cluster
   configuration type. On the next eval, the cluster is generated as the
   initial state of the simulation.

2. "rand_ranges" [placement_type=(even|skewed|random|weighted_rand)]
	[replication_factor=<int>] [range_gen_type=(uniform|zipf)]
	[keyspace_gen_type=(uniform|zipf)] [weighted_rand=(<[]float64>)]
	e.g. rand_ranges placement_type=weighted_rand weighted_rand=(0.1,0.2,0.7)
	e.g. rand_ranges placement_type=skewed replication_factor=1
		 range_gen_type=zipf keyspace_gen_type=uniform
	- rand_ranges: randomly generate a distribution of ranges across stores
   based on the specified parameters. On the next call to eval, ranges and
   their replica placement are generated and loaded to initial state.
	- placement_type(default value is even): defines the type of range placement
	  distribution across stores. Once set, it remains constant across
	  iterations with no randomization involved.
	- replication_factor(default value is 3): represents the replication factor
	  of each range. Once set, it remains constant across iterations with no
	  randomization involved.
	- range_gen_type(default value is uniform): represents the type of
	  distribution used to yield the range parameter as ranges are generated
   across iterations (range ∈[1, 1000]).
	- keyspace_gen_type: represents the type of distribution used to yield the
   keyspace parameter as ranges are generated across iterations
   (keyspace ∈[1000,200000]).
	- weighted_rand: specifies the weighted random distribution among stores.
	  Requirements (will panic otherwise): 1. weighted_rand should only be
   used with placement_type=weighted_rand and vice versa. 2. Must specify a
   weight between [0.0, 1.0] for each element in the array, with each element
   corresponding to a store 3. len(weighted_rand) cannot be greater than
   number of stores 4. sum of weights in the array should be equal to 1

3. "eval" [seed=<int64>] [num_iterations=<int>] [duration=<time.Duration>]
[verbose=<bool>]
e.g. eval seed=20 duration=30m2s verbose=true
   - eval: generates a simulation based on the configuration set with the given
   commands.
   - seed(default value is int64(42)): used to create a new random number
   generator which will then be used to create a new seed for each iteration.
   - num_iterations(default value is 3): specifies the number of simulations to
   run.
   - duration(default value is 10m): defines duration of each iteration.
   - verbose(default value is false): if set to true, plots all stat(as
   specified by defaultStat) history.
```

RandTestingFramework is initialized with specified testSetting and maintains
its state across all iterations. It repeats the test with different random
configurations. Each iteration in RandTestingFramework executes the following
steps:
1. Generates a random configuration: based on whether randOption is on and
the specific settings for randomized generation.
2. Executes the simulation and checks the assertions on the final state.
3. Stores any outputs and assertion failures in a buffer.

Release note: None
Part Of: #106311

108185: server: remove support for sticky engines r=itsbilal a=jbowens

Remove support for reusing engines from the StickyVFSRegistry. Tests should not
depend on ephemeral, in-memory engine state between server restarts, or read
closed Engine state.

Close #108119.

108467: sql: implement oidvectortypes builtin r=fqazi a=fqazi

Previously, the oidvectortypes builtin in wasn't implemented, causing a compatibility gap for tools
that need to format oidvectors. To address this, this patch adds the oidvectortypes built in.

Fixes: #107942

Release note (sql change): The oidvectortypes built-in has been implemented, which can format oidvector.

108678: closedts: make settings TenantReadOnly and public r=erikgrinaker a=erikgrinaker

It doesn't make sense for these to be `TenantWritable`, since the side transport runs below KV. Furthermore, these settings are referenced throughout our documentation, so make them public.

These should really be set only for the system tenant, and secondary tenants could simply read the system tenant's setting. This functionality runs in the host cluster below KV and it doesn't make any sense to set individual settings for tenants here. Unfortunately, this isn't currently possible with the existing settings classes, there is no way for secondary tenants to access the host's settings.

Touches #108677.

Epic: none
Release note (ops change): The following closed timestamp side-transport settings can no longer be set from secondary tenants (they did not have an effect in secondary tenants): kv.closed_timestamp.target_duration, kv.closed_timestamp.side_transport_interval, and kv.closed_timestamp.lead_for_global_reads_override.

108845: sql: add last_updated column to crdb_internal.kv_protected_ts_records r=jayshrivastava a=jayshrivastava

This change adds a `last_updated` column to the protected timestamps virtual table. This column contains the mvcc timestamp of the row. Having this column present in this table, which is included in debug zips, improves observability when debugging issues.

Informs: #104161
Release note: None
Epic: None

109029: sql: fix TestCreateStatisticsCanBeCancelled txn retry hang r=fqazi a=fqazi

Previously, this test could hang if there was an automatic
stats came in concurrently with a manual stats collection,
where the request filter would end up hanging and being called twice.
To address this patch will disable automatic stats collections
on the table.


Fixes: #109007

Release note: None

109049: concurrency: allow multiple transactions to hold locks on a single key  r=nvanbenschoten a=arulajmani

Locks on a single key are stored in the `lockState` struct. Prior to
this patch, the lock table only expected a single transaction to hold
a lock on a given key at any point in time. This restriction needs to
be lifted for shared locks, whose semantics allow multiple transactions
to hold locks on a single key.

This patch changes the `lockState` datastructure so that it can be
generalized in the future. We don't actually allow multiple transactions
to acquire locks on a single key just yet -- that'll come in a subsequent
patch.

Informs #91545

Release note: None

109087: storage: defer putBuffer release in all cases r=nvanbenschoten a=nvanbenschoten

Minor cleanup.

This commit switches the remainder of the calls to putBuffer.release to be deferred, instead of being manually called at the end of their function. The comments mentioning that the defer was "measurably slower" were introduced in 4444618, which was before Go 1.14 optimized the performance of defer. Most of these, including the more performance-sensitive calls, were already switched over to use defer in fbe8852.

Epic: None
Release note: None

109142: roachtest: Cast snapshot-recd bytes to int in disagg-rebalance r=jbowens a=itsbilal

Previously we were reading a float value as an int, which would trip up the Scan() method if the float value was large enough to be wired over in scientified notation eg. `2.3456E7`. This change ensures that Cockroach prints out the value as an integer to avoid the scan-time error in the roachtest.

Fixes #109114.

Epic: none

Release note: None

109152: build: update some configurations for remote build execution r=rail a=rickystewart

1. Use the `large` pool of executors for `enormous` test targets
2. Add (temporary) network access to the following tests: `amazon_test`,
   `base_test`, `cloudprivilege_test`, `externalconn_test`, and
   `cockroach-go-testserver-upgrade-to-master` logictests. These
   erroneously have a dependency on network assets; bugs have been
   filed for each of these.

Epic: CRDB-8308
Release note: None

109156: sql: version gate UNIQUE constraint with json column r=rafiss a=rafiss

This prevents the usage of a json column in a unique constraint, until after the upgrade is finalized.

fixes #108978
Release note: None

109157:  ci,ui: don't lint `e2e-tests` r=sjbarag a=rickystewart

This workspace has a huge download of `cypress` which was causing
CI to flake.

Epic: none
Release note: None

109161: workload: add background qos to kv workload r=bananabrick a=bananabrick

A --background-qos flag can be used in the kv workload to ensure that the generated work is treated as low priority by admission control.

Epic: none
Release note: None

109165: Revert "rangefeed/changefeed: Enable mux rangefeeds by default." r=erikgrinaker a=erikgrinaker

This reverts commit de65c54.

We decided to keep these disabled for another release, to get more real-world experience with it first.

Touches #95781.
Touches #105270.

Release note (performance improvement): The following release note no longer applies: "mux range feeds reuse connection and workers across multiple range feeds.  This mode is now enabled by default."

109166: build: more resources for building AWS dependency r=rail a=rickystewart

This is a huge package with apparently a lot of auto-generated code that was causing OOM's on EngFlow RBE. This fixes it.

Epic: none
Release note: CRDB-8308

109172: storage: Fix panic in MVCCHistories test r=jbowens a=itsbilal

storage_test.intentPrintingReadWriter previously did not support ReaderWithMustIterators.

Epic: none

Release note: None

Co-authored-by: wenyihu6 <[email protected]>
Co-authored-by: Jackson Owens <[email protected]>
Co-authored-by: Faizan Qazi <[email protected]>
Co-authored-by: Erik Grinaker <[email protected]>
Co-authored-by: Jayant Shrivastava <[email protected]>
Co-authored-by: Arul Ajmani <[email protected]>
Co-authored-by: Nathan VanBenschoten <[email protected]>
Co-authored-by: Bilal Akhtar <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>
Co-authored-by: Arjun Nair <[email protected]>
craig bot pushed a commit that referenced this issue Sep 28, 2023
110377: pkg/server: add an extra server startup guardrail r=knz a=healthy-pod

This code change prevents a SQL server from starting if its minimum supported binary version is greater than the tenant active version.

Release note: None
Epic: CRDB-26691

111143: cli: add --experimental-secondary-cache flag r=RaduBerinde a=itsbilal

This change adds a new `--experimental-secondary-cache` flag for use with `--experimental-shared-storage` to enable the use of a secondary cache to speed up reads of objects in shared storage. This option sets the max cache size for each store using disaggregated storage; for per-store granularity, pebble options can also be used.

Epic: none

Release note: None

111173: sql: add some tests for lookup join when eq cols are key r=yuzefovich a=yuzefovich

These tests are "regression tests" for #108489 if it were to be implemented.

Epic: None

Release note: None

111325: roachtest: add test to ruby-pg blocklist r=fqazi a=andyyang890

Fixes #111116

Release note: None

111384: process-bep-file: create binary for fetching test results from EngFlow r=healthy-pod a=rickystewart

I'll extend this to additionally allow posting GitHub issues.

Part of: DEVINF-871
Epic: CRDB-8308
Release note: None

111433: kv: add shared, replicated, and shared-replicated locks to TestEvaluateBatch r=nvanbenschoten a=nvanbenschoten

Informs #91545.
Informs #100193.

This commit extends TestEvaluateBatch to include shared, replicated, and shared-replicated lock acquisition using Get, Scan, and ReverseScan requests.

Release note: None

111434: roachtest: automatically profile CPU in restore tests r=msbutler a=pavelkalinnikov

See https://www.cockroachlabs.com/docs/v23.1/automatic-cpu-profiler.

NB: the `server.cpu_profile.enabled` setting is not used because it was recently removed in #107717. It should be used if this change is backported.

Touches #111160, #111159
Epic: none
Release note: none

Co-authored-by: healthy-pod <[email protected]>
Co-authored-by: Bilal Akhtar <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>
Co-authored-by: Andy Yang <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>
Co-authored-by: Nathan VanBenschoten <[email protected]>
Co-authored-by: Pavel Kalinnikov <[email protected]>
nvanbenschoten added a commit to nvanbenschoten/cockroach that referenced this issue Sep 28, 2023
…tRequests

Informs cockroachdb#91545.
Informs cockroachdb#100193.

This commit expands TestLockTableConcurrentRequests to support Shared,
Exclusive, and Intent locking strength, in addition to both Replicated
and Unreplicated locking durabilities. This provides randomized coverage
of the lock table with these combinations.

The commit then temporarily disables Shared locks in the test, which
occasionally fail with the panic: `tryMakeNewDistinguished called with new claimant txn`.
This is related to cockroachdb#111144, so we can re-enable Shared locks when that issue
is resolved.

Release note: None
arulajmani added a commit to arulajmani/cockroach that referenced this issue Sep 29, 2023
Previously, FOR SHARE and FOR KEY SHARE would use non-locking KV scans.
Now that the lock table supports shared locks, we can use lock.Shared as
the locking strength for KV scans. This patch does that, and in doing
so, wires up SHARED locks end to end.

By default, we turn of this functionality for serializable transactions.
Instead, it's gated behind a session setting called
`enable_shared_locking_for_serializable`.

Informs cockroachdb#91545

Release note (sql change): Shared locks are a thing.
nvanbenschoten added a commit to nvanbenschoten/cockroach that referenced this issue Sep 29, 2023
…tRequests

Informs cockroachdb#91545.
Informs cockroachdb#100193.

This commit expands TestLockTableConcurrentRequests to support Shared,
Exclusive, and Intent locking strength, in addition to both Replicated
and Unreplicated locking durabilities. This provides randomized coverage
of the lock table with these combinations.

The commit then temporarily disables Shared locks in the test, which
occasionally fail with the panic: `tryMakeNewDistinguished called with new claimant txn`.
This is related to cockroachdb#111144, so we can re-enable Shared locks when that issue
is resolved.

Release note: None
craig bot pushed a commit that referenced this issue Sep 29, 2023
111465: kv: add all lock strengths and durabilities to TestLockTableConcurrentRequests r=nvanbenschoten a=nvanbenschoten

Informs #91545.
Informs #100193.

This commit expands `TestLockTableConcurrentRequests` to support Shared, Exclusive, and Intent locking strength, in addition to both Replicated and Unreplicated locking durabilities. This provides randomized coverage of the lock table with these combinations.

The commit then temporarily disables Shared locks in the test, which occasionally fail with the panic: `tryMakeNewDistinguished called with new claimant txn`. This is related to #111144, so we can re-enable Shared locks when that issue is resolved.

Release note: None

Co-authored-by: Nathan VanBenschoten <[email protected]>
arulajmani added a commit to arulajmani/cockroach that referenced this issue Sep 29, 2023
Previously, FOR SHARE and FOR KEY SHARE would use non-locking KV scans.
Now that the lock table supports shared locks, we can use lock.Shared as
the locking strength for KV scans. This patch does that, and in doing
so, wires up SHARED locks end to end.

By default, we turn off this functionality for serializable transactions.
Instead, it's gated behind a session setting called
`enable_shared_locking_for_serializable`.

Informs cockroachdb#91545

Release note (sql change): SELECT FOR SHARE and SELECT FOR UPDATE
previously did not acquire any locks. Users issuing these statements
would expect them to acquire shared locks (multiple readers allowed,
no writers though). This patch switches over the behavior to acquire
such read locks.

For serializable transactions, we default to the old behaviour, unless
the `enable_shared_locking_for_serializable` session setting is set
to true. We'll probably switch this behavior in the future, but for
now, given shared locks are a preview feature, we gate things behind
a session setting.
arulajmani added a commit to arulajmani/cockroach that referenced this issue Sep 29, 2023
Previously, FOR SHARE and FOR KEY SHARE would use non-locking KV scans.
Now that the lock table supports shared locks, we can use lock.Shared as
the locking strength for KV scans. This patch does that, and in doing
so, wires up SHARED locks end to end.

By default, we turn off this functionality for serializable transactions.
Instead, it's gated behind a session setting called
`enable_shared_locking_for_serializable`.

Informs cockroachdb#91545

Release note (sql change): SELECT FOR SHARE and SELECT FOR KEY SHARE
previously did not acquire any locks. Users issuing these statements
would expect them to acquire shared locks (multiple readers allowed,
no writers though). This patch switches over the behavior to acquire
such read locks.

For serializable transactions, we default to the old behaviour, unless
the `enable_shared_locking_for_serializable` session setting is set
to true. We'll probably switch this behavior in the future, but for
now, given shared locks are a preview feature, we gate things behind
a session setting.
arulajmani added a commit to arulajmani/cockroach that referenced this issue Sep 29, 2023
Previously, FOR SHARE and FOR KEY SHARE would use non-locking KV scans.
Now that the lock table supports shared locks, we can use lock.Shared as
the locking strength for KV scans. This patch does that, and in doing
so, wires up SHARED locks end to end.

By default, we turn off this functionality for serializable transactions.
Instead, it's gated behind a session setting called
`enable_shared_locking_for_serializable`.

Informs cockroachdb#91545

Release note (sql change): SELECT FOR SHARE and SELECT FOR KEY SHARE
previously did not acquire any locks. Users issuing these statements
would expect them to acquire shared locks (multiple readers allowed,
no writers though). This patch switches over the behavior to acquire
such read locks.

For serializable transactions, we default to the old behaviour, unless
the `enable_shared_locking_for_serializable` session setting is set
to true. We'll probably switch this behavior in the future, but for
now, given shared locks are a preview feature, we gate things behind
a session setting.
arulajmani added a commit to arulajmani/cockroach that referenced this issue Sep 29, 2023
Previously, FOR SHARE and FOR KEY SHARE would use non-locking KV scans.
Now that the lock table supports shared locks, we can use lock.Shared as
the locking strength for KV scans. This patch does that, and in doing
so, wires up SHARED locks end to end.

By default, we turn off this functionality for serializable transactions.
Instead, it's gated behind a session setting called
`enable_shared_locking_for_serializable`.

Informs cockroachdb#91545

Release note (sql change): SELECT FOR SHARE and SELECT FOR KEY SHARE
previously did not acquire any locks. Users issuing these statements
would expect them to acquire shared locks (multiple readers allowed,
no writers though). This patch switches over the behavior to acquire
such read locks.

For serializable transactions, we default to the old behaviour, unless
the `enable_shared_locking_for_serializable` session setting is set
to true. We'll probably switch this behavior in the future, but for
now, given shared locks are a preview feature, we gate things behind
a session setting.
arulajmani added a commit to arulajmani/cockroach that referenced this issue Sep 29, 2023
Previously, FOR SHARE and FOR KEY SHARE would use non-locking KV scans.
Now that the lock table supports shared locks, we can use lock.Shared as
the locking strength for KV scans. This patch does that, and in doing
so, wires up SHARED locks end to end.

By default, we turn off this functionality for serializable transactions.
Instead, it's gated behind a session setting called
`enable_shared_locking_for_serializable`.

Informs cockroachdb#91545

Release note (sql change): SELECT FOR SHARE and SELECT FOR KEY SHARE
previously did not acquire any locks. Users issuing these statements
would expect them to acquire shared locks (multiple readers allowed,
no writers though). This patch switches over the behavior to acquire
such read locks.

For serializable transactions, we default to the old behaviour, unless
the `enable_shared_locking_for_serializable` session setting is set
to true. We'll probably switch this behavior in the future, but for
now, given shared locks are a preview feature, we gate things behind
a session setting.
craig bot pushed a commit that referenced this issue Sep 29, 2023
109638: sql: use the correct locking strength for FOR SHARE clause r=michae2 a=arulajmani

Previously, FOR SHARE and FOR KEY SHARE would use non-locking KV scans.
Now that the lock table supports shared locks, we can use lock.Shared as
the locking strength for KV scans. This patch does that, and in doing
so, wires up SHARED locks end to end.

By default, we turn of this functionality for serializable transactions.
Instead, it's gated behind a session setting called
`enable_shared_locking_for_serializable`.

Informs #91545

Release note (sql change): SELECT FOR SHARE and SELECT FOR UPDATE
previously did not acquire any locks. Users issuing these statements
would expect them to acquire shared locks (multiple readers allowed,
no writers though). This patch switches over the behavior to acquire
such read locks.

For serializable transactions, we default to the old behaviour, unless
the `enable_shared_locking_for_serializable` session setting is set
to true. We'll probably switch this behavior in the future, but for
now, given shared locks are a preview feature, we gate things behind
a session setting.

111441: goschedstats: reduce underloadedRunnablePerProcThreshold r=aadityasondhi a=sumeerbhola

By reducing this threshold, we more often sample the stats at 1ms intervals, which is desirable for admission control slot adjustment.

Fixes #111125

Epic: none

Release note: None

111466: ui: update default timescale to 1h r=maryliag a=maryliag

On the Metrics page, the default value was 10min, which was too small. This commit updates it to 1h to match the most selected value on the SQL Activity.

Fixes #96479

Release note: None

111521: roachprod: fix createTenantCertBundle script r=andy-kimball a=herkolategan

The `createTenantCertBundle` function has a script in it that has erroneous "+" characters which results in an error when trying to start a secure cluster. This change removes those characters.

Epic: None
Release Note: None

111526: kv/spanset: permit writes to lock table by shared lock latching configuration r=nvanbenschoten a=nvanbenschoten

Fixes #111409.
Fixes #111492.

This commit updates addLockTableSpans to account for the way that shared locking requests declare their latches. Even though they declare a "read" latch, they do so at max timestamp, which gives them sufficient isolation to write to the lock table without having to declare a write latch and be serialized with other shared lock acquisitions.

Release note: None

Co-authored-by: Arul Ajmani <[email protected]>
Co-authored-by: sumeerbhola <[email protected]>
Co-authored-by: maryliag <[email protected]>
Co-authored-by: Herko Lategan <[email protected]>
Co-authored-by: Nathan VanBenschoten <[email protected]>
@craig craig bot closed this as completed in #109638 Sep 29, 2023
THardy98 pushed a commit to THardy98/cockroach that referenced this issue Oct 6, 2023
…teBatch

Informs cockroachdb#91545.
Informs cockroachdb#100193.

This commit extends TestEvaluateBatch to include shared, replicated, and
shared-replicated lock acquisition using Get, Scan, and ReverseScan requests.

Release note: None
THardy98 pushed a commit to THardy98/cockroach that referenced this issue Oct 6, 2023
…tRequests

Informs cockroachdb#91545.
Informs cockroachdb#100193.

This commit expands TestLockTableConcurrentRequests to support Shared,
Exclusive, and Intent locking strength, in addition to both Replicated
and Unreplicated locking durabilities. This provides randomized coverage
of the lock table with these combinations.

The commit then temporarily disables Shared locks in the test, which
occasionally fail with the panic: `tryMakeNewDistinguished called with new claimant txn`.
This is related to cockroachdb#111144, so we can re-enable Shared locks when that issue
is resolved.

Release note: None
THardy98 pushed a commit to THardy98/cockroach that referenced this issue Oct 6, 2023
Previously, FOR SHARE and FOR KEY SHARE would use non-locking KV scans.
Now that the lock table supports shared locks, we can use lock.Shared as
the locking strength for KV scans. This patch does that, and in doing
so, wires up SHARED locks end to end.

By default, we turn off this functionality for serializable transactions.
Instead, it's gated behind a session setting called
`enable_shared_locking_for_serializable`.

Informs cockroachdb#91545

Release note (sql change): SELECT FOR SHARE and SELECT FOR KEY SHARE
previously did not acquire any locks. Users issuing these statements
would expect them to acquire shared locks (multiple readers allowed,
no writers though). This patch switches over the behavior to acquire
such read locks.

For serializable transactions, we default to the old behaviour, unless
the `enable_shared_locking_for_serializable` session setting is set
to true. We'll probably switch this behavior in the future, but for
now, given shared locks are a preview feature, we gate things behind
a session setting.
aliher1911 pushed a commit to aliher1911/cockroach that referenced this issue Oct 9, 2023
…teBatch

Informs cockroachdb#91545.
Informs cockroachdb#100193.

This commit extends TestEvaluateBatch to include shared, replicated, and
shared-replicated lock acquisition using Get, Scan, and ReverseScan requests.

Release note: None
aliher1911 pushed a commit to aliher1911/cockroach that referenced this issue Oct 9, 2023
…tRequests

Informs cockroachdb#91545.
Informs cockroachdb#100193.

This commit expands TestLockTableConcurrentRequests to support Shared,
Exclusive, and Intent locking strength, in addition to both Replicated
and Unreplicated locking durabilities. This provides randomized coverage
of the lock table with these combinations.

The commit then temporarily disables Shared locks in the test, which
occasionally fail with the panic: `tryMakeNewDistinguished called with new claimant txn`.
This is related to cockroachdb#111144, so we can re-enable Shared locks when that issue
is resolved.

Release note: None
@exalate-issue-sync exalate-issue-sync bot added the P-1 Issues/test failures with a fix SLA of 1 month label Jan 4, 2024
@github-project-automation github-project-automation bot moved this to Closed in KV Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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) P-1 Issues/test failures with a fix SLA of 1 month T-kv KV Team
Projects
No open projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants