HDDS-15766. Make Recon OM DB large tarball transfer reliable by devmadhuu · Pull Request #10696 · apache/ozone

devmadhuu · 2026-07-09T07:17:31Z

What changes were proposed in this pull request?

Problem:

Recon keeps its own copy of the OM DB and syncs it two ways: incremental delta updates (getDBUpdates) for the normal case, and a full OM DB snapshot download as the fallback when delta sync cannot be
used (Recon fell too far behind, OM no longer has the requested updates, etc.).

The full-snapshot fallback was Recon's own implementation:

It used a one-shot GET /dbCheckpoint call with Recon-specific download-and-extract code (TarExtractor), separate from the checkpoint-transfer path OM itself uses internally.
On failure it did not degrade gracefully (it surfaced the error rather than falling back to the existing DB).

Maintaining a separate downloader in Recon meant it could deviate from OM's own, well tested checkpoint-transfer code and had to be kept in sync manually over time.

Approach:

Instead of maintaining Recon's own download logic, Recon now reuses the same OM checkpoint-download path an OM follower uses to bootstrap from the leader — POST /v2/dbCheckpoint — through the shared
RDBSnapshotProvider base class that OmRatisSnapshotProvider also extends:

ReconRDBSnapshotProvider overrides only the download step (POST /v2/dbCheckpoint) and the step that assembles and promotes the finished DB; the surrounding logic (driver loop, untar) is the shared OM code.
Recon requests only the active OM DB (includeSnapshotData=false, since it does not need OM's snapshots), which OM sends as a single tarball.
On failure, Recon keeps its current DB and retries on the next sync cycle instead of crashing.

The assembled DB is always the complete OM DB, so a full snapshot still triggers a full task reprocess in Recon, exactly as before. This change is about reusing OM's proven transfer code and handling
failure cleanly — not about changing how the bytes move (the active DB is still transferred as one tarball).

What is the link to the Apache JIRA

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

How was this patch tested?

Testing:

New unit test TestReconRDBSnapshotProvider — covers the post-download assembly path: promoting the untarred checkpoint into a stable om.snapshot.db_ directory while clearing the candidate dir
(testGetCheckpointPromotesDbAndClearsCandidate), installing the leader's hardLinkFile inventory (testGetCheckpointInstallsHardLinks), and the candidate-dir layout (testCandidateDirLocation).
Updated TestOzoneManagerServiceProviderImpl — drives the new provider path for both the success and the download-failure cases (via a stub ReconRDBSnapshotProvider).
Updated TestTriggerDBSyncEndpoint — stubs reconUtils.getReconDbDir(...) so the now-eagerly-constructed provider receives a real snapshot dir (the provider is created in the
OzoneManagerServiceProviderImpl constructor).
Existing TestReconWithOzoneManager.testOmDBSyncWithSeqNumberMismatch — end-to-end coverage of the full-snapshot fallback against a mini-cluster: it forces getDBUpdates to fail and asserts Recon
recovers via a full snapshot and normalizes the OM/Recon sequence numbers.
TestReconWithOzoneManagerHA — dropped the stale v1-URL assertion, keeping its end-to-end leader-sync verification.

…. - initial commit.

- TestReconRDBSnapshotProvider: guard File.list() results with requireNonNull. - OzoneManagerServiceProviderImpl: make omSnapshotDBParentDir a local variable.

jojochuang

It would be nice to respect configration property ozone.om.db.checkpoint.use.inode.based.transfer and switch to the old endpoint if it's false.

priyeshkaratha

Thanks @devmadhuu for working on this. Changes overall LGTM. Please check some minor inline comments provided.

priyeshkaratha · 2026-07-14T04:09:34Z

      <groupId>org.apache.hadoop</groupId>
      <artifactId>hadoop-hdfs-client</artifactId>
    </dependency>
+    <dependency>


Can avoid adding this dependency since it is used only to use URIBuilder in buildCheckpointUrl?

priyeshkaratha · 2026-07-14T04:13:02Z

          targetFile, unTarredDb.toAbsolutePath());
      if (ratisSnapshotComplete(unTarredDb)) {
-        LOG.info("Ratis snapshot transfer is complete.");
+        LOG.info("DB snapshot transfer is complete.");


These log changes will also impact the OM follower logs. Do we really need this change? The existing check for ratisSnapshotComplete already aligns with the info logs.

Downloading active OM DB by OM followers should not be tied up with this terminology "RATIS". It is just full active OM DB download from leader. Since it was originally written based in install snapshot flow, but this API can be used y any http client. So updating log makes sense.

sadanand48

Thanks @devmadhuu for the patch, left few minor comments inline. Like @jojochuang said lets make the behaviour switchable i.e retain both v1 and v2 code

sadanand48 · 2026-07-14T07:54:35Z

-    // Before fetching full snapshot again and create a new OM DB snapshot directory, check and delete
-    // any existing OM DB snapshot directories under recon om db dir location and delete all such
-    // om db snapshot dirs including the last known om db snapshot dir returned by reconUtils.getLastKnownDB
-    File lastKnownDB = reconUtils.getLastKnownDB(omSnapshotDBParentDir, RECON_OM_SNAPSHOT_DB);


We can still keep these 2 code blocks i.e deleting lastKnownDB and leftover staging dirs as it can happen b/w switching v1 and v2 , there leftover artifacts from v1. I think keeping this should be safe

sadanand48 · 2026-07-14T07:56:00Z

+          ReconContext.ErrorCode.GET_OM_DB_SNAPSHOT_FAILED);
+      // Reset the candidate dir so the next attempt starts from a clean state.
+      try {
+        reconSnapshotProvider.init();


We don't support chunking/batched transfers from active om.db today so this should not be a problem but in case we did this would be a problem as failure in a single batch would delete the whole candidate dir. Just want to call it out.

Good catch, this was left behind to remove when moved from chunk transfer impl to single tar ball transfer same as OM follower. I have fixed it.

devmadhuu · 2026-07-14T16:15:53Z

It would be nice to respect configration property ozone.om.db.checkpoint.use.inode.based.transfer and switch to the old endpoint if it's false.

Thanks @jojochuang for catching that. Fixed it in latest push.

priyeshkaratha

Thanks @devmadhuu for improving the patch. Changes LGTM

sadanand48

Thanks @devmadhuu for updating the patch . Overall changes LGTM

sadanand48

LGTM

jojochuang · 2026-07-17T17:17:21Z

+    ServiceInfo leader = leaderInfoSupplier.get();
+    URL checkpointUrl = buildCheckpointUrl(leader);
+    LOG.info("Downloading OM DB checkpoint from leader {}. Checkpoint: {}, "
+        + "URL: {}", leaderNodeID, targetFile.getName(), checkpointUrl);


Question regarding leader transition corner case @devmadhuu --
this method is called multiple times during a checkpoint, each time it returns partial checkpoint.
The leader may change at different time, and it may attempt to download partial checkpoint from another OM, will that be a problem?

jojochuang · 2026-07-17T17:48:38Z

Sample repro test for the Bugbot finding about leader change mid chunked transfer (ReconRDBSnapshotProvider re-resolves leaderInfoSupplier.get() on every chunk while checkLeaderConsistency runs only once).

Pushed to my fork: jojochuang@fa988a0

The test simulates a two-chunk transfer where the supplier returns om-leader-a for chunk 1 and om-leader-b for chunk 2. It asserts the safe behavior (pin original leader for all chunks or reset the candidate dir). On current code it fails, which demonstrates the bug:

leadersUsedPerChunk=[om-leader-a, om-leader-b], initCountBefore=1, initCountAfter=1

Run locally:

mvn -pl :ozone-recon test \
  -Dtest=TestReconRDBSnapshotProvider#testLeaderChangeMidTransferUsesNewLeaderWithoutResettingCandidate \
  -DskipShade -DskipRecon -DskipDocs

Core test code (TestReconRDBSnapshotProvider.java):

@Test
public void testLeaderChangeMidTransferUsesNewLeaderWithoutResettingCandidate(
    @TempDir File snapshotDir) throws IOException {
  ServiceInfo leaderA = mock(ServiceInfo.class);
  when(leaderA.getHostname()).thenReturn("om-leader-a");
  when(leaderA.getPort(any())).thenReturn(9874);
  ServiceInfo leaderB = mock(ServiceInfo.class);
  when(leaderB.getHostname()).thenReturn("om-leader-b");
  when(leaderB.getPort(any())).thenReturn(9874);

  AtomicInteger supplierCalls = new AtomicInteger();
  Supplier<ServiceInfo> leaderSupplier = () ->
      supplierCalls.getAndIncrement() == 0 ? leaderA : leaderB;

  List<String> leadersUsedPerChunk = new ArrayList<>();

  ReconRDBSnapshotProvider provider =
      new ReconRDBSnapshotProvider(snapshotDir, null, false,
          HttpConfig.Policy.HTTP_ONLY, false, true, leaderSupplier) {
        @Override
        public void downloadSnapshot(String leaderNodeID, File targetFile)
            throws IOException {
          ServiceInfo leader = leaderSupplier.get();
          leadersUsedPerChunk.add(leader.getHostname());
          if (leadersUsedPerChunk.size() == 1) {
            Map<String, String> chunkOne = new HashMap<>();
            chunkOne.put("fromLeaderA.sst", "partial-a");
            chunkOne.put("CURRENT", "MANIFEST");
            writeTar(targetFile, chunkOne);
          } else {
            Map<String, String> chunkTwo = new HashMap<>();
            chunkTwo.put(HddsServerUtil.OZONE_RATIS_SNAPSHOT_COMPLETE_FLAG_NAME, "");
            writeTar(targetFile, chunkTwo);
          }
        }
      };

  long initCountBefore = provider.getInitCount();
  provider.downloadDBSnapshotFromLeader("leader-a-node-id");

  boolean pinnedLeader = leadersUsedPerChunk.stream()
      .allMatch("om-leader-a"::equals);
  boolean resetCandidate = provider.getInitCount() > initCountBefore;
  assertTrue(pinnedLeader || resetCandidate,
      "When the resolved OM leader changes mid-transfer, Recon must either "
          + "pin the original leader for all chunks or reset the candidate "
          + "dir before continuing; leadersUsedPerChunk=" + leadersUsedPerChunk
          + ", initCountBefore=" + initCountBefore
          + ", initCountAfter=" + provider.getInitCount());
}

This is a repro-only sketch for discussion — not intended to land as-is on the PR branch until the fix is in place.

devmadhuu · 2026-07-18T12:57:53Z

It would be nice to respect configration property ozone.om.db.checkpoint.use.inode.based.transfer and switch to the old endpoint if it's false.

Thanks @jojochuang for catching that. Fixed it in latest push.

Sample repro test for the Bugbot finding about leader change mid chunked transfer (ReconRDBSnapshotProvider re-resolves leaderInfoSupplier.get() on every chunk while checkLeaderConsistency runs only once).

Pushed to my fork: jojochuang@fa988a0

The test simulates a two-chunk transfer where the supplier returns om-leader-a for chunk 1 and om-leader-b for chunk 2. It asserts the safe behavior (pin original leader for all chunks or reset the candidate dir). On current code it fails, which demonstrates the bug:

leadersUsedPerChunk=[om-leader-a, om-leader-b], initCountBefore=1, initCountAfter=1

Run locally:

mvn -pl :ozone-recon test \
  -Dtest=TestReconRDBSnapshotProvider#testLeaderChangeMidTransferUsesNewLeaderWithoutResettingCandidate \
  -DskipShade -DskipRecon -DskipDocs

Core test code (TestReconRDBSnapshotProvider.java):

@Test
public void testLeaderChangeMidTransferUsesNewLeaderWithoutResettingCandidate(
    @TempDir File snapshotDir) throws IOException {
  ServiceInfo leaderA = mock(ServiceInfo.class);
  when(leaderA.getHostname()).thenReturn("om-leader-a");
  when(leaderA.getPort(any())).thenReturn(9874);
  ServiceInfo leaderB = mock(ServiceInfo.class);
  when(leaderB.getHostname()).thenReturn("om-leader-b");
  when(leaderB.getPort(any())).thenReturn(9874);

  AtomicInteger supplierCalls = new AtomicInteger();
  Supplier<ServiceInfo> leaderSupplier = () ->
      supplierCalls.getAndIncrement() == 0 ? leaderA : leaderB;

  List<String> leadersUsedPerChunk = new ArrayList<>();

  ReconRDBSnapshotProvider provider =
      new ReconRDBSnapshotProvider(snapshotDir, null, false,
          HttpConfig.Policy.HTTP_ONLY, false, true, leaderSupplier) {
        @Override
        public void downloadSnapshot(String leaderNodeID, File targetFile)
            throws IOException {
          ServiceInfo leader = leaderSupplier.get();
          leadersUsedPerChunk.add(leader.getHostname());
          if (leadersUsedPerChunk.size() == 1) {
            Map<String, String> chunkOne = new HashMap<>();
            chunkOne.put("fromLeaderA.sst", "partial-a");
            chunkOne.put("CURRENT", "MANIFEST");
            writeTar(targetFile, chunkOne);
          } else {
            Map<String, String> chunkTwo = new HashMap<>();
            chunkTwo.put(HddsServerUtil.OZONE_RATIS_SNAPSHOT_COMPLETE_FLAG_NAME, "");
            writeTar(targetFile, chunkTwo);
          }
        }
      };

  long initCountBefore = provider.getInitCount();
  provider.downloadDBSnapshotFromLeader("leader-a-node-id");

  boolean pinnedLeader = leadersUsedPerChunk.stream()
      .allMatch("om-leader-a"::equals);
  boolean resetCandidate = provider.getInitCount() > initCountBefore;
  assertTrue(pinnedLeader || resetCandidate,
      "When the resolved OM leader changes mid-transfer, Recon must either "
          + "pin the original leader for all chunks or reset the candidate "
          + "dir before continuing; leadersUsedPerChunk=" + leadersUsedPerChunk
          + ", initCountBefore=" + initCountBefore
          + ", initCountAfter=" + provider.getInitCount());
}

This is a repro-only sketch for discussion — not intended to land as-is on the PR branch until the fix is in place.

Thanks @jojochuang for the repro and putting details here.. Agree the concern is valid at the framework level, though it isn't reachable for Recon today: Recon requests includeSnapshotData=false, so the server sets maxTotalSstSize=Long.MAX_VALUE and sends the active OM DB as a single batch with the completion flag in the same response. That makes the transfer always single-chunk — downloadSnapshot runs once and the leader resolves once, so there's no second chunk that could come from a different leader (which is why your repro has to stub two chunks). Most important is this behavior of Recon for downloading active OM DB is same as OM follower for active OM DB where OM follower also downloads the active OM DB in one single tar ball transfer. Only snapshots data and checkpoint logs are downloaded by OM follower in multi chunked transfer fashion.

That said, relying on single-chunk is fragile. ReconRDBSnapshotProvider re-resolves the leader via the supplier on every call instead of pinning the leaderNodeID the loop was invoked with (like OmRatisSnapshotProvider does). I'll pin the leader for the whole transfer so it's correct by construction regardless of chunk count. Will push in the next revision.

Devesh Kumar Singh added 2 commits July 9, 2026 11:20

HDDS-15766. Recon - Make Recon OM DB large tar ball transfer reliable…

1072a79

…. - initial commit.

HDDS-15766. Updated log and comment - initial commit.

48e1b28

devmadhuu requested review from ArafatKhan2198 and sadanand48 July 9, 2026 07:17

devmadhuu added the AI-gen label Jul 9, 2026

devmadhuu changed the title ~~HDDS-15766 - Recon - Make Recon OM DB large tar ball transfer reliable.~~ HDDS-15766. Recon - Make Recon OM DB large tar ball transfer reliable. Jul 9, 2026

HDDS-15766. Fix SpotBugs NP and PMD SingularField findings.

0e9f990

- TestReconRDBSnapshotProvider: guard File.list() results with requireNonNull. - OzoneManagerServiceProviderImpl: make omSnapshotDBParentDir a local variable.

devmadhuu force-pushed the HDDS-15766 branch from ccf77d2 to 0e9f990 Compare July 9, 2026 07:44

Devesh Kumar Singh added 2 commits July 9, 2026 13:58

HDDS-15766. findbugs fixed.

d234dab

HDDS-15766. test failures fixed.

46012aa

devmadhuu marked this pull request as ready for review July 9, 2026 11:53

adoroszlai changed the title ~~HDDS-15766. Recon - Make Recon OM DB large tar ball transfer reliable.~~ HDDS-15766. Make Recon OM DB large tar ball transfer reliable. Jul 9, 2026

adoroszlai changed the title ~~HDDS-15766. Make Recon OM DB large tar ball transfer reliable.~~ HDDS-15766. Make Recon OM DB large tarball transfer reliable Jul 9, 2026

HDDS-15766. Removed seeding of existing sst files behavior.

a992775

devmadhuu marked this pull request as draft July 10, 2026 13:51

devmadhuu marked this pull request as ready for review July 13, 2026 04:05

devmadhuu force-pushed the HDDS-15766 branch from 3566a4e to a992775 Compare July 13, 2026 13:42

jojochuang reviewed Jul 13, 2026

View reviewed changes

priyeshkaratha reviewed Jul 14, 2026

View reviewed changes

sadanand48 reviewed Jul 14, 2026

View reviewed changes

HDDS-15766. Added flag for v1 and v2 API.

29325f1

devmadhuu requested review from jojochuang, priyeshkaratha and sadanand48 July 14, 2026 16:29

priyeshkaratha approved these changes Jul 15, 2026

View reviewed changes

sadanand48 reviewed Jul 16, 2026

View reviewed changes

Comment thread ...recon/src/test/java/org/apache/hadoop/ozone/recon/spi/impl/TestReconRDBSnapshotProvider.java Outdated

Comment thread ...on/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/OzoneManagerServiceProviderImpl.java

HDDS-15766. Fixing tests

129648c

devmadhuu requested a review from sadanand48 July 17, 2026 08:38

sadanand48 approved these changes Jul 17, 2026

View reviewed changes

jojochuang reviewed Jul 17, 2026

View reviewed changes

HDDS-15766. Fixing leader consistency check and use while transfer.

aa5db9a

devmadhuu requested a review from jojochuang July 18, 2026 13:15

Uh oh!

Conversation

devmadhuu commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

jojochuang left a comment

Choose a reason for hiding this comment

Uh oh!

priyeshkaratha left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sadanand48 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devmadhuu commented Jul 14, 2026

Uh oh!

priyeshkaratha left a comment

Choose a reason for hiding this comment

Uh oh!

sadanand48 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sadanand48 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jojochuang commented Jul 17, 2026

Uh oh!

devmadhuu commented Jul 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

devmadhuu commented Jul 9, 2026 •

edited

Loading