You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2021/08/29 07:20:16 UTC

[GitHub] [ozone] mukul1987 opened a new pull request #2594: HDDS-5660. Separate container caches for Open and Closed containers.

mukul1987 opened a new pull request #2594:
URL: https://github.com/apache/ozone/pull/2594


   ## What changes were proposed in this pull request?
   
   It has been seen in deployments that with large number of containers on datanodes. we have seen cache misses on the rocksDB cache.
   
   This patch proposes
   a) to have separate caches for open and closed container
   b) rocksDB containers in open container cache are never evicted.
   c) Open containers are moved from open to close container cache it they are closed/quasi closed.
   d) Closed container are cached in LRU fashion as they are cached right now
   
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-5660
   
   ## How was this patch tested?
   Added a new unit test
   


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] kerneltime commented on a change in pull request #2594: HDDS-5660. Separate container caches for Open and Closed containers.

Posted by GitBox <gi...@apache.org>.
kerneltime commented on a change in pull request #2594:
URL: https://github.com/apache/ozone/pull/2594#discussion_r697973812



##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerCache.java
##########
@@ -227,9 +246,68 @@ private boolean cleanupDb(ReferenceCountedDB db) {
   public void addDB(String containerDBPath, ReferenceCountedDB db) {
     lock.lock();
     try {
-      this.putIfAbsent(containerDBPath, db);
+      openContainerCache.putIfAbsent(containerDBPath, db);
+      metrics.incNumOpenContainerInserts();
+    } finally {
+      lock.unlock();
+    }
+  }
+
+  public void markContainerClosed(String containerPath) {
+    lock.lock();
+    try {
+      ReferenceCountedDB db = openContainerCache.remove(containerPath);
+      Preconditions.checkNotNull(db, "Container not found in open cache:" +
+          containerPath);
+      closeContainerCache.putIfAbsent(containerPath, db);
+      metrics.incNumOpenContainerRemoves();
     } finally {
       lock.unlock();
     }
   }
+
+  public boolean isContainerOpen(State state) {

Review comment:
       Would a better separation between the container states be `isContainerWritable`? Open is becoming an over loaded term.




-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] adoroszlai commented on pull request #2594: HDDS-5660. Separate container caches for Open and Closed containers.

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #2594:
URL: https://github.com/apache/ozone/pull/2594#issuecomment-1049892553


   /pending


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] mukul1987 commented on a change in pull request #2594: HDDS-5660. Separate container caches for Open and Closed containers.

Posted by GitBox <gi...@apache.org>.
mukul1987 commented on a change in pull request #2594:
URL: https://github.com/apache/ozone/pull/2594#discussion_r698355347



##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerCache.java
##########
@@ -227,9 +246,68 @@ private boolean cleanupDb(ReferenceCountedDB db) {
   public void addDB(String containerDBPath, ReferenceCountedDB db) {
     lock.lock();
     try {
-      this.putIfAbsent(containerDBPath, db);
+      openContainerCache.putIfAbsent(containerDBPath, db);
+      metrics.incNumOpenContainerInserts();
+    } finally {
+      lock.unlock();
+    }
+  }
+
+  public void markContainerClosed(String containerPath) {
+    lock.lock();
+    try {
+      ReferenceCountedDB db = openContainerCache.remove(containerPath);
+      Preconditions.checkNotNull(db, "Container not found in open cache:" +
+          containerPath);
+      closeContainerCache.putIfAbsent(containerPath, db);
+      metrics.incNumOpenContainerRemoves();
     } finally {
       lock.unlock();
     }
   }
+
+  public boolean isContainerOpen(State state) {

Review comment:
       ok will do that,




-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] mukul1987 commented on a change in pull request #2594: HDDS-5660. Separate container caches for Open and Closed containers.

Posted by GitBox <gi...@apache.org>.
mukul1987 commented on a change in pull request #2594:
URL: https://github.com/apache/ozone/pull/2594#discussion_r698362152



##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerCache.java
##########
@@ -227,9 +246,68 @@ private boolean cleanupDb(ReferenceCountedDB db) {
   public void addDB(String containerDBPath, ReferenceCountedDB db) {
     lock.lock();
     try {
-      this.putIfAbsent(containerDBPath, db);
+      openContainerCache.putIfAbsent(containerDBPath, db);
+      metrics.incNumOpenContainerInserts();
+    } finally {
+      lock.unlock();
+    }
+  }
+
+  public void markContainerClosed(String containerPath) {
+    lock.lock();
+    try {
+      ReferenceCountedDB db = openContainerCache.remove(containerPath);
+      Preconditions.checkNotNull(db, "Container not found in open cache:" +
+          containerPath);
+      closeContainerCache.putIfAbsent(containerPath, db);
+      metrics.incNumOpenContainerRemoves();
     } finally {
       lock.unlock();
     }
   }
+
+  public boolean isContainerOpen(State state) {

Review comment:
       done in the followup patch




-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] bshashikant commented on a change in pull request #2594: HDDS-5660. Separate container caches for Open and Closed containers.

Posted by GitBox <gi...@apache.org>.
bshashikant commented on a change in pull request #2594:
URL: https://github.com/apache/ozone/pull/2594#discussion_r698375971



##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerCache.java
##########
@@ -219,17 +238,80 @@ private boolean cleanupDb(ReferenceCountedDB db) {
   }
 
   /**
-   * Add a DB handler into cache.
+   * Add a DB handler into container cache
+   * only if the container is in OPEN State.
    *
    * @param containerDBPath - DB path of the container.
    * @param db - DB handler
    */
-  public void addDB(String containerDBPath, ReferenceCountedDB db) {
+  public void addDB(String containerDBPath, ReferenceCountedDB db,
+                    State state) {
+    if (isContainerWriteable(state)) {
+      lock.lock();
+      try {
+        openContainerCache.putIfAbsent(containerDBPath, db);
+        metrics.incNumOpenContainerInserts();
+      } finally {
+        lock.unlock();
+      }
+    }
+  }
+
+  public void markContainerClosed(String containerPath) {
     lock.lock();
     try {
-      this.putIfAbsent(containerDBPath, db);
+      ReferenceCountedDB db = openContainerCache.remove(containerPath);
+      Preconditions.checkNotNull(db, "Container not found in open cache:" +
+          containerPath);
+      closeContainerCache.putIfAbsent(containerPath, db);
+      metrics.incNumOpenContainerRemoves();
     } finally {
       lock.unlock();
     }
   }
+
+  private boolean isContainerWriteable(State state) {
+    switch (state) {
+    case CLOSED:
+    case QUASI_CLOSED:
+      return false;
+    case CLOSING:
+    case OPEN:
+    case UNHEALTHY:
+      return true;

Review comment:
       Unhealthy containers are writeable?




-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] mukul1987 commented on pull request #2594: HDDS-5660. Separate container caches for Open and Closed containers.

Posted by GitBox <gi...@apache.org>.
mukul1987 commented on pull request #2594:
URL: https://github.com/apache/ozone/pull/2594#issuecomment-1067860271


   /ready


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org