sodonnel commented on a change in pull request #1147:
URL: https://github.com/apache/hadoop-ozone/pull/1147#discussion_r454957121



##########
File path: 
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerCache.java
##########
@@ -113,22 +113,41 @@ protected boolean removeLRU(LinkEntry entry) {
    * @return ReferenceCountedDB.
    */
   public ReferenceCountedDB getDB(long containerID, String containerDBType,
-                             String containerDBPath, ConfigurationSource conf)
+      String containerDBPath, ConfigurationSource conf)
+      throws IOException {
+    return getDB(containerID, containerDBType, containerDBPath, conf, true);
+  }
+  /**
+   * Returns a DB handle if available, create the handler otherwise.
+   *
+   * @param containerID - ID of the container.
+   * @param containerDBType - DB type of the container.
+   * @param containerDBPath - DB path of the container.
+   * @param acquireLock - false only for one-time ContainerReader execution
+   *                    during datanode initialization. Don't set it to false
+   *                    in other cases.
+   * @param conf - Hadoop Configuration.
+   * @return ReferenceCountedDB.
+   */
+  public ReferenceCountedDB getDB(long containerID, String containerDBType,
+      String containerDBPath, ConfigurationSource conf, boolean acquireLock)
       throws IOException {
     Preconditions.checkState(containerID >= 0,
         "Container ID cannot be negative.");
-    lock.lock();
+    ReferenceCountedDB db;
+    if (acquireLock) {
+      lock.lock();
+    }
     try {
-      ReferenceCountedDB db = (ReferenceCountedDB) this.get(containerDBPath);
-
+      db = (ReferenceCountedDB) this.get(containerDBPath);
       if (db == null) {
         MetadataStore metadataStore =
             MetadataStoreBuilder.newBuilder()
-            .setDbFile(new File(containerDBPath))
-            .setCreateIfMissing(false)
-            .setConf(conf)
-            .setDBType(containerDBType)
-            .build();
+                .setDbFile(new File(containerDBPath))
+                .setCreateIfMissing(false)
+                .setConf(conf)
+                .setDBType(containerDBType)
+                .build();
         db = new ReferenceCountedDB(metadataStore, containerDBPath);
         this.put(containerDBPath, db);

Review comment:
       Thinking about this some more, it might be safest to wrap addDB and 
removeDB with the strippedLock too, as that way we ensure only one DB instance 
can be initialized and added to the map at any given time.
   
   @adoroszlai might have some better idea than me on this too?




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

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



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

Reply via email to