[
https://issues.apache.org/jira/browse/HDDS-16052?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Ivan Andika updated HDDS-16052:
-------------------------------
Description:
ContainerData#updateWriteStats is not atomic. It is updated for each
WriteChunk, but WriteChunk is only serialized for a single block (see
ContainerStateMachine#getChunkExecutor). If there are two concurrent WriteChunk
for the same container, but with different blocks, this can trigger race
conditions which can cause updateWriteStats to be interleaved which can end up
causing inconsistency in ContainerData. We also need to specify the thread
safety guarantee of ContainerData.
Additionally, ContainerData is also has memory visibility (no happens-before
guarantee) issue since the shared variables like committedSpace is not volatile
(or protected by synchronization), although ContainerData can be accessed by
multiple threads.
Example: container has 100 bytes unused, T1 and T2 each write 100 bytes.
* T1 reads {{{}unused = 100{}}}, gets preempted.
* T2 writes: {{bytesUsed}} goes to +100, releases 100 from
{{{}committedBytes{}}}. Container is now full, committed space fully released.
Correct so far.
* T1 resumes with its stale {{{}unused = 100{}}}, writes (now {{bytesUsed}} =
+200), and releases _another_ 100 from {{{}committedBytes{}}}.
End state: {{committedBytes}} was decremented by 200 when only 100 was ever
reserved for that container. The volume-level {{committedBytes}} under-reports
by 100, so {{VolumeUsage}} thinks there's more free space for new containers
than there really is.
A few things worth keeping in mind:
* The error compounds over many concurrent writes rather than being a one-off,
but it's bounded by how much committed space was actually reserved — it can't
go arbitrarily wrong from a single container.
* It self-corrects partially: DU/{{{}DedicatedDiskSpaceUsage{}}} periodically
recomputes real used space, and {{committedBytes}} is released wholesale on
container close via {{releaseCommitSpace}} (which has its own races with the
same stale-read pattern).
* The cleanest fix is to compute the decrement from the return value of
{{incrBytesUsed}} (the post-increment total) instead of the pre-read
{{{}unused{}}}, or guard the whole release block with the container's lock —
though that also needs {{{}commitSpace{}}}/{{{}releaseCommitSpace{}}} to
participate in the same locking to close the check-then-act window on the
{{committedSpace}} flag.
was:
ContainerData#updateWriteStats is not atomic. It is updated for each
WriteChunk, but WriteChunk is only serialized for a single block. If there are
two concurrent WriteChunk for the same container, but with different blocks,
this can trigger race conditions which can cause updateWriteStats to be
interleaved which can end up causing inconsistency in ContainerData. We also
need to specify the thread safety guarantee of ContainerData.
Additionally, ContainerData is also has memory visibility (no happens-before
guarantee) issue since the shared variables like committedSpace is not volatile
(or protected by synchronization), although ContainerData can be accessed by
multiple threads.
Example: container has 100 bytes unused, T1 and T2 each write 100 bytes.
* T1 reads {{{}unused = 100{}}}, gets preempted.
* T2 writes: {{bytesUsed}} goes to +100, releases 100 from
{{{}committedBytes{}}}. Container is now full, committed space fully released.
Correct so far.
* T1 resumes with its stale {{{}unused = 100{}}}, writes (now {{bytesUsed}} =
+200), and releases _another_ 100 from {{{}committedBytes{}}}.
End state: {{committedBytes}} was decremented by 200 when only 100 was ever
reserved for that container. The volume-level {{committedBytes}} under-reports
by 100, so {{VolumeUsage}} thinks there's more free space for new containers
than there really is.
A few things worth keeping in mind:
* The error compounds over many concurrent writes rather than being a one-off,
but it's bounded by how much committed space was actually reserved — it can't
go arbitrarily wrong from a single container.
* It self-corrects partially: DU/{{{}DedicatedDiskSpaceUsage{}}} periodically
recomputes real used space, and {{committedBytes}} is released wholesale on
container close via {{releaseCommitSpace}} (which has its own races with the
same stale-read pattern).
* The cleanest fix is to compute the decrement from the return value of
{{incrBytesUsed}} (the post-increment total) instead of the pre-read
{{{}unused{}}}, or guard the whole release block with the container's lock —
though that also needs {{{}commitSpace{}}}/{{{}releaseCommitSpace{}}} to
participate in the same locking to close the check-then-act window on the
{{committedSpace}} flag.
> ContainerData updateWriteStats is not atomic
> --------------------------------------------
>
> Key: HDDS-16052
> URL: https://issues.apache.org/jira/browse/HDDS-16052
> Project: Apache Ozone
> Issue Type: Sub-task
> Reporter: Ivan Andika
> Assignee: Ivan Andika
> Priority: Major
>
> ContainerData#updateWriteStats is not atomic. It is updated for each
> WriteChunk, but WriteChunk is only serialized for a single block (see
> ContainerStateMachine#getChunkExecutor). If there are two concurrent
> WriteChunk for the same container, but with different blocks, this can
> trigger race conditions which can cause updateWriteStats to be interleaved
> which can end up causing inconsistency in ContainerData. We also need to
> specify the thread safety guarantee of ContainerData.
> Additionally, ContainerData is also has memory visibility (no happens-before
> guarantee) issue since the shared variables like committedSpace is not
> volatile (or protected by synchronization), although ContainerData can be
> accessed by multiple threads.
> Example: container has 100 bytes unused, T1 and T2 each write 100 bytes.
> * T1 reads {{{}unused = 100{}}}, gets preempted.
> * T2 writes: {{bytesUsed}} goes to +100, releases 100 from
> {{{}committedBytes{}}}. Container is now full, committed space fully
> released. Correct so far.
> * T1 resumes with its stale {{{}unused = 100{}}}, writes (now {{bytesUsed}}
> = +200), and releases _another_ 100 from {{{}committedBytes{}}}.
> End state: {{committedBytes}} was decremented by 200 when only 100 was ever
> reserved for that container. The volume-level {{committedBytes}}
> under-reports by 100, so {{VolumeUsage}} thinks there's more free space for
> new containers than there really is.
> A few things worth keeping in mind:
> * The error compounds over many concurrent writes rather than being a
> one-off, but it's bounded by how much committed space was actually reserved —
> it can't go arbitrarily wrong from a single container.
> * It self-corrects partially: DU/{{{}DedicatedDiskSpaceUsage{}}}
> periodically recomputes real used space, and {{committedBytes}} is released
> wholesale on container close via {{releaseCommitSpace}} (which has its own
> races with the same stale-read pattern).
> * The cleanest fix is to compute the decrement from the return value of
> {{incrBytesUsed}} (the post-increment total) instead of the pre-read
> {{{}unused{}}}, or guard the whole release block with the container's lock —
> though that also needs {{{}commitSpace{}}}/{{{}releaseCommitSpace{}}} to
> participate in the same locking to close the check-then-act window on the
> {{committedSpace}} flag.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
