HDDS-15394. Sequential Reader of from and to snapshots for full snapshot diff.

[SaketaChalamchala]

What changes were proposed in this pull request?

Developed with the help of Cursor AI.

This PR is intended as the foundation for a more efficient snapshot diff (HDDS-9154).

Baseline full diff performs random reads against snapshot DBs and holds large in-memory maps. The optimized design replaces that with:

1. Sequential table scans over both snapshots
2. Persisted per-job intermediate lists keyed by objectId
3. Update-id gating (OM HA) to limit compare-signature work to likely-changed objects
4. Deferred path resolution via FSO edge column families

• FullDiffSequentialReader scans raw snapshot tables (Table<byte[], byte[]>) so SnapshotDiffValueParser operates on exact persisted protobuf bytes (no decode/re-encode round trip). Scan order: file/key tables first, then directory tables (FSO). Each table pair runs to-side, then from-side.
• HA gating: admits rows with updateID > fromSnapshotDbTxSequenceNumber. Rows with missing, zero, or DEFAULT_OM_UPDATE_ID (-1) are always candidates (conservative fallback).
• SnapDiffJobStore owns per-job temporary RocksDB column families:
  • {jobId}-new-list, {jobId}-old-list — keyed by objectId
  • {jobId}-to-edges, {jobId}-from-edges — FSO directory edges (parentId, objectId) → name
  • {jobId}-cand-ids — spill target when the in-memory diff-candidate set exceeds the configured limit
• EntryValue — compact fixed-layout (parentId, name, isDir, signature) (shared with the future DAG diff sequential reader).
• Config: ozone.om.snapshot.diff.max.in.memory.entries.per.job (default 1M) bounds the in-memory diff-candidate set before spill to RocksDB.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-15394

How was this patch tested?

Unit Tests.

[Copilot]

Pull request overview

This PR lays groundwork for an optimized full snapshot diff implementation in OM by introducing a stage-1 sequential reader that scans snapshot RocksDB tables in order and writes compact, per-job intermediate structures (new/old lists, candidate IDs, and optional FSO directory edges). It also adds configuration to bound in-memory candidate retention and extends test utilities to support raw byte[] table iteration.

Changes:

• Added FullDiffSequentialReader plus compact EntryValue encoding to support sequential scan–based stage-1 full diff processing (with optional HA updateID gating).
• Added SnapDiffJobStore to manage per-job temporary RocksDB column families and controlled in-memory vs spilled diff-candidate tracking.
• Added/updated unit tests and supporting test utilities, plus a new OM config key and default XML property for in-memory candidate limits.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/diff/TestSnapshotDiffValueParser.java	Fixes package to match location under snapshot.diff.
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/diff/TestFullDiffSequentialReader.java	Adds unit tests for sequential scan stage-1 behavior, gating, spilling, and FSO edge population.
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/diff/SnapshotDiffValueParser.java	Fixes package to match snapshot.diff namespace.
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/diff/SnapDiffJobStore.java	Introduces per-job temporary CF store with batched writes and candidate spill-to-RocksDB behavior.
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/diff/FullDiffSequentialReader.java	Implements sequential scans over to/from tables and writes stage-1 intermediates with optional HA gating.
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/diff/EntryValue.java	Adds compact fixed-layout byte encoding for stage intermediates.
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java	Adds config key + default for max in-memory entries per diff job before spill.
hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/InMemoryTestTable.java	Adds raw byte[] table factory with unsigned ordering and implements iterator(prefix, type) for tests.
hadoop-hdds/common/src/main/resources/ozone-default.xml	Documents default value and description for the new in-memory entry limit property.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[jojochuang]

Thanks for putting together the Stage 1 scaffolding for the optimized full snapshot diff path. The overall shape looks good: sequential raw-table scans, HA updateID gating, per-job intermediate column families, and compact EntryValue records align well with the efficient-snapdiff design doc.

This is a reasonable foundation PR — the abstractions are clear and the unit tests cover the main classification behaviors. A few items below are worth addressing before or shortly after merge.

PR description / hygiene

The Jira link is present, but the template line is still there:

(Please replace this section with the link to the Apache JIRA)

Please remove that placeholder.

Also, per ASF generative-tooling guidance, consider adding an explicit disclosure line, e.g. Generated-by: Cursor (...) instead of only "Developed with the help of Cursor AI."

Config key not wired

