Siyao Meng created HDDS-17062:
---------------------------------
Summary: ICRs put back after a failed datanode heartbeat are sent
after the newer full container report that replaced them
Key: HDDS-17062
URL: https://issues.apache.org/jira/browse/HDDS-17062
Project: Apache Ozone
Issue Type: Bug
Reporter: Siyao Meng
Attachments:
MC-1-drop-put-back-icrs-covered-by-newer-full-report.patch,
TestBugMC1StalePutBackIcr.java
h3. Mechanism
{{StateContext.getFullContainerReportDiscardPendingICR}} drops the ICRs still
queued for each endpoint before it builds a new full container report (FCR), so
that no ICR older than the FCR is sent after it (HDDS-5267). It cannot see the
ICRs that a heartbeat in flight has already taken from the queue. If that
heartbeat fails with an {{IOException}},
{{HeartbeatEndpointTask.putBackIncrementalReports}} returns those ICRs to the
head of the queue through {{StateContext.putBackReports}}. The next heartbeat
then carries the new FCR followed by the older ICRs, and Recon and SCM apply
the ICRs on top of the FCR.
h3. Trigger
# A container changes state on the datanode and its ICR is queued (for example
the container is marked UNHEALTHY, or deleted).
# A heartbeat takes the ICR and its RPC is in flight.
# While the RPC is in flight, the datanode builds a new FCR (the periodic
container report, or a volume failure) from a container state that already
includes later changes (the container was then deleted, or recreated by
replication).
# The RPC fails (an RPC timeout or a short network failure) and the ICR is put
back.
# The next heartbeat sends the FCR, then the stale ICR.
At Recon, an outage of 3 heartbeat intervals or more (180s by default) makes
Recon ask the datanode to re-register, and the register request carries a fresh
container report, which repairs the state seconds later. So at Recon the
trigger is a single failed heartbeat. SCM has no such gap check (read, not run).
h3. Impact
* A stale UNHEALTHY or CLOSED ICR re-adds a replica that the FCR has just
removed (the datanode no longer has the container), and a stale DELETED ICR
removes a replica that the FCR has just reported (the datanode has it again).
Both were run against Recon.
* The wrong replica stays until the next FCR from that datanode, which comes 60
to 120 minutes after the previous one ({{hdds.container.report.interval}} plus
a random delay of up to the same interval).
* SCM processes the same reports with the same handler code, so the same
phantom or missing replica appears there for the same time. That can hold a
DELETING container in DELETING, or skew the replica counts used by
ReplicationManager (argued from the code, not run). I found no realistic
sequence in which this makes SCM move a DELETING or DELETED container back: SCM
is in DELETING only after it took this datanode's replica as empty or gone,
which only the FCR in the same heartbeat can tell it, so ReplicationManager
would have to run between the FCR and the stale ICR handlers (argued from the
code, not run).
h3. Reproduction
PASS, deterministic, unmodified source. [^TestBugMC1StalePutBackIcr.java]
({{hadoop-ozone/recon}}) runs the real datanode {{StateContext}},
{{HeartbeatEndpointTask}} and {{ContainerSet}} against a stub RPC proxy that
feeds the real {{SCMDatanodeHeartbeatDispatcher}},
{{ReconContainerReportQueue}} and Recon FCR and ICR handlers. The stub fails
one heartbeat with a {{SocketTimeoutException}}, and runs the concurrent
datanode work (container state change and FCR build) on a separate thread while
that RPC is in flight. The first test shows Recon keeping an UNHEALTHY replica
of a container the datanode no longer has, across further heartbeats, until the
next periodic FCR. The second shows Recon dropping a CLOSED replica that the
datanode still holds. A passing test means the defect is present.
h3. Patch
[^MC-1-drop-put-back-icrs-covered-by-newer-full-report.patch], against
916aa1d9949cbb10f2750a40c8704340893131a2. It also applies to master at
62d982ff2d9. The open pull request for HDDS-16723 (apache/ozone#11423) also
changes {{StateContext}}; the main code of this patch applies cleanly on its
head commit.
{{StateContext}} remembers, under the lock of the incremental report queue, the
endpoints whose ICRs taken by the heartbeat in flight are older than the latest
FCR: {{getFullContainerReportDiscardPendingICR}} marks every endpoint when it
drops the queued ICRs, {{getIncrementalReports}} clears the mark when a
heartbeat takes the endpoint's ICRs, and {{putBackReports}} leaves out ICRs
(but still puts back command status reports) while the endpoint is marked.
Heartbeats to one endpoint are serialized, so the mark means that an FCR was
built after the failed heartbeat took its ICRs, and that FCR already covers
them. Nothing changes when no FCR is built while a heartbeat fails. Checking
the FCR ready flag instead would be racy, because {{refreshFullReport}} sets it
after the drop, outside that lock.
Covered by
{{testFailedHeartbeatDropsIncrementalReportsCoveredByNewerFullReport}} in the
existing {{TestHeartbeatEndpointTask}}: two failed heartbeats, the second with
an FCR rebuild and a newer ICR during the RPC; the third heartbeat must carry
the FCR, only the newer ICR, and the command status report. Without the change
it fails because the older ICR is also sent. With it
{{TestHeartbeatEndpointTask}}, {{TestStateContext}}, {{TestContainerSet}},
{{TestReportPublisher}}, {{TestVolumeSetDiskChecks}}, {{TestOzoneContainer}},
{{TestDatanodeStateMachine}}, {{TestRunningDatanodeState}},
{{TestHddsDispatcher}} and {{TestKeyValueHandler}} (165 tests) and
{{TestEndPoint}} in server-scm (18 tests) pass, checkstyle is clean. With the
patch both tests of the reproduction fail, because the stale ICR is no longer
sent.
Found by TLA+ model checking and code review of the datanode container report
and Recon container tracking paths under HDDS-15926, on commit
916aa1d9949cbb10f2750a40c8704340893131a2. Checked against HDDS issues and
apache/ozone pull requests for duplicates before filing. 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]
