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 2022/08/29 03:46:20 UTC

[GitHub] [iotdb] Cpaulyz opened a new pull request, #7154: [IOTDB-3949] Sync data collection process in standalone version

Cpaulyz opened a new pull request, #7154:
URL: https://github.com/apache/iotdb/pull/7154

   WIP


-- 
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: reviews-unsubscribe@iotdb.apache.org

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


[GitHub] [iotdb] Cpaulyz commented on a diff in pull request #7154: [IOTDB-3224][IOTDB-3949] Sync pipe execution and data collection process in standalone version

Posted by GitBox <gi...@apache.org>.
Cpaulyz commented on code in PR #7154:
URL: https://github.com/apache/iotdb/pull/7154#discussion_r962509655


##########
server/src/main/java/org/apache/iotdb/db/engine/storagegroup/DataRegion.java:
##########
@@ -2375,8 +2374,9 @@ private void deleteDataInFiles(
         tsFileResource.getProcessor().deleteDataInMemory(deletion, devicePaths);
       }
 
-      if (tsFileSyncManager.isEnableSync()) {
-        tsFileSyncManager.collectRealTimeDeletion(deletion, storageGroupName);
+      for (ISyncManager syncManager :
+          SyncService.getInstance().getOrCreateSyncManager(dataRegionId)) {
+        syncManager.syncRealTimeDeletion(deletion);

Review Comment:
   SyncService is aways enabled☑️



-- 
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: reviews-unsubscribe@iotdb.apache.org

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


[GitHub] [iotdb] Cpaulyz commented on a diff in pull request #7154: [IOTDB-3224][IOTDB-3949] Sync pipe execution and data collection process in standalone version

Posted by GitBox <gi...@apache.org>.
Cpaulyz commented on code in PR #7154:
URL: https://github.com/apache/iotdb/pull/7154#discussion_r962507323


##########
node-commons/src/main/java/org/apache/iotdb/commons/sync/SyncConstant.java:
##########
@@ -54,7 +56,7 @@ public class SyncConstant {
 
   // data config
   public static final String DEFAULT_PIPE_SINK_IP = "127.0.0.1";
-  public static final int DEFAULT_PIPE_SINK_PORT = 6670;
+  public static final int DEFAULT_PIPE_SINK_PORT = 6667;

Review Comment:
   Because `thrift-sync` has been merged into ClientRPC(#7004) and default ClientRPC port is 6667.



-- 
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: reviews-unsubscribe@iotdb.apache.org

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


[GitHub] [iotdb] HTHou commented on a diff in pull request #7154: [IOTDB-3224][IOTDB-3949] Sync pipe execution and data collection process in standalone version

Posted by GitBox <gi...@apache.org>.
HTHou commented on code in PR #7154:
URL: https://github.com/apache/iotdb/pull/7154#discussion_r962455537


##########
server/src/main/java/org/apache/iotdb/db/sync/transport/client/SenderManager.java:
##########
@@ -96,7 +96,6 @@ private void takePipeDataAndTransport() {
                       PipeMessage.MsgType.WARN,
                       String.format(
                           "Transfer piepdata %s error, skip it.", pipeData.getSerialNumber()));
-              continue;

Review Comment:
   Why do you remove this?



##########
server/src/main/java/org/apache/iotdb/db/engine/storagegroup/DataRegion.java:
##########
@@ -2375,8 +2374,9 @@ private void deleteDataInFiles(
         tsFileResource.getProcessor().deleteDataInMemory(deletion, devicePaths);
       }
 
-      if (tsFileSyncManager.isEnableSync()) {
-        tsFileSyncManager.collectRealTimeDeletion(deletion, storageGroupName);
+      for (ISyncManager syncManager :
+          SyncService.getInstance().getOrCreateSyncManager(dataRegionId)) {
+        syncManager.syncRealTimeDeletion(deletion);

Review Comment:
   SyncService is aways enabled? 



##########
node-commons/src/main/java/org/apache/iotdb/commons/sync/SyncConstant.java:
##########
@@ -54,7 +56,7 @@ public class SyncConstant {
 
   // data config
   public static final String DEFAULT_PIPE_SINK_IP = "127.0.0.1";
-  public static final int DEFAULT_PIPE_SINK_PORT = 6670;
+  public static final int DEFAULT_PIPE_SINK_PORT = 6667;

Review Comment:
   Why change the port to `6667`?



-- 
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: reviews-unsubscribe@iotdb.apache.org

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


[GitHub] [iotdb] HTHou merged pull request #7154: [IOTDB-3224][IOTDB-3949] Sync pipe execution and data collection process in standalone version

Posted by GitBox <gi...@apache.org>.
HTHou merged PR #7154:
URL: https://github.com/apache/iotdb/pull/7154


-- 
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: reviews-unsubscribe@iotdb.apache.org

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


[GitHub] [iotdb] Cpaulyz commented on a diff in pull request #7154: [IOTDB-3224][IOTDB-3949] Sync pipe execution and data collection process in standalone version

Posted by GitBox <gi...@apache.org>.
Cpaulyz commented on code in PR #7154:
URL: https://github.com/apache/iotdb/pull/7154#discussion_r962508432


##########
server/src/main/java/org/apache/iotdb/db/sync/transport/client/SenderManager.java:
##########
@@ -96,7 +96,6 @@ private void takePipeDataAndTransport() {
                       PipeMessage.MsgType.WARN,
                       String.format(
                           "Transfer piepdata %s error, skip it.", pipeData.getSerialNumber()));
-              continue;

Review Comment:
   If continue directly when failed to `send(pipeData),` the call to the `commit()` interface will be missed, resulting in a dead loop.



-- 
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: reviews-unsubscribe@iotdb.apache.org

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