[HDDS-6334] Remove ContainerID to Proto to ContainerID conversion in ContainerStateManagerImpl

• Type: Improvement
• Status: Resolved
• Priority: Major
• Resolution: Fixed
• Fix Version/s: 1.3.0
• Component/s: SCM
• Labels: pull-request-available
• Assignee: Stephen O'Donnell
• Reporter: Stephen O'Donnell
• Created: 16/Feb/22 20:06
• Updated: 24/Feb/22 20:57
• Resolved: 24/Feb/22 20:57

Description

In ContainerManagerImpl, most of the methods accept a ContainerID object. This is then converted into `HddsProtos.ContainerID` to pass to ContainerStateManagerImpl. The data is stored in ContainerStateMap, where it is all keyed on ContainerID, so the `HddsProtos.ContainerID` has to be converted back to ContainerID again, which seems wasteful.

I think this is left over from some previous implementation, but is there there any reason we cannot change ContainerStateManagerImpl to accept ContainerID directly? This is an internal class to ContainerManager, so the change should have minimal impact.

There are also some TODO's in the code suggesting it was always the intention to make this change, but it never happened, eg:

  @Override
  public boolean contains(final HddsProtos.ContainerID id) {
    lock.readLock().lock();
    try {
      // TODO: Remove the protobuf conversion after fixing ContainerStateMap.
      return containers.contains(ContainerID.getFromProtobuf(id));
    } finally {
      lock.readLock().unlock();
    }
  }

 

Issue Links

• links to: GitHub Pull Request #3110