Skip to content

concurrency: improve findNextLockAfter naming and add some commentary#104600

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
arulajmani:find-next-lock-after-improve-naming
Jun 12, 2023
Merged

concurrency: improve findNextLockAfter naming and add some commentary#104600
craig[bot] merged 1 commit intocockroachdb:masterfrom
arulajmani:find-next-lock-after-improve-naming

Conversation

@arulajmani
Copy link
Copy Markdown
Collaborator

The new names/commentary better reflect some of the intentions here. These will compose better once we get rid of the tryActiveWait naming as well.

Epic: none

Release note: None

@arulajmani arulajmani requested a review from nvb June 8, 2023 15:26
@arulajmani arulajmani requested a review from a team as a code owner June 8, 2023 15:26
@blathers-crl
Copy link
Copy Markdown

blathers-crl bot commented Jun 8, 2023

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

Copy link
Copy Markdown
Contributor

@nvb nvb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @arulajmani)

The new names/commentary better reflect some of the intentions here.
These will compose better once we get rid of the `tryActiveWait`
naming as well.

Epic: none

Release note: None
@arulajmani arulajmani force-pushed the find-next-lock-after-improve-naming branch from b7a5225 to 6276b0c Compare June 12, 2023 14:08
@arulajmani
Copy link
Copy Markdown
Collaborator Author

TFTR!

bors r+

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Jun 12, 2023

This PR was included in a batch that successfully built, but then failed to merge into master. It will not be retried.

Additional information:

{"message":"Changes must be made through a pull request.","documentation_url":"https://docs.github.com/articles/about-protected-branches"}

@arulajmani
Copy link
Copy Markdown
Collaborator Author

bors r+ single on

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Jun 12, 2023

Build succeeded:

@craig craig bot merged commit 3d4db0a into cockroachdb:master Jun 12, 2023
craig bot pushed a commit that referenced this pull request Jun 27, 2023
104620: concurrency: use lock modes to find conflicts during lock table scans r=nvanbenschoten a=arulajmani

Previous attempt over at: #104261

First commit from: #104537
Second commit from: #104600

This patch majorly refactors the lock table scan codepath, all in the
name of shared locks. At its core is a desire to use lock modes to
perform conflict resolution between an incoming request and locks held
on one particular key. In doing so, we rip out tryActiveWait.

At a high level (for a particular key), a request's journey looks like
the following:

- It first checks if the transaction already holds a lock at a equal or
higher lock strength (read: It's good enough for its use). If this is
the case, it can proceed without any bookkeeping.
- It then checks if any finalized transactions hold locks on the key.
Such locks do not conflict, but need to be resolved before the
transaction can evaluate. They're thus accumulated for later.
- The request then enqueues itself in the appropriate wait queue.
- It then determines if it needs to actively wait at this lock because
of a conflict. If that's the case, the lock table scan short circuits.
- Otherwise, the request lays a claim (if it can) before proceeding with
its scan.

Closes #102210

Release note: None

105482: sqltelemetry: add missing schema telemetry r=postamar a=rafiss

CREATE [ SCHEMA | INDEX | FUNCTION | TYPE ] and ALTER FUNCTION did not
have any telemetry, but they should.

Epic: None
Release note: None

105579: sql: disallow cross-database type references in CTAS r=chengxiong-ruan a=chengxiong-ruan

Fixes: #105393

Release note (bug fix): reviously, cross-database type references could sneaked in through `CREATE TABLE...AS` statements if the source table is from another database and any of its columns is of a user defined type. This introduced bug where the source table can be dropped and type could not be found for the CTAS table. This commit disallow such CTAS as a fix.

105581: optbuilder: reset annotations when building CREATE FUNCTION r=rafiss a=rafiss

In 22dabb0 we started overriding the annotations for each statement in the UDF body. We should reset them to the original values, so we don't accidentally leave the old reference.

Epic: None
Release note: None

105596: storage: make `storage.max_sync_duration` public and `TenantReadOnly` r=erikgrinaker a=erikgrinaker

Users have asked why this setting is not public, this patch makes it so.

Furthermore, these settings were `TenantWritable`. We do not want these to be writable by tenants, where they can potentially cause problems on SQL nodes, considering e.g. SQL disk spilling uses Pebble.

Epic: none
Release note: None

Co-authored-by: Arul Ajmani <arulajmani@gmail.com>
Co-authored-by: Rafi Shamim <rafi@cockroachlabs.com>
Co-authored-by: Chengxiong Ruan <chengxiongruan@gmail.com>
Co-authored-by: Erik Grinaker <grinaker@cockroachlabs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants