Siyao Meng created HDDS-16795:
---------------------------------
Summary: 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
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]
