You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by GitBox <gi...@apache.org> on 2020/07/22 13:51:01 UTC

[GitHub] [incubator-iotdb] HTHou commented on a change in pull request #1524: [IOTDB-776] Control the memory usage of flushing the memtable

HTHou commented on a change in pull request #1524:
URL: https://github.com/apache/incubator-iotdb/pull/1524#discussion_r458807037



##########
File path: server/src/main/java/org/apache/iotdb/db/engine/storagegroup/TsFileProcessor.java
##########
@@ -241,34 +272,91 @@ public void insertTablet(InsertTabletPlan insertTabletPlan, int start, int end,
       workMemTable = MemTablePool.getInstance().getAvailableMemTable(this);
     }
 
-    // insert insertRowPlan to the work memtable
-    try {
-      workMemTable.insertTablet(insertTabletPlan, start, end);
-      if (IoTDBDescriptor.getInstance().getConfig().isEnableWal()) {
-        insertTabletPlan.setStart(start);
-        insertTabletPlan.setEnd(end);
-        getLogNode().write(insertTabletPlan);
+    if (workMemTable.checkIfArrayIsEnough(insertTabletPlan)) {
+      // insert insertRowPlan to the work memtable
+      try {
+        workMemTable.insertTablet(insertTabletPlan, start, end);
+        if (IoTDBDescriptor.getInstance().getConfig().isEnableWal()) {
+          insertTabletPlan.setStart(start);
+          insertTabletPlan.setEnd(end);
+          getLogNode().write(insertTabletPlan);
+        }
+      } catch (Exception e) {
+        for (int i = start; i < end; i++) {
+          results[i] = RpcUtils.getStatus(TSStatusCode.INTERNAL_SERVER_ERROR, e.getMessage());
+        }
+        throw new WriteProcessException(e);
       }
-    } catch (Exception e) {
+  
       for (int i = start; i < end; i++) {
-        results[i] = RpcUtils.getStatus(TSStatusCode.INTERNAL_SERVER_ERROR, e.getMessage());
+        results[i] = RpcUtils.SUCCESS_STATUS;
+      }
+  
+      tsFileResource
+          .updateStartTime(insertTabletPlan.getDeviceId(), insertTabletPlan.getTimes()[start]);
+  
+      //for sequence tsfile, we update the endTime only when the file is prepared to be closed.
+      //for unsequence tsfile, we have to update the endTime for each insertion.
+      if (!sequence) {
+        tsFileResource
+            .updateEndTime(
+                insertTabletPlan.getDeviceId(), insertTabletPlan.getTimes()[end - 1]);
       }
-      throw new WriteProcessException(e);
     }
-
-    for (int i = start; i < end; i++) {
-      results[i] = RpcUtils.SUCCESS_STATUS;
+    else {
+      // if there are available buffered arrays in array pool
+      boolean isbufferedArrayEnough = true;
+      // TODO: isbufferedArrayEnough = checkBufferedArray();

Review comment:
       > Hi, according to previous design, `checkBufferedArray` is to be done by `ArrayManager` module.
   > 
   > When TSP is asking for one array, it could call `ArrayManager.getDataListByType` (and it is the only interface for asking array). This method will check whether there are enough buffered arrays and return one if true; or apply an out of buffer array and return one if system module agrees; or return null if system module refuses to offer one.
   > 
   > In this way, TSP does not need to know whether the array is buffered or out of buffer. The management of arrays is responsibility of `ArrayManager`. What do you think?
   
   Yes, you are right. That's why I write `TODO` here. I'll fix it very soon. Thanks. :)




----------------------------------------------------------------
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:
users@infra.apache.org