ozone.om.snapshot.diff.max.in.memory.entries.per.job is added to OMConfigKeys and ozone-default.xml, but SnapDiffJobStore.open(...) always uses OZONE_OM_SNAPSHOT_DIFF_MAX_IN_MEMORY_ENTRIES_PER_JOB_DEFAULT.

Other snapshot-diff limits are read from OzoneConfiguration in SnapshotDiffManager. Either wire this key now, or add a short TODO in SnapDiffJobStore.open so the integration PR doesn't miss it.

keyPrefix is untested

scanFileTables / scanDirectoryTables accept a bucket keyPrefix, but all tests scan with null. A small test with a non-null prefix would help validate the InMemoryTestTable iterator behavior before integration with real bucket-scoped scans.

From-side oldList writes all rows (confirm intent)

The design doc says the from-side directory scan "only processes entries in DiffCandidateSet," but this implementation writes every from-side row to oldList — candidates get a full signature, non-candidates get metadata with an empty signature.

That seems intentional and necessary for delete detection (e.g. object 4 in testDeleteCandidateHasMetadataWithoutSignature). Could you add a brief comment in processFromSideEntry clarifying that all from-side rows are persisted for merge-join/delete detection, not only gated candidates?

Naming: FullDiffSequentialReader vs FullDiffComputer

There is already a FullDiffComputer under om.snapshot.diff.delta (SST delta file computation). Consider a one-line class javadoc cross-reference to distinguish this Stage 1 sequential table scanner from the existing delta/SST path.

Double protobuf scan on candidates (CPU / hot path)

For to-side candidates, the hot path does two full passes over the same protobuf value:

ParsedRequiredInfo info = parseRequired(value, ...);   // pass 1: walk all tags
...
byte[] signature = computeSignature(value, ...);       // pass 2: walk all tags again

parseKeyInfoRequiredFields extracts objectId / parentId / name / updateID and skips other fields, but still visits every tag. computeKeyInfoCompareSignature then restarts from the beginning and walks the message again for digest inputs.

HA gating avoids pass 2 for unchanged rows (present marker), which is good. But for every candidate row we still pay ~2× protobuf traversal. On large buckets that can be a substantial fraction of rows.

Suggestion: Consider a single-pass parser that returns both ParsedRequiredInfo and the compare signature in one loop, or defer signature computation until merge-join when both sides are present. Same applies to from-side candidates in processFromSideEntry.

Prefer reducing copies with ByteString on the parse path (GC)

The implementation is mostly byte[], which matches RocksDB's raw-table API (Table<byte[], byte[]>). That's appropriate at the storage boundary. However, several spots add avoidable allocations on a millions-of-rows scan:

1. SnapshotDiffValueParser: Already uses ByteString internally in places, but paths like input.readBytes().toByteArray() copy nested protobuf fields unnecessarily. Prefer ByteString inputs/overloads and CodedInputStream.newInstance(ByteString) through the parse pipeline.
2. FullDiffSequentialReader.nameBytes(): getBytes(UTF_8) allocates per FSO edge row. Consider ByteString.copyFromUtf8(name) and materialize byte[] only at batchPut.
3. SnapDiffJobStore.objectIdKey() / edgeKey(): ByteBuffer.allocate(...).array() allocates per key. A fixed stack buffer or reusable encoder would be cheaper.
4. EntryValue: Store signature as ByteString; expose byte[] only in toBytes() at write time.

Guidance: Keep byte[] at the RocksDB JNI boundary; use ByteString (or zero-copy views) inside the parse/compare pipeline to avoid intermediate copies.

Minor nits

• testDiffCandidatesSpillToRocksDb: Consider asserting that spill actually occurred (e.g. via areDiffCandidatesSpilled()).
• InMemoryTestTable: Good addition for raw scans; please confirm unit tests pass with the new iterator() implementation.
• EntryValue: Consider a small dedicated round-trip test (empty name, empty signature).
• FullDiffSequentialReader.scanFromTable: A brief comment on why the pre-scan flushWrites() is required would help future readers.

Overall this is solid foundation work for HDDS-9154. Happy to approve; the performance items (single-pass parsing, reduced byte copying) could land in this PR or an immediate follow-up before SnapshotDiffManager integration.

[jojochuang]

nore sure how this is going to be used, but a getter method should not be a O(n) operation. Either return signature object as is, or rename the method to copySignature().

[SaketaChalamchala]

Addressed