Gargi-jais11 commented on code in PR #10109:
URL: https://github.com/apache/ozone/pull/10109#discussion_r3233389538


##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/ContainerSet.java:
##########
@@ -279,6 +285,49 @@ public Container<?> getContainer(long containerId) {
     return containerMap.get(containerId);
   }
 
+  /**
+   * Returns the max retry for a container map swap while acquiring container 
lock.
+   * @return max retry count
+   */
+  public static int maxContainerMapSwapRetries() {
+    return MAX_CONTAINER_MAP_SWAP_RETRIES;
+  }
+
+  /**
+   * Locks the container mapped to {@code containerId} for write, and verifies 
that the instance locked is still
+   * the one stored in this set. If the mapping is swapped or the container no 
longer exists, unlocks and retries up to
+   * {@link #maxContainerMapSwapRetries()} times, then returns {@code null}.
+   *
+   * @return the locked container, or {@code null} if none mapped, or mapping 
could not be stabilized
+   */
+  @Nullable
+  public Container<?> acquireContainerLock(long containerId) {
+    for (int retry = 0; retry < MAX_CONTAINER_MAP_SWAP_RETRIES; retry++) {
+      Container<?> candidate = getContainer(containerId);
+      if (candidate == null) {
+        LOG.info("Container {} no longer present in ContainerSet, skipping.", 
containerId);
+        return null;
+      }
+      candidate.writeLock();
+      Container<?> current = getContainer(containerId);
+      if (current == null) {
+        candidate.writeUnlock();
+        LOG.info("Container {} no longer exists in ContainerSet while 
acquiring lock.", containerId);
+        return null;
+      }
+      if (current != candidate) {
+        candidate.writeUnlock();
+        if (LOG.isDebugEnabled()) {
+          LOG.debug("Container {} mapping changed during lock acquisition 
(attempt {}); retrying.",

Review Comment:
   @sodonnel but for now there is only diskbalancer which will swap container 
in in-memory. for one diskbalancer task it operates only one time on that 
container. It's very rarest case that the same container will be eligible to 
move to other disk again but that too will happen in next iteration of balancer 
and since we already writeLock wound be working on current then diskbalancer 
won't able to acquire readLock again on same container. 
   considering the above I don't think there is any gap left for this. Please 
correct me if I am wrong.
   



-- 
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]

Reply via email to