cockroachdb / cockroach

CockroachDB — the cloud native, distributed SQL database designed for high availability, effortless scale, and control over data placement.
https://www.cockroachlabs.com
Other
30.11k stars 3.81k forks source link

kv: disallow non-transactional locking requests #109614

Open arulajmani opened 1 year ago

arulajmani commented 1 year ago

Describe the problem

Non-transactional requests are not allow to acquire locks. However, we do allow non-transactional read requests to have a locking strength associated with them. They use this locking strength to declare lock spans and perform conflict resolution during their scan of the lock table. They also use this locking strength to check which keys to skip over (or not to skip over) when running with the SKIP LOCKED option.

We should reject non-transactional requests that have an associated locking strength. They aren't used by SQL and there's very little benefit in supporting them. On the flip side, rejecting them reduces the testing space we need to account for in the lock table. It also allows us to tighten some assertions we removed in https://github.com/cockroachdb/cockroach/pull/109610.

Jira issue: CRDB-31020

arulajmani commented 1 year ago

Should've added to earlier, but non-transactional writes shouldn't be included in the definition of "non-transactional locking requests" for what the issue is asking for.

arulajmani commented 1 month ago

We've since changed our mind on this. Paraphrasing a conversation with @nvanbenschoten, the benefit of keeping this around is that it allows us to hit 1PC for batches that have locking reads and writes in them, as hitting 1PC involves running the batch non-transactionally.

Note that currently, SQL doesn't construct batches which have both reads/writes in them, so the benefit above is a bit theoretical. However, non-transactional writes are fully supported and tested, so there's not much benefit to go out of our way to rip them out right now.

arulajmani commented 1 month ago

cc @tbg -- your comment reminded me of this issue.