Add lazy wide-column read API (GetEntityLazy / MultiGetEntityLazy)

[pdillinger]

Summary:
Reading a large blob-backed wide-column entity today (GetEntity /
MultiGetEntity) eagerly resolves every blob column, even when the caller
only needs a few of them. This adds a lazy variant that returns the entity
with its inline columns materialized zero-copy and its blob-backed columns
left as unresolved references; the caller resolves only the columns it
needs, so blobs for columns that are never pulled are never read from
storage.

The lazy result is a new type (LazyWideColumns / LazyWideColumnsBatch in
rocksdb/lazy_wide_columns.h), kept separate from the ubiquitous eager
PinnableWideColumns so the eager path stays zero-overhead. Unlike GetEntity,
the result may be used after the call returns: it pins the SuperVersion (as
an iterator does) so deferred blob reads stay resolvable. The API requires
max_open_files == -1 so that same-file/embedded table readers remain valid
for later resolution, and returns InvalidArgument otherwise.

Mechanism: an optional lazy signal is threaded through GetImpl ->
Version::Get -> GetContext; on that path a found wide-column entity is left
with its blob references unresolved and the originating SST's
SameFileBlobReader is captured, and GetImpl transfers an extra SuperVersion
reference into the result. The eager GetContext path is unchanged (the
signal defaults off). Resolution reuses ReadPathBlobResolver, extended to
carry a SameFileBlobReader so same-file/embedded references resolve
uniformly with separate-file ones. MultiGetEntityLazy currently loops
single-key lookups under one pinned snapshot for a consistent view.

Scope is single column family with whole-column resolution (a requested
byte range is served by resolving the whole column and slicing it for now).

Limitations (documented on DB::GetEntityLazy in include/rocksdb/db.h):

• User-defined timestamps: wide-column entities do not support UDT at all
  (PutEntity returns InvalidArgument on a UDT column family; see
  DBWideBasicTest.PutEntityTimestampError). The lazy API inherits this and,
  on a UDT column family, behaves at parity with GetEntity (both go through
  the same read-path timestamp validation).
• Transactions: there is no Transaction counterpart; the lazy read APIs are
  available only directly on DB. Use GetEntity (which Transaction provides)
  for reads within a transaction.

Directions / follow-ups (marked with TODO(lazy-blob-resolution-phaseN) and
tracked in the lazy blob resolution plan): byte-range/partial reads (read
only the requested bytes, skipping CRC and cache-fill) for separate-file and
embedded (SimpleGen2) blobs; resolve-time ReadOptions (read_tier / verify);
cross-key coalescing with a shared per-CF pin; asynchronous execution; a
cross-column-family (CF**) MultiGetEntityLazy; a Transaction-level lazy read
API; and a column-granular lazy iterator view (Iterator::columns() resolves
all blob columns eagerly today). A separate TODO(get-context-args-struct)
will replace GetContext's telescoping constructors with a parameter struct.

Test Plan:
db/wide/db_lazy_entity_test.cc covers: enumerating an entity's columns
without triggering any blob I/O; whole-column and multi-range resolution;
resolving several blob columns in one MultiResolve call; cross-key batch
resolution; the result staying valid after the producing call returns and
after being moved; columns that are never pulled never being read from
storage (asserted via blob-read counters); InvalidArgument when
max_open_files != -1; out-of-range column index; and
UserTimestampParityWithGetEntity (PutEntity rejects a UDT column family, and
GetEntityLazy is at parity with GetEntity there). Non-OK statuses are
asserted via Status::code() against the enumerators for clearer failures.
One test (BlockCacheTierYieldsIncompleteOnMiss) stays DISABLED_ until
resolve-time ReadOptions are honored (a later phase). Also adds an
unreleased_history note for the new public API.

