You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iotdb.apache.org by qi...@apache.org on 2020/01/09 07:02:36 UTC

[incubator-iotdb] 01/01: add synchronized

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

qiaojialin pushed a commit to branch fix_query_bug
in repository https://gitbox.apache.org/repos/asf/incubator-iotdb.git

commit 3a2047a1f898c8b1e0174cab001eaec80ecc3596
Author: qiaojialin <64...@qq.com>
AuthorDate: Thu Jan 9 15:02:18 2020 +0800

    add synchronized
---
 .../iotdb/db/query/context/QueryContext.java       |  5 +++--
 .../iotdb/db/query/control/FileReaderManager.java  | 25 +++++++++-------------
 .../iotdb/db/query/control/QueryFileManager.java   |  5 ++---
 .../iotdb/tsfile/read/TsFileSequenceReader.java    |  3 ++-
 4 files changed, 17 insertions(+), 21 deletions(-)

diff --git a/server/src/main/java/org/apache/iotdb/db/query/context/QueryContext.java b/server/src/main/java/org/apache/iotdb/db/query/context/QueryContext.java
index 10f6cbc..fe78825 100644
--- a/server/src/main/java/org/apache/iotdb/db/query/context/QueryContext.java
+++ b/server/src/main/java/org/apache/iotdb/db/query/context/QueryContext.java
@@ -23,6 +23,7 @@ import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 import org.apache.iotdb.db.engine.modification.Modification;
 import org.apache.iotdb.db.engine.modification.ModificationFile;
 import org.apache.iotdb.tsfile.file.metadata.ChunkMetaData;
@@ -36,13 +37,13 @@ public class QueryContext {
    * The outer key is the path of a ModificationFile, the inner key in the name of a timeseries and
    * the value is the Modifications of a timeseries in this file.
    */
-  private Map<String, Map<String, List<Modification>>> filePathModCache = new HashMap<>();
+  private Map<String, Map<String, List<Modification>>> filePathModCache = new ConcurrentHashMap<>();
   /**
    * The key is the path of a ModificationFile and the value is all Modifications in this file. We
    * use this field because each call of Modification.getModifications() return a copy of the
    * Modifications, and we do not want it to create multiple copies within a query.
    */
-  private Map<String, List<Modification>> fileModCache = new HashMap<>();
+  private Map<String, List<Modification>> fileModCache = new ConcurrentHashMap<>();
 
   private long queryId;
 
diff --git a/server/src/main/java/org/apache/iotdb/db/query/control/FileReaderManager.java b/server/src/main/java/org/apache/iotdb/db/query/control/FileReaderManager.java
index c961a64..b3c546f 100644
--- a/server/src/main/java/org/apache/iotdb/db/query/control/FileReaderManager.java
+++ b/server/src/main/java/org/apache/iotdb/db/query/control/FileReaderManager.java
@@ -164,15 +164,12 @@ public class FileReaderManager implements IService {
    * Increase the reference count of the reader specified by filePath. Only when the reference count
    * of a reader equals zero, the reader can be closed and removed.
    */
-  void increaseFileReaderReference(TsFileResource tsFile, boolean isClosed) {
-    // TODO : this should be called in get()
+  synchronized void increaseFileReaderReference(TsFileResource tsFile, boolean isClosed) {
     tsFile.getWriteQueryLock().readLock().lock();
-    synchronized (this) {
-      if (!isClosed) {
-        unclosedReferenceMap.computeIfAbsent(tsFile, k -> new AtomicInteger()).getAndIncrement();
-      } else {
-        closedReferenceMap.computeIfAbsent(tsFile, k -> new AtomicInteger()).getAndIncrement();
-      }
+    if (!isClosed) {
+      unclosedReferenceMap.computeIfAbsent(tsFile, k -> new AtomicInteger()).getAndIncrement();
+    } else {
+      closedReferenceMap.computeIfAbsent(tsFile, k -> new AtomicInteger()).getAndIncrement();
     }
   }
 
@@ -180,13 +177,11 @@ public class FileReaderManager implements IService {
    * Decrease the reference count of the reader specified by filePath. This method is latch-free.
    * Only when the reference count of a reader equals zero, the reader can be closed and removed.
    */
-  void decreaseFileReaderReference(TsFileResource tsFile, boolean isClosed) {
-    synchronized (this) {
-      if (!isClosed && unclosedReferenceMap.containsKey(tsFile)) {
-        unclosedReferenceMap.get(tsFile).getAndDecrement();
-      } else if (closedReferenceMap.containsKey(tsFile)){
-        closedReferenceMap.get(tsFile).getAndDecrement();
-      }
+  synchronized void decreaseFileReaderReference(TsFileResource tsFile, boolean isClosed) {
+    if (!isClosed && unclosedReferenceMap.containsKey(tsFile)) {
+      unclosedReferenceMap.get(tsFile).getAndDecrement();
+    } else if (closedReferenceMap.containsKey(tsFile)) {
+      closedReferenceMap.get(tsFile).getAndDecrement();
     }
     tsFile.getWriteQueryLock().readLock().unlock();
   }
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 5c58351..74ca896 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
@@ -48,7 +48,7 @@ public class QueryFileManager {
    * Set job id for current request thread. When a query request is created firstly,
    * this method must be invoked.
    */
-  void addQueryId(long queryId) {
+  synchronized void addQueryId(long queryId) {
     sealedFilePathsMap.computeIfAbsent(queryId, x -> new HashSet<>());
     unsealedFilePathsMap.computeIfAbsent(queryId, x -> new HashSet<>());
   }
@@ -109,10 +109,9 @@ public class QueryFileManager {
    * so <code>sealedFilePathsMap.get(queryId)</code> or <code>unsealedFilePathsMap.get(queryId)</code>
    * must not return null.
    */
-  void addFilePathToMap(long queryId, TsFileResource tsFile, boolean isClosed) {
+  synchronized void addFilePathToMap(long queryId, TsFileResource tsFile, boolean isClosed) {
     Map<Long, Set<TsFileResource>> pathMap = isClosed ? unsealedFilePathsMap :
         sealedFilePathsMap;
-    //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);
diff --git a/tsfile/src/main/java/org/apache/iotdb/tsfile/read/TsFileSequenceReader.java b/tsfile/src/main/java/org/apache/iotdb/tsfile/read/TsFileSequenceReader.java
index 3b18933..1e15713 100644
--- a/tsfile/src/main/java/org/apache/iotdb/tsfile/read/TsFileSequenceReader.java
+++ b/tsfile/src/main/java/org/apache/iotdb/tsfile/read/TsFileSequenceReader.java
@@ -28,6 +28,7 @@ import java.util.Comparator;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 import org.apache.iotdb.tsfile.common.conf.TSFileConfig;
 import org.apache.iotdb.tsfile.common.conf.TSFileDescriptor;
 import org.apache.iotdb.tsfile.compress.IUnCompressor;
@@ -113,7 +114,7 @@ public class TsFileSequenceReader implements AutoCloseable {
     this(file, loadMetadata);
     this.cacheDeviceMetadata = cacheDeviceMetadata;
     if (cacheDeviceMetadata) {
-      deviceMetadataMap = new HashMap<>();
+      deviceMetadataMap = new ConcurrentHashMap<>();
     }
   }