Skip to content

[kv] Improve point lookup concurrency with KV writes #4057

Description

@platinumhamburg

Search before asking

  • I searched in the issues and found nothing similar.

Motivation

KvTablet currently uses one read-write lock to protect both the tablet lifecycle and the mutable write pipeline.

A point lookup acquires the read lock, while putAsLeader acquires the write lock for the complete write path, including row merging, auto-increment handling, pre-write-buffer updates, and WAL append. As a result, point lookups and writes to the same bucket are fully serialized, even though ordinary lookups only read the committed RocksDB state and do not access the pre-write buffer.

This unnecessarily limits lookup throughput and increases lookup tail latency under mixed read/write workloads.

Solution

Split the current lock responsibilities into two locks:

  • kvStateLock

    • Protects the RocksDB-visible state and the KvTablet lifecycle.
    • Ordinary lookups and puts acquire its read lock.
    • Flush, snapshot, and close acquire its write lock.
  • mutationLock

    • Serializes the non-thread-safe write pipeline, including row merging, auto-increment allocation, pre-write-buffer mutation, WAL append, and rollback.
    • Puts are still serialized with each other.

When both locks are required, they must always be acquired in the following order:

kvStateLock -> mutationLock

The expected concurrency behavior is:

Operations Concurrent
Point lookup and put Yes
Point lookup and point lookup Yes
Put and put No
Point lookup and flush No
Put and flush No
Point lookup and close No

A logical flush must hold kvStateLock in write mode across all RocksDB writes and the corresponding flushedLogOffset and row-count updates. This ensures that lookups cannot observe a partially applied flush.

Ordinary point lookups should continue reading only RocksDB, preserving the existing committed-data visibility semantics. Internal reads that also inspect the pre-write buffer must additionally acquire mutationLock.

RocksDB close must remain protected by kvStateLock in write mode so that native close cannot race with point lookup JNI calls. Long-lived scan iterators must continue holding a ResourceGuard.Lease because they outlive a single lock acquisition.

Callbacks that may enter Replica lifecycle code must be invoked after the internal locks are released. Backpressure-triggered flush requests must also be scheduled after releasing the read lock to avoid read-to-write lock upgrading.

Expected benefit

This allows point lookups to overlap the expensive merge, pre-write, and WAL portions of a put while preserving existing write ordering and committed-read semantics.

The main expected benefit is improved lookup throughput and tail latency for mixed read/write workloads on the same bucket. Pure-read and pure-write workloads are not expected to change significantly.

Correctness requirements

The change must preserve the following properties:

  • Ordinary lookups never expose unflushed pre-write-buffer records.
  • A lookup observes either the state before a logical flush or the completed state, never an intermediate flush segment.
  • Put ordering, WAL ordering, duplicate handling, and failure rollback remain serialized.
  • Snapshot data and tablet metadata represent the same state.
  • No RocksDB JNI operation races with native close.
  • No new lock-order cycle or read-to-write lock upgrade is introduced.
  • Scanner native resources are not released while an RPC is still using them.

Anything else?

The implementation should include concurrency tests covering:

  • Point lookup overlapping an in-progress put.
  • Put-put serialization.
  • Lookup exclusion during flush and close.
  • Visibility during a segmented RocksDB flush.
  • Fatal-error callbacks executing outside internal locks.
  • Scanner close racing with an in-flight continuation.

Willingness to contribute

  • I'm willing to submit a PR!

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions