Siyao Meng created HDDS-16794:
---------------------------------
Summary: KeyDeletingService run that outlives a lost and regained
OM leadership purges keys again and reduces bucket usage twice
Key: HDDS-16794
URL: https://issues.apache.org/jira/browse/HDDS-16794
Project: Apache Ozone
Issue Type: Bug
Reporter: Siyao Meng
Attachments:
MC-2-do-not-submit-key-purge-after-term-change-during-run.patch,
TestBugMC2StaleRunDoublePurge.java
h3. Mechanism
{{KeyDeletingService.KeyDeletingTask.call}} checks {{shouldRun}} (leader ready)
and the flush of the previous purge once, at the start of a run. It then scans
{{deletedTable}}, calls SCM {{deleteKeyBlocks}} and submits {{PurgeKeys}}.
Nothing ties the request to the leadership under which the scan was made, and
Ratis accepts it whenever this OM is the leader at the time of the submit.
{{OMKeyPurgeRequest.validateAndUpdateCache}} subtracts the sizes in
{{bucketPurgeKeysSize}} from {{snapshotUsedBytes}} and
{{snapshotUsedNamespace}} whether or not the keys still exist, and deleting the
absent {{deletedTable}} rows is a no-op.
If the OM loses the leadership and gets it back while a run is between its
checks and its submit, and the leader in between has purged the same keys, the
usage is subtracted a second time on every OM.
HDDS-13760 added the flush check "so that duplicate purge requests are not
submitted". It covers a second scan on the same OM before the flush, not a run
that has already passed the check.
h3. Trigger
# OM1 is leader and a key is deleted. A KeyDeletingService run on OM1 passes
its checks and scans the key.
# The run stalls before it submits the purge, for example in the SCM call. OM1
loses the leadership, OM2 becomes leader and its own run purges the key.
# OM1 becomes leader again.
# The stalled run continues and submits its purge of the key, which is applied
in the new term.
The stall has to cover both leadership changes and the run of the other leader,
which comes on that leader's own schedule, so with the default service interval
of 60s it has to last up to about a minute. I expect this to be rare (argued,
not measured).
By history the defect is present since 2.1.0: {{ozone-2.1.0}} is the first
release tag in which the purge reduces the bucket usage (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 stale request repeats the whole batch of the stalled run, not a single
key.
* The wrong values are written to 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. [^TestBugMC2StaleRunDoublePurge.java]
({{hadoop-ozone/integration-test}}) starts a mini cluster with three OMs and
one datanode. The deleting services get a long interval and are run by hand
with {{runPeriodicalTaskNow}}, and the leadership is moved with
{{OzoneManager.transferLeadership}}. The run on the first leader is made by a
second {{KeyDeletingService}} on that OM whose SCM client delays the reply of
{{deleteKeyBlocks}}, so the run is held in the SCM call. After the sequence
above the client reads {{usedBytes = -1024}} and {{usedNamespace = -1}} for the
empty bucket, the bucket row holds -1024 pending deletion bytes in RocksDB on
all three OMs, 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.
h3. Patch
[^MC-2-do-not-submit-key-purge-after-term-change-during-run.patch], against
a6332fb59879b80615b9233e953d6813d03ae7a8. It also applies to master at
3a22acfaa21.
A run of {{KeyDeletingService}} records the Raft term right after the leader
check, before the flush check and the scan, and compares it with the current
term immediately before each submit of the run ({{PurgeKeys}} and the snapshot
property updates, for the active object store and for the snapshot tasks). If
the term is different the request is not submitted and the run ends like a run
whose purge failed. The term is read from the same Ratis division info that
{{shouldRun}} uses for leader ready. There is no proto change and nothing
changes at apply time. The keys stay in {{deletedTable}} and the next run,
which starts in the new term, purges them once (asserted in the new test).
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. An
earlier form of the reproduction, which holds the run inside the submit call
after the comparison, still passes with the patch. The stricter alternative is
a new optional field in {{PurgeKeysRequest}} with the term of the run, and a
check in {{OMKeyPurgeRequest}} that rejects the request when the term of its
log entry is different, which is exact and decided identically on every OM. It
is not proposed here because it adds a field without a layout feature gate to
an existing request, and in a ring where OMs of two versions apply the same log
an OM without the check applies a stale request that an OM with the check
rejects, so the OMs disagree.
Subtracting only for keys that are still in {{deletedTable}} was also
considered and not taken. It cannot be decided consistently at apply time,
because {{deletedTable}} has 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
{{TestKeyDeletingService}}: the term changes while the run is in the SCM call,
the run does not purge, and the next run purges the key once. It fails when the
comparison in {{KeyDeletingService.submitRequest}} is removed, because the
first run then purges the key. With the patch 173 unit tests of
{{hadoop-ozone/ozone-manager}} (deleting service, purge request, open key
cleanup, key manager, snapshot manager and quota repair suites) and 22 mini
cluster tests (key purging, OM HA with stopped nodes and the snapshot deleting
service suite) pass, checkstyle is clean. With the patch the reproduction fails
at its first assertion of the buggy outcome, because the stale run is not
submitted and the usage stays 0.
Not covered by the patch: {{DirectoryDeletingService}} submits
{{PurgeDirectories}} with the same single check at the start of a run, and
{{OMDirectoriesPurgeRequestWithFSO}} also reduces the bucket usage without a
presence check (code reading, not run). It submits from its worker threads, so
the term would have to be handed to them, and it is left out here. A term
change between two requests of one run now stops the later requests, which is
one more way into HDDS-16790 (a key purged by one request with its size in the
next) until that is fixed.
The flush check that misses a purge which is not applied yet on a new leader is
a different defect and is filed separately. Neither fix covers the other: that
one covers a run that starts after a leader change, this 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]
