Skip to content

kv: acquire latches during READ_UNCOMMITTED scans#160257

Closed
MASA-JAPAN wants to merge 1 commit intocockroachdb:masterfrom
MASA-JAPAN:fix-read-uncommitted-latches
Closed

kv: acquire latches during READ_UNCOMMITTED scans#160257
MASA-JAPAN wants to merge 1 commit intocockroachdb:masterfrom
MASA-JAPAN:fix-read-uncommitted-latches

Conversation

@MASA-JAPAN
Copy link

@MASA-JAPAN MASA-JAPAN commented Dec 29, 2025

kv: acquire latches during READ_UNCOMMITTED scans

Fixes #98862

Summary

This PR modifies READ_UNCOMMITTED requests to acquire latches, providing real-time ordering guarantees while still allowing dirty reads. Previously, READ_UNCOMMITTED requests skipped latches (behaving identically to INCONSISTENT), which sacrificed real-time ordering guarantees.

Problem

READ_UNCOMMITTED scans could violate real-time ordering by missing recently written data. Consider this scenario:

Before this change (buggy behavior):

Time 1: Process A writes key 'x' = 100
        └─> Acquires write latch
        └─> Write completes, releases latch
        └─> Returns success to client

Time 2: Process B reads key 'x' with READ_UNCOMMITTED
        └─> Skips latches (no synchronization!)
        └─> Runs in parallel with write being applied
        └─> Gets old value or nothing! ❌

The issue: Process B's read happened AFTER Process A's write completed, but Process B didn't see the write because READ_UNCOMMITTED skipped latches. The read could execute in parallel with the write being applied to storage, leading to stale data being returned.

After this change (correct behavior):

Time 1: Process A writes key 'x' = 100
        └─> Acquires write latch on 'x'
        └─> Write completes, releases latch
        └─> Returns success to client

Time 2: Process B reads key 'x' with READ_UNCOMMITTED
        └─> Acquires read latch on 'x' ✓
        └─> WAITS for any conflicting write latches
        └─> Sees the updated value: 100 ✓

Real-time ordering guaranteed: Reads that start after a write completes will see that write's data.

Real-world example:

In an e-commerce system using READ_UNCOMMITTED for performance:

-- Process 1: Update inventory
UPDATE products SET stock = stock - 1 WHERE id = 'widget-123';
-- Returns success ✓

-- Process 2: Check stock (uses READ_UNCOMMITTED)
SELECT stock FROM products WHERE id = 'widget-123';
-- Without latches: might see the old stock value!
-- This violates real-time ordering

Two customers could both see "1 in stock" simultaneously and both successfully order, even though one update completed before the other's read started.

Solution

This PR makes READ_UNCOMMITTED acquire latches (as shown in the "After" section above), ensuring real-time ordering guarantees while still differing from CONSISTENT reads in that it can see uncommitted/in-flight data (dirty reads).

Changes

  1. pkg/kv/kvserver/concurrency/concurrency_manager.go: Modified shouldIgnoreLatches() to check specifically for INCONSISTENT rather than != CONSISTENT, allowing READ_UNCOMMITTED to acquire latches.

  2. pkg/kv/kvpb/api.proto: Updated documentation to clarify that READ_UNCOMMITTED now acquires latches for real-time ordering, distinguishing it from INCONSISTENT reads.

  3. pkg/kv/kvserver/concurrency/concurrency_manager_test.go: Added support for read-uncommitted flag in the test DSL.

  4. pkg/kv/kvserver/concurrency/testdata/concurrency_manager/no_latches: Added test case verifying READ_UNCOMMITTED acquires latches while INCONSISTENT does not.

Background: Latches vs Locks

CockroachDB uses two synchronization mechanisms:

Feature Latches Locks
Duration Microseconds (during operation) Milliseconds to seconds (transaction lifetime)
Scope Single command execution Entire transaction
Purpose Prevent concurrent I/O on same keys Prevent conflicting transactions
Released Immediately after operation completes When transaction commits/aborts

This PR is about latches, which provide ordering for individual operations. Without latches, READ_UNCOMMITTED operations could run in parallel with writes on the same keys, breaking real-time ordering guarantees.

Behavior Comparison

Behavior CONSISTENT READ_UNCOMMITTED (after fix) INCONSISTENT
Acquires latches? ✓ Yes ✓ Yes ✗ No
Sees uncommitted data? ✗ No ✓ Yes (dirty reads) ✗ No
Requires valid lease? ✓ Yes ✓ Yes ✗ No
Real-time ordering? ✓ Yes ✓ Yes ✗ No

Testing

  • Updated TestConcurrencyManagerBasic/no_latches to verify READ_UNCOMMITTED acquires latches
  • Test confirms INCONSISTENT reads still skip latches (existing behavior preserved)
  • Test confirms READ_UNCOMMITTED reads now acquire latches and scan the lock table

Release Notes: None
Epic: None

@MASA-JAPAN MASA-JAPAN requested a review from a team as a code owner December 29, 2025 02:37
@blathers-crl
Copy link

blathers-crl bot commented Dec 29, 2025

Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR.

Before a member of our team reviews your PR, I have some potential action items for you:

  • Please ensure your git commit message contains a release note.
  • When CI has completed, please ensure no errors have appeared.

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

@blathers-crl blathers-crl bot added the O-community Originated from the community label Dec 29, 2025
@cockroachlabs-cla-agent
Copy link

cockroachlabs-cla-agent bot commented Dec 29, 2025

CLA assistant check
All committers have signed the CLA.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@stevendanna
Copy link
Collaborator

Thanks for your interest in contributing to CockroachDB!

While this change looks straightforward on the surface, changes to transaction isolation require analysis and testing. If you are interested in pursuing this change, let's start by having a discussion on the motivation and desired end state in the original issue (#98862) so that we can make sure we don't ask you do to wasted work.

A few preliminary notes:

READ_UNCOMMITTED is only used internally in CockroachDB as at the SQL layer we translate READ UNCOMMITTED transactions to READ COMMITTED. As such, the real world example your pull request description should not be possible.

This may still be a worthwhile change, but we'll probably want to first discuss a issues:

  1. What is the motivation for the change and given that motivation what is the full set of requirements? That will help us create tests that will ensure we've achieved those goals.
  2. For existing users of READ_UNCOMMITTED in the database, is the new behaviour preferable or should they be converted to use INCONSISTENT?
  3. Should READ_UNCOMMITTED requests be able to drop latches early (see canReadOnlyRequestDropLatchesBeforeEval in pkg/kv/kvserver/replica_evaluate.go)?

If you are interested in pursuing this change, please do follow up with a discussion on the original issue (#98862) about the motivation for this change at this time.

For tracking purposes, I'm going to close this PR for now, but it can be re-opened once we are sure about the direction.

@stevendanna stevendanna closed this Jan 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

O-community Originated from the community T-kv KV Team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

kv: acquire latches during READ_UNCOMMITTED scans

4 participants