Siyao Meng created HDDS-17670:
---------------------------------
Summary: DirectoryDeletingService run that outlives a lost and
regained OM leadership purges directories again and reduces bucket usage twice
Key: HDDS-17670
URL: https://issues.apache.org/jira/browse/HDDS-17670
Project: Apache Ozone
Issue Type: Bug
Reporter: Siyao Meng
Attachments:
MC-1-do-not-submit-dir-purge-after-term-change-during-run.patch,
TestBugMC1MC2StaleDirPurge.java
h3. Mechanism
{{DirectoryDeletingService.DirDeletingTask.call}} checks {{shouldRun}} (leader
ready) once, at the start of a run, for the active object store task and for
each snapshot deep clean task. The task then scans {{deletedDirTable}} with its
worker threads, each worker submits {{PurgeDirectories}} requests, and the task
submits a {{SetSnapshotProperty}} request at the end. Nothing ties these
requests to the leadership under which the scan was made, and Ratis accepts
them whenever this OM is the leader at the time of the submit.
{{OMDirectoriesPurgeRequestWithFSO}} only checks that the bucket still has the
same object ID (and, for a purge from a snapshot, that the snapshot chain has
not changed). It does not check that the directories and files of the request
are still pending, and applies the usage change for every entry again: a
sub-file moves its size and one namespace from {{usedBytes}} and
{{usedNamespace}} to {{snapshotUsedBytes}} and {{snapshotUsedNamespace}}, a
sub-directory moves one namespace, and a purged directory subtracts one from
{{snapshotUsedNamespace}}.
If the OM loses the leadership and gets it back while a run is between its
check and its submit, and the leader in between has purged the same
directories, the request is applied a second time on every OM. HDDS-16794
describes this window for {{KeyDeletingService}} and leaves this service out of
its patch.
h3. Trigger
# OM1 is leader and directories of an FSO bucket are deleted (for the snapshot
path, a snapshot of the bucket is taken after the delete, so the deleted
directories belong to the snapshot).
# A DirectoryDeletingService run on OM1 passes its checks, scans the deleted
directories and builds its request.
# The run stalls before it submits. OM1 loses the leadership, OM2 becomes
leader and its own run purges the same directories.
# OM1 becomes leader again. Its log holds the purge of OM2, otherwise it could
not be elected, so the stale request is appended after it.
# The stalled run continues and submits its request, which is applied in the
new term on every OM.
The stall has to cover both leadership changes and the run of the other leader,
which comes on that leader's own schedule (default interval 60s), so it has to
last for tens of seconds up to about a minute. It also has to hold the thread
of the run and not the whole OM, because an OM that is paused as a whole cannot
be elected again before the run continues. Waits for snapshot locks or a slow
opening of snapshot DBs in the reclaim filters are possible causes. I expect
this to be rare (argued, not measured). Snapshot deep cleaning is enabled by
default.
By reading the release tags (not run on released code): the purge has released
the usage of the sub-files and sub-directories it moves since {{ozone-1.3.0}}
(HDDS-4565), so a repeated purge of a directory with content reduces
{{usedBytes}} and {{usedNamespace}} again since then. The reduction of
{{snapshotUsedNamespace}} for purged directories (HDDS-13764) and the snapshot
deep clean tasks of this service (HDDS-13160) are in {{ozone-2.1.0}} and later.
h3. Impact
Reproduced on unmodified source:
* *Active object store.* An empty directory and a directory with one 1024 byte
file are deleted, OM2 purges both, then the stale run of OM1 purges them again.
On all three OMs the bucket row in RocksDB holds {{usedBytes = -1024}},
{{usedNamespace = -1}}, {{snapshotUsedBytes = 2048}} and
{{snapshotUsedNamespace = 0}} (correct: 0, 0, 1024, 1). After
{{KeyDeletingService}} has purged the file and {{deletedTable}} is empty, the
row stays at -1024, -1, 1024 and -1 (correct: all 0), the client reads
{{usedNamespace = -2}} for the empty bucket, and with a namespace quota of 1
the bucket accepts three keys and rejects the fourth with {{QUOTA_EXCEEDED}}.
* *Snapshot deep clean.* An empty directory is deleted and a snapshot is taken,
the deep clean of OM2 purges the directory from the snapshot, then the stale
run of OM1 purges it again. On all three OMs {{snapshotUsedNamespace = -1}}
(correct: 0), the client reads {{usedNamespace = -1}}, and with a namespace
quota of 1 the bucket accepts two directories and rejects the third.
* The stale request repeats the whole batch of the stalled worker, not a single
directory.
* Total bytes, which the client and the space quota check read
({{OmBucketInfo.getTotalBucketSpace}}), are not changed by the repeated move of
a file: it lowers {{usedBytes}} and raises {{snapshotUsedBytes}} by the same
amount. In the reproduction the client reads {{usedBytes = 0}} after the file
is purged, which is correct. By code reading, not run: the total bytes drop
when {{KeyDeletingService}} of the other leader has already purged the file,
because the stale request puts it back into {{deletedTable}} and it is purged a
second time, and in 1.3.0 to 2.0.x, where the purge subtracted {{usedBytes}}
directly.
* The wrong values are in the bucket table on all OMs and nothing corrects them
automatically. The quota repair recomputes these counters when an operator
starts it (code reading, not run).
h3. Reproduction
PASS, unmodified source. [^TestBugMC1MC2StaleDirPurge.java]
({{hadoop-ozone/integration-test}}) starts a mini cluster with three OMs and
one datanode, shared by two tests (active object store and snapshot deep
clean). The deleting services get a long interval and are run by hand with
{{runPeriodicalTaskNow}}, and the leadership is moved with
{{OzoneManager.transferLeadership}}. The stale run is made by a second
{{DirectoryDeletingService}} on the first leader whose {{submitPurgeRequest}}
waits on a latch and then calls the unmodified implementation, so the request
is built by the real service from a real scan. All state is created through the
client API. A passing test means the defect is present. With the patch both
tests fail at their first assertion of the buggy outcome, because the stale run
does not submit and all OMs keep the correct counters.
h3. Patch
[^MC-1-do-not-submit-dir-purge-after-term-change-during-run.patch], against
ec5ed5f7ea0e4b406aee53aae4906616819cba2b. It also applies to master at
6ab9ed82a81.
Same approach as the patch on HDDS-16794 for {{KeyDeletingService}}. A run of
{{DirectoryDeletingService}} records the Raft term right before the leader
check, and the service compares it with the current term immediately before
each submit of the run ({{PurgeDirectories}} from the worker threads and
{{SetSnapshotProperty}} from the task, for the active object store and for the
snapshot tasks). Reading it before the check means that a leader change between
the two fails the comparison instead of being recorded as the term of the run.
The HDDS-16794 patch reads the term after its leader check, and the same order
applies there. An OM without a Ratis server yet (as in
{{OzoneManager.restart}}) reads -1, which never matches. The term is kept per
thread, as in the HDDS-16794 patch, so that the helpers between the task and
the submit keep their signatures; it is handed to the worker threads of the run
when they start and cleared when the task and each worker end. If the term is
different the request is not submitted and the worker stops as after a failed
submit. The directories stay in {{deletedDirTable}} and the next run, which
starts in the new term, purges them once. There is no proto change and nothing
changes at apply time. Stopping after a failed submit already exists in this
service and leaves no partial accounting: every {{PurgePathRequest}} carries
the usage of its own entries, and a sub-directory marked by an applied request
is processed by a later run.
This narrows the window, it does not close it. The comparison is not atomic
with the append to the Ratis log, so a run that stalls between the comparison
and the append for as long as described under Trigger still purges twice. The
stricter alternative is an optional field in {{PurgeDirectoriesRequest}} with
the term of the run, checked in {{OMDirectoriesPurgeRequestWithFSO}} against
the term of the log entry. It is exact and decided identically on every OM, but
it adds a field without a layout feature gate to an existing request, and OMs
of two versions applying the same log would disagree. Applying the usage only
for entries that are still pending was not taken for the reason given in
HDDS-16794: the deleted tables have no table cache entries and the DB lags the
apply by the double buffer, so OMs could decide differently.
Covered by {{testNoPurgeAfterTermChangeDuringRun}} in the existing
{{TestDirectoryDeletingService}}, for the active object store and for a
snapshot, with two worker threads: the term changes after the run has started,
the run does not purge and {{snapshotUsedNamespace}} stays 1, and the next run
purges the directory once. It fails when the comparison is removed, and also
when the term is not handed to the worker threads, because the first run then
purges the directory. With the patch {{TestDirectoryDeletingService}},
{{TestKeyDeletingService}}, {{TestSnapshotDeletingService}} and
{{TestOMDirectoriesPurgeRequestAndResponse}} (65 tests) and the mini cluster
suites {{TestDirectoryDeletingServiceWithFSO}} and {{TestRootedDDSWithFSO}} (9
tests) pass, and checkstyle is clean. {{TestSnapshotDirectoryCleaningService}}
timed out in my environment with and without the patch, before the service is
involved, so it is not counted.
The patch and the one on HDDS-16794 change different files and apply together.
If both are taken, the term check can move into {{AbstractKeyDeletingService}}.
HDDS-16795 (a run that starts after a leader change, before an earlier purge is
applied) is a different window: neither patch covers the other. Its patch
covers the active object store of this service through the shared flush check,
not the snapshot tasks.
Found by TLA+ model checking and code review of the OM directory and key
deleting services and their bucket usage accounting under HDDS-15926, on commit
ec5ed5f7ea0e4b406aee53aae4906616819cba2b. Checked against HDDS issues and
apache/ozone pull requests for duplicates before filing. Related but not
duplicates: HDDS-16794 (the same window for {{KeyDeletingService}}, names this
service as not covered), HDDS-16795 (a run that starts after a leader change),
HDDS-16790 ({{KeyDeletingService}} request split) and HDDS-13760 (the flush
check at the start of a run). 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]
