[
https://issues.apache.org/jira/browse/HDDS-17670?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17670:
------------------------------
Attachment: MC-1-do-not-submit-dir-purge-after-term-change-during-run.patch
> 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
> Affects Versions: 1.3.0, 1.4.0, 2.0.0, 1.4.1, 2.1.0, 2.2.0, 2.1.1, 2.1.2,
> 2.2.1, 2.2.2
> Reporter: Siyao Meng
> Priority: Minor
> 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]
