Skip to content

HDDS-17468. Rename responses miss an earlier snapshotRenamedTable put in the same flush batch, so blocks a snapshot still holds can be reclaimed - #11779

Merged
adoroszlai merged 4 commits into
apache:masterfrom
smengcl:HDDS-17468
Oct 30, 2026

Conversation

@smengcl

@smengcl smengcl commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Generated-by: Claude Code (Opus 5.5)

What changes were proposed in this pull request?

In a bucket that has a snapshot, snapshotRenamedTable maps /volume/bucket/objectID to the name the object has in the latest snapshot, and only the first rename after the snapshot may write it. OMKeyRenameResponse, OMKeysRenameResponse and OMKeyRenameResponseWithFSO decide that on the double buffer flush thread: each reads the entry with snapshotRenamedTable.get and puts its source name only if the read returns null. Nothing adds cache entries to snapshotRenamedTable, so the read does not see a put that an earlier response added to the same batch. When two renames of one object are flushed in the same batch, the second also reads null and replaces the entry with the intermediate name, which the object never had in the snapshot. KeyDeletingService then looks the deleted key up in the snapshot under that name, finds nothing, and reclaims blocks the snapshot still holds.

This change keeps the decision on the flush thread and adds one condition to all three responses: the committed DB must still hold the same object (same object ID) at the rename's source key. The check reads with getSkipCache from the key table, the file table or, for an FSO directory rename, the directory table. If an earlier rename of the object is in the same batch, the committed table does not hold it at the source, and the later rename leaves the entry that the first one wrote. The snapshotRenamedTable update, which was duplicated in the three responses, is now one helper, OMKeyRenameResponse.addToSnapshotRenamedTable.

The decision is not moved to apply time: snapshot create clears the bucket's snapshotRenamedTable range only when its own batch is flushed, so a rename applied between a snapshot create and that flush would read the stale entry and leave the object without one.

There is no request, response or proto change. The cost is one extra point read per rename, only in a bucket that has a snapshot and only while the object has no entry. Entries already stored with a wrong name are not repaired. HDDS-17467 is a separate defect in the same table with an independent fix.

What is the link to the Apache JIRA

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

How was this patch tested?

  • New TestOMKeyRenameResponse.testSecondRenameKeepsSnapshotName (also run through TestOMKeyRenameResponseWithFSO): two rename responses for one key in a snapshotted bucket, in one batch and, as a control, with a commit between them. Without the change the one batch case fails.
  • New TestOMKeysRenameResponse.testSecondRenameInSameBatchKeepsSnapshotName and TestOMKeyRenameResponseWithFSO.testSecondDirectoryRenameInSameBatchKeepsSnapshotName: the same for OMKeysRenameResponse and for a directory rename. Both fail without the change.
  • New TestOMKeyRenameResponse.testRenameOfObjectNotCommittedAtSourceRecordsNothing (also run through TestOMKeyRenameResponseWithFSO): when the committed source name holds a different object, no entry is written. Without the change an entry is written.
  • TestOMSnapshotCreateRequest.testEntryRenamedKeyTable renamed sources that were never committed and had another object ID than the destination, so the new check wrote no entry; its fixtures now commit the source with the destination's object ID.
  • The rename request and response suites, the TestOMSnapshot* request and response suites and the TestReclaimable*Filter suites (620 tests) pass locally on master; checkstyle is clean.

… in the same flush batch, so blocks a snapshot still holds can be reclaimed

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@smengcl smengcl added om snapshot https://issues.apache.org/jira/browse/HDDS-6517 AI-gen labels Oct 26, 2026
Copilot AI lite review requested due to automatic review settings October 26, 2026 15:12

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

@smengcl
smengcl requested a review from sadanand48 October 27, 2026 08:24

@sadanand48 sadanand48 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @smengcl for the patch, LGTM, left a minor comment

@sadanand48 sadanand48 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@smengcl
smengcl marked this pull request as ready for review October 29, 2026 08:09
@jojochuang

Copy link
Copy Markdown
Contributor

@smengcl TestOMSnapshotCreateRequest failed, it might be related can you please check again

@jojochuang

Copy link
Copy Markdown
Contributor

I reproduced the OM CI failure locally: TestOMSnapshotCreateRequest.testEntryRenamedKeyTable fails with expected 4 entries, actual 0.

I pushed a test-only fix on top of this PR to my fork: jojochuang@54b7030

Branch: jojochuang:codex/HDDS-17468-snapshot-fixture-fix. Feel free to cherry-pick it.

The fixture now commits source keys/directories in the correct tables and preserves object IDs across renames, satisfying the new committed-source guard. Its FSO DB-key helper also uses the entry's actual bucket rather than always using the first bucket. All existing assertions and production code are unchanged.

Validation on JDK 21: 47 tests passed across TestOMSnapshotCreateRequest, TestOMSnapshotDeleteRequest, TestOMKeyRenameResponse, TestOMKeyRenameResponseWithFSO, and TestOMKeysRenameResponse; repository checkstyle passed. This fixes the deterministic OM failure; it does not address the separate Recon Derby-deadlock failure.

Generated-by: Codex (GPT-6)

@smengcl
smengcl marked this pull request as draft October 29, 2026 23:56
Co-authored-by: Codex (GPT-6)
Change-Id: I82c838785795684e08e7dbcbc30f57ee68656ee7
@smengcl

smengcl commented Oct 30, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for reproducing and fixing it. The fixture renamed a source that was never committed and had a different object ID from the destination, which a real rename never does, so the new committed source check rightly wrote no entry. I cherry-picked your commit; the snapshot, rename and reclaim suites (620 tests) pass locally. The Recon failure is a Derby deadlock in TestContainerSizeCountTask, unrelated to this change.

@smengcl
smengcl marked this pull request as ready for review October 30, 2026 06:52
@adoroszlai
adoroszlai merged commit eb84eda into apache:master Oct 30, 2026
134 of 135 checks passed
@adoroszlai

Copy link
Copy Markdown
Contributor

Thanks @smengcl for the patch, @jojochuang the test fix, @sadanand48 for the review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI-gen om snapshot https://issues.apache.org/jira/browse/HDDS-6517

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants