[iotdb] branch rel_0.12_debug_compaction_stop updated: fix concurrent issue

[ta...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

tanxinyu pushed a commit to branch rel_0.12_debug_compaction_stop
in repository https://gitbox.apache.org/repos/asf/iotdb.git


The following commit(s) were added to refs/heads/rel_0.12_debug_compaction_stop by this push:
     new 41e2ab5  fix concurrent issue
41e2ab5 is described below

commit 41e2ab5290e5608259bb9db344d62672da86a01c
Author: LebronAl <TX...@gmail.com>
AuthorDate: Thu Nov 11 10:23:45 2021 +0800

    fix concurrent issue
---
 .../iotdb/db/query/control/QueryFileManager.java   | 35 ++++++++++++----------
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/server/src/main/java/org/apache/iotdb/db/query/control/QueryFileManager.java b/server/src/main/java/org/apache/iotdb/db/query/control/QueryFileManager.java
index b7604d6..c9a4e14 100644
--- a/server/src/main/java/org/apache/iotdb/db/query/control/QueryFileManager.java
+++ b/server/src/main/java/org/apache/iotdb/db/query/control/QueryFileManager.java
@@ -21,11 +21,9 @@ package org.apache.iotdb.db.query.control;
 import org.apache.iotdb.db.engine.querycontext.QueryDataSource;
 import org.apache.iotdb.db.engine.storagegroup.TsFileResource;
 
-import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 
 /**
@@ -36,9 +34,9 @@ import java.util.concurrent.ConcurrentHashMap;
 public class QueryFileManager {
 
   /** Map<queryId, Set<filePaths>> */
-  private Map<Long, Set<TsFileResource>> sealedFilePathsMap;
+  private Map<Long, Map<TsFileResource, TsFileResource>> sealedFilePathsMap;
 
-  private Map<Long, Set<TsFileResource>> unsealedFilePathsMap;
+  private Map<Long, Map<TsFileResource, TsFileResource>> unsealedFilePathsMap;
 
   QueryFileManager() {
     sealedFilePathsMap = new ConcurrentHashMap<>();
@@ -50,8 +48,8 @@ public class QueryFileManager {
    * must be invoked.
    */
   void addQueryId(long queryId) {
-    sealedFilePathsMap.computeIfAbsent(queryId, x -> new HashSet<>());
-    unsealedFilePathsMap.computeIfAbsent(queryId, x -> new HashSet<>());
+    sealedFilePathsMap.computeIfAbsent(queryId, x -> new ConcurrentHashMap<>());
+    unsealedFilePathsMap.computeIfAbsent(queryId, x -> new ConcurrentHashMap<>());
   }
 
   /** Add the unique file paths to sealedFilePathsMap and unsealedFilePathsMap. */
@@ -73,10 +71,10 @@ public class QueryFileManager {
 
       // this file may be deleted just before we lock it
       if (tsFileResource.isDeleted()) {
-        Map<Long, Set<TsFileResource>> pathMap =
+        Map<Long, Map<TsFileResource, TsFileResource>> pathMap =
             !isClosed ? unsealedFilePathsMap : sealedFilePathsMap;
         // This resource may be removed by other threads of this query.
-        if (pathMap.get(queryId).remove(tsFileResource)) {
+        if (pathMap.get(queryId).remove(tsFileResource) != null) {
           FileReaderManager.getInstance()
               .decreaseFileReaderReference(tsFileResource, isClosed, queryId);
         }
@@ -94,7 +92,7 @@ public class QueryFileManager {
     sealedFilePathsMap.computeIfPresent(
         queryId,
         (k, v) -> {
-          for (TsFileResource tsFile : v) {
+          for (TsFileResource tsFile : v.keySet()) {
             FileReaderManager.getInstance().decreaseFileReaderReference(tsFile, true, queryId);
           }
           return null;
@@ -102,7 +100,7 @@ public class QueryFileManager {
     unsealedFilePathsMap.computeIfPresent(
         queryId,
         (k, v) -> {
-          for (TsFileResource tsFile : v) {
+          for (TsFileResource tsFile : v.keySet()) {
             FileReaderManager.getInstance().decreaseFileReaderReference(tsFile, false, queryId);
           }
           return null;
@@ -116,11 +114,16 @@ public class QueryFileManager {
    * not return null.
    */
   void addFilePathToMap(long queryId, TsFileResource tsFile, boolean isClosed) {
-    Map<Long, Set<TsFileResource>> pathMap = isClosed ? sealedFilePathsMap : unsealedFilePathsMap;
-    // TODO this is not an atomic operation, is there concurrent problem?
-    if (!pathMap.get(queryId).contains(tsFile)) {
-      pathMap.get(queryId).add(tsFile);
-      FileReaderManager.getInstance().increaseFileReaderReference(tsFile, isClosed, queryId);
-    }
+    Map<Long, Map<TsFileResource, TsFileResource>> pathMap =
+        isClosed ? sealedFilePathsMap : unsealedFilePathsMap;
+    pathMap
+        .get(queryId)
+        .computeIfAbsent(
+            tsFile,
+            k -> {
+              FileReaderManager.getInstance()
+                  .increaseFileReaderReference(tsFile, isClosed, queryId);
+              return k;
+            });
   }
 }