[
https://issues.apache.org/jira/browse/HDDS-16744?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16744:
------------------------------
Attachment: MC-4-wait-for-earlier-attempt-before-retry-cache-check.patch
> A client retry that arrives before the first attempt reaches Ratis runs
> preExecute twice and leaks inFlightSnapshotCount
> ------------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-16744
> URL: https://issues.apache.org/jira/browse/HDDS-16744
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Major
> Attachments:
> MC-4-wait-for-earlier-attempt-before-retry-cache-check.patch
>
>
> h3. Mechanism
> The retry cache check, {{preExecute}} and the Ratis submit are three separate
> steps with nothing tying them together.
> {{OzoneManagerProtocolServerSideTranslatorPB.java:204}} calls
> {{checkRetryCache()}}, which is a read only {{getIfPresent}}
> ({{OzoneManagerRatisServer.java:548}}) and creates nothing. The record it
> looks for is created by Ratis only when it accepts the request, inside
> {{submitClientRequestAsync}} ({{OzoneManagerRatisServer.java:319}}).
> {{OMExecutionFlow}} reaches that at {{:80}}, after it has already run
> {{preExecute}} at {{:67}}.
> A second attempt of the same call, carrying the same {{(ClientId, callId)}},
> that arrives while the first is still in {{preExecute}} therefore misses the
> retry cache, runs {{preExecute}} again, and is only then absorbed by Ratis as
> a duplicate. At most once apply still holds, but whatever {{preExecute}} did
> outside the Raft log has now been done twice.
> For snapshot create that side effect is a counter.
> {{OMSnapshotCreateRequest.java:125}} calls {{snapshotLimitCheck()}}, which
> increments {{inFlightSnapshotCount}} ({{OmSnapshotManager.java:991}}). The
> only decrement is in {{validateAndUpdateCache}}
> ({{OMSnapshotCreateRequest.java:202}}). The absorbed duplicate never applies
> and its reply reports success, so {{handleRequestFailure}} is not reached
> either, and the second increment is never taken back.
> h3. Trigger
> An ordinary client retry after an RPC timeout or a connection loss, while the
> first attempt is still being served. Hadoop RPC reuses the call ID across
> retries, which is what makes the retry cache work at all. The window is the
> whole of the first attempt's {{preExecute}}: RocksDB reads, bucket link
> resolution and ACL checks for snapshot create, and a synchronous SCM
> {{allocateBlock}} RPC for key create and allocate block.
> h3. Impact
> {{inFlightSnapshotCount}} leaks by one per occurrence, and
> {{snapshotLimitCheck()}} counts it against
> {{ozone.om.fs.snapshot.max.limit}}, so {{createSnapshot}} starts failing with
> {{TOO_MANY_SNAPSHOTS}} below the configured limit. Reproduced with the limit
> set to 8: the eighth create was refused with "Current snapshots: 7, In-flight
> creations: 1" and nothing in flight.
> The leak stays until a leadership change, since {{notifyLeaderReady}}
> ({{OzoneManagerStateMachine.java:199}}) is the only thing that resets the
> counter.
> Only the snapshot counter consequence was demonstrated. The same ordering
> applies to every other {{preExecute}} side effect, such as a second SCM block
> allocation, but those were not exercised.
> h3. Reproduction
> PASS. Two attempts of one {{CreateSnapshot}} call with the same client ID and
> call ID, released together from a latch against a mini cluster, with no
> product code modified. The duplicate landed in the window on the first round
> in both runs. A sequential retry of the same call is absorbed by the retry
> cache as designed, which is the control. Artifacts under
> {{confirmation/MC-4/}} and {{repro/}}, driven from the counterexample
> {{spec/output/hunt_s1_bfs2.out}} against invariant {{NoOrphanedSideEffect}}.
> h3. Suggested fix
> Close the window instead of compensating for each side effect: make a later
> attempt of a call wait for an earlier attempt that is past the retry cache
> check, then check the retry cache itself. By then the earlier attempt's
> record exists, so the later one is answered from the cache and never reaches
> {{preExecute}}.
> h3. Patch
> [^MC-4-wait-for-earlier-attempt-before-retry-cache-check.patch], against
> 20e6a1c0f6039248373efd3eb1d7f2afcf7f1535.
> {{OzoneManagerRatisServer}} gains {{startAttempt()}}, which registers the
> current call's {{ClientInvocationId}} in a map and first waits for any
> attempt already registered under it.
> {{OzoneManagerProtocolServerSideTranslatorPB}} holds that registration across
> the existing retry cache check and the submit. An attempt with no concurrent
> duplicate pays one map insert and one remove. Calls on the gRPC transport
> carry a fresh client ID per request, so they never wait.
> Covered by {{testSnapshotInFlightCountWithConcurrentRetry}} in the existing
> {{TestOzoneManagerHASnapshot}}: ten rounds of two concurrent attempts of one
> {{CreateSnapshot}} call, asserting both succeed with the same snapshot ID and
> that the in flight count returns to 0. Without the change it fails with the
> count at 1. The OM Ratis server unit suites and the existing
> {{TestOzoneManagerHASnapshot}}, {{TestOzoneManagerHAWithAllRunning}} and
> {{TestOzoneManagerHAWithStoppedNodes}} pass. Checkstyle clean.
> Found by TLA+ model checking and code review of the OM client retry cache and
> at most once mutation paths under HDDS-15926, on commit
> 20e6a1c0f6039248373efd3eb1d7f2afcf7f1535. Checked against open HDDS issues
> for duplicates before filing, in addition to the repository's merged git
> history. Generated with Specula (Claude Opus 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]
