[
https://issues.apache.org/jira/browse/HDDS-17063?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17063:
------------------------------
Attachment: TestBugMC3LateIcrAfterVolumeFailure.java
> ICR sent after the volume failure full container report re-adds the lost
> replica in Recon and SCM
> -------------------------------------------------------------------------------------------------
>
> Key: HDDS-17063
> URL: https://issues.apache.org/jira/browse/HDDS-17063
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Minor
> Attachments: MC-3-no-icr-for-container-on-failed-volume.patch,
> TestBugMC3LateIcrAfterVolumeFailure.java
>
>
> h3. Mechanism
> When a volume fails, {{ContainerSet.handleVolumeFailures}} removes its
> containers, then {{StateContext.getFullContainerReportDiscardPendingICR}}
> drops the queued ICRs and builds a full container report (FCR) without them.
> A thread that already holds one of these containers past its lock section,
> such as {{KeyValueHandler.closeContainer}} after the container is closed, or
> {{quasiCloseContainer}} and {{markContainerForClose}}, can still call the ICR
> sender afterwards. The sender builds the ICR from the container object
> without checking the volume, and it is queued after the FCR. The next
> heartbeat carries the FCR without the container followed by the ICR, so the
> receiver removes the replica and then adds it back.
> The drop of queued ICRs is meant to prevent this ("Block ICRs from being
> generated"), but it synchronizes on the {{OzoneContainer}} object while the
> ICR sender synchronizes on the {{ContainerSet}}, so they do not exclude each
> other. Only the FCR build in {{ContainerSet.getContainerReport}} takes the
> container set monitor, and any ICR still queued at that point is sent after
> the FCR anyway.
> h3. Trigger
> # SCM asks the datanode to close a container; {{markContainerForClose}} moves
> it to CLOSING.
> # {{closeContainer}} closes it and releases the container lock, and is about
> to send the ICR.
> # The volume holding it fails its health check; {{handleVolumeFailures}}
> removes the container and queues an FCR without it.
> # {{closeContainer}} sends the CLOSED ICR.
> No fault other than the disk failure is needed; the window is a few
> statements wide.
> h3. Impact
> * Recon lists a CLOSED replica on the datanode that no longer holds the
> container (run). The next periodic FCR from that datanode removes it, 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 ICR with the same handler code, so it counts the lost
> replica for the same time and does not re-replicate that container until then
> (argued from the code, not run).
> * One container per occurrence.
> h3. Reproduction
> PASS, deterministic, unmodified source.
> [^TestBugMC3LateIcrAfterVolumeFailure.java] ({{hadoop-ozone/recon}}) uses the
> real datanode {{OzoneContainer}}, {{ContainerController}},
> {{KeyValueHandler}}, {{MutableVolumeSet}}, {{StorageVolumeChecker}} and
> {{StateContext}}. The volume fails for real: the test removes the permissions
> of its directory and the volume checker fails it. The only timing aid is a
> log4j appender on the {{ContainerLog}} logger that parks the close thread at
> {{ContainerLogger.logClosed}}, after the lock is released and right before
> the ICR is sent, until the volume failure has been handled. The reports are
> applied in heartbeat order to the real Recon FCR and ICR handlers. The
> heartbeat carries the FCR without the container followed by its CLOSED ICR,
> and Recon ends with a CLOSED replica that the datanode does not have. It must
> run as a user that file permissions apply to (not root). A passing test means
> the defect is present.
> h3. Patch
> [^MC-3-no-icr-for-container-on-failed-volume.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.
> The ICR sender in {{OzoneContainer}} sends nothing for a container whose
> volume has failed (the rule {{KeyValueHandler.markContainerUnhealthy}}
> already follows: "The container has already been discarded and SCM
> notified"), and {{getFullContainerReportDiscardPendingICR}} now synchronizes
> on the container set, the monitor the ICR sender holds. The volume is marked
> failed before {{handleVolumeFailures}} runs, so either the sender sees the
> failed volume, or it queues its ICR before the drop starts and the drop
> removes it. Checking instead whether the container is still in the container
> set would suppress the DELETED ICR, which is sent after the container is
> removed from the set, and would not be ordered against the drop. The volume
> check can suppress a DELETED ICR only when the volume fails between the
> delete's own failed volume check and its ICR; if no other container is left
> on a failed volume, no FCR follows either, and SCM keeps the replica (and
> resends the delete) until the next periodic FCR (argued, not run). A
> container added to a volume after it failed (HDDS-17018) sends no ICRs with
> this patch, but is still in the FCR until that issue is fixed.
> Covered by two tests in existing classes.
> {{TestStateContext.testFullContainerReportDropsIcrQueuedWhileContainerSetLocked}}
> queues an ICR while holding the container set monitor and the FCR build
> waits on it; without the change the ICR survives the build.
> {{TestOzoneContainer.testNoIncrementalReportForContainerOnFailedVolume}}
> fails the volume of a real container and then quasi closes it; without the
> change a second ICR is sent. With the change the container service suites
> around reports, volumes, close and delete commands and the container handlers
> (365 tests in 13 classes), {{TestDatanodeHddsVolumeFailureDetection}} and
> {{TestDatanodeHddsVolumeFailureToleration}} on a mini cluster, and
> {{TestEndPoint}} in server-scm pass, checkstyle is clean. With the patch the
> reproduction fails, because the heartbeat carries only the FCR.
> 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]
