Siyao Meng created HDDS-17605:
---------------------------------
Summary: Deleting services can mark the wrong snapshot deep
cleaned after a snapshot rename, or undo the deep clean reset of a snapshot
purge
Key: HDDS-17605
URL: https://issues.apache.org/jira/browse/HDDS-17605
Project: Apache Ozone
Issue Type: Bug
Reporter: Siyao Meng
Attachments:
MC-3-recheck-snapshot-before-setting-deep-clean-flag.patch,
TestBugCR3StaleDirDeepCleanFlagAfterPurge.java,
TestBugMC3RenamedSnapshotDeepCleanFlag.java
h3. Mechanism
{{KeyDeletingService}} and {{DirectoryDeletingService}} deep clean one snapshot
per task. When a task has nothing left to purge, it sends
{{SetSnapshotProperty}} with {{deepCleanedDeletedKey=true}} or
{{deepCleanedDeletedDir=true}}, and every later task skips a snapshot that
carries the flag. The flag request names the snapshot only by the table key
({{/volume/bucket/name}}) that the task read when it started, and carries
nothing about the previous snapshot the scan ran against.
{{OMSnapshotSetPropertyRequest.validateAndUpdateCache}} sets the flag on
whatever row has that key when it is applied. Unlike {{PurgeKeys}} and
{{PurgeDirectories}}, which call {{SnapshotUtils.validatePreviousSnapshotId}}
at apply time, nothing checks that the snapshot is still the one that was
scanned, against the same previous snapshot.
Two cases follow.
* *Rename.* {{OMSnapshotRenameRequest}} takes the bucket lock and the snapshot
name locks, not the {{SNAPSHOT_GC_LOCK}} the deleting services use, so a rename
of the scanned snapshot followed by a new snapshot with the old name can be
applied while the task runs. The flag then lands on the new snapshot, which was
never scanned. The renamed snapshot stays unmarked and is scanned again
(harmless).
* *Purge of the previous snapshot.* When a deleted snapshot is purged,
{{OMSnapshotPurgeRequest}} sets {{deepClean}} and {{deepCleanedDeletedDir}}
back to false on the next two snapshots so they are cleaned again against their
new previous snapshot. {{SnapshotDeletingService}} submits the purge after it
releases its lock, the purge apply takes no lock, and
{{DirectoryDeletingService}} submits the directory flag after its workers have
released their GC locks. A flag built against the old previous snapshot can
therefore be applied after the purge and overwrite the reset.
h3. Trigger
* *Rename:* from ozone-2.2.0 it needs {{ozone.om.snapshot.rename.allowed=true}}
(default false). ozone-2.0.0 to ozone-2.1.2 have no rename switch, but there
snapshot deep cleaning is off by default
({{ozone.snapshot.deep.cleaning.enabled=false}}; HDDS-15187 turned it on by
default in 2.2.0), so they are affected only with it enabled. A user renames
snapshot S and creates a new snapshot named S in the same bucket while a
deleting service task on S is running. For a {{DirectoryDeletingService}} task,
which sends the flag only when it purged nothing, and for a
{{KeyDeletingService}} task that purges nothing, the window is the whole task,
from the start of the scan to the apply of the flag. For a
{{KeyDeletingService}} task that purges, it starts at the apply of its last
{{PurgeKeys}}: a {{PurgeKeys}} applied after the rename goes to the old name,
fails its previous snapshot check, and no flag follows.
* *Purge:* no special config from ozone-2.2.0; on ozone-2.1.x only with
{{ozone.snapshot.deep.cleaning.enabled=true}}. Snapshot S1 is deleted while
{{DirectoryDeletingService}} deep cleans the next snapshot S2 against S1 and
finds nothing it can purge, then {{SnapshotDeletingService}} purges S1 (it has
nothing to move) and the purge is applied before the directory flag. The
directory workers hold GC read locks on S1 and S2 while they scan, so
{{SnapshotDeletingService}} can only evaluate S1 after they release them; its
purge then races the coordinator's flag request through Ratis. This is argued
from the code; the reproductions control the order and do not measure how often
the race is won in a running cluster.
h3. Impact
The wrong snapshot carries a deep clean flag until a later purge in its chain
resets it or the snapshot itself is deleted.
* Rename, key flag: the new snapshot is skipped by every {{KeyDeletingService}}
run, so its deleted keys that are reclaimable against the renamed snapshot are
not reclaimed. Rename, directory flag: the same for the deleted directories of
the new snapshot (shown at flag level only).
* Purge, directory flag: a deleted directory that became reclaimable against
the new previous snapshot stays in the snapshot's {{deletedDirTable}}, together
with whatever it holds.
No data is lost and nothing is reclaimed early; the effect is space that is not
reclaimed. The same purge mechanism for the key flag is HDDS-16464. At this
commit that case is already narrow: {{KeyDeletingService}} keeps its GC read
locks on the snapshot and its previous snapshots until its
{{SetSnapshotProperty}} is applied, and its filter refuses a previous snapshot
that is not active, so {{SnapshotDeletingService}} cannot purge the previous
snapshot of a run that read any entry before the key flag is applied. The
{{DirectoryDeletingService}} coordinator submits its flag after the workers
have released those locks, which is why the directory flag is the purge case
reported here.
h3. Reproduction
Two self contained classes for {{hadoop-ozone/ozone-manager}} (target path and
Maven command in each class comment) run one real OM through
{{OmTestManagers}}, real {{KeyDeletingTask}} / {{DirDeletingTask}} runs, and
requests sent through {{OzoneManagerProtocol}} or Ratis. No state is injected;
the only aid is a hook that runs the rename or purge at a chosen point of the
task.
* [^TestBugMC3RenamedSnapshotDeepCleanFlag.java]: with rename allowed, snapshot
snapa (id1) is renamed to snapb and a new snapa (id2) holding one reclaimable
deleted key is created after the task's {{PurgeKeys}} request. id2 gets
{{deepCleanedDeletedKey=true}}, id1 stays false, and a later task on id2 leaves
the key in id2's {{deletedTable}}.
* [^TestBugCR3StaleDirDeepCleanFlagAfterPurge.java]: snap0, snap1, snap2 with a
directory that only snap1 references in snap2's {{deletedDirTable}}. snap1 is
deleted during the {{DirDeletingTask}} on snap2 and purged with the request
{{SnapshotDeletingService}} sends, applied before the flag. snap2's previous
becomes snap0 with {{deepCleanedDeletedDir=true}} in RocksDB, and a second task
on snap2 skips it and leaves the directory in place.
Both classes also run a case where the change happens after the flag request
has been built; see Patch.
h3. Suggested fix
The strict fix is for {{SetSnapshotPropertyRequest}} to carry the snapshot ID
and the expected previous snapshot ID, checked at apply time like the purge
requests. That is a new request field, so it needs a new layout feature to keep
OMs of two versions applying the same log in agreement (HDDS-16464 proposes the
previous snapshot ID field for the key flag only, without a layout feature).
The attached patch stays on the submitting side.
h3. Patch
[^MC-3-recheck-snapshot-before-setting-deep-clean-flag.patch], against
284fad307bfd129fca3f4966d854ac45ea3b0d11. It also applies to master at
6135e81a09ac6d618dc0ad4d220dd8cf254586b6.
A new helper {{SnapshotUtils.getTableKeyToMarkDeepCleaned}}, used by both
services right before they build the flag request, looks the snapshot's table
key up again by snapshot ID and returns nothing, so no flag is sent, when the
snapshot is gone, its previous snapshot is not the one the scan used, or that
previous snapshot is no longer active. The last condition covers a purge
already on its way: {{SnapshotDeletingService}} only purges deleted snapshots.
A skipped flag only means the snapshot is scanned again in the next run;
exclusive size updates in the same request are sent as before. No request,
config or apply path changes. Because the helper is shared,
{{KeyDeletingService}} gets the same check for the key flag, which also narrows
what is left of the HDDS-16464 case (a run that read no entry and so took no GC
lock) on the submitting side.
This narrows the windows, it does not close them: a rename plus a create, or a
delete, its flush and a {{SnapshotDeletingService}} run, all landing between
the check and the apply of the flag request (one Ratis round trip) still mark
the wrong snapshot. Each reproduction class has a case for that order, and it
still shows the bug with the patch. A rename plus a create in the few lines
between a task reading the snapshot by ID and opening its DB by name also
remains: the task then scans the new snapshot while the flag goes to the
renamed one, and any purge in that run is rejected.
New tests in existing suites:
{{TestDirectoryDeletingService#testDeepCleanFlagAfterSnapshotChangeDuringRun}}
(no change, previous purged during the run, previous deleted with its purge
applied just before the flag, rename and reuse of the name) ,
{{TestKeyDeletingService.SnapshotDbHandleLifecycle#testDeepCleanFlagFollowsRenamedSnapshot}}
and {{TestSnapshotUtils#testTableKeyToMarkDeepCleanedWhenPreviousIsPurged}}
(the previous snapshot purged between the helper's two lookups counts as not
active instead of failing the whole request). Without the change the three
change cases and the rename case fail; with it they pass, as do
{{TestKeyDeletingService}}, {{TestDirectoryDeletingService}},
{{TestSnapshotDeletingService}},
{{TestOMSnapshotSetPropertyRequestAndResponse}},
{{TestOMSnapshotPurgeRequestAndResponse}}, {{TestSnapshotUtils}} and the
{{TestReclaimable*}} filter suites (578 tests), and checkstyle is clean.
Found by TLA+ model checking and code review of the OM snapshot purge, set
property and double buffer apply paths under HDDS-15926, on commit
284fad307bfd129fca3f4966d854ac45ea3b0d11. Checked against HDDS issues and
apache/ozone pull requests for duplicates before filing. Related but not a
duplicate: HDDS-16464 (the purge case for the key flag, with a proposed apply
side check of the previous snapshot ID; it does not cover the directory flag,
and its text does not discuss renames, though its proposed check would reject
most of them for the key flag because the new snapshot's previous is the
renamed one). The attached patch is a proposal for review. Generated with
Specula (Claude Opus 5.5).
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
