[GitHub] [iotdb] CRZbulabula commented on a diff in pull request #6328: [IOTDB-3522] Set heartbeat as a periodic scheduled task

[GitBox <gi...@apache.org>]

CRZbulabula commented on code in PR #6328:
URL: https://github.com/apache/iotdb/pull/6328#discussion_r901195964


##########
confignode/src/main/java/org/apache/iotdb/confignode/manager/load/LoadManager.java:
##########
@@ -128,32 +146,59 @@ public Map<String, DataPartitionTable> allocateDataPartition(
     return partitionBalancer.allocateDataPartition(unassignedDataPartitionSlotsMap);
   }
 
-  @Override
-  public void run() {
-    int balanceCount = 0;
-    while (true) {
-      try {
-
-        if (getConsensusManager().isLeader()) {
-          // Send heartbeat requests to all the online DataNodes
-          pingOnlineDataNodes(getNodeManager().getOnlineDataNodes(-1));
-          // TODO: Send heartbeat requests to all the online ConfigNodes
-
-          // Do load balancing
-          doLoadBalancing(balanceCount);
-          balanceCount += 1;
-        } else {
-          // Discard all cache when current ConfigNode is not longer the leader
-          heartbeatCacheMap.clear();
-          balanceCount = 0;
-        }
+  /** start the heartbeat service */
+  public void start() {
+    LOGGER.debug("Start Heartbeat Service of LoadManager");
+    synchronized (isRunningMonitor) {
+      isRunning = true;
+      // only 1 thread can start the heartbeat service
+      isRunningMonitor.notifyAll();
+    }
+  }
+
+  /** stop the heartbeat service */
+  public void stop() {
+    LOGGER.debug("Stop Heartbeat Service of LoadManager");
+    synchronized (isRunningMonitor) {
+      isRunning = false;
+    }
+  }
 
-        TimeUnit.MILLISECONDS.sleep(heartbeatInterval);
-      } catch (InterruptedException e) {
-        LOGGER.error("Heartbeat thread has been interrupted, stopping ConfigNode...", e);
-        System.exit(-1);
+  /** loop body of the heartbeat thread */
+  private void heartbeatLoopBody() {
+    synchronized (isRunningMonitor) {
+      // avoid spurious wakeup
+      while (!isRunning) {
+        // not longer the leader, discard all cache
+        heartbeatCacheMap.clear();
+        balanceCount = 0;
+        try {
+          isRunningMonitor.wait();
+        } catch (InterruptedException e) {
+          throw new RuntimeException(e);
+        }

Review Comment:
   Perhaps we should stop the heartbeat service rather then block some heartbeat task?



##########
confignode/src/main/java/org/apache/iotdb/confignode/manager/ConfigManager.java:
##########
@@ -143,7 +143,7 @@ public ConfigManager() throws IOException {
     // We are on testing.......
     if (ConfigNodeDescriptor.getInstance().getConf().isEnableHeartbeat()) {
       // Start asking for heartbeat
-      new Thread(this.loadManager).start();
+      this.loadManager.start();
     }
   }

Review Comment:
   This code block should move to PartitionRegionStateMachine.notifyLeaderChanged



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