You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "Ivan Andika (Jira)" <ji...@apache.org> on 2024/01/25 03:53:00 UTC
[jira] [Updated] (HDDS-10210) Ensure atomic update in StateContext#updateCommandStatus
[ https://issues.apache.org/jira/browse/HDDS-10210?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Ivan Andika updated HDDS-10210:
-------------------------------
Description:
The cmdStatusMap in StateContext is a ConcurrentHashMap
Currently the update logic for command status is as follows:
{code:java}
public boolean updateCommandStatus(Long cmdId,
Consumer<CommandStatus> cmdStatusUpdater) {
if (cmdStatusMap.containsKey(cmdId)) {
cmdStatusUpdater.accept(cmdStatusMap.get(cmdId));
return true;
}
return false;
} {code}
It does not seem correct. Although the get containsKey and get is protected by the ConcurrentHashMap, the CommandStatus retrieved is mutated directly using the cmdStatusUpdater and not protected by the ConcurrentHashMap. This might trigger race condition.
We should use atomic operation like computeIfPresent.
Note: The cmdStatusMap is also exposed to public (getCommandStatusMap) which might not be a good idea since we cannot guarantee correct usage of the map. We might need to encapsulate properly inside StateContext.
was:
The cmdStatusMap in StateContext is a ConcurrentHashMap
Currently the update logic for command status is as follows:
{code:java}
public boolean updateCommandStatus(Long cmdId,
Consumer<CommandStatus> cmdStatusUpdater) {
if (cmdStatusMap.containsKey(cmdId)) {
cmdStatusUpdater.accept(cmdStatusMap.get(cmdId));
return true;
}
return false;
} {code}
It does not seem correct. Although the get containsKey and get is protected by the ConcurrentHashMap, the CommandStatus retrieved is mutated directly using the cmdStatusUpdater and not protected by the ConcurrentHashMap. This might trigger race condition.
We should use atomic operation like computeIfPresent.
> Ensure atomic update in StateContext#updateCommandStatus
> --------------------------------------------------------
>
> Key: HDDS-10210
> URL: https://issues.apache.org/jira/browse/HDDS-10210
> Project: Apache Ozone
> Issue Type: Improvement
> Components: DN, Ozone Datanode
> Affects Versions: 1.4.0
> Reporter: Ivan Andika
> Assignee: Ivan Andika
> Priority: Major
>
> The cmdStatusMap in StateContext is a ConcurrentHashMap
> Currently the update logic for command status is as follows:
> {code:java}
> public boolean updateCommandStatus(Long cmdId,
> Consumer<CommandStatus> cmdStatusUpdater) {
> if (cmdStatusMap.containsKey(cmdId)) {
> cmdStatusUpdater.accept(cmdStatusMap.get(cmdId));
> return true;
> }
> return false;
> } {code}
> It does not seem correct. Although the get containsKey and get is protected by the ConcurrentHashMap, the CommandStatus retrieved is mutated directly using the cmdStatusUpdater and not protected by the ConcurrentHashMap. This might trigger race condition.
> We should use atomic operation like computeIfPresent.
> Note: The cmdStatusMap is also exposed to public (getCommandStatusMap) which might not be a good idea since we cannot guarantee correct usage of the map. We might need to encapsulate properly inside StateContext.
--
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