HDDS-15939. [OEP] Design for accounting per-snapshot trapped deleted data by sadanand48 · Pull Request #10838 · apache/ozone

sadanand48 · 2026-07-22T08:17:23Z

What changes were proposed in this pull request?

Design doc for accounting per-snapshot trapped deleted data.

What is the link to the Apache JIRA

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

SaketaChalamchala · 2026-07-31T20:59:16Z

+* Increment snapshot counters **only** when `putIfAbsent` on ledger succeeds.
+* Decrement **only** when CAS `ACCOUNTED_* → PURGED` (or `DIR_EXPAND_ACCOUNTED → PURGED` for dir
+  root after purge) succeeds.
+* One ledger row per objectID; prevents double counting across create, expand, DDS promote, and purge.


How do you propose to deal with multiple versions of an Object? deletedTable can can hold multiple OmKeyInfo versions for one DB key.

Good point instead of using the objectID in the ledger, we may use deleteDbKey which is currently used as a key in deletedTable to distinguish b/w multiple KeyInfo of same dbKey

SaketaChalamchala · 2026-07-31T22:11:45Z

+Per file promoted:
+
+```
+pinningSnapshot = resolve pinning snapshot (same logic as ReclaimableKeyFilter)


ReclaimableKeyFilter only checks the immediate previous snapshot but in Section 4 pinning snapshot is said to be "The oldest snapshot in the chain that still references the object's data".
I assume we want pinning snapshot to be the latter definition. If that's the case we can't use ReclaimableKeyFilter as it is. We may need to extend it to find the oldest snapshot with references to the file.

Right, instead of pinning-snapshot being oldest snapshot that holds the key it will be just the snapshot that has the deletedKey. I will update this

SaketaChalamchala · 2026-07-31T22:17:51Z

+┌─────────────────────────────────────────────────────────────────┐
+│ ExpandAndAccountDirService (new)                                │
+│   read-only subtree account → trappedKey* on pinning snapshot    │
+└─────────────────────────────────────────────────────────────────┘


Wouldn't it be simpler to just credit the trappedKeys to the store snapshot after expansion as well?
Since there is no overlap of deleted entries among snapshots, whatever deletes are captured in the snapshot(store snapshot) will account towards the trapped space for that snapshot even after expansion.

Any thoughts? I might be missing something here?

Correct, this is what was intended that trappedKeys is credited to store snapshot.

SaketaChalamchala · 2026-07-31T22:23:09Z

+│ SnapshotDeletingService                                         │
+│   move tables S1→S2/AOS; update ledger snapshot_id on rows     │
+│   transfer trappedDirNamespace between store snapshots          │
+│   file entry snapshot_id unchanged until purge (pinning)        │
+└─────────────────────────────────────────────────────────────────┘


For each key moved, updating snapshot_id in ledger_table for store snapshots and for pinning snapshots(It may be the after current snapshot is purged the next snapshot can be the oldest snapshot that holds reference to subfiles under a deleted root) and consistently transferring trapped* counters to the next snapshot for the entries that have been moved should all be an atomic operation.

SaketaChalamchala · 2026-07-31T22:27:42Z

+|-------|------|-------------------|-------------|
+| `trappedKeyBytes` | `long` | Snapshot create (`deletedTable` sum); ExpandAndAccountDirService; DDS promote to `deletedTable` (reclaimable dir) | Successful key purge |
+| `trappedKeyNamespace` | `long` | Same | Successful key purge |
+| `trappedDirNamespace` | `long` | Snapshot create (count of **root** rows in `deletedDirTable`) | DDS removes root from `deletedDirTable` |


Since trapped* fields can be updated after snapshot create and after ExpandAndAccountDirService and/or DDS, should we add another field to indicate "pending deleted directory expansion" and "expanded" just so that it is clear for client reading this info off of the snapshot info.

This is tricky as the 'expanded' state would be momentary because say there is a snapshot delete after the snapshot has been marked expanded and that snapshot's dirs are moved to the 'expanded' snapshot its state would again be pending expansion

SaketaChalamchala · 2026-07-31T22:31:28Z

+|-------|------|-------------------|-------------|
+| `trappedKeyBytes` | `long` | Snapshot create (`deletedTable` sum); ExpandAndAccountDirService; DDS promote to `deletedTable` (reclaimable dir) | Successful key purge |
+| `trappedKeyNamespace` | `long` | Same | Successful key purge |
+| `trappedDirNamespace` | `long` | Snapshot create (count of **root** rows in `deletedDirTable`) | DDS removes root from `deletedDirTable` |


To the trappedDirNamespace we could add the sub-directories that have been expanded once the subtree walk finishes. This can give some info about how much namespace is trapped.

priyeshkaratha

Thanks @sadanand48 for working on this design. I have few comments please check it.

priyeshkaratha · 2026-08-03T05:48:38Z

+inline `SnapshotInfo` update per task — **not** one transaction per file.
+
+### 6.5 Integration with existing services
+


can you explain the flow with help of some flow charts like https://ozone.apache.org/docs/next/system-internals/data-operations/delete#interaction-with-ozone-snapshots

generated one in the PR description

priyeshkaratha · 2026-08-03T08:00:24Z

+
+```
+┌─────────────────────────────────────────────────────────────────┐
+│ Snapshot create (createOmSnapshotCheckpoint)                    │


