sodonnel opened a new pull request #3071:
URL: https://github.com/apache/ozone/pull/3071
## What changes were proposed in this pull request?
Inside ContainerStateMap, the replicas for a container are stored in a Set
backed by a ConcurrentHashMap.
When you ask for the current replicas of a container, this method is used:
```
public Set<ContainerReplica> getContainerReplicas(
final ContainerID containerID) {
Preconditions.checkNotNull(containerID);
final Set<ContainerReplica> replicas = replicaMap.get(containerID);
return replicas == null ? null : Collections.unmodifiableSet(replicas);
}
```
Note that it pulls out the Set, wraps it as unmodifiable and returns it.
There is a problem here, in that if the Set is updated by ICR / FCR at the
same time as another part of the code has taken a reference to it, the other
part of the code can make incorrect decisions. Eg:
```
Set<> replicas = getContainerReplicas(...)
replicaCount = replicas.size()
// continue to do something based on the size
```
ReplicationManger has run into a race condition like this. We also use the
Replicas to form pipelines for closed containers, so I worry there could be
some strange issues if the set if mutated during the pipeline creation.
I see two possible solutions here. `GetContainerReplicas` should create a
copy of the Set and return that, so the copy the other part of the code gets is
its own copy and nothing can change it.
Or, we make the Set immutable, so that each new replica details are
received, we create the new copy of the set and store that. Then any other
parts of the code can get a reference to it, and know it will never change.
Mutations to the replicas for a closed container will only happen with FCR,
which is relatively rare.
However we may ask for read pipelines very frequently, so it would be
cheaper overall to use option 2.
It we go with option 2, I think we can move from a concurrentHashMap to a
plain hashMap too, which may make the memory footprint slightly smaller.
Note access to the replicas is via ContainerStateManagerImpl, which already
has a course RW lock protecting access to the container manager. Quite possibly
FCR reporting could be improved by a finer grained or striped lock.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-6292
## How was this patch tested?
Existing tests cover this area.
--
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]
