[ArafatKhan2198]

What changes were proposed in this pull request?

Problem

Recon fetches the OM DB by downloading a tar file from the OM leader and unpacking it into a candidate dir (om.snapshot.db.candidate). If the unpacking fails halfway, for example because the disk fills up or the leader sends a tar that is cut short (HDDS-16864), tar leaves a partially written SST file in the candidate dir.

Since HDDS-15766, Recon keeps the candidate dir after a failed fetch, so the retry can resume: it tells the leader which files it already has, and the leader skips them. That list is built from file names only, so the truncated SST is on it too, and the leader does not send it again. Recon then builds the new OM DB with the truncated SST, and RocksDB refuses to open it. The retry is wasted, and a corrupt om.snapshot.db_<ts> is left on disk.

Fix

The fix keeps the resume behaviour of HDDS-15766 and makes sure only complete files are ever kept in the candidate dir.

• ReconRDBSnapshotProvider now overrides untarContentsOfTarball. It extracts each file to <name>.tmp and renames it to its real name only after all its bytes are written. If the untar fails partway, the .tmp file is deleted. It uses the existing Archiver helpers, which Recon already uses to unpack the SCM DB.
• On the retry, the candidate dir holds only fully extracted files. The leader skips those, and sends the file that failed in full along with the remaining files.
• RDBSnapshotProvider.untarContentsOfTarball is changed from private to protected so that Recon can override it. The OM follower is unchanged.
• The comment in OzoneManagerServiceProviderImpl.getOzoneManagerDBSnapshot said keeping the dir "matches how the OM follower handles a failed download". That is wrong: OzoneManager.installSnapshotFromLeader deletes the candidate dir after a failed download. The comment now says that the dir only holds fully extracted files.

Only master is affected: HDDS-15766 is not in any release. OM is not affected.

Related: HDDS-15766 (introduced resuming from the candidate dir), HDDS-16882 (Recon reports the failed open of such a DB as success), HDDS-16864 (a failed untar also leaves the downloaded tarball behind, which is not changed here).

What is the link to the Apache JIRA

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

How was this patch tested?

testPartiallyUntarredFileNotReportedOnRetry was added to TestOzoneManagerServiceProviderImpl. The download returns a tar that holds one complete SST (000009.sst) and ends in the middle of a second one (000010.sst), so the untar fails. The test checks that the retry lists only 000009.sst as already received, and that the candidate dir holds only that file, at its full size.

• On the original code it fails with [[], ["000010.sst", "000009.sst"]]: the truncated file is listed.
• If the candidate dir is wiped instead, it fails with [[], []]: nothing is resumed.

TestOzoneManagerServiceProviderImpl, TestReconRDBSnapshotProvider, TestTriggerDBSyncEndpoint and TestRDBSnapshotProvider pass (24 tests), and checkstyle is clean. The reproduction test attached to the Jira fails without the fix and passes with it: the retry builds a complete OM DB that Recon opens.

[smengcl]

+1. Thanks @ArafatKhan2198 for the patch.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[smengcl]

Thanks @ArafatKhan2198 for posting this.

[devmadhuu]

@smengcl @ArafatKhan2198 I wanted to have a review of this PR, so added myself as reviewer. So here are 2 points:

1. This PR has done a override in Recon specifically to handle partial transfer cleanup for Recon. since currently OM cleanup the complete candidate dir, we better should have either fixed it same way, but since we have made it a resume option to not transfer the already extracted sst files from tar, we have fixed in this PR by overriding the OM method and using the .tmp rename mechanism. This would have taken care at OM side as well and no need to maintain separate logic in Recon.
2. Also there is no guarantee that using this resume option, we are going to save some transfer cost. Lets try to understand with below example:

Example

The leader's DB has 10 SST files, 1 GB each: F1 … F10. The leader stops sending in the middle of F4.

First attempt:
File | What Recon got

F1, F2, F3 | Complete. Unpacked and kept.
F4 | Only half. The untar fails here, and the PR deletes the .tmp file.
F5...F10 | Never arrived

Recon's candidate dir now holds F1, F2, F3.

Retry on the next sync cycle:

• Recon tells the leader: "I already have F1, F2, F3."
• The leader sends F4 … F10, which is 7 GB.

So resume saves 3 GB of the 10 GB.

Why I called it "a little"

1. Only the files before the cut are saved. The cut could happen anywhere. Stopping early saves almost nothing. Stopping near the end saves most of it. On average that is about half the DB.
2. Everything after the cut is sent again. The retry is one more tar with all the missing files.
3. The saving may not last. If the leader compacts F2 into a new file before the retry, Recon's F2 no longer matches anything on the leader. The leader sends the new file, and Recon's old copy is useless.

IMO, For this case, resume saves some download time, depending on where the cut happened. For the more likely case, Recon's disk filling up, it saves almost nothing and keeps the space in use. That's why I'd still go with wiping on failure for Recon as it works in OM today or else if we still think, we can save some transfer with best case of cut, we should change in base method of OM rather specifically override for Recon.

Keeping the OM and Recon logic as was before PR is recommended, so that logic doesn't drift for same OM db snapshot transfer either by OM follower or by Recon. No need to have separate overridden logic.

cc: @sadanand48 @umamaheswararao

[smengcl]

@devmadhuu Agreed. Addendum PR opened: #11624