[
https://issues.apache.org/jira/browse/HDDS-16795?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16795:
------------------------------
Attachment: TestBugMC1PurgeRepeatedAfterRestart.java
> KeyDeletingService can purge keys twice after a leader change to an OM still
> applying a backlog, or after an OM restart
> -----------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-16795
> URL: https://issues.apache.org/jira/browse/HDDS-16795
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Major
> Attachments:
> MC-1-wait-for-flush-of-earlier-terms-before-aos-purge.patch,
> TestBugMC1PurgeRepeatedAfterRestart.java
>
>
> h3. Mechanism
> Before a run on the active object store, {{KeyDeletingService}} and
> {{DirectoryDeletingService}} call
> {{AbstractKeyDeletingService.isPreviousPurgeTransactionFlushed}}
> (HDDS-13760). It compares the last purge transaction, kept in memory in
> {{DeletingServiceMetrics}}, with the transaction info flushed to the DB. The
> in-memory value is set when this OM process applies a purge. After a restart
> it is empty, and on an OM that has just become leader it does not include a
> purge that is committed in the Ratis log but not applied yet.
> The OM reports leader ready as soon as the Ratis state machine updater has
> passed the first entry of the new term. The updater hands entries to the
> single OM executor and does not wait for them, so earlier entries, including
> a {{PurgeKeys}}, can still be queued at that time. A run that starts then
> passes {{shouldRun}} and the flush check, finds the same keys in
> {{deletedTable}} and submits a second {{PurgeKeys}}. {{OMKeyPurgeRequest}}
> subtracts the sizes in {{bucketPurgeKeysSize}} from {{snapshotUsedBytes}} and
> {{snapshotUsedNamespace}} for both requests. Deleting the already deleted
> rows is a no-op and nothing rejects the second request.
> h3. Trigger
> # A key is deleted and the delete is flushed.
> # A KeyDeletingService run submits {{PurgeKeys}} for it. The request is
> committed in the Ratis log.
> # Before the purge is applied on another OM that OM becomes leader, or before
> the purge is flushed the OM restarts.
> # The (new) leader reports leader ready while the purge is still waiting
> behind earlier entries in its executor.
> # A KeyDeletingService run starts in that window and purges the key again.
> The window is short, from leader ready until the executor has caught up,
> which is bounded by {{ozone.om.unflushed.transaction.max.count}} entries.
> With the default service interval of 60s a run rarely starts inside it. I
> expect the leader change onto an OM that is still applying a backlog (for
> example during a rolling restart under load) to be the realistic form, and
> the restart of a single OM to be practically unreachable at defaults, because
> its first run comes before leader ready and the next one 60s later (both
> argued, not run). The reproduction uses a restart and keeps the replay
> waiting.
> By history the defect is present since 2.1.0: {{ozone-2.1.0}} is the first
> release tag that has both the flush check (HDDS-13760) and the usage
> accounting in the purge (HDDS-13764). Not run on older code.
> h3. Impact
> * The pending deletion bytes and namespace of the bucket become too low,
> negative for a bucket with nothing else pending.
> {{OmBucketInfo.getTotalBucketSpace}} and {{getTotalBucketNamespace}} include
> them without a lower bound, so clients see wrong (in the reproduction
> negative) used bytes and namespace, and the quota checks in {{OMKeyRequest}}
> admit more than the quota. In the reproduction a bucket with a namespace
> quota of 1 accepted two keys. The space quota check reads the same total
> (code reading, not run).
> * The second request repeats the whole batch of the run, not a single key.
> * The wrong values are flushed to the bucket table and replicated, and
> nothing corrects them automatically. The quota repair recomputes these
> counters when an operator starts it (code reading, not run).
> * {{DirectoryDeletingService}} uses the same check and
> {{OMDirectoriesPurgeRequestWithFSO}} also reduces the bucket usage without a
> presence check, so it has the same exposure (code reading, not run).
> h3. Reproduction
> PASS, unmodified source. [^TestBugMC1PurgeRepeatedAfterRestart.java]
> ({{hadoop-ozone/ozone-manager}}) runs one real OM with a recording SCM block
> client and no datanodes. The KeyDeletingService gets a long interval and is
> run by hand. A key is deleted and flushed, then the double buffer is paused,
> a key is created in a second bucket and the service purges the deleted key,
> and the OM is stopped. A new {{OzoneManager}} is created on the same storage
> while the test holds the lock of the second bucket, so the replayed key
> create and the replayed purge behind it wait while the OM is already ready
> leader. The service run in that state sends the blocks to SCM again and
> submits a second purge. Afterwards the bucket row has {{snapshotUsedBytes}}
> of minus three times the key size and {{snapshotUsedNamespace = -1}} in
> RocksDB, the client reads the same negative usage, and with a namespace quota
> of 1 the bucket accepts two keys and rejects the third with
> {{QUOTA_EXCEEDED}}. A passing test means the defect is present.
> Note that {{OzoneManager.restart}} on the same object keeps the in-memory
> value and therefore does not show the problem.
> h3. Patch
> [^MC-1-wait-for-flush-of-earlier-terms-before-aos-purge.patch], against
> a6332fb59879b80615b9233e953d6813d03ae7a8. It also applies to master at
> 3a22acfaa21.
> {{isPreviousPurgeTransactionFlushed}} now also returns false while the last
> applied term index of the OM state machine is from an earlier term than the
> current term of the Ratis server. In OM that index is advanced by the double
> buffer after a flush, so once it is in the current term everything committed
> before this OM became leader is applied and flushed, and the scan sees it.
> After that the existing in-memory value covers the purges of the current
> term. A new leader with nothing to do is not held back: the first entry of
> every term is a Ratis configuration entry, and the state machine moves the
> last applied index onto it as soon as everything before it is flushed, so the
> check opens without any OM transaction in the new term. No new state is
> added, and both services are covered for the active object store. A run that
> starts in the window is skipped and logged at INFO.
> Subtracting only for keys that are still in {{deletedTable}}, or flooring the
> counters at 0, was considered and not taken: {{deletedTable}} has no table
> cache entries and the DB lags the apply by the double buffer, so an earlier
> purge that is not flushed would still look present, and OMs could decide
> differently.
> Covered by {{testNoPurgeUntilReplayedPurgeIsFlushed}} in a new nested class
> of the existing {{TestKeyDeletingService}}. It asserts that the check is
> closed for both services while the replayed purge is not applied, and that
> the key is then purged once. Without the change to
> {{AbstractKeyDeletingService}} it fails at the first of these assertions,
> because the check is open. {{OmTestManagers}} gets a small static helper so
> that a test can create a second OM on the same storage. With the patch 131
> unit tests of {{hadoop-ozone/ozone-manager}} (deleting service, purge
> request, key manager, snapshot manager and quota repair suites) and 31 mini
> cluster tests (key purging, OM HA with stopped nodes, snapshot and directory
> deleting service suites) pass, checkstyle is clean. With the patch the
> reproduction fails at its first assertion of the buggy outcome (SCM gets the
> blocks of the key once, not twice), because the run after the restart is
> skipped.
> Not covered by the patch, on purpose: the snapshot tasks of the two services
> use a different check, {{OmSnapshotManager.areSnapshotChangesFlushedToDB}},
> which reads the last transaction recorded in the snapshot info when a purge
> is applied. It has the same weakness, because on a new leader a snapshot
> purge that is committed but not applied is not recorded yet (code reading,
> not run). That check needs its own change and test and is left for a follow
> up.
> A run that has already passed the check when the leadership moves away and
> comes back is a different defect, filed separately as HDDS-16794. Neither fix
> covers the other: this patch covers a run that starts after the leader
> change, the other one a run that started before it.
> Found by TLA+ model checking and code review of the OM key deletion service
> and key purge paths under HDDS-15926, on commit
> a6332fb59879b80615b9233e953d6813d03ae7a8. Checked against HDDS issues and
> apache/ozone pull requests for duplicates before filing. The attached patch
> is a proposal for review. Generated with Specula (Claude Opus 5.5).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