What happens during upgrade? If you have chains of snapshots?

We need a tool to update the counters on upgrade like the quota repair tool which we could run, We could also add another task to the QuotaRepair tool itself.

smengcl · 2026-08-04T06:04:27Z

Thanks @sadanand48 for the design doc

smengcl · 2026-08-04T06:04:44Z

+
+1. Re-validate snapshot chain (`expectedPreviousSnapshotId`).
+2. Re-check `ReclaimableDirFilter`; if now reclaimable → exit (DDS owns expansion).
+3. Read-only walk `fileTable` under dir root (and subdirs still in `directoryTable`).


DDS already scans child directories when the root is not reclaimable. DDS also calls ReclaimableKeyFilter for each file. DDS can move a child directory before this service reads directoryTable. The new walk can then miss the child subtree and mark the parent complete. Define coordination with DDS, or collect the accounting during the existing DDS traversal.

DDS already scans child directories when the root is not reclaimable

This is wrong, it doesn't scan it

boolean isDirReclaimable = reclaimableDirFilter.apply(pendingDeletedDirInfo); Optional<PurgePathRequest> request = prepareDeleteDirRequest( pendingDeletedDirInfo.getValue(), pendingDeletedDirInfo.getKey(), isDirReclaimable, allSubDirList, getOzoneManager().getKeyManager(), reclaimableFileFilter, remainNum);

And inside prepareDeleteDirRequest (lines 485-501, which I can see from the surrounding context), it does:

