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