Siyao Meng created HDDS-17175:
---------------------------------
Summary: Expired delegation token remover can drop a renew applied
during its scan, so the OM rejects the token before its renewed expiry
Key: HDDS-17175
URL: https://issues.apache.org/jira/browse/HDDS-17175
Project: Apache Ozone
Issue Type: Bug
Reporter: Siyao Meng
h3. Mechanism
{{OzoneDelegationTokenSecretManager.removeExpiredToken}} runs on its own thread
on every OM. For each entry of the in-memory token map it reads the renew date,
and if that date has passed it calls {{Iterator.remove()}} and then deletes the
row from {{delegationTokenTable}} directly in RocksDB, outside Ratis and the
double buffer. On a {{ConcurrentHashMap}} the iterator's remove deletes the key
whatever value it holds by then. The renew apply
({{OMRenewDelegationTokenRequest.validateAndUpdateCache}}, which calls
{{updateRenewToken}}) shares no lock with the remover, so a renew applied
between the check and the remove is dropped from memory. Its DB row survives
only if the renew's flush lands after the remover's delete.
h3. Trigger
# A client renews a token shortly before its renew date. The leader's
{{renewToken}} check passes and the client gets a new expiry.
# The renew is applied after the old renew date, while that OM's expired token
remover is checking this token.
No fault is needed, only timing. The window is the gap between reading the
entry and removing it within one scan (microseconds per token per scan; the
scan runs every {{ozone.manager.delegation.remover.scan.interval}}, 1 hour by
default). Only a renew applied after the token's old renew date is exposed, so
renewals made well ahead of the renew date never hit it. I did not measure how
often it happens in a real cluster.
h3. Impact
* On that OM, before the acknowledged expiry, {{retrievePassword}} (token
authentication) and {{renewToken}} fail with "token ... can't be found in
cache", and {{cancelToken}} fails with "Token not found in-memory map of
tokens". The in-memory map is only rebuilt when the secret manager is created
(OM start, or installing a checkpoint from the leader), and the client cannot
recover by renewing again.
* If the renew's flush landed after the remover's delete, the DB row holds the
new expiry and a restart of that OM brings the token back. If it landed before,
the row is gone too and the token stays lost on that OM until it would have
expired, or until that OM installs a checkpoint from an OM that kept the row.
* Each OM runs its own remover, so the OMs can disagree about the token. A
client can get a new token; no other data is affected.
h3. Reproduction
PASS, deterministic, unmodified source.
[^TestBugMC3RemoverDropsConcurrentRenew.java] ({{hadoop-ozone/ozone-manager}})
drives one {{OzoneDelegationTokenSecretManager}} with its real remover thread
and calls the methods the request path calls for preExecute ({{createToken}},
{{renewToken}}), apply ({{updateToken}}, {{updateRenewToken}}) and flush (a
{{delegationTokenTable}} put). The renew is accepted 300 ms before the old
renew date. A test {{TokenInfo}} in the token map holds the remover thread
inside {{getRenewDate()}}, on its first read after the old renew date, until
the renew has been applied. The first test flushes the renew before releasing
the remover: {{retrievePassword}} is rejected about 1.2 s before the
acknowledged expiry, the DB row is gone and a restart does not help. The second
flushes it after the remover finished: the token is rejected the same way, the
DB row holds the new expiry and a restart brings the token back. A passing test
means the defect is present. Ratis and the double buffer are not running in the
test; their calls are made directly, and no multi OM cluster was run.
h3. Patch
[^MC-3-remove-only-checked-entry-in-expired-token-remover.patch], against
b94df213f20ec0d675f079b57a4566e4e64e4060. It also applies to master at
df42f7f60a2.
The remover now removes an expired entry with
{{currentTokens.computeIfPresent}}: if the map still holds the {{TokenInfo}}
whose renew date it checked, it deletes the DB row and removes the entry inside
that call, otherwise it keeps the entry. A renew applied after the check has
replaced the value, so the token and its row are left to the renew. A renew
applied while the remover is deleting the row waits for the map entry
({{updateRenewToken}} puts the same key), so its flush always lands after the
delete and the row is kept. Behaviour without a concurrent renew is unchanged.
The cost is one RocksDB delete per expired token done while holding that map
bin; reads of the map do not wait, and only writes to keys in the same bin wait
for it.
Covered by two tests in the existing {{TestOzoneDelegationTokenSecretManager}}:
{{testExpiredTokenRemoverKeepsConcurrentRenew}} (renew applied between the
check and the removal; also checks that an expired token that is not renewed is
still removed from memory and DB) and
{{testExpiredTokenRemoverKeepsRenewAppliedDuringRowDelete}} (renew applied and
flushed while the remover is inside its row delete). Without the change the
first fails with "can't be found in cache" and the second finds the row
deleted. With it {{TestOzoneDelegationTokenSecretManager}},
{{TestOMGetDelegationTokenRequest}}, {{TestOMGetDelegationTokenResponse}} and
{{TestS3SecurityUtil}} (38 tests) pass and checkstyle is clean. With the patch
both tests of the reproduction fail, because the token stays valid. The secure
mini cluster suites were not run.
The remover still deletes rows outside Ratis, so the OMs still remove expired
tokens at different times; moving that to the leader through Ratis is
HDDS-13457 and is not needed for this fix.
Found by TLA+ model checking and code review of the OM delegation token secret
manager and request paths under HDDS-15926, on commit
b94df213f20ec0d675f079b57a4566e4e64e4060. Checked against HDDS issues and
apache/ozone pull requests for duplicates before filing (HDDS-13457, and its
duplicate HDDS-13450, cover the uncoordinated removal, not a lost renew). 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]
