[HDDS-10210] Ensure atomic update in StateContext#updateCommandStatus

• Type: Improvement
• Status: Resolved
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 1.4.0
• Fix Version/s: 1.5.0
• Component/s: DN, Ozone Datanode
• Labels: pull-request-available
• Assignee: Ivan Andika
• Reporter: Ivan Andika
• Created: 25/Jan/24 03:44
• Updated: 26/Jan/24 11:36
• Resolved: 26/Jan/24 11:15

Description

While tracing the block deletion logic, I found a possible problem regarding command status update logic.

The cmdStatusMap in StateContext is a ConcurrentHashMap

Currently the update logic for command status is as follows:

public boolean updateCommandStatus(Long cmdId,
    Consumer<CommandStatus> cmdStatusUpdater) {
  if (cmdStatusMap.containsKey(cmdId)) {
    cmdStatusUpdater.accept(cmdStatusMap.get(cmdId));
    return true;
  }
  return false;
} 

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 it properly inside StateContext.

Issue Links

• links to: GitHub Pull Request #6091