You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "Siyao Meng (Jira)" <ji...@apache.org> on 2026/09/16 03:13:00 UTC

[jira] [Updated] (HDDS-16445) SCM over replication split lock read can delete a live replica and drop a CLOSED container below RF

     [ https://issues.apache.org/jira/browse/HDDS-16445?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Siyao Meng updated HDDS-16445:
------------------------------
    Description: 
h3. Finding
A split-lock read in processOverReplicatedContainer issues an extra delete against a live replica, dropping a CLOSED RF=3 container below RF (DurableReplicationFloor safety violation). Reachable from normal concurrent over-replication processing racing a container-report handler; harm is bounded because a later cycle re-replicates, so High rather than Critical.

h3. Classification
* Verdict: REPRODUCED
* Severity: High
* Source: Specula TLA+ model checking and confirmation debate, finding MC-1

h3. Reproduce
{noformat}
Ozone commit: 9fbf9ee0cb1bd2f5f5d437b6719ebbe5309351fb
Specula:      v1.1.0 (commit c6aa3dfa)
Target:       scm-ratis-replication
Guidance:     campaigns/ozone-9fbf9ee/targets/012-scm-ratis-replication/.prompt-extra.md
{noformat}
{code:none}
specula run --agent=claude-code --effort=medium --keep-original --max-parallel=2 \
  --enable-reviews --confirm-debate --tlc-memory-limit=28G --tlc-worker-limit=8 \
  "scm-ratis-replication|apache/ozone|Java|Use the target-specific .prompt-extra.md"
{code}
Discovered under HDDS-16431 (Specula TLA+ verification effort). The TLA+ specification, counterexample, and confirmation debate live in the Specula run artifacts.

Generated with Specula (Claude Opus 4.8).

h3. Steps to reproduce

Precondition: a CLOSED Ratis container (RF=3) with 4 healthy replicas (legitimately over-replicated by 1). A delete for one replica is already in flight and tracked as a pending DELETE op.
Interleaving (two SCM threads, the ReplicationManager over-replication processor and the container-report / ICR handler):
1. ReplicationManager.processOverReplicatedContainer reads the replica set: 4 replicas.
2. The in-flight delete is confirmed: ContainerStateManagerImpl.removeContainerReplica removes the replica AND completes its pending DELETE op, atomically under the container-state-manager lock.
3. processOverReplicatedContainer then reads the pending ops: the delete is gone (empty list).
Observed wrong outcome: the handler sees 4 replicas and 0 pending deletes, computes an excess of 1, and issues an extra delete against a still-live replica. Only 3 replicas physically remain, so applying that command drops the container to 2 replicas, below the replication factor (a DurableReplicationFloor safety violation). A later replication cycle re-replicates, so the harm is bounded but real.

h3. Root cause

processOverReplicatedContainer (hadoop-hdds/server-scm/.../ReplicationManager.java) reads the replica set (guarded by the container-state-manager striped lock) and the pending ops (guarded by the separate ContainerReplicaPendingOps striped lock) with no enclosing lock and no re-validation. A confirmed delete removes the replica and completes its pending DELETE op atomically (ContainerStateManagerImpl.removeContainerReplica calls completeDeleteReplica while holding the container-state-manager write lock), but that atomic unit can land in the gap between the two unsynchronized reads. The resulting view still counts the removed replica while its in-flight delete has vanished from the pending set, so RatisContainerReplicaCount.getExcessRedundancy is inflated by one and an extra delete is issued.

h3. Suggested fix

The attached patch [^HDDS-16445.001.patch] reads the pending ops before the replica set in processOverReplicatedContainer. Because a confirmed delete removes the replica and its pending DELETE op together, reading pending ops first guarantees that if such a delete lands between the two reads the view is either fully consistent or conservative (a still-pending delete for a replica that has already been removed), never the dangerous combination of a stale replica counted while its in-flight delete has disappeared. In the conservative case the still-tracked delete keeps the effective count at the replication factor, so no extra delete is scheduled; a later cycle removes any genuine excess. A test is added to TestReplicationManager that models the confirmed delete landing at the replica read and asserts no extra delete command is issued (it fails before this change, issuing one over-delete). A fully atomic snapshot of both structures under a single lock is a heavier alternative that would change the ContainerManager read interface.

Patch is an AI-drafted proposal (Specula + Claude); pending human build, full test, and review before merge.


  was:
h3. Finding
A split-lock read in processOverReplicatedContainer issues an extra delete against a live replica, dropping a CLOSED RF=3 container below RF (DurableReplicationFloor safety violation). Reachable from normal concurrent over-replication processing racing a container-report handler; harm is bounded because a later cycle re-replicates, so High rather than Critical.

h3. Classification
* Verdict: REPRODUCED
* Severity: High
* Source: Specula TLA+ model checking and confirmation debate, finding MC-1

h3. Reproduce
{noformat}
Ozone commit: 9fbf9ee0cb1bd2f5f5d437b6719ebbe5309351fb
Specula:      v1.1.0 (commit c6aa3dfa)
Target:       scm-ratis-replication
Guidance:     campaigns/ozone-9fbf9ee/targets/012-scm-ratis-replication/.prompt-extra.md
{noformat}
{code:none}
specula run --agent=claude-code --effort=medium --keep-original --max-parallel=2 \
  --enable-reviews --confirm-debate --tlc-memory-limit=28G --tlc-worker-limit=8 \
  "scm-ratis-replication|apache/ozone|Java|Use the target-specific .prompt-extra.md"
{code}
Discovered under HDDS-16431 (Specula TLA+ verification effort). The TLA+ specification, counterexample, and confirmation debate live in the Specula run artifacts.

Generated with Specula (Claude Opus 4.8).


> SCM over replication split lock read can delete a live replica and drop a CLOSED container below RF
> ---------------------------------------------------------------------------------------------------
>
>                 Key: HDDS-16445
>                 URL: https://issues.apache.org/jira/browse/HDDS-16445
>             Project: Apache Ozone
>          Issue Type: Bug
>            Reporter: Siyao Meng
>            Priority: Major
>         Attachments: HDDS-16445.001.patch
>
>
> h3. Finding
> A split-lock read in processOverReplicatedContainer issues an extra delete against a live replica, dropping a CLOSED RF=3 container below RF (DurableReplicationFloor safety violation). Reachable from normal concurrent over-replication processing racing a container-report handler; harm is bounded because a later cycle re-replicates, so High rather than Critical.
> h3. Classification
> * Verdict: REPRODUCED
> * Severity: High
> * Source: Specula TLA+ model checking and confirmation debate, finding MC-1
> h3. Reproduce
> {noformat}
> Ozone commit: 9fbf9ee0cb1bd2f5f5d437b6719ebbe5309351fb
> Specula:      v1.1.0 (commit c6aa3dfa)
> Target:       scm-ratis-replication
> Guidance:     campaigns/ozone-9fbf9ee/targets/012-scm-ratis-replication/.prompt-extra.md
> {noformat}
> {code:none}
> specula run --agent=claude-code --effort=medium --keep-original --max-parallel=2 \
>   --enable-reviews --confirm-debate --tlc-memory-limit=28G --tlc-worker-limit=8 \
>   "scm-ratis-replication|apache/ozone|Java|Use the target-specific .prompt-extra.md"
> {code}
> Discovered under HDDS-16431 (Specula TLA+ verification effort). The TLA+ specification, counterexample, and confirmation debate live in the Specula run artifacts.
> Generated with Specula (Claude Opus 4.8).
> h3. Steps to reproduce
> Precondition: a CLOSED Ratis container (RF=3) with 4 healthy replicas (legitimately over-replicated by 1). A delete for one replica is already in flight and tracked as a pending DELETE op.
> Interleaving (two SCM threads, the ReplicationManager over-replication processor and the container-report / ICR handler):
> 1. ReplicationManager.processOverReplicatedContainer reads the replica set: 4 replicas.
> 2. The in-flight delete is confirmed: ContainerStateManagerImpl.removeContainerReplica removes the replica AND completes its pending DELETE op, atomically under the container-state-manager lock.
> 3. processOverReplicatedContainer then reads the pending ops: the delete is gone (empty list).
> Observed wrong outcome: the handler sees 4 replicas and 0 pending deletes, computes an excess of 1, and issues an extra delete against a still-live replica. Only 3 replicas physically remain, so applying that command drops the container to 2 replicas, below the replication factor (a DurableReplicationFloor safety violation). A later replication cycle re-replicates, so the harm is bounded but real.
> h3. Root cause
> processOverReplicatedContainer (hadoop-hdds/server-scm/.../ReplicationManager.java) reads the replica set (guarded by the container-state-manager striped lock) and the pending ops (guarded by the separate ContainerReplicaPendingOps striped lock) with no enclosing lock and no re-validation. A confirmed delete removes the replica and completes its pending DELETE op atomically (ContainerStateManagerImpl.removeContainerReplica calls completeDeleteReplica while holding the container-state-manager write lock), but that atomic unit can land in the gap between the two unsynchronized reads. The resulting view still counts the removed replica while its in-flight delete has vanished from the pending set, so RatisContainerReplicaCount.getExcessRedundancy is inflated by one and an extra delete is issued.
> h3. Suggested fix
> The attached patch [^HDDS-16445.001.patch] reads the pending ops before the replica set in processOverReplicatedContainer. Because a confirmed delete removes the replica and its pending DELETE op together, reading pending ops first guarantees that if such a delete lands between the two reads the view is either fully consistent or conservative (a still-pending delete for a replica that has already been removed), never the dangerous combination of a stale replica counted while its in-flight delete has disappeared. In the conservative case the still-tracked delete keeps the effective count at the replication factor, so no extra delete is scheduled; a later cycle removes any genuine excess. A test is added to TestReplicationManager that models the confirmed delete landing at the replica read and asserts no extra delete command is issued (it fails before this change, issuing one over-delete). A fully atomic snapshot of both structures under a single lock is a heavier alternative that would change the ContainerManager read interface.
> Patch is an AI-drafted proposal (Specula + Claude); pending human build, full test, and review before merge.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org