[
https://issues.apache.org/jira/browse/HDDS-15066?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Gargi Jaiswal updated HDDS-15066:
---------------------------------
Description:
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}
was:
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}
> 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]
