[ 
https://issues.apache.org/jira/browse/HDDS-17215?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Siyao Meng updated HDDS-17215:
------------------------------
    Affects Version/s: 2.2.1
                       2.0.0
                       1.4.0
                       2.1.2
                       2.1.0
                       1.4.1
                       2.1.1
                       2.2.0

> OMs call a shared external S3 secret store while applying, so a late OM can 
> diverge from the leader and a store failure terminates the OM
> -----------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: HDDS-17215
>                 URL: https://issues.apache.org/jira/browse/HDDS-17215
>             Project: Apache Ozone
>          Issue Type: Bug
>    Affects Versions: 1.4.0, 2.0.0, 1.4.1, 2.1.0, 2.2.0, 2.1.1, 2.1.2, 2.2.1
>            Reporter: Siyao Meng
>            Priority: Major
>         Attachments: TestBugMC6LaggingOmSharedSecretStore.java
>
>
> h3. Mechanism
> With an S3 secret store without batch support, such as 
> {{VaultS3SecretStore}}, every OM applies every log entry and reads and writes 
> the store itself. {{S3GetSecretRequest}} writes the store while applying 
> since HDDS-10469; the patch on HDDS-17214 moves the flush time store updates 
> of revoke, set and the tenant access ID requests to apply time as well. The 
> documented Vault configuration has one address and one secret path, so in OM 
> HA all OMs share one store. The checks that decide a request's outcome read 
> that store on a cache miss: {{S3SecretManager.hasS3Secret}} in 
> {{OMSetSecretRequest}} and {{S3RevokeSecretRequest}}, and 
> {{S3SecretManager.getSecret}} in {{S3GetSecretRequest}} and 
> {{OMTenantAssignUserAccessIdRequest}}. The store already reflects entries 
> that other OMs applied, and after a crash it also reflects entries this OM 
> applied but did not flush. So an OM that applies an entry late (a lagging 
> follower, or a single OM replaying after a crash) evaluates it against later 
> state, can take a different branch than the leader took, and writes values 
> the leader never wrote. A store failure while applying is fatal, which is the 
> second symptom below.
> h3. Symptom 1: an OM that applies late diverges from the leader
> * *A tenant access ID revoke terminates the OM, again after every restart.* A 
> tenant admin assigns an access ID (index 2), revokes it (index 3) and assigns 
> it again (index 4). An OM that applies index 2 after the store holds the 
> secret of index 4 rejects the assign with 
> TENANT_USER_ACCESS_ID_ALREADY_EXISTS, so its tenant tables lack the access 
> ID, and the rejected response is flushed. Applying index 3 then fails on 
> {{Objects.requireNonNull(omDBAccessIdInfo)}} in 
> {{OMTenantRevokeUserAccessIdRequest.validateAndUpdateCache}}. The 
> NullPointerException reaches the {{catch (Throwable)}} in 
> {{OzoneManagerStateMachine.runCommand}}, which terminates the OM. Index 3 was 
> never flushed, so every restart applies it again and terminates again, until 
> the OM is rebuilt from another OM's checkpoint (the restart loop is argued 
> from the code). A follower that is down or behind while the access ID is 
> revoked and assigned again is enough. With the patch on HDDS-17214, a single 
> OM that crashes with indexes 2 to 4 applied but not flushed and replays them 
> hits the same; without that patch a single OM hits it only when it crashes 
> inside a flush, between the store calls and the RocksDB commit. Without that 
> patch a follower also rejects every tenant assign it applies after the leader 
> flushed it, with no revoke needed; that patch fixes this case.
> * *A set that the leader rejected overwrites the issued secret.* Two callers 
> act on one access ID: a revoke (index 2) and a set (index 3) both pass 
> {{preExecute}} while the secret exists. The leader applies the revoke and 
> rejects the set with ACCESS_ID_NOT_FOUND. A getsecret (index 4) then writes a 
> new secret and returns it to the client. An OM that applies index 3 after the 
> store holds that secret accepts the set and overwrites it. A single OM is 
> enough: it crashes with indexes 3 and 4 applied but not flushed and replays 
> them. A follower that restarts after index 2 and applies index 3 after the 
> leader applied index 4 does the same. The store then holds the value of a set 
> the leader rejected, S3 requests signed with the issued secret fail on every 
> OM, and nothing reconciles the store with the log. Fail closed; a revoke and 
> a new getsecret recover.
> h3. Symptom 2: a store failure while applying terminates the OM
> The store reports a failure as an {{IOException}}. 
> {{OzoneManagerRatisUtils.exceptionToResponseStatus}} maps an {{IOException}} 
> that is not an {{OMException}} to INTERNAL_ERROR, and 
> {{OzoneManagerStateMachine.processResponse}} terminates the OM on 
> INTERNAL_ERROR. This applies to the getsecret store write and to every store 
> read on a cache miss, and with the patch on HDDS-17214 also to the store 
> updates of revoke, set and the tenant access ID requests (without that patch 
> these terminate the OM at flush time instead, which that issue covers). The 
> comment in {{S3GetSecretRequest}} says that the request will fail; instead 
> the OM stops, for all clients, not only S3. 
> {{RequestHandler.handleWriteRequest}} adds the error response to the double 
> buffer before {{processResponse}} terminates, so the response may be flushed 
> first. A restart then does not replay the entry, and in OM HA the failing OM 
> has skipped an entry that the other OMs applied, which is symptom 1 again.
> h3. Affected versions
> The per OM store calls exist since the external store was added in 1.4.0 
> (HDDS-7814). The apply time getsecret write, which the set overwrite and the 
> getsecret part of symptom 2 depend on, exists since 2.0.0 (HDDS-10469).
> h3. Reproduction
> [^TestBugMC6LaggingOmSharedSecretStore.java] 
> ({{hadoop-ozone/ozone-manager}}): OMs in one JVM, each with its own metadata 
> DB, S3 secret manager and cache, and a running double buffer, sharing one in 
> memory store without batch support. Requests go through the leader's 
> {{preExecute}} and every OM's {{validateAndUpdateCache}}; a restart reopens 
> the OM's DB and resumes after its persisted transaction info, and a crash 
> before the flush is modelled by not handing the responses to the double 
> buffer. Ratis is not run, Ranger calls are mocked and no Vault server was 
> used. All three tests fail on df04bc0e309f625a7dcdcd60790a42f34e2743d9 and 
> with the patch on HDDS-17214:
> * {{laggingFollowerAppliesTenantAssignAfterReassign}}: 
> {{NullPointerException: omDBAccessIdInfo == null}} when the follower applies 
> the tenant revoke.
> * {{replayedSetAppliesAgainstLaterStoreState}} (one OM) and 
> {{restartedFollowerAppliesSetAgainstLaterStoreState}} (two OMs): the store 
> holds {{set-by-another-caller-0123456789}} instead of the issued secret.
> Symptom 2 is argued from the code above; no reproduction is attached for it.
> h3. Suggested fix
> The store is shared state that every OM updates on its own while applying, so 
> decisions made at apply time are not deterministic across OMs and replays, 
> and a store failure on one OM cannot simply become a non fatal result: OMs 
> applying the same entry would then disagree. A fix needs a design decision, 
> for example calling the external store once on the leader before the entry is 
> replicated (as S3 multi tenancy does with Ranger in {{preExecute}}, and as 
> suggested in the review of HDDS-10469) and only updating caches when 
> applying. A leader side call can leave the store changed for an entry that 
> never commits (a leader change, or an entry rejected when applied), so it 
> needs reconciliation, as {{OMRangerBGSyncService}} does for Ranger, or an 
> apply step that checks that the store holds the entry's value. Storing the 
> log index with each secret and refusing to overwrite a newer one would cover 
> only the overwrite, not the diverging decisions such as the rejected tenant 
> assign. Such a design would also replace the apply time store calls that the 
> patch on HDDS-17214 adds. No patch is attached.
> Found by TLA+ model checking and code review of the OM S3 secret cache and 
> external S3 secret store paths under HDDS-15926, on commit 
> df04bc0e309f625a7dcdcd60790a42f34e2743d9. The S3 secret request, response, 
> cache and state machine code involved is unchanged on master at 2cb1b6dd4e9. 
> Checked against HDDS issues and apache/ozone pull requests for duplicates 
> before filing. Related but not duplicates: HDDS-10469 (apply time store write 
> for getsecret; it aimed to keep the OM working when the store is unavailable, 
> which this path does not), HDDS-7814 (remote S3 secret store) and HDDS-1989 
> (terminate on INTERNAL_ERROR). 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]

Reply via email to