Gargi-jais11 opened a new pull request, #10109:
URL: https://github.com/apache/ozone/pull/10109
## What changes were proposed in this pull request?
On a datanode, some work runs under a container read lock (or otherwise
changes which replica directory / in-memory Container is authoritative) while
other threads look up a container once and later take a write lock. 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 DiskBalancer 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 on writeLock() 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:**
1. **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.
2. **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.
```
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
}
```
**For BlockDeletingService:**
```
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;
}
```
**For DiskBalancer:**
Move the container state check after acquiring readLock() to prevent stale
references about container.
```
container.readLock();
try {
// Double check container state before acquiring lock to start move process.
// Container state may have changed after selection.
State containerState = container.getContainerData().getState();
if (!movableContainerStates.contains(containerState)) {
LOG.warn("Container {} is in {} state, skipping move process.",
containerId, containerState);
postCall(false, startTime);
return BackgroundTaskResult.EmptyTaskResult.newResult();
} // Step 1: Copy container to new Volume's tmp Dir
diskBalancerTmpDir = getDiskBalancerTmpDir(destVolume)
.resolve(String.valueOf(containerId));
ozoneContainer.getController().copyContainer(containerData,
diskBalancerTmpDir);
....
```
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-15066
## How was this patch tested?
Added unit tests on diskBalancer side as an example for the race condition
which can happen with any services read/write Lock race.
Test file: `TestDiskBalancerWithConcurrentBackgroundTasks`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
