Siyao Meng created HDDS-17279:
---------------------------------
Summary: Adding or removing an SCM on a running datanode can make
report producers throw, losing reports and stopping command status reports
until restart
Key: HDDS-17279
URL: https://issues.apache.org/jira/browse/HDDS-17279
Project: Apache Ozone
Issue Type: Bug
Reporter: Siyao Meng
Attachments:
MC-7-synchronize-scm-endpoint-add-remove-with-report-fan-out.patch,
TestBugMC7AddEndpointRace.java
h3. Mechanism
{{StateContext}} keeps one queue per SCM endpoint for incremental reports,
container actions and pipeline actions, plus a plain {{HashSet}} of endpoints.
The producers ({{addIncrementalReport}}, {{addContainerAction}},
{{addContainerActionIfAbsent}}, {{addPipelineActionIfAbsent}}) iterate that set
and look up each endpoint's queue. {{StateContext.addEndpoint}} adds the
endpoint to the set before it creates the queues, and neither {{addEndpoint}}
nor {{removeEndpoint}} takes the locks the producers hold. A producer running
at the same time therefore gets {{null}} for the new (or just removed) endpoint
and throws a {{NullPointerException}}, or throws
{{ConcurrentModificationException}} from the set iterator. The endpoints after
it in the iteration miss that report or action. The queue size getters read by
{{DatanodeQueueMetrics}} iterate the same maps without the locks, so a metrics
read can also throw {{ConcurrentModificationException}}.
{{ReportPublisher.publishReport}} catches only {{IOException}}, and the
publishers run with {{scheduleAtFixedRate}}. When the exception is thrown
inside {{CommandStatusReportPublisher}}, the scheduled task ends and is never
rescheduled. The command statuses it had just removed from {{cmdStatusMap}}
reach no SCM, and no command status report is sent to any SCM until the
datanode restarts. {{refreshFullReport}} iterates the plain {{HashMap}}
{{isFullReportReadyToBeSent}} that {{addEndpoint}} inserts into, so the full
report publishers can end the same way (argued from the code, not run).
h3. Trigger
An Ozone administrator adds or removes an SCM with {{ozone admin reconfig}} of
{{ozone.scm.nodes.<service>}} on a running datanode
({{HddsDatanodeService.reconfigScmNodes}}, added by HDDS-13890), while another
thread is fanning out a report or action. Producers include container state
changes (ICRs), pipeline reports, container and pipeline actions, and the
periodic command status report (default every 60s). The window is the few
microseconds {{addEndpoint}} or {{removeEndpoint}} take, so a single
reconfiguration hits it rarely.
h3. Impact
* The existing SCMs miss the report or action that hit the window (for example
an ICR).
* If the command status report publisher hits it, command status reports stop
for the life of the process. They are how SCM learns that block deletion
commands completed, so SCM gets no more block deletion acknowledgements from
this datanode until it restarts, and completed command statuses accumulate in
{{cmdStatusMap}} (argued from the code, not run).
* If a full report publisher hits it, SCM gets no more refreshed node reports
(capacity and usage) or periodic full container reports, depending on the
publisher, from this datanode until it restarts (argued from the code, not run).
* A throw inside container request handling (the close container action that
{{HddsDispatcher}} adds for a write to a full or unhealthy container, or the
ICR sent when a write creates a container) fails that write and marks the Ratis
container state machine unhealthy, which closes the pipeline on this datanode.
The client retries on another pipeline (argued from the code, not run).
* No crash of the datanode process and no data loss.
h3. Reproduction
Deterministic, on unmodified source: [^TestBugMC7AddEndpointRace.java]
({{hadoop-hdds/container-service}}). It uses the public {{StateContext}} and
{{CommandStatusReportPublisher}} APIs as production calls them. The window is
held open by adding an endpoint whose {{hashCode}} parks the reconfiguration
thread on the first map update after the endpoint entered the set. The report
interval is lowered to 100 ms. Both tests assert the correct behaviour and fail
on the unmodified code: {{addIncrementalReport}} throws
{{NullPointerException}}, and the scheduled {{CommandStatusReportPublisher}}
task ends after its run. With the patch both pass.
h3. Patch
[^MC-7-synchronize-scm-endpoint-add-remove-with-report-fan-out.patch], against
b10a7d427bdf83807a7e48a1be507d3b02d5a503. It also applies to master at
e0de543cbe9.
Each producer now iterates the values of the per endpoint map it writes to,
under the lock that already guards that map, instead of iterating the separate
endpoint set. {{addEndpoint}} and {{removeEndpoint}} update those maps under
the same locks, and {{isFullReportReadyToBeSent}} becomes a
{{ConcurrentHashMap}}. The queue size getters used by {{DatanodeQueueMetrics}}
now read those maps under the same locks too. No new lock and no change in
behaviour when the SCM set does not change. {{DatanodeQueueMetrics}} has the
same pattern on its own maps (iterated in {{getMetrics}}, written by its
{{addEndpoint}} and {{removeEndpoint}}) and is not covered by this patch.
Covered by {{testEndpointChangeDuringReportFanOut}} in the existing
{{TestStateContext}}: the test thread fans out reports and actions and reads
the queue sizes for 10000 rounds while another thread keeps adding and removing
an endpoint, then checks that the existing endpoint got every report and
action. Without the change it failed in 4 of 4 runs with a
{{ConcurrentModificationException}} (a {{NullPointerException}} is also
possible); with only the getters left unlocked it failed in 3 of 3 runs in a
getter; with the patch it passes in about 3s. {{TestStateContext}},
{{TestHddsDispatcher}}, {{TestHeartbeatEndpointTask}}, {{TestReportPublisher}},
{{TestReportManager}}, {{TestRunningDatanodeState}} and the mini cluster test
{{TestDatanodeSCMNodesReconfiguration}} pass, checkstyle is clean.
Found by TLA+ model checking and code review of the datanode registration and
heartbeat paths under HDDS-15926, on commit
b10a7d427bdf83807a7e48a1be507d3b02d5a503. Checked against HDDS issues and
apache/ozone pull requests for duplicates before filing. Related but not
duplicates: HDDS-13890 (added the runtime SCM add and remove) and HDDS-15369
(datanode shutdown on {{ozone.scm.nodes}} reconfiguration). 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]