if (isDirReclaimable) { // It expands the directory and adds files to subFiles and dirs to subDirs ... }

Define coordination with DDS, or collect the accounting during the existing DDS traversal.

Yes this is addressed on line 241

smengcl · 2026-08-04T06:04:44Z

+* Call SCM or purge blocks.
+* Update `trappedDirNamespace` before subtree walk completion.
+
+**Ratis pressure mitigation:** accumulate counter deltas in memory; one batched `SetSnapshotProperty` or


putIfAbsent and the counter delta must use the same OM Ratis request and active DB batch. A separate SetSnapshotProperty request creates a failure window. A persisted ledger entry can cause a retry to skip a missing counter increment. Define the request, cache update, DB batch, and replay behavior.

Added explicit text that putIfAbsent and the counter delta must use the same OM Ratis request and active DB batch to prevent failure windows

smengcl · 2026-08-04T06:04:44Z

+
+New OM table: `snapshotTrappedLedgerTable` (name TBD).
+
+**Key:** `deleteDbKey` (row key in `deletedTable` / `deletedDirTable`)


A raw deleteDbKey has two prefix formats. deletedTable uses a volume and bucket name prefix. deletedDirTable uses an FSO ID prefix. Snapshot SST filtering supports one prefix for each table. Define a normalized ledger key and a filtering rule. Also define when OM removes PURGED rows.

Updated the format, the the ledger key to be /{volumeId}/{bucketId}/{deleteDbKey}

smengcl · 2026-08-04T06:04:44Z

+|-------|--------------|
+| `referencedSize` | Live data at create — orthogonal |
+| `exclusiveSize` | Legacy deep-clean side effect — keep for reconciliation; do not sum with `trappedKeyBytes` |
+| `snapshotUsedBytes` (bucket) | Superset across snapshots + AOS; authoritative for quota |


snapshotUsedBytes is not a superset after the new service counts files that remain in fileTable. Those bytes remain in usedBytes until DDS moves the files. trappedKeyBytes can exceed snapshotUsedBytes. Correct this relationship and state which totals operators can compare.

Added a note for this

smengcl · 2026-08-04T06:30:07Z

+
+## 3. Goals and Non-Goals
+
+### Goals


I don't see a non-goal section anymore?

smengcl · 2026-08-04T06:34:09Z

+| **Store snapshot** | The snapshot whose DB holds a `deletedTable` / `deletedDirTable` row |
+| **Pinning snapshot** | In this design, counter ownership follows the **store snapshot** (snapshot DB currently holding the deleted row) |


If counters always belong to the store snapshot, just remove Pinning snapshot and use store snapshot throughout?

sadanand48 · 2026-08-19T06:57:36Z

About why the ledger/cache for the accounted keys is needed:

Table presence only tells us that a deleted row exists; it does not tell us whether trapped accounting has already been applied for that row. We need exactly-once semantics across retries/replays and across multiple paths (snapshot create, DDS expand/promote, purge, snapshot-delete move). This is especially important for FSO where only dir roots are accounted at create, while subtree files can be accounted later during DDS traversal and may be revisited multiple times. A persisted ledger provides durable “already-accounted / purged” state and preserves ownership when rows move between snapshots, which table membership alone cannot represent.

sadanand48 · 2026-08-26T07:12:00Z

@smengcl @jojochuang I was considering the performance concerns that were discussed around having a separate service and additional implementation with new table etc and thought that we could have a tool instead similar to du that will give out on-demand space distribution for the bucket. That way it doesn't always stress the system and only will be run when the user wants an explanation. I've raised a PR for it in #11122 . Let me know what you think.

sadanand48 added 2 commits July 22, 2026 13:39

HDDS-15939. Design for accounting per-snapshot trapped deleted data.

5267fe1

add details

5bc2871

sadanand48 added AI-gen snapshot https://issues.apache.org/jira/browse/HDDS-6517 labels Jul 22, 2026

jojochuang requested review from SaketaChalamchala, jojochuang and smengcl July 23, 2026 15:04

errose28 added the design label Jul 27, 2026

SaketaChalamchala reviewed Jul 31, 2026

View reviewed changes

priyeshkaratha reviewed Aug 3, 2026

View reviewed changes

address comments

bcc7793

smengcl reviewed Aug 4, 2026

View reviewed changes

address comments

df83167

jojochuang changed the title ~~HDDS-15939. Design for accounting per-snapshot trapped deleted data~~ HDDS-15939. [OEP] Design for accounting per-snapshot trapped deleted data Aug 20, 2026

		inline `SnapshotInfo` update per task — not one transaction per file.

		### 6.5 Integration with existing services


		New OM table: `snapshotTrappedLedgerTable` (name TBD).

		Key: `deleteDbKey` (row key in `deletedTable` / `deletedDirTable`)

		\| Store snapshot \| The snapshot whose DB holds a `deletedTable` / `deletedDirTable` row \|
		\| Pinning snapshot \| In this design, counter ownership follows the store snapshot (snapshot DB currently holding the deleted row) \|

Uh oh!

Conversation

sadanand48 commented Jul 22, 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

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!

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!

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smengcl commented Aug 4, 2026

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sadanand48 commented Aug 19, 2026

Uh oh!

sadanand48 commented Aug 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sadanand48 commented Jul 22, 2026 •

edited

Loading