[
https://issues.apache.org/jira/browse/HDDS-17175?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17175:
------------------------------
Attachment: MC-3-remove-only-checked-entry-in-expired-token-remover.patch
> 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
> Priority: Minor
> Attachments:
> MC-3-remove-only-checked-entry-in-expired-token-remover.patch,
> TestBugMC3RemoverDropsConcurrentRenew.java
>
>
> 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]
