[
https://issues.apache.org/jira/browse/HDDS-15066?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18075055#comment-18075055
]
Gargi Jaiswal commented on HDDS-15066:
--------------------------------------
h2. Race Condition Timeline For RM writeLock and Balancer readLock
*Container C* is CLOSED / QUASI_CLOSED. It lives on *disk-A* (source volume).
DiskBalancer wants to move it to *disk-B* (dest volume). *RM* wants to delete
it because it is *over-replicated.*
**
|*Time*|*DiskBalancer (background thread)*|*RM Delete Command (handler
thread)*|*Shared State*|
|1|getContainer(C) → gets disk-A ref|—|map: C → disk-A|
|2|State check: CLOSED / QUASI_CLOSED ✓|—|map: C → disk-A|
|3|disk-A.readLock() acquired|—|map: C → disk-A|
|4|Copying disk-A → destVolume/tmp/C/ ...|—|map: C → disk-A|
|5|Copy still in progress|getContainer(C) → returns disk-A ref ← which is STALE
REF CAPTURED|map: C → disk-A|
|6|Copy still in progress|deleteInternal(disk-A) calls disk-A.writeLock() →
BLOCKED (DiskBalancer holds readLock)|map: C → disk-A|
|7|Verify checksum on tmp copy|(blocked)|map: C → disk-A|
|8|ATOMIC_MOVE: tmp/C/ → destVolume/C/|(blocked)|map: C → disk-A|
|9|importContainer() → creates disk-B object|(blocked)|map: C → disk-A|
|10|containerSet.updateContainer(disk-B) ← MAP SWAPPED|(blocked)|map: C →
disk-B|
|11|destVolume.incrementUsedSpace(size)|(blocked)|map: C → disk-B|
|12|disk-A.readUnlock() ← LOCK RELEASED|(blocked)|map: C → disk-B|
|13|disk-A.markContainerForDelete() — writes DELETED state to disk-A .container
file and balancer deletes after 5min this old container.|UNBLOCKED → acquires
writeLock on disk-A|map: C → disk-B|
|14|—|disk-A.markContainerForDelete() → idempotent, already DELETED, no-op|map:
C → disk-B|
|15|—|containerSet.removeContainer(C) → map has C → disk-B → removes disk-B
from map ← BUG|map: C → (empty)|
|16|—|KeyValueContainerUtil.removeContainer(disk-A.data) → moves disk-A files
to deleted/ folder|map: C → (empty)|
|17|—|disk-A.writeUnlock()|map: C → (empty)|
|18|—|sendICR(disk-A) → SCM told "container C deleted" using disk-A's info|map:
C → (empty)|
|19|—|disk-A.delete() → disk-A physically removed from disk;
sourceVolume.decrementUsedSpace(size)|map: C → (empty)|
|20|finally: pendingDeletionContainers.put(disk-A, ttl) → disk-A is already
gone from disk!|—|map: C → (empty)|
h2. Final State After the Race
|*What*|*Expected*|*Actual*|*Impact*|
|containerMap entry for C|Gone (correct)|Gone|Appears correct but for the wrong
reason. Container C entry should be gone when the newly moved container is
deleted by RM after lock release but C lies orphaned at new disk until the dn
restarts and detects this.|
|disk-A files|Removed|Removed ✓|Correct — but this was the source, not dest.
Disk-A files should be gone due to cleanup by diskBalancer but actually it is
gone due to RM having stale reference to old container path.|
|disk-B files on dest volume|Removed|Still on disk|Ghost / orphan replica as RM
receives the updated path and only removes it from the container map and not
physically.|
|SCM replica knowledge|C is deleted|C is deleted (via ICR at Step 18)|SCM
believes it, but disk-B still has orphan data|
|Container heartbeat report|C absent|C absent (map has no entry for C)|SCM
never learns about the orphan|
|destVolume space accounting|0|Overcounted by size bytes (Step 11 was never
reversed)|Disk pressure skewed|
|pendingDeletionContainers|disk-A cleaned up by DiskBalancer|disk-A was already
deleted — cleanup is a no-op or error|Harmless but wasteful|
disk-B is an orphan: invisible to SCM, not tracked in memory, not cleaned up.
Every heartbeat SCM receives confirms the container is gone, even though the
data directory exists on disk.
> Read-Write Lock race leave stale references to container creating orphan
> replicas
> ---------------------------------------------------------------------------------
>
> Key: HDDS-15066
> URL: https://issues.apache.org/jira/browse/HDDS-15066
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Gargi Jaiswal
> Assignee: Gargi Jaiswal
> Priority: Major
>
> On a datanode, some work runs under a container {color:#de350b}read
> lock{color} (or otherwise changes which replica directory / in-memory
> {{Container}} is authoritative) while other threads look up a container once
> and later take a {color:#de350b}write lock{color}. If the in-memory container
> mapping or on-disk location changes in between, the second thread can still
> use a stale *{{Container}}* or *{{KeyValueContainerData}}* reference. That is
> a classic *TOCTOU* problem: the map and the caller disagree about where the
> replica lives.
> Worst cases include wrong {{*ContainerSet*}} updates, deleting or updating
> the wrong paths, ghost / orphan replica data on disk that SCM no longer
> tracks, and block deletion targeting the wrong RocksDB/chunks tree so pending
> deletes on the live replica are never applied (space not reclaimed).
> Applies to CLOSED and QUASI_CLOSED (and any path where balancing/replication
> overlaps lifecycle commands), not a single state.
> *This can be easily explained with help of DiskBalancer as an example,
> although it applies to other read write races as well:*
> While {color:#de350b}DiskBalance{color}r moves a container between volumes on
> the same datanode, it {*}holds the container read lock{*}, copies data, then
> calls *{{ContainerSet.updateContainer(...)}}* to point the *in-memory map* at
> a new {{Container}} instance (new volume / paths).
> Whereas it is seen that other threads often look up the {*}container once{*},
> then block {color:#de350b}on {{writeLock()}}{color} until the move finishes.
> After they unblock, they still hold a reference to the old container (source
> volume). They then run logic and/or *{{removeContainer(containerId)}}* using
> stale paths and stale object identity.
> *Impacts:*
> * *Replication Manager {{{}DeleteContainerCommand{}}}:* can remove the wrong
> map entry (live replica on the destination volume), delete/move source files,
> and leave an orphan replica on disk that SCM no longer sees — plus wrong
> volume accounting and ICRs keyed off the old replica.
> * *{{{}BlockDeletingTask{}}}:* uses a task-scoped
> *{{KeyValueContainerData}}* snapshot (source paths) while {{getContainer()}}
> may return the new replica; block cleanup can target the wrong chunks path
> and never clear pending-deletion state on the live replica → space not
> reclaimed.
> *Suggested Fix:*
> For RM side: After acquiring writeLock(), {*}re-fetch from the map and
> compare by identity{*}. If they differ, the container was moved — abort and
> let the caller retry.
> {code:java}
> container.writeLock();
> try {
> Container<?> current containerSet.getContainer(
> container.getContainerData().getContainerID());
> if (current != container) {
>
> // Container was relocated by DiskBalancer while we waited for the lock.Our
> reference is stale; the operation will be retried on the next cycle.
>
> return;
> }
> // ... proceed with the actual operation
> }{code}
> For BlockDeletingService:
> {code:java}
> container.writeLock();
> // Re-fetch AFTER lock: map already has disk-B in scenarios.
> // current.getContainerData() is disk-B's ContainerData ≠ this.containerData
> (disk-A snapshot).
> Container<?> current = containerSet.getContainer(containerData
> .getContainerID());
> if (current == null || current.getContainerData() != containerData) {
> // containerData is stale — DiskBalancer relocated this container.
> BlockDeletingService will reschedule with disk-B's fresh containerData.
> return;
> } {code}
>
>
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