Crash test (db_stress): adds a lazy-vs-eager differential wired into every
mode's TestGetEntity / TestMultiGetEntity (no_batched_ops, batched_ops,
cf_consistency; multi_ops_txns stays a no-op stub). It is gated by the new
--lazy_entity_read_one_in knob and only runs when open_files == -1 (the
API's requirement), user_timestamp_size == 0, and transactions are off. When
it triggers it pins a snapshot, always runs GetEntityLazy /
MultiGetEntityLazy plus a random-subset MultiResolve to exercise the code
for crashes -- this happens regardless of --skip_verifydb, whose job is to
disable verification, not code coverage -- and, when DB verification is
enabled, checks the lazily-resolved result (enumeration with no blob I/O,
plus a random resolved subset that may be empty/partial/all) against an
eager reference. Where the mode already read the entity under a pinned
snapshot, that already-verified eager result is reused as the reference so
no redundant eager read is issued; otherwise the helper reads its own
reference under a snapshot it pins. tools/db_crashtest.py adds the knob and
biases open_files to -1 ~2/3 of the time so the path exercises often. The
single-CF-only lazy API (no cross-CF overload yet) and the UDT/txn
exclusions are noted at the call sites and in the plan.

[github-actions]

⚠️ clang-tidy: 2 warning(s) on changed lines

Completed in 371.0s.

Summary by check

Check	Count
readability-braces-around-statements	2
Total	2

Details

db_stress_tool/db_stress_test_base.cc (2 warning(s))

db_stress_tool/db_stress_test_base.cc:574:17: warning: statement should be inside braces [readability-braces-around-statements]
db_stress_tool/db_stress_test_base.cc:575:25: warning: statement should be inside braces [readability-braces-around-statements]

[meta-codesync]

@pdillinger has imported this pull request. If you are a Meta employee, you can view this in D113792365.

[github-actions]

✅ Claude Code Review

Auto-triggered after CI passed — reviewing commit e84333f

---

Summary

Well-structured API scaffolding for lazy blob resolution. The design follows existing RocksDB patterns (PinnableWideColumns, forward_list backing, move-only semantics). The batch-first API shape (MultiResolve as primary, per-column as sugar) is appropriate for coalesced/async blob reads. All tests are correctly DISABLED pending implementation. Two issues need attention before merge.

High-severity findings (2):

• [stackable_db.h] StackableDB does not forward GetEntityLazy / MultiGetEntityLazy, breaking the transparent wrapper pattern for TransactionDB and all other StackableDB subclasses.
• [src.mk:~105] New db/wide/lazy_wide_columns.cc entry has inconsistent trailing whitespace alignment compared to neighboring lines (cosmetic but will fail CI style checks).

Full review (click to expand)

Findings

🔴 HIGH

H1. Missing StackableDB forwarding for GetEntityLazy / MultiGetEntityLazy -- include/rocksdb/utilities/stackable_db.h

• Issue: StackableDB forwards every GetEntity and MultiGetEntity overload (lines 111-168) to the wrapped db_, but neither GetEntityLazy nor MultiGetEntityLazy is forwarded. Any DB accessed through a StackableDB wrapper (TransactionDB, OptimisticTransactionDB, BlobDB wrapper, etc.) will silently return Status::NotSupported from the DB base class default, even when the underlying DBImpl implements it.
• Root cause: New virtual methods added to DB without corresponding forwarding in StackableDB.
• Suggested fix: Add forwarding overrides in stackable_db.h following the existing GetEntity/MultiGetEntity pattern. Will also need forward declarations or include for the new types.

H2. src.mk trailing whitespace misalignment -- src.mk:~105

• Issue: The diff adds db/wide/lazy_wide_columns.cc with trailing whitespace that doesn't match the column alignment of surrounding lines. The LIB_SOURCES and TEST_MAIN_SOURCES sections use backslashes aligned to a consistent column. The new entries appear misaligned.
• Suggested fix: Run make format-auto or manually align backslashes to match neighboring lines.

🟡 MEDIUM

M1. verify field defaults to false -- include/rocksdb/lazy_wide_columns.h

• Issue: Partial byte-range reads skip CRC verification by default. This is performance-optimal but diverges from RocksDB's typical correctness-by-default philosophy.
• Suggested fix: Consider defaulting to true (safe default, opt-out for performance).

M2. LazyBatchColumnReadRequest public inheritance enables silent slicing -- include/rocksdb/lazy_wide_columns.h

• Issue: A LazyBatchColumnReadRequest* can be implicitly passed as LazyColumnReadRequest* to LazyWideColumns::MultiResolve, silently dropping the entity_index field.
• Suggested fix: Consider composition or documenting the hazard.

M3. No cross-CF MultiGetEntityLazy overload -- include/rocksdb/db.h

• Issue: Only the single-CF variant exists, though the internal Rep is designed for cross-CF use. Acceptable for WIP but should be tracked.

🟢 LOW / NIT

L1. Verbose forward-declaration comment in db.h

L2. std::map for cf_pins (minor; revisit if cross-CF batching is profiled)

L3. Aggregate initialization in tests relies on field order (mitigated by /*field=*/ comments)

Cross-Component Analysis

Context	Affected?	Action needed?
WritePreparedTxnDB / TransactionDB	YES (via StackableDB)	H1: Add forwarding
ReadOnly DB	Returns NotSupported (safe)	None
BlobDB wrapper (StackableDB)	YES	H1

Positive Observations

• Batch-first API design (MultiResolve primary) is well-suited for future coalesced/async blob I/O
• Consistent forward_list<PinnableSlice> backing pattern from PinnableWideColumns
• Clean pimpl separation keeps internal dependencies out of public header
• Well-documented DISABLED_ test convention with phased enablement plan
• Build system completeness: all four systems updated

---

ℹ️ About this response

Generated by Claude Code.
Review methodology: claude_md/code_review.md

Limitations:

• Claude may miss context from files not in the diff
• Large PRs may be truncated
• Always apply human judgment to AI suggestions

Commands:

• /claude-review [context] — Request a code review
• /claude-query <question> — Ask about the PR or codebase