You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by "SzyWilliam (via GitHub)" <gi...@apache.org> on 2025/02/17 09:15:54 UTC
[PR] [RTO/RPO] Add Phi Accrual for Node failure detection [iotdb]
SzyWilliam opened a new pull request, #14866:
URL: https://github.com/apache/iotdb/pull/14866
See the phi accrual algo at https://doc.akka.io/libraries/akka-core/current/typed/failure-detector.html
See the overall design at https://apache-iotdb-project.feishu.cn/docx/RdRhdAwUOoeccBx2FfzcHG00nwd
--
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
Re: [PR] [RTO/RPO] Add Phi Accrual for Node failure detection [iotdb]
Posted by "SzyWilliam (via GitHub)" <gi...@apache.org>.
SzyWilliam commented on code in PR #14866:
URL: https://github.com/apache/iotdb/pull/14866#discussion_r1962771598
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/cache/node/AINodeHeartbeatCache.java:
##########
@@ -54,7 +56,8 @@ public void updateCurrentStatistics(boolean forceUpdate) {
long currentNanoTime = System.nanoTime();
if (lastSample != null && NodeStatus.Removing.equals(lastSample.getStatus())) {
status = NodeStatus.Removing;
- } else if (currentNanoTime - lastSendTime > HEARTBEAT_TIMEOUT_TIME_IN_NS) {
+ } else if (failureDetector.isAvailable(heartbeatHistory)) {
Review Comment:
Good Catch! Thanks very much for the careful review.
--
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
Re: [PR] [RTO/RPO] Add Phi Accrual for Node failure detection [iotdb]
Posted by "SzyWilliam (via GitHub)" <gi...@apache.org>.
SzyWilliam commented on code in PR #14866:
URL: https://github.com/apache/iotdb/pull/14866#discussion_r1971724921
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/cache/detector/PhiAccrualDetector.java:
##########
@@ -54,8 +56,21 @@ public boolean isAvailable(List<AbstractHeartbeatSample> history) {
return true;
}
final PhiAccrual phiAccrual = create(history);
- // TODO(szywilliam) we should log the status change and dump the heartbeat history
- return phiAccrual.phi() < (double) this.threshold;
+ final boolean isAvailable = phiAccrual.phi() < (double) this.threshold;
+ if (!isAvailable) {
+ // log the status change and dump the heartbeat history for analysis use
+ final StringBuilder builder = new StringBuilder();
+ builder.append("[");
+ for (double interval : phiAccrual.heartbeatIntervals) {
+ final long msInterval = (long) interval / 1000_000;
+ builder.append(msInterval).append(", ");
+ }
+ builder.append(phiAccrual.timeElapsedSinceLastHeartbeat / 1000_000);
+ builder.append("]");
+ LOGGER.info(String.format("Node Down, heartbeat history (ms): %s", builder));
+ }
+
Review Comment:
It is auto-generated by spotless.
--
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
Re: [PR] [RTO/RPO] Add Phi Accrual for Node failure detection [iotdb]
Posted by "SzyWilliam (via GitHub)" <gi...@apache.org>.
SzyWilliam commented on code in PR #14866:
URL: https://github.com/apache/iotdb/pull/14866#discussion_r1970021804
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/cache/detector/PhiAccrualDetector.java:
##########
@@ -0,0 +1,160 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.confignode.manager.load.cache.detector;
+
+import org.apache.iotdb.confignode.manager.load.cache.AbstractHeartbeatSample;
+import org.apache.iotdb.confignode.manager.load.cache.IFailureDetector;
+import org.apache.iotdb.confignode.manager.load.cache.node.NodeHeartbeatSample;
+
+import org.apache.commons.lang3.ArrayUtils;
+import org.apache.commons.math3.stat.descriptive.DescriptiveStatistics;
+import org.apache.tsfile.utils.Preconditions;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public class PhiAccrualDetector implements IFailureDetector {
+
+ private final long threshold;
+ private final long acceptableHeartbeatPauseNs;
+ private final long firstHeartbeatEstimateNs;
+ private final long minHeartbeatStdNs;
+
+ public PhiAccrualDetector(
+ long threshold,
+ long acceptableHeartbeatPauseNs,
+ long firstHeartbeatEstimateNs,
+ long minHeartbeatStdNs) {
+ this.threshold = threshold;
+ this.acceptableHeartbeatPauseNs = acceptableHeartbeatPauseNs;
+ this.firstHeartbeatEstimateNs = firstHeartbeatEstimateNs;
+ this.minHeartbeatStdNs = minHeartbeatStdNs;
+ }
+
+ @Override
+ public boolean isAvailable(List<AbstractHeartbeatSample> history) {
+ if (history.isEmpty()) {
+ /* We haven't received the first heartbeat reply. We cannot decide the node availability. */
+ return true;
+ }
+ final PhiAccrual phiAccrual = create(history);
+ // TODO(szywilliam) we should log the status change and dump the heartbeat history
Review Comment:
Done
--
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
Re: [PR] [RTO/RPO] Add Phi Accrual for Node failure detection [iotdb]
Posted by "SzyWilliam (via GitHub)" <gi...@apache.org>.
SzyWilliam commented on code in PR #14866:
URL: https://github.com/apache/iotdb/pull/14866#discussion_r1962773625
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/cache/detector/PhiAccrualDetector.java:
##########
@@ -0,0 +1,160 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.confignode.manager.load.cache.detector;
+
+import org.apache.iotdb.confignode.manager.load.cache.AbstractHeartbeatSample;
+import org.apache.iotdb.confignode.manager.load.cache.IFailureDetector;
+import org.apache.iotdb.confignode.manager.load.cache.node.NodeHeartbeatSample;
+
+import org.apache.commons.lang3.ArrayUtils;
+import org.apache.commons.math3.stat.descriptive.DescriptiveStatistics;
+import org.apache.tsfile.utils.Preconditions;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public class PhiAccrualDetector implements IFailureDetector {
+
+ private final long threshold;
+ private final long acceptableHeartbeatPauseNs;
+ private final long firstHeartbeatEstimateNs;
+ private final long minHeartbeatStdNs;
+
+ public PhiAccrualDetector(
+ long threshold,
+ long acceptableHeartbeatPauseNs,
+ long firstHeartbeatEstimateNs,
+ long minHeartbeatStdNs) {
+ this.threshold = threshold;
+ this.acceptableHeartbeatPauseNs = acceptableHeartbeatPauseNs;
+ this.firstHeartbeatEstimateNs = firstHeartbeatEstimateNs;
+ this.minHeartbeatStdNs = minHeartbeatStdNs;
+ }
+
+ @Override
+ public boolean isAvailable(List<AbstractHeartbeatSample> history) {
+ if (history.isEmpty()) {
+ /* We haven't received the first heartbeat reply. We cannot decide the node availability. */
+ return true;
Review Comment:
When a node starts and haven't reply 1st heartbeat, the status should be UNKNOWN and unchanged unless heartbeat received, right?
--
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
Re: [PR] [RTO/RPO] Add Phi Accrual for Node failure detection [iotdb]
Posted by "OneSizeFitsQuorum (via GitHub)" <gi...@apache.org>.
OneSizeFitsQuorum commented on code in PR #14866:
URL: https://github.com/apache/iotdb/pull/14866#discussion_r1971293325
##########
iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/manager/load/cache/detector/DetectorTest.java:
##########
@@ -0,0 +1,157 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.confignode.manager.load.cache.detector;
+
+import org.apache.iotdb.commons.cluster.NodeStatus;
+import org.apache.iotdb.confignode.manager.load.cache.AbstractHeartbeatSample;
+import org.apache.iotdb.confignode.manager.load.cache.node.NodeHeartbeatSample;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.Collections;
+import java.util.LinkedList;
+import java.util.List;
+
+public class DetectorTest {
+
+ final long sec = 1_000_000_000L;
+ final PhiAccrualDetector phiAccrualDetector =
+ new PhiAccrualDetector(35, 10 * sec, sec, (long) (0.2 * sec));
+ final FixedDetector fixedDetector = new FixedDetector(20 * sec);
+
+ private double getPhi(long elapsed, double[] intervals, long minStd, long pause) {
+ final PhiAccrualDetector.PhiAccrual p =
+ new PhiAccrualDetector.PhiAccrual(intervals, elapsed, minStd, pause);
+ return p.phi();
+ }
+
+ private void assertInRange(double value, double start, double end) {
+ Assert.assertTrue(value > start);
+ Assert.assertTrue(value < end);
+ }
+
+ @Test
+ public void testFixedDetector() {
+ final long lastHeartbeatTs = System.nanoTime() - 21 * sec;
+ final List<AbstractHeartbeatSample> history =
+ Collections.singletonList(new NodeHeartbeatSample(lastHeartbeatTs, NodeStatus.Unknown));
+ Assert.assertFalse(fixedDetector.isAvailable(history));
+ }
+
+ @Test
+ public void testPhiCalculation1() {
+ /* (min, std, acceptable_pause) = (1000, 200, 0) */
+ final double[] heartbeatIntervals = {1000, 1000, 1000, 1000, 1000};
+ final long minStd = 200;
+ final long pause = 0;
+
+ assertInRange(getPhi(1000, heartbeatIntervals, minStd, pause), 0, 1);
+ assertInRange(getPhi(2000, heartbeatIntervals, minStd, pause), 5, 10);
+ assertInRange(getPhi(3000, heartbeatIntervals, minStd, pause), 35, 50);
+ }
+
+ @Test
+ public void testPhiCalculation2() {
+ /* (min, std, acceptable_pause) = (1000, 300, 0) */
+ /* (min, std, acceptable_pause) = (1000, 300, 5000) */
Review Comment:
where is 5000?
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/cache/detector/PhiAccrualDetector.java:
##########
@@ -0,0 +1,180 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.confignode.manager.load.cache.detector;
+
+import org.apache.iotdb.confignode.manager.load.cache.AbstractHeartbeatSample;
+import org.apache.iotdb.confignode.manager.load.cache.IFailureDetector;
+import org.apache.iotdb.confignode.manager.load.cache.node.NodeHeartbeatSample;
+
+import org.apache.commons.math3.stat.descriptive.DescriptiveStatistics;
+import org.apache.tsfile.utils.Preconditions;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public class PhiAccrualDetector implements IFailureDetector {
+ private static final Logger LOGGER = LoggerFactory.getLogger(PhiAccrualDetector.class);
+ private final long threshold;
+ private final long acceptableHeartbeatPauseNs;
+ private final long firstHeartbeatEstimateNs;
+ private final long minHeartbeatStdNs;
+
+ public PhiAccrualDetector(
+ long threshold,
+ long acceptableHeartbeatPauseNs,
+ long firstHeartbeatEstimateNs,
+ long minHeartbeatStdNs) {
+ this.threshold = threshold;
+ this.acceptableHeartbeatPauseNs = acceptableHeartbeatPauseNs;
+ this.firstHeartbeatEstimateNs = firstHeartbeatEstimateNs;
+ this.minHeartbeatStdNs = minHeartbeatStdNs;
+ }
+
+ @Override
+ public boolean isAvailable(List<AbstractHeartbeatSample> history) {
+ if (history.isEmpty()) {
+ /* We haven't received the first heartbeat reply. We cannot decide the node availability. */
+ return true;
+ }
+ final PhiAccrual phiAccrual = create(history);
+ final boolean isAvailable = phiAccrual.phi() < (double) this.threshold;
+ if (!isAvailable) {
+ // log the status change and dump the heartbeat history for analysis use
+ final StringBuilder builder = new StringBuilder();
+ builder.append("[");
+ for (double interval : phiAccrual.heartbeatIntervals) {
+ final long msInterval = (long) interval / 1000_000;
+ builder.append(msInterval).append(", ");
+ }
+ builder.append(phiAccrual.timeElapsedSinceLastHeartbeat / 1000_000);
+ builder.append("]");
+ LOGGER.info(String.format("Node Down, heartbeat history (ms): %s", builder));
+ }
+
+ return isAvailable;
+ }
+
+ PhiAccrual create(List<AbstractHeartbeatSample> history) {
+ final int size = history.size();
+
+ final List<Double> heartbeatIntervals = new ArrayList<>();
+ /*
+ * During cold start, the heartbeat history may contain not enough samples for wise decisions.
+ * Therefore, we manually add data samples for better estimation.
+ * 1. mean = heartbeat interval (1000ms)
+ * 2. std = mean / 4
+ */
+ if (size <= 2) {
Review Comment:
try to make the cold start process smoother
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/cache/region/RegionCache.java:
##########
@@ -44,8 +44,7 @@ public synchronized void updateCurrentStatistics(boolean forceUpdate) {
long currentNanoTime = System.nanoTime();
if (lastSample == null) {
status = RegionStatus.Unknown;
- } else if (currentNanoTime - lastSample.getSampleLogicalTimestamp()
- > HEARTBEAT_TIMEOUT_TIME_IN_NS) {
+ } else if (currentNanoTime - lastSample.getSampleLogicalTimestamp() > heartbeatTimeoutNs) {
Review Comment:
try to use same detector
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/cache/AbstractLoadCache.java:
##########
@@ -33,16 +38,35 @@ public abstract class AbstractLoadCache {
// Max heartbeat cache samples store size
private static final int MAXIMUM_WINDOW_SIZE = 100;
// The Status will be set to Unknown when the response time of heartbeat is more than 20s
- protected static final long HEARTBEAT_TIMEOUT_TIME_IN_NS = 20_000_000_000L;
+ protected final long heartbeatTimeoutNs;
// Caching the recent MAXIMUM_WINDOW_SIZE heartbeat sample
protected final List<AbstractHeartbeatSample> slidingWindow;
// The current statistics calculated by the latest heartbeat sample
protected final AtomicReference<AbstractStatistics> currentStatistics;
+ protected final IFailureDetector failureDetector;
+
+ private static final ConfigNodeConfig CONF = ConfigNodeDescriptor.getInstance().getConf();
+
protected AbstractLoadCache() {
this.currentStatistics = new AtomicReference<>();
this.slidingWindow = Collections.synchronizedList(new LinkedList<>());
+ this.heartbeatTimeoutNs = CONF.getFailureDetectorFixedThresholdInMs() * 1000_000L;
+ switch (CONF.getFailureDetector()) {
+ case IFailureDetector.PHI_ACCRUAL_DETECTOR:
+ this.failureDetector =
+ new PhiAccrualDetector(
+ CONF.getFailureDetectorPhiThreshold(),
+ CONF.getFailureDetectorPhiAcceptablePauseInMs() * 1000_000L,
+ CONF.getHeartbeatIntervalInMs() * 1000_000L,
+ CONF.getFailureDetectorPhiAcceptablePauseInMs() * 1000_000L);
Review Comment:
too big for std???
##########
iotdb-core/node-commons/src/assembly/resources/conf/iotdb-system.properties.template:
##########
@@ -684,6 +684,26 @@ time_partition_interval=604800000
# Datatype: long
heartbeat_interval_in_ms=1000
+# Default failure detector, enum from {fixed, phi_accrual}
+# effectiveMode: restart
+# Datatype: string
+failure_detector=fixed
+
+# Fixed failure detector threshold of time elapsed without receiving heartbeat replies
+# effectiveMode: restart
+# Datatype: long
+failure_detector_fixed_threshold_in_ms=20000
+
+# Phi accrual: threshold to determine a node is down
+# effectiveMode: restart
+# Datatype: long
+failure_detector_phi_threshold=30
Review Comment:
Got it
##########
iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/manager/load/cache/detector/DetectorTest.java:
##########
@@ -0,0 +1,157 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.confignode.manager.load.cache.detector;
+
+import org.apache.iotdb.commons.cluster.NodeStatus;
+import org.apache.iotdb.confignode.manager.load.cache.AbstractHeartbeatSample;
+import org.apache.iotdb.confignode.manager.load.cache.node.NodeHeartbeatSample;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.Collections;
+import java.util.LinkedList;
+import java.util.List;
+
+public class DetectorTest {
Review Comment:
add cold start test
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/cache/AbstractLoadCache.java:
##########
@@ -40,9 +45,27 @@ public abstract class AbstractLoadCache {
// The current statistics calculated by the latest heartbeat sample
Review Comment:
For the regionCache, it's logical clock used nanotime too. It's best to unify them, or you might end up with a strange state where the node is unknown but the region is running
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/cache/detector/PhiAccrualDetector.java:
##########
@@ -54,8 +56,21 @@ public boolean isAvailable(List<AbstractHeartbeatSample> history) {
return true;
}
final PhiAccrual phiAccrual = create(history);
- // TODO(szywilliam) we should log the status change and dump the heartbeat history
- return phiAccrual.phi() < (double) this.threshold;
+ final boolean isAvailable = phiAccrual.phi() < (double) this.threshold;
+ if (!isAvailable) {
+ // log the status change and dump the heartbeat history for analysis use
+ final StringBuilder builder = new StringBuilder();
+ builder.append("[");
+ for (double interval : phiAccrual.heartbeatIntervals) {
+ final long msInterval = (long) interval / 1000_000;
+ builder.append(msInterval).append(", ");
+ }
+ builder.append(phiAccrual.timeElapsedSinceLastHeartbeat / 1000_000);
+ builder.append("]");
+ LOGGER.info(String.format("Node Down, heartbeat history (ms): %s", builder));
+ }
+
Review Comment:
remove empty line
--
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
Re: [PR] [RTO/RPO] Add Phi Accrual for Node failure detection [iotdb]
Posted by "SzyWilliam (via GitHub)" <gi...@apache.org>.
SzyWilliam commented on code in PR #14866:
URL: https://github.com/apache/iotdb/pull/14866#discussion_r1971767999
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/cache/detector/PhiAccrualDetector.java:
##########
@@ -0,0 +1,180 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.confignode.manager.load.cache.detector;
+
+import org.apache.iotdb.confignode.manager.load.cache.AbstractHeartbeatSample;
+import org.apache.iotdb.confignode.manager.load.cache.IFailureDetector;
+import org.apache.iotdb.confignode.manager.load.cache.node.NodeHeartbeatSample;
+
+import org.apache.commons.math3.stat.descriptive.DescriptiveStatistics;
+import org.apache.tsfile.utils.Preconditions;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public class PhiAccrualDetector implements IFailureDetector {
+ private static final Logger LOGGER = LoggerFactory.getLogger(PhiAccrualDetector.class);
+ private final long threshold;
+ private final long acceptableHeartbeatPauseNs;
+ private final long firstHeartbeatEstimateNs;
+ private final long minHeartbeatStdNs;
+
+ public PhiAccrualDetector(
+ long threshold,
+ long acceptableHeartbeatPauseNs,
+ long firstHeartbeatEstimateNs,
+ long minHeartbeatStdNs) {
+ this.threshold = threshold;
+ this.acceptableHeartbeatPauseNs = acceptableHeartbeatPauseNs;
+ this.firstHeartbeatEstimateNs = firstHeartbeatEstimateNs;
+ this.minHeartbeatStdNs = minHeartbeatStdNs;
+ }
+
+ @Override
+ public boolean isAvailable(List<AbstractHeartbeatSample> history) {
+ if (history.isEmpty()) {
+ /* We haven't received the first heartbeat reply. We cannot decide the node availability. */
+ return true;
+ }
+ final PhiAccrual phiAccrual = create(history);
+ final boolean isAvailable = phiAccrual.phi() < (double) this.threshold;
+ if (!isAvailable) {
+ // log the status change and dump the heartbeat history for analysis use
+ final StringBuilder builder = new StringBuilder();
+ builder.append("[");
+ for (double interval : phiAccrual.heartbeatIntervals) {
+ final long msInterval = (long) interval / 1000_000;
+ builder.append(msInterval).append(", ");
+ }
+ builder.append(phiAccrual.timeElapsedSinceLastHeartbeat / 1000_000);
+ builder.append("]");
+ LOGGER.info(String.format("Node Down, heartbeat history (ms): %s", builder));
+ }
+
+ return isAvailable;
+ }
+
+ PhiAccrual create(List<AbstractHeartbeatSample> history) {
+ final int size = history.size();
+
+ final List<Double> heartbeatIntervals = new ArrayList<>();
+ /*
+ * During cold start, the heartbeat history may contain not enough samples for wise decisions.
+ * Therefore, we manually add data samples for better estimation.
+ * 1. mean = heartbeat interval (1000ms)
+ * 2. std = mean / 4
+ */
+ if (size <= 2) {
Review Comment:
Let's assume that once the first heartbeat is received, the node is probably up in the next 10 secs. So we can start the phi accrual after 10 heartbeats.
--
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
Re: [PR] [RTO/RPO] Add Phi Accrual for Node failure detection [iotdb]
Posted by "SzyWilliam (via GitHub)" <gi...@apache.org>.
SzyWilliam commented on code in PR #14866:
URL: https://github.com/apache/iotdb/pull/14866#discussion_r1969567205
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/cache/AbstractLoadCache.java:
##########
@@ -40,9 +45,27 @@ public abstract class AbstractLoadCache {
// The current statistics calculated by the latest heartbeat sample
Review Comment:
Sure let's remove `HEARTBEAT_TIMEOUT_TIME_IN_NS` and keep it same with `failure_detector_fixed_threshold_in_ms`.
It seems that we cannot simply apply Node failure to RegionCache. RegionCache uses logical timestamp instead of physical timestamp. Let's keep `regionCache` unchanged.
--
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
Re: [PR] [RTO/RPO] Add Phi Accrual for Node failure detection [iotdb]
Posted by "SzyWilliam (via GitHub)" <gi...@apache.org>.
SzyWilliam commented on code in PR #14866:
URL: https://github.com/apache/iotdb/pull/14866#discussion_r1970814846
##########
iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/manager/load/cache/detector/PhiAccrualDetectorTest.java:
##########
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.confignode.manager.load.cache.detector;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class PhiAccrualDetectorTest {
Review Comment:
Done
--
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
Re: [PR] [RTO/RPO] Add Phi Accrual for Node failure detection [iotdb]
Posted by "SzyWilliam (via GitHub)" <gi...@apache.org>.
SzyWilliam commented on code in PR #14866:
URL: https://github.com/apache/iotdb/pull/14866#discussion_r1972706193
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/cache/AbstractLoadCache.java:
##########
@@ -33,16 +38,35 @@ public abstract class AbstractLoadCache {
// Max heartbeat cache samples store size
private static final int MAXIMUM_WINDOW_SIZE = 100;
// The Status will be set to Unknown when the response time of heartbeat is more than 20s
- protected static final long HEARTBEAT_TIMEOUT_TIME_IN_NS = 20_000_000_000L;
+ protected final long heartbeatTimeoutNs;
// Caching the recent MAXIMUM_WINDOW_SIZE heartbeat sample
protected final List<AbstractHeartbeatSample> slidingWindow;
// The current statistics calculated by the latest heartbeat sample
protected final AtomicReference<AbstractStatistics> currentStatistics;
+ protected final IFailureDetector failureDetector;
+
+ private static final ConfigNodeConfig CONF = ConfigNodeDescriptor.getInstance().getConf();
+
protected AbstractLoadCache() {
this.currentStatistics = new AtomicReference<>();
this.slidingWindow = Collections.synchronizedList(new LinkedList<>());
+ this.heartbeatTimeoutNs = CONF.getFailureDetectorFixedThresholdInMs() * 1000_000L;
+ switch (CONF.getFailureDetector()) {
+ case IFailureDetector.PHI_ACCRUAL_DETECTOR:
+ this.failureDetector =
+ new PhiAccrualDetector(
+ CONF.getFailureDetectorPhiThreshold(),
+ CONF.getFailureDetectorPhiAcceptablePauseInMs() * 1000_000L,
+ CONF.getHeartbeatIntervalInMs() * 1000_000L,
+ CONF.getFailureDetectorPhiAcceptablePauseInMs() * 1000_000L);
Review Comment:
Fixed
##########
iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/manager/load/cache/detector/DetectorTest.java:
##########
@@ -0,0 +1,157 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.confignode.manager.load.cache.detector;
+
+import org.apache.iotdb.commons.cluster.NodeStatus;
+import org.apache.iotdb.confignode.manager.load.cache.AbstractHeartbeatSample;
+import org.apache.iotdb.confignode.manager.load.cache.node.NodeHeartbeatSample;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.Collections;
+import java.util.LinkedList;
+import java.util.List;
+
+public class DetectorTest {
Review Comment:
Done
--
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
Re: [PR] [RTO/RPO] Add Phi Accrual for Node failure detection [iotdb]
Posted by "SzyWilliam (via GitHub)" <gi...@apache.org>.
SzyWilliam commented on code in PR #14866:
URL: https://github.com/apache/iotdb/pull/14866#discussion_r1970082933
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/cache/detector/PhiAccrualDetector.java:
##########
@@ -0,0 +1,160 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.confignode.manager.load.cache.detector;
+
+import org.apache.iotdb.confignode.manager.load.cache.AbstractHeartbeatSample;
+import org.apache.iotdb.confignode.manager.load.cache.IFailureDetector;
+import org.apache.iotdb.confignode.manager.load.cache.node.NodeHeartbeatSample;
+
+import org.apache.commons.lang3.ArrayUtils;
+import org.apache.commons.math3.stat.descriptive.DescriptiveStatistics;
+import org.apache.tsfile.utils.Preconditions;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public class PhiAccrualDetector implements IFailureDetector {
+
+ private final long threshold;
+ private final long acceptableHeartbeatPauseNs;
+ private final long firstHeartbeatEstimateNs;
+ private final long minHeartbeatStdNs;
+
+ public PhiAccrualDetector(
+ long threshold,
+ long acceptableHeartbeatPauseNs,
+ long firstHeartbeatEstimateNs,
+ long minHeartbeatStdNs) {
+ this.threshold = threshold;
+ this.acceptableHeartbeatPauseNs = acceptableHeartbeatPauseNs;
+ this.firstHeartbeatEstimateNs = firstHeartbeatEstimateNs;
+ this.minHeartbeatStdNs = minHeartbeatStdNs;
+ }
+
+ @Override
+ public boolean isAvailable(List<AbstractHeartbeatSample> history) {
+ if (history.isEmpty()) {
+ /* We haven't received the first heartbeat reply. We cannot decide the node availability. */
+ return true;
+ }
+ final PhiAccrual phiAccrual = create(history);
+ // TODO(szywilliam) we should log the status change and dump the heartbeat history
+ return phiAccrual.phi() < (double) this.threshold;
+ }
+
+ PhiAccrual create(List<AbstractHeartbeatSample> history) {
+ final int size = history.size();
+
+ final List<Double> heartbeatIntervals = new ArrayList<>();
+ /*
+ * During cold start, the heartbeat history may contain not enough samples for wise decisions.
+ * Therefore, we manually add data samples for better estimation.
+ * 1. mean = heartbeat interval (1000ms)
+ * 2. std = mean / 4
+ */
+ if (size <= 2) {
+ double mean = firstHeartbeatEstimateNs;
+ double std = mean / 4.0;
+ heartbeatIntervals.add(mean + std);
+ heartbeatIntervals.add(mean - std);
+ }
+
+ for (int i = 1; i < size; i++) {
+ // ensure getSampleLogicalTimestamp() will return system nano timestamp
+ Preconditions.checkArgument(history.get(i) instanceof NodeHeartbeatSample);
+ double interval =
Review Comment:
Let's use iterator to avoid `get` call.
--
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
Re: [PR] [RTO/RPO] Add Phi Accrual for Node failure detection [iotdb]
Posted by "SzyWilliam (via GitHub)" <gi...@apache.org>.
SzyWilliam commented on code in PR #14866:
URL: https://github.com/apache/iotdb/pull/14866#discussion_r1972729590
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/cache/detector/PhiAccrualDetector.java:
##########
@@ -0,0 +1,160 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.confignode.manager.load.cache.detector;
+
+import org.apache.iotdb.confignode.manager.load.cache.AbstractHeartbeatSample;
+import org.apache.iotdb.confignode.manager.load.cache.IFailureDetector;
+import org.apache.iotdb.confignode.manager.load.cache.node.NodeHeartbeatSample;
+
+import org.apache.commons.lang3.ArrayUtils;
+import org.apache.commons.math3.stat.descriptive.DescriptiveStatistics;
+import org.apache.tsfile.utils.Preconditions;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public class PhiAccrualDetector implements IFailureDetector {
+
+ private final long threshold;
+ private final long acceptableHeartbeatPauseNs;
+ private final long firstHeartbeatEstimateNs;
+ private final long minHeartbeatStdNs;
+
+ public PhiAccrualDetector(
+ long threshold,
+ long acceptableHeartbeatPauseNs,
+ long firstHeartbeatEstimateNs,
+ long minHeartbeatStdNs) {
+ this.threshold = threshold;
+ this.acceptableHeartbeatPauseNs = acceptableHeartbeatPauseNs;
+ this.firstHeartbeatEstimateNs = firstHeartbeatEstimateNs;
+ this.minHeartbeatStdNs = minHeartbeatStdNs;
+ }
+
+ @Override
+ public boolean isAvailable(List<AbstractHeartbeatSample> history) {
+ if (history.isEmpty()) {
+ /* We haven't received the first heartbeat reply. We cannot decide the node availability. */
+ return true;
+ }
+ final PhiAccrual phiAccrual = create(history);
+ // TODO(szywilliam) we should log the status change and dump the heartbeat history
Review Comment:
You are really a born genius
--
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
Re: [PR] [RTO/RPO] Add Phi Accrual for Node failure detection [iotdb]
Posted by "SzyWilliam (via GitHub)" <gi...@apache.org>.
SzyWilliam commented on code in PR #14866:
URL: https://github.com/apache/iotdb/pull/14866#discussion_r1971720934
##########
iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/manager/load/cache/detector/DetectorTest.java:
##########
@@ -0,0 +1,157 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.confignode.manager.load.cache.detector;
+
+import org.apache.iotdb.commons.cluster.NodeStatus;
+import org.apache.iotdb.confignode.manager.load.cache.AbstractHeartbeatSample;
+import org.apache.iotdb.confignode.manager.load.cache.node.NodeHeartbeatSample;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.Collections;
+import java.util.LinkedList;
+import java.util.List;
+
+public class DetectorTest {
+
+ final long sec = 1_000_000_000L;
+ final PhiAccrualDetector phiAccrualDetector =
+ new PhiAccrualDetector(35, 10 * sec, sec, (long) (0.2 * sec));
+ final FixedDetector fixedDetector = new FixedDetector(20 * sec);
+
+ private double getPhi(long elapsed, double[] intervals, long minStd, long pause) {
+ final PhiAccrualDetector.PhiAccrual p =
+ new PhiAccrualDetector.PhiAccrual(intervals, elapsed, minStd, pause);
+ return p.phi();
+ }
+
+ private void assertInRange(double value, double start, double end) {
+ Assert.assertTrue(value > start);
+ Assert.assertTrue(value < end);
+ }
+
+ @Test
+ public void testFixedDetector() {
+ final long lastHeartbeatTs = System.nanoTime() - 21 * sec;
+ final List<AbstractHeartbeatSample> history =
+ Collections.singletonList(new NodeHeartbeatSample(lastHeartbeatTs, NodeStatus.Unknown));
+ Assert.assertFalse(fixedDetector.isAvailable(history));
+ }
+
+ @Test
+ public void testPhiCalculation1() {
+ /* (min, std, acceptable_pause) = (1000, 200, 0) */
+ final double[] heartbeatIntervals = {1000, 1000, 1000, 1000, 1000};
+ final long minStd = 200;
+ final long pause = 0;
+
+ assertInRange(getPhi(1000, heartbeatIntervals, minStd, pause), 0, 1);
+ assertInRange(getPhi(2000, heartbeatIntervals, minStd, pause), 5, 10);
+ assertInRange(getPhi(3000, heartbeatIntervals, minStd, pause), 35, 50);
+ }
+
+ @Test
+ public void testPhiCalculation2() {
+ /* (min, std, acceptable_pause) = (1000, 300, 0) */
+ /* (min, std, acceptable_pause) = (1000, 300, 5000) */
Review Comment:
Fixed
--
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
Re: [PR] [RTO/RPO] Add Phi Accrual for Node failure detection [iotdb]
Posted by "SzyWilliam (via GitHub)" <gi...@apache.org>.
SzyWilliam commented on code in PR #14866:
URL: https://github.com/apache/iotdb/pull/14866#discussion_r1970024500
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/cache/detector/PhiAccrualDetector.java:
##########
@@ -0,0 +1,160 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
Review Comment:
Done
--
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
Re: [PR] [RTO/RPO] Add Phi Accrual for Node failure detection [iotdb]
Posted by "SzyWilliam (via GitHub)" <gi...@apache.org>.
SzyWilliam commented on code in PR #14866:
URL: https://github.com/apache/iotdb/pull/14866#discussion_r1969556185
##########
iotdb-core/node-commons/src/assembly/resources/conf/iotdb-system.properties.template:
##########
@@ -684,6 +684,26 @@ time_partition_interval=604800000
# Datatype: long
heartbeat_interval_in_ms=1000
+# Default failure detector, enum from {fixed, phi_accrual}
+# effectiveMode: restart
+# Datatype: string
+failure_detector=fixed
Review Comment:
Done
--
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
Re: [PR] [RTO/RPO] Add Phi Accrual for Node failure detection [iotdb]
Posted by "SzyWilliam (via GitHub)" <gi...@apache.org>.
SzyWilliam commented on code in PR #14866:
URL: https://github.com/apache/iotdb/pull/14866#discussion_r1970026970
##########
iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/manager/load/cache/detector/PhiAccrualDetectorTest.java:
##########
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.confignode.manager.load.cache.detector;
Review Comment:
Done
--
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
Re: [PR] [RTO/RPO] Add Phi Accrual for Node failure detection [iotdb]
Posted by "SzyWilliam (via GitHub)" <gi...@apache.org>.
SzyWilliam commented on code in PR #14866:
URL: https://github.com/apache/iotdb/pull/14866#discussion_r1969559832
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/cache/node/AINodeHeartbeatCache.java:
##########
@@ -54,7 +56,8 @@ public void updateCurrentStatistics(boolean forceUpdate) {
long currentNanoTime = System.nanoTime();
if (lastSample != null && NodeStatus.Removing.equals(lastSample.getStatus())) {
status = NodeStatus.Removing;
- } else if (currentNanoTime - lastSendTime > HEARTBEAT_TIMEOUT_TIME_IN_NS) {
+ } else if (!failureDetector.isAvailable(heartbeatHistory)) {
+ /* Failure detector decides that this DataNode is UNKNOWN */
Review Comment:
Good Catch！
--
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
Re: [PR] [RTO/RPO] Add Phi Accrual for Node failure detection [iotdb]
Posted by "SzyWilliam (via GitHub)" <gi...@apache.org>.
SzyWilliam commented on code in PR #14866:
URL: https://github.com/apache/iotdb/pull/14866#discussion_r1970071233
##########
iotdb-core/node-commons/src/assembly/resources/conf/iotdb-system.properties.template:
##########
@@ -684,6 +684,26 @@ time_partition_interval=604800000
# Datatype: long
heartbeat_interval_in_ms=1000
+# Default failure detector, enum from {fixed, phi_accrual}
+# effectiveMode: restart
+# Datatype: string
+failure_detector=fixed
+
+# Fixed failure detector threshold of time elapsed without receiving heartbeat replies
+# effectiveMode: restart
+# Datatype: long
+failure_detector_fixed_threshold_in_ms=20000
+
+# Phi accrual: threshold to determine a node is down
+# effectiveMode: restart
+# Datatype: long
+failure_detector_phi_threshold=30
Review Comment:
Actually I think threshold = 30 is a little bit small. From the experiment, we can see that threshold=30 only tolerates about 2-3 seconds of not receiving heartbeats.
<img width="569" alt="image" src="https://github.com/user-attachments/assets/9be1df37-e10e-4d16-bfde-21579be2dde3" />
--
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
Re: [PR] [RTO/RPO] Add Phi Accrual for Node failure detection [iotdb]
Posted by "SzyWilliam (via GitHub)" <gi...@apache.org>.
SzyWilliam commented on code in PR #14866:
URL: https://github.com/apache/iotdb/pull/14866#discussion_r1969556987
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/conf/ConfigNodeConfig.java:
##########
@@ -180,6 +181,18 @@ public class ConfigNodeConfig {
/** The heartbeat interval in milliseconds. */
private long heartbeatIntervalInMs = 1000;
+ /** Failure detector implementation */
+ private String failureDetector = IFailureDetector.FIXED_DETECTOR;
Review Comment:
Done.
--
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
Re: [PR] [RTO/RPO] Add Phi Accrual for Node failure detection [iotdb]
Posted by "SzyWilliam (via GitHub)" <gi...@apache.org>.
SzyWilliam commented on code in PR #14866:
URL: https://github.com/apache/iotdb/pull/14866#discussion_r1974599903
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/cache/detector/PhiAccrualDetector.java:
##########
@@ -0,0 +1,171 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.confignode.manager.load.cache.detector;
+
+import org.apache.iotdb.confignode.manager.load.cache.AbstractHeartbeatSample;
+import org.apache.iotdb.confignode.manager.load.cache.IFailureDetector;
+import org.apache.iotdb.confignode.manager.load.cache.node.NodeHeartbeatSample;
+import org.apache.iotdb.confignode.manager.load.cache.region.RegionHeartbeatSample;
+
+import org.apache.commons.math3.stat.descriptive.DescriptiveStatistics;
+import org.apache.tsfile.utils.Preconditions;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public class PhiAccrualDetector implements IFailureDetector {
Review Comment:
Done
--
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
Re: [PR] [RTO/RPO] Add Phi Accrual for Node failure detection [iotdb]
Posted by "MiniSho (via GitHub)" <gi...@apache.org>.
MiniSho commented on code in PR #14866:
URL: https://github.com/apache/iotdb/pull/14866#discussion_r1969579120
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/cache/detector/PhiAccrualDetector.java:
##########
@@ -0,0 +1,160 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.confignode.manager.load.cache.detector;
+
+import org.apache.iotdb.confignode.manager.load.cache.AbstractHeartbeatSample;
+import org.apache.iotdb.confignode.manager.load.cache.IFailureDetector;
+import org.apache.iotdb.confignode.manager.load.cache.node.NodeHeartbeatSample;
+
+import org.apache.commons.lang3.ArrayUtils;
+import org.apache.commons.math3.stat.descriptive.DescriptiveStatistics;
+import org.apache.tsfile.utils.Preconditions;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public class PhiAccrualDetector implements IFailureDetector {
+
+ private final long threshold;
+ private final long acceptableHeartbeatPauseNs;
+ private final long firstHeartbeatEstimateNs;
+ private final long minHeartbeatStdNs;
+
+ public PhiAccrualDetector(
+ long threshold,
+ long acceptableHeartbeatPauseNs,
+ long firstHeartbeatEstimateNs,
+ long minHeartbeatStdNs) {
+ this.threshold = threshold;
+ this.acceptableHeartbeatPauseNs = acceptableHeartbeatPauseNs;
+ this.firstHeartbeatEstimateNs = firstHeartbeatEstimateNs;
+ this.minHeartbeatStdNs = minHeartbeatStdNs;
+ }
+
+ @Override
+ public boolean isAvailable(List<AbstractHeartbeatSample> history) {
+ if (history.isEmpty()) {
+ /* We haven't received the first heartbeat reply. We cannot decide the node availability. */
+ return true;
+ }
+ final PhiAccrual phiAccrual = create(history);
+ // TODO(szywilliam) we should log the status change and dump the heartbeat history
Review Comment:
elegant
--
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
Re: [PR] [RTO/RPO] Add Phi Accrual for Node failure detection [iotdb]
Posted by "CRZbulabula (via GitHub)" <gi...@apache.org>.
CRZbulabula commented on code in PR #14866:
URL: https://github.com/apache/iotdb/pull/14866#discussion_r1961656423
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/cache/detector/PhiAccrualDetector.java:
##########
@@ -0,0 +1,160 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.confignode.manager.load.cache.detector;
+
+import org.apache.iotdb.confignode.manager.load.cache.AbstractHeartbeatSample;
+import org.apache.iotdb.confignode.manager.load.cache.IFailureDetector;
+import org.apache.iotdb.confignode.manager.load.cache.node.NodeHeartbeatSample;
+
+import org.apache.commons.lang3.ArrayUtils;
+import org.apache.commons.math3.stat.descriptive.DescriptiveStatistics;
+import org.apache.tsfile.utils.Preconditions;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public class PhiAccrualDetector implements IFailureDetector {
+
+ private final long threshold;
+ private final long acceptableHeartbeatPauseNs;
+ private final long firstHeartbeatEstimateNs;
+ private final long minHeartbeatStdNs;
+
+ public PhiAccrualDetector(
+ long threshold,
+ long acceptableHeartbeatPauseNs,
+ long firstHeartbeatEstimateNs,
+ long minHeartbeatStdNs) {
+ this.threshold = threshold;
+ this.acceptableHeartbeatPauseNs = acceptableHeartbeatPauseNs;
+ this.firstHeartbeatEstimateNs = firstHeartbeatEstimateNs;
+ this.minHeartbeatStdNs = minHeartbeatStdNs;
+ }
+
+ @Override
+ public boolean isAvailable(List<AbstractHeartbeatSample> history) {
+ if (history.isEmpty()) {
+ /* We haven't received the first heartbeat reply. We cannot decide the node availability. */
+ return true;
+ }
+ final PhiAccrual phiAccrual = create(history);
+ return phiAccrual.phi() < (double) this.threshold;
+ }
+
+ PhiAccrual create(List<AbstractHeartbeatSample> history) {
+ final int size = history.size();
+ Preconditions.checkArgument(size >= 1);
Review Comment:
Why setup a redundant check here?
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/cache/node/AINodeHeartbeatCache.java:
##########
@@ -54,7 +56,8 @@ public void updateCurrentStatistics(boolean forceUpdate) {
long currentNanoTime = System.nanoTime();
if (lastSample != null && NodeStatus.Removing.equals(lastSample.getStatus())) {
status = NodeStatus.Removing;
- } else if (currentNanoTime - lastSendTime > HEARTBEAT_TIMEOUT_TIME_IN_NS) {
+ } else if (failureDetector.isAvailable(heartbeatHistory)) {
Review Comment:
This line of code conflicts with the definition of "isAvailable"
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/cache/detector/PhiAccrualDetector.java:
##########
@@ -0,0 +1,160 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.confignode.manager.load.cache.detector;
+
+import org.apache.iotdb.confignode.manager.load.cache.AbstractHeartbeatSample;
+import org.apache.iotdb.confignode.manager.load.cache.IFailureDetector;
+import org.apache.iotdb.confignode.manager.load.cache.node.NodeHeartbeatSample;
+
+import org.apache.commons.lang3.ArrayUtils;
+import org.apache.commons.math3.stat.descriptive.DescriptiveStatistics;
+import org.apache.tsfile.utils.Preconditions;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public class PhiAccrualDetector implements IFailureDetector {
+
+ private final long threshold;
+ private final long acceptableHeartbeatPauseNs;
+ private final long firstHeartbeatEstimateNs;
+ private final long minHeartbeatStdNs;
+
+ public PhiAccrualDetector(
+ long threshold,
+ long acceptableHeartbeatPauseNs,
+ long firstHeartbeatEstimateNs,
+ long minHeartbeatStdNs) {
+ this.threshold = threshold;
+ this.acceptableHeartbeatPauseNs = acceptableHeartbeatPauseNs;
+ this.firstHeartbeatEstimateNs = firstHeartbeatEstimateNs;
+ this.minHeartbeatStdNs = minHeartbeatStdNs;
+ }
+
+ @Override
+ public boolean isAvailable(List<AbstractHeartbeatSample> history) {
+ if (history.isEmpty()) {
+ /* We haven't received the first heartbeat reply. We cannot decide the node availability. */
+ return true;
Review Comment:
It's better to invoke the FixedDetector here. Since there is a corner case that some nodes are stuck in the network partition at the begining.
--
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
Re: [PR] [RTO/RPO] Add Phi Accrual for Node failure detection [iotdb]
Posted by "Cpaulyz (via GitHub)" <gi...@apache.org>.
Cpaulyz commented on code in PR #14866:
URL: https://github.com/apache/iotdb/pull/14866#discussion_r1969571505
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/cache/detector/PhiAccrualDetector.java:
##########
@@ -0,0 +1,160 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.confignode.manager.load.cache.detector;
+
+import org.apache.iotdb.confignode.manager.load.cache.AbstractHeartbeatSample;
+import org.apache.iotdb.confignode.manager.load.cache.IFailureDetector;
+import org.apache.iotdb.confignode.manager.load.cache.node.NodeHeartbeatSample;
+
+import org.apache.commons.lang3.ArrayUtils;
+import org.apache.commons.math3.stat.descriptive.DescriptiveStatistics;
+import org.apache.tsfile.utils.Preconditions;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public class PhiAccrualDetector implements IFailureDetector {
+
+ private final long threshold;
+ private final long acceptableHeartbeatPauseNs;
+ private final long firstHeartbeatEstimateNs;
+ private final long minHeartbeatStdNs;
+
+ public PhiAccrualDetector(
+ long threshold,
+ long acceptableHeartbeatPauseNs,
+ long firstHeartbeatEstimateNs,
+ long minHeartbeatStdNs) {
+ this.threshold = threshold;
+ this.acceptableHeartbeatPauseNs = acceptableHeartbeatPauseNs;
+ this.firstHeartbeatEstimateNs = firstHeartbeatEstimateNs;
+ this.minHeartbeatStdNs = minHeartbeatStdNs;
+ }
+
+ @Override
+ public boolean isAvailable(List<AbstractHeartbeatSample> history) {
+ if (history.isEmpty()) {
+ /* We haven't received the first heartbeat reply. We cannot decide the node availability. */
+ return true;
+ }
+ final PhiAccrual phiAccrual = create(history);
+ // TODO(szywilliam) we should log the status change and dump the heartbeat history
Review Comment:
```suggestion
```
--
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
Re: [PR] [RTO/RPO] Add Phi Accrual for Node failure detection [iotdb]
Posted by "OneSizeFitsQuorum (via GitHub)" <gi...@apache.org>.
OneSizeFitsQuorum commented on code in PR #14866:
URL: https://github.com/apache/iotdb/pull/14866#discussion_r1969341039
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/cache/node/AINodeHeartbeatCache.java:
##########
@@ -54,7 +56,8 @@ public void updateCurrentStatistics(boolean forceUpdate) {
long currentNanoTime = System.nanoTime();
if (lastSample != null && NodeStatus.Removing.equals(lastSample.getStatus())) {
status = NodeStatus.Removing;
- } else if (currentNanoTime - lastSendTime > HEARTBEAT_TIMEOUT_TIME_IN_NS) {
+ } else if (!failureDetector.isAvailable(heartbeatHistory)) {
+ /* Failure detector decides that this DataNode is UNKNOWN */
Review Comment:
AINode
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/conf/ConfigNodeConfig.java:
##########
@@ -180,6 +181,18 @@ public class ConfigNodeConfig {
/** The heartbeat interval in milliseconds. */
private long heartbeatIntervalInMs = 1000;
+ /** Failure detector implementation */
+ private String failureDetector = IFailureDetector.FIXED_DETECTOR;
Review Comment:
Phi?
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/cache/detector/FixedDetector.java:
##########
@@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.confignode.manager.load.cache.detector;
Review Comment:
add empty line
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/cache/detector/PhiAccrualDetector.java:
##########
@@ -0,0 +1,160 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.confignode.manager.load.cache.detector;
+
+import org.apache.iotdb.confignode.manager.load.cache.AbstractHeartbeatSample;
+import org.apache.iotdb.confignode.manager.load.cache.IFailureDetector;
+import org.apache.iotdb.confignode.manager.load.cache.node.NodeHeartbeatSample;
+
+import org.apache.commons.lang3.ArrayUtils;
+import org.apache.commons.math3.stat.descriptive.DescriptiveStatistics;
+import org.apache.tsfile.utils.Preconditions;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public class PhiAccrualDetector implements IFailureDetector {
+
+ private final long threshold;
+ private final long acceptableHeartbeatPauseNs;
+ private final long firstHeartbeatEstimateNs;
+ private final long minHeartbeatStdNs;
+
+ public PhiAccrualDetector(
+ long threshold,
+ long acceptableHeartbeatPauseNs,
+ long firstHeartbeatEstimateNs,
+ long minHeartbeatStdNs) {
+ this.threshold = threshold;
+ this.acceptableHeartbeatPauseNs = acceptableHeartbeatPauseNs;
+ this.firstHeartbeatEstimateNs = firstHeartbeatEstimateNs;
+ this.minHeartbeatStdNs = minHeartbeatStdNs;
+ }
+
+ @Override
+ public boolean isAvailable(List<AbstractHeartbeatSample> history) {
+ if (history.isEmpty()) {
+ /* We haven't received the first heartbeat reply. We cannot decide the node availability. */
+ return true;
+ }
+ final PhiAccrual phiAccrual = create(history);
+ // TODO(szywilliam) we should log the status change and dump the heartbeat history
Review Comment:
do not leave todo? just do it?
##########
iotdb-core/node-commons/src/assembly/resources/conf/iotdb-system.properties.template:
##########
@@ -684,6 +684,26 @@ time_partition_interval=604800000
# Datatype: long
heartbeat_interval_in_ms=1000
+# Default failure detector, enum from {fixed, phi_accrual}
+# effectiveMode: restart
+# Datatype: string
+failure_detector=fixed
Review Comment:
The default detecter should be set to phi_accrual?
##########
iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/manager/load/cache/detector/PhiAccrualDetectorTest.java:
##########
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.confignode.manager.load.cache.detector;
Review Comment:
add empty line
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/cache/detector/PhiAccrualDetector.java:
##########
@@ -0,0 +1,160 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
Review Comment:
add empty line
##########
iotdb-core/node-commons/src/assembly/resources/conf/iotdb-system.properties.template:
##########
@@ -684,6 +684,26 @@ time_partition_interval=604800000
# Datatype: long
heartbeat_interval_in_ms=1000
+# Default failure detector, enum from {fixed, phi_accrual}
+# effectiveMode: restart
+# Datatype: string
+failure_detector=fixed
+
+# Fixed failure detector threshold of time elapsed without receiving heartbeat replies
+# effectiveMode: restart
+# Datatype: long
+failure_detector_fixed_threshold_in_ms=20000
+
+# Phi accrual: threshold to determine a node is down
+# effectiveMode: restart
+# Datatype: long
+failure_detector_phi_threshold=30
Review Comment:
is it too large?
<img width="1121" alt="image" src="https://github.com/user-attachments/assets/99840f32-7a63-490b-a89a-8b0ef459d1b4" />
##########
iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/manager/load/cache/detector/PhiAccrualDetectorTest.java:
##########
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.confignode.manager.load.cache.detector;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class PhiAccrualDetectorTest {
Review Comment:
1. rename to DetecterTest
2. to compare the difference between PhiAccrual and Fixed under same heartbeat interval list，to list some case dentifing each other's strengths and weaknesses, for example, Phi may detect the failure earlier, but there will be false positives
3. Each test should be complemented with a description of what the HeartbeatInterval corresponds to in the real world (e.g., a node going down, or a 5s/10s gc, etc.).
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/cache/detector/PhiAccrualDetector.java:
##########
@@ -0,0 +1,160 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.confignode.manager.load.cache.detector;
+
+import org.apache.iotdb.confignode.manager.load.cache.AbstractHeartbeatSample;
+import org.apache.iotdb.confignode.manager.load.cache.IFailureDetector;
+import org.apache.iotdb.confignode.manager.load.cache.node.NodeHeartbeatSample;
+
+import org.apache.commons.lang3.ArrayUtils;
+import org.apache.commons.math3.stat.descriptive.DescriptiveStatistics;
+import org.apache.tsfile.utils.Preconditions;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public class PhiAccrualDetector implements IFailureDetector {
+
+ private final long threshold;
+ private final long acceptableHeartbeatPauseNs;
+ private final long firstHeartbeatEstimateNs;
+ private final long minHeartbeatStdNs;
+
+ public PhiAccrualDetector(
+ long threshold,
+ long acceptableHeartbeatPauseNs,
+ long firstHeartbeatEstimateNs,
+ long minHeartbeatStdNs) {
+ this.threshold = threshold;
+ this.acceptableHeartbeatPauseNs = acceptableHeartbeatPauseNs;
+ this.firstHeartbeatEstimateNs = firstHeartbeatEstimateNs;
+ this.minHeartbeatStdNs = minHeartbeatStdNs;
+ }
+
+ @Override
+ public boolean isAvailable(List<AbstractHeartbeatSample> history) {
+ if (history.isEmpty()) {
+ /* We haven't received the first heartbeat reply. We cannot decide the node availability. */
+ return true;
+ }
+ final PhiAccrual phiAccrual = create(history);
+ // TODO(szywilliam) we should log the status change and dump the heartbeat history
+ return phiAccrual.phi() < (double) this.threshold;
+ }
+
+ PhiAccrual create(List<AbstractHeartbeatSample> history) {
+ final int size = history.size();
+
+ final List<Double> heartbeatIntervals = new ArrayList<>();
+ /*
+ * During cold start, the heartbeat history may contain not enough samples for wise decisions.
+ * Therefore, we manually add data samples for better estimation.
+ * 1. mean = heartbeat interval (1000ms)
+ * 2. std = mean / 4
+ */
+ if (size <= 2) {
+ double mean = firstHeartbeatEstimateNs;
+ double std = mean / 4.0;
+ heartbeatIntervals.add(mean + std);
+ heartbeatIntervals.add(mean - std);
+ }
+
+ for (int i = 1; i < size; i++) {
+ // ensure getSampleLogicalTimestamp() will return system nano timestamp
+ Preconditions.checkArgument(history.get(i) instanceof NodeHeartbeatSample);
+ double interval =
Review Comment:
1.It seems that as long as the duration of a heartbeat is less than 1 second, the interval should be about 1 second by default. Is this reasonable? Does a faster heartbeat predict earlier detection of problems?
2. This history is a linkedlist and cannot support get(i) requests efficiently. I recommend just creating a circular array of size 100 for slidingWindow
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/cache/detector/PhiAccrualDetector.java:
##########
@@ -0,0 +1,160 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.confignode.manager.load.cache.detector;
+
+import org.apache.iotdb.confignode.manager.load.cache.AbstractHeartbeatSample;
+import org.apache.iotdb.confignode.manager.load.cache.IFailureDetector;
+import org.apache.iotdb.confignode.manager.load.cache.node.NodeHeartbeatSample;
+
+import org.apache.commons.lang3.ArrayUtils;
+import org.apache.commons.math3.stat.descriptive.DescriptiveStatistics;
+import org.apache.tsfile.utils.Preconditions;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public class PhiAccrualDetector implements IFailureDetector {
+
+ private final long threshold;
+ private final long acceptableHeartbeatPauseNs;
+ private final long firstHeartbeatEstimateNs;
+ private final long minHeartbeatStdNs;
+
+ public PhiAccrualDetector(
+ long threshold,
+ long acceptableHeartbeatPauseNs,
+ long firstHeartbeatEstimateNs,
+ long minHeartbeatStdNs) {
+ this.threshold = threshold;
+ this.acceptableHeartbeatPauseNs = acceptableHeartbeatPauseNs;
+ this.firstHeartbeatEstimateNs = firstHeartbeatEstimateNs;
+ this.minHeartbeatStdNs = minHeartbeatStdNs;
+ }
+
+ @Override
+ public boolean isAvailable(List<AbstractHeartbeatSample> history) {
+ if (history.isEmpty()) {
+ /* We haven't received the first heartbeat reply. We cannot decide the node availability. */
+ return true;
Review Comment:
then we should return false?
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/cache/AbstractLoadCache.java:
##########
@@ -40,9 +45,27 @@ public abstract class AbstractLoadCache {
// The current statistics calculated by the latest heartbeat sample
Review Comment:
remove `HEARTBEAT_TIMEOUT_TIME_IN_NS` default value? to make it controlled by `failure_detector_fixed_threshold_in_ms`?
BTW, regionCache may need to be reconstruced too
--
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
Re: [PR] [RTO/RPO] Add Phi Accrual for Node failure detection [iotdb]
Posted by "SzyWilliam (via GitHub)" <gi...@apache.org>.
SzyWilliam commented on code in PR #14866:
URL: https://github.com/apache/iotdb/pull/14866#discussion_r1970022813
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/cache/detector/FixedDetector.java:
##########
@@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.confignode.manager.load.cache.detector;
Review Comment:
Done
--
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
Re: [PR] [RTO/RPO] Add Phi Accrual for Node failure detection [iotdb]
Posted by "OneSizeFitsQuorum (via GitHub)" <gi...@apache.org>.
OneSizeFitsQuorum merged PR #14866:
URL: https://github.com/apache/iotdb/pull/14866
--
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
Re: [PR] [RTO/RPO] Add Phi Accrual for Node failure detection [iotdb]
Posted by "SzyWilliam (via GitHub)" <gi...@apache.org>.
SzyWilliam commented on code in PR #14866:
URL: https://github.com/apache/iotdb/pull/14866#discussion_r1971773037
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/cache/region/RegionCache.java:
##########
@@ -44,8 +44,7 @@ public synchronized void updateCurrentStatistics(boolean forceUpdate) {
long currentNanoTime = System.nanoTime();
if (lastSample == null) {
status = RegionStatus.Unknown;
- } else if (currentNanoTime - lastSample.getSampleLogicalTimestamp()
- > HEARTBEAT_TIMEOUT_TIME_IN_NS) {
+ } else if (currentNanoTime - lastSample.getSampleLogicalTimestamp() > heartbeatTimeoutNs) {
Review Comment:
Fixed.
--
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
Re: [PR] [RTO/RPO] Add Phi Accrual for Node failure detection [iotdb]
Posted by "SzyWilliam (via GitHub)" <gi...@apache.org>.
SzyWilliam commented on code in PR #14866:
URL: https://github.com/apache/iotdb/pull/14866#discussion_r1972707247
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/cache/detector/PhiAccrualDetector.java:
##########
@@ -0,0 +1,180 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.confignode.manager.load.cache.detector;
+
+import org.apache.iotdb.confignode.manager.load.cache.AbstractHeartbeatSample;
+import org.apache.iotdb.confignode.manager.load.cache.IFailureDetector;
+import org.apache.iotdb.confignode.manager.load.cache.node.NodeHeartbeatSample;
+
+import org.apache.commons.math3.stat.descriptive.DescriptiveStatistics;
+import org.apache.tsfile.utils.Preconditions;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public class PhiAccrualDetector implements IFailureDetector {
+ private static final Logger LOGGER = LoggerFactory.getLogger(PhiAccrualDetector.class);
+ private final long threshold;
+ private final long acceptableHeartbeatPauseNs;
+ private final long firstHeartbeatEstimateNs;
+ private final long minHeartbeatStdNs;
+
+ public PhiAccrualDetector(
+ long threshold,
+ long acceptableHeartbeatPauseNs,
+ long firstHeartbeatEstimateNs,
+ long minHeartbeatStdNs) {
+ this.threshold = threshold;
+ this.acceptableHeartbeatPauseNs = acceptableHeartbeatPauseNs;
+ this.firstHeartbeatEstimateNs = firstHeartbeatEstimateNs;
+ this.minHeartbeatStdNs = minHeartbeatStdNs;
+ }
+
+ @Override
+ public boolean isAvailable(List<AbstractHeartbeatSample> history) {
+ if (history.isEmpty()) {
+ /* We haven't received the first heartbeat reply. We cannot decide the node availability. */
+ return true;
+ }
+ final PhiAccrual phiAccrual = create(history);
+ final boolean isAvailable = phiAccrual.phi() < (double) this.threshold;
+ if (!isAvailable) {
+ // log the status change and dump the heartbeat history for analysis use
+ final StringBuilder builder = new StringBuilder();
+ builder.append("[");
+ for (double interval : phiAccrual.heartbeatIntervals) {
+ final long msInterval = (long) interval / 1000_000;
+ builder.append(msInterval).append(", ");
+ }
+ builder.append(phiAccrual.timeElapsedSinceLastHeartbeat / 1000_000);
+ builder.append("]");
+ LOGGER.info(String.format("Node Down, heartbeat history (ms): %s", builder));
+ }
+
+ return isAvailable;
+ }
+
+ PhiAccrual create(List<AbstractHeartbeatSample> history) {
+ final int size = history.size();
+
+ final List<Double> heartbeatIntervals = new ArrayList<>();
+ /*
+ * During cold start, the heartbeat history may contain not enough samples for wise decisions.
+ * Therefore, we manually add data samples for better estimation.
+ * 1. mean = heartbeat interval (1000ms)
+ * 2. std = mean / 4
+ */
+ if (size <= 2) {
Review Comment:
Use fixed detector as fallback in the initial 60s
--
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
Re: [PR] [RTO/RPO] Add Phi Accrual for Node failure detection [iotdb]
Posted by "OneSizeFitsQuorum (via GitHub)" <gi...@apache.org>.
OneSizeFitsQuorum commented on code in PR #14866:
URL: https://github.com/apache/iotdb/pull/14866#discussion_r1972730667
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/cache/detector/PhiAccrualDetector.java:
##########
@@ -0,0 +1,171 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.confignode.manager.load.cache.detector;
+
+import org.apache.iotdb.confignode.manager.load.cache.AbstractHeartbeatSample;
+import org.apache.iotdb.confignode.manager.load.cache.IFailureDetector;
+import org.apache.iotdb.confignode.manager.load.cache.node.NodeHeartbeatSample;
+import org.apache.iotdb.confignode.manager.load.cache.region.RegionHeartbeatSample;
+
+import org.apache.commons.math3.stat.descriptive.DescriptiveStatistics;
+import org.apache.tsfile.utils.Preconditions;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public class PhiAccrualDetector implements IFailureDetector {
Review Comment:
Add some javadoc for this class, describe the fi and cold start processes
##########
iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/manager/load/cache/detector/DetectorTest.java:
##########
@@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.confignode.manager.load.cache.detector;
+
+import org.apache.iotdb.commons.cluster.NodeStatus;
+import org.apache.iotdb.confignode.manager.load.cache.AbstractHeartbeatSample;
+import org.apache.iotdb.confignode.manager.load.cache.node.NodeHeartbeatSample;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.Collections;
+import java.util.LinkedList;
+import java.util.List;
+
+public class DetectorTest {
+
+ final long sec = 1_000_000_000L;
+ final FixedDetector fixedDetector = new FixedDetector(20 * sec);
+ final PhiAccrualDetector phiAccrualDetector =
+ new PhiAccrualDetector(30, 10 * sec, (long) (0.2 * sec), 0, fixedDetector);
+
+ private double getPhi(long elapsed, double[] intervals, long minStd, long pause) {
+ final PhiAccrualDetector.PhiAccrual p =
+ new PhiAccrualDetector.PhiAccrual(intervals, elapsed, minStd, pause);
+ return p.phi();
+ }
+
+ private void assertInRange(double value, double start, double end) {
+ Assert.assertTrue(value > start);
+ Assert.assertTrue(value < end);
+ }
+
+ @Test
+ public void testFixedDetector() {
Review Comment:
add a available test
##########
iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/manager/load/cache/detector/DetectorTest.java:
##########
@@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.confignode.manager.load.cache.detector;
+
+import org.apache.iotdb.commons.cluster.NodeStatus;
+import org.apache.iotdb.confignode.manager.load.cache.AbstractHeartbeatSample;
+import org.apache.iotdb.confignode.manager.load.cache.node.NodeHeartbeatSample;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.Collections;
+import java.util.LinkedList;
+import java.util.List;
+
+public class DetectorTest {
+
+ final long sec = 1_000_000_000L;
+ final FixedDetector fixedDetector = new FixedDetector(20 * sec);
+ final PhiAccrualDetector phiAccrualDetector =
+ new PhiAccrualDetector(30, 10 * sec, (long) (0.2 * sec), 0, fixedDetector);
+
+ private double getPhi(long elapsed, double[] intervals, long minStd, long pause) {
+ final PhiAccrualDetector.PhiAccrual p =
+ new PhiAccrualDetector.PhiAccrual(intervals, elapsed, minStd, pause);
+ return p.phi();
+ }
+
+ private void assertInRange(double value, double start, double end) {
+ Assert.assertTrue(value > start);
+ Assert.assertTrue(value < end);
+ }
+
+ @Test
+ public void testFixedDetector() {
+ final long lastHeartbeatTs = System.nanoTime() - 21 * sec;
+ final List<AbstractHeartbeatSample> history =
+ Collections.singletonList(new NodeHeartbeatSample(lastHeartbeatTs, NodeStatus.Running));
+ Assert.assertFalse(fixedDetector.isAvailable(history));
+ }
+
+ @Test
+ public void testPhiCalculation1() {
+ /* (min, std, acceptable_pause) = (1000, 200, 0) */
+ final double[] heartbeatIntervals = {1000, 1000, 1000, 1000, 1000};
+ final long minStd = 200;
+ final long pause = 0;
+
+ assertInRange(getPhi(1000, heartbeatIntervals, minStd, pause), 0, 1);
+ assertInRange(getPhi(2000, heartbeatIntervals, minStd, pause), 5, 10);
+ assertInRange(getPhi(3000, heartbeatIntervals, minStd, pause), 35, 50);
+ }
+
+ @Test
+ public void testPhiCalculation2() {
+ /* (min, std, acceptable_pause) = (1000, 300, 0) */
+ final double[] heartbeatIntervals = {1000, 1000, 1000, 1000, 1000};
+ final long minStd = 300;
+ final long pause = 0;
+
+ assertInRange(getPhi(1000, heartbeatIntervals, minStd, pause), 0, 1);
+ assertInRange(getPhi(2000, heartbeatIntervals, minStd, pause), 1, 5);
+ assertInRange(getPhi(3000, heartbeatIntervals, minStd, pause), 10, 15);
+ }
+
+ @Test
+ public void testPhiCalculation3() {
+ /* (min, std, acceptable_pause) = (1000, 200, 3000) */
+ final double[] heartbeatIntervals = {1000, 1000, 1000, 1000, 1000};
+ final long minStd = 200;
+ final long pause = 3000;
+
+ assertInRange(getPhi(1000 + pause, heartbeatIntervals, minStd, pause), 0, 1);
+ assertInRange(getPhi(2000 + pause, heartbeatIntervals, minStd, pause), 5, 10);
+ assertInRange(getPhi(3000 + pause, heartbeatIntervals, minStd, pause), 35, 50);
+ }
+
+ @Test
+ public void testComparisonQuickFailureDetection() {
+ // When a node hasn't responded with interval longer than accepted GC pause
+ // Phi Accrual can detect the problem quicker than Fix
+ // In this case, the accepted pause is 10s, but we haven't received heartbeats for 13s
+ long[] interval = new long[] {sec, sec, sec};
+ List<AbstractHeartbeatSample> history = fromInterval(interval, 13 * sec);
+ Assert.assertTrue(fixedDetector.isAvailable(history));
+ Assert.assertFalse(phiAccrualDetector.isAvailable(history));
+ }
+
+ @Test
+ public void testFalsePositiveOnExceptionallyLongGCPause() {
+ // When the system load is high, we may observe exceptionally long GC pause
Review Comment:
move to function doc
##########
iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/manager/load/cache/detector/DetectorTest.java:
##########
@@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.confignode.manager.load.cache.detector;
+
+import org.apache.iotdb.commons.cluster.NodeStatus;
+import org.apache.iotdb.confignode.manager.load.cache.AbstractHeartbeatSample;
+import org.apache.iotdb.confignode.manager.load.cache.node.NodeHeartbeatSample;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.Collections;
+import java.util.LinkedList;
+import java.util.List;
+
+public class DetectorTest {
+
+ final long sec = 1_000_000_000L;
+ final FixedDetector fixedDetector = new FixedDetector(20 * sec);
+ final PhiAccrualDetector phiAccrualDetector =
+ new PhiAccrualDetector(30, 10 * sec, (long) (0.2 * sec), 0, fixedDetector);
+
+ private double getPhi(long elapsed, double[] intervals, long minStd, long pause) {
+ final PhiAccrualDetector.PhiAccrual p =
+ new PhiAccrualDetector.PhiAccrual(intervals, elapsed, minStd, pause);
+ return p.phi();
+ }
+
+ private void assertInRange(double value, double start, double end) {
+ Assert.assertTrue(value > start);
+ Assert.assertTrue(value < end);
+ }
+
+ @Test
+ public void testFixedDetector() {
+ final long lastHeartbeatTs = System.nanoTime() - 21 * sec;
+ final List<AbstractHeartbeatSample> history =
+ Collections.singletonList(new NodeHeartbeatSample(lastHeartbeatTs, NodeStatus.Running));
+ Assert.assertFalse(fixedDetector.isAvailable(history));
+ }
+
+ @Test
+ public void testPhiCalculation1() {
+ /* (min, std, acceptable_pause) = (1000, 200, 0) */
+ final double[] heartbeatIntervals = {1000, 1000, 1000, 1000, 1000};
+ final long minStd = 200;
+ final long pause = 0;
+
+ assertInRange(getPhi(1000, heartbeatIntervals, minStd, pause), 0, 1);
+ assertInRange(getPhi(2000, heartbeatIntervals, minStd, pause), 5, 10);
+ assertInRange(getPhi(3000, heartbeatIntervals, minStd, pause), 35, 50);
+ }
+
+ @Test
+ public void testPhiCalculation2() {
+ /* (min, std, acceptable_pause) = (1000, 300, 0) */
+ final double[] heartbeatIntervals = {1000, 1000, 1000, 1000, 1000};
+ final long minStd = 300;
+ final long pause = 0;
+
+ assertInRange(getPhi(1000, heartbeatIntervals, minStd, pause), 0, 1);
+ assertInRange(getPhi(2000, heartbeatIntervals, minStd, pause), 1, 5);
+ assertInRange(getPhi(3000, heartbeatIntervals, minStd, pause), 10, 15);
+ }
+
+ @Test
+ public void testPhiCalculation3() {
+ /* (min, std, acceptable_pause) = (1000, 200, 3000) */
+ final double[] heartbeatIntervals = {1000, 1000, 1000, 1000, 1000};
+ final long minStd = 200;
+ final long pause = 3000;
+
+ assertInRange(getPhi(1000 + pause, heartbeatIntervals, minStd, pause), 0, 1);
+ assertInRange(getPhi(2000 + pause, heartbeatIntervals, minStd, pause), 5, 10);
+ assertInRange(getPhi(3000 + pause, heartbeatIntervals, minStd, pause), 35, 50);
+ }
+
+ @Test
+ public void testComparisonQuickFailureDetection() {
+ // When a node hasn't responded with interval longer than accepted GC pause
+ // Phi Accrual can detect the problem quicker than Fix
+ // In this case, the accepted pause is 10s, but we haven't received heartbeats for 13s
+ long[] interval = new long[] {sec, sec, sec};
+ List<AbstractHeartbeatSample> history = fromInterval(interval, 13 * sec);
+ Assert.assertTrue(fixedDetector.isAvailable(history));
+ Assert.assertFalse(phiAccrualDetector.isAvailable(history));
+ }
+
+ @Test
+ public void testFalsePositiveOnExceptionallyLongGCPause() {
+ // When the system load is high, we may observe exceptionally long GC pause
+ // The first exceptionally long GC pause will be a false positive to Phi
+ // the GC pause is 15 (longer than the expected 10s)
+ // Phi will report false positive
+ long[] interval = new long[] {sec, sec, sec};
+ long gcPause = 15 * sec;
+ List<AbstractHeartbeatSample> history = fromInterval(interval, gcPause + 2 * sec);
+ Assert.assertTrue(fixedDetector.isAvailable(history));
+ Assert.assertFalse(phiAccrualDetector.isAvailable(history));
+ }
+
+ @Test
+ public void testPhiAdaptionToFrequentGCPause() {
+ // When the system load is high, we may observe exceptionally long GC pause
Review Comment:
move to function doc
##########
iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/manager/load/cache/detector/DetectorTest.java:
##########
@@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.confignode.manager.load.cache.detector;
+
+import org.apache.iotdb.commons.cluster.NodeStatus;
+import org.apache.iotdb.confignode.manager.load.cache.AbstractHeartbeatSample;
+import org.apache.iotdb.confignode.manager.load.cache.node.NodeHeartbeatSample;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.Collections;
+import java.util.LinkedList;
+import java.util.List;
+
+public class DetectorTest {
+
+ final long sec = 1_000_000_000L;
+ final FixedDetector fixedDetector = new FixedDetector(20 * sec);
+ final PhiAccrualDetector phiAccrualDetector =
+ new PhiAccrualDetector(30, 10 * sec, (long) (0.2 * sec), 0, fixedDetector);
+
+ private double getPhi(long elapsed, double[] intervals, long minStd, long pause) {
+ final PhiAccrualDetector.PhiAccrual p =
+ new PhiAccrualDetector.PhiAccrual(intervals, elapsed, minStd, pause);
+ return p.phi();
+ }
+
+ private void assertInRange(double value, double start, double end) {
+ Assert.assertTrue(value > start);
+ Assert.assertTrue(value < end);
+ }
+
+ @Test
+ public void testFixedDetector() {
+ final long lastHeartbeatTs = System.nanoTime() - 21 * sec;
+ final List<AbstractHeartbeatSample> history =
+ Collections.singletonList(new NodeHeartbeatSample(lastHeartbeatTs, NodeStatus.Running));
+ Assert.assertFalse(fixedDetector.isAvailable(history));
+ }
+
+ @Test
+ public void testPhiCalculation1() {
+ /* (min, std, acceptable_pause) = (1000, 200, 0) */
+ final double[] heartbeatIntervals = {1000, 1000, 1000, 1000, 1000};
+ final long minStd = 200;
+ final long pause = 0;
+
+ assertInRange(getPhi(1000, heartbeatIntervals, minStd, pause), 0, 1);
+ assertInRange(getPhi(2000, heartbeatIntervals, minStd, pause), 5, 10);
+ assertInRange(getPhi(3000, heartbeatIntervals, minStd, pause), 35, 50);
+ }
+
+ @Test
+ public void testPhiCalculation2() {
+ /* (min, std, acceptable_pause) = (1000, 300, 0) */
+ final double[] heartbeatIntervals = {1000, 1000, 1000, 1000, 1000};
+ final long minStd = 300;
+ final long pause = 0;
+
+ assertInRange(getPhi(1000, heartbeatIntervals, minStd, pause), 0, 1);
+ assertInRange(getPhi(2000, heartbeatIntervals, minStd, pause), 1, 5);
+ assertInRange(getPhi(3000, heartbeatIntervals, minStd, pause), 10, 15);
+ }
+
+ @Test
+ public void testPhiCalculation3() {
+ /* (min, std, acceptable_pause) = (1000, 200, 3000) */
+ final double[] heartbeatIntervals = {1000, 1000, 1000, 1000, 1000};
+ final long minStd = 200;
+ final long pause = 3000;
+
+ assertInRange(getPhi(1000 + pause, heartbeatIntervals, minStd, pause), 0, 1);
+ assertInRange(getPhi(2000 + pause, heartbeatIntervals, minStd, pause), 5, 10);
+ assertInRange(getPhi(3000 + pause, heartbeatIntervals, minStd, pause), 35, 50);
+ }
+
+ @Test
+ public void testComparisonQuickFailureDetection() {
+ // When a node hasn't responded with interval longer than accepted GC pause
Review Comment:
move to function doc
--
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
Re: [PR] [RTO/RPO] Add Phi Accrual for Node failure detection [iotdb]
Posted by "SzyWilliam (via GitHub)" <gi...@apache.org>.
SzyWilliam commented on code in PR #14866:
URL: https://github.com/apache/iotdb/pull/14866#discussion_r1974612210
##########
iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/manager/load/cache/detector/DetectorTest.java:
##########
@@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.confignode.manager.load.cache.detector;
+
+import org.apache.iotdb.commons.cluster.NodeStatus;
+import org.apache.iotdb.confignode.manager.load.cache.AbstractHeartbeatSample;
+import org.apache.iotdb.confignode.manager.load.cache.node.NodeHeartbeatSample;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.Collections;
+import java.util.LinkedList;
+import java.util.List;
+
+public class DetectorTest {
+
+ final long sec = 1_000_000_000L;
+ final FixedDetector fixedDetector = new FixedDetector(20 * sec);
+ final PhiAccrualDetector phiAccrualDetector =
+ new PhiAccrualDetector(30, 10 * sec, (long) (0.2 * sec), 0, fixedDetector);
+
+ private double getPhi(long elapsed, double[] intervals, long minStd, long pause) {
+ final PhiAccrualDetector.PhiAccrual p =
+ new PhiAccrualDetector.PhiAccrual(intervals, elapsed, minStd, pause);
+ return p.phi();
+ }
+
+ private void assertInRange(double value, double start, double end) {
+ Assert.assertTrue(value > start);
+ Assert.assertTrue(value < end);
+ }
+
+ @Test
+ public void testFixedDetector() {
+ final long lastHeartbeatTs = System.nanoTime() - 21 * sec;
+ final List<AbstractHeartbeatSample> history =
+ Collections.singletonList(new NodeHeartbeatSample(lastHeartbeatTs, NodeStatus.Running));
+ Assert.assertFalse(fixedDetector.isAvailable(history));
+ }
+
+ @Test
+ public void testPhiCalculation1() {
+ /* (min, std, acceptable_pause) = (1000, 200, 0) */
+ final double[] heartbeatIntervals = {1000, 1000, 1000, 1000, 1000};
+ final long minStd = 200;
+ final long pause = 0;
+
+ assertInRange(getPhi(1000, heartbeatIntervals, minStd, pause), 0, 1);
+ assertInRange(getPhi(2000, heartbeatIntervals, minStd, pause), 5, 10);
+ assertInRange(getPhi(3000, heartbeatIntervals, minStd, pause), 35, 50);
+ }
+
+ @Test
+ public void testPhiCalculation2() {
+ /* (min, std, acceptable_pause) = (1000, 300, 0) */
+ final double[] heartbeatIntervals = {1000, 1000, 1000, 1000, 1000};
+ final long minStd = 300;
+ final long pause = 0;
+
+ assertInRange(getPhi(1000, heartbeatIntervals, minStd, pause), 0, 1);
+ assertInRange(getPhi(2000, heartbeatIntervals, minStd, pause), 1, 5);
+ assertInRange(getPhi(3000, heartbeatIntervals, minStd, pause), 10, 15);
+ }
+
+ @Test
+ public void testPhiCalculation3() {
+ /* (min, std, acceptable_pause) = (1000, 200, 3000) */
+ final double[] heartbeatIntervals = {1000, 1000, 1000, 1000, 1000};
+ final long minStd = 200;
+ final long pause = 3000;
+
+ assertInRange(getPhi(1000 + pause, heartbeatIntervals, minStd, pause), 0, 1);
+ assertInRange(getPhi(2000 + pause, heartbeatIntervals, minStd, pause), 5, 10);
+ assertInRange(getPhi(3000 + pause, heartbeatIntervals, minStd, pause), 35, 50);
+ }
+
+ @Test
+ public void testComparisonQuickFailureDetection() {
+ // When a node hasn't responded with interval longer than accepted GC pause
Review Comment:
Done
--
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
Re: [PR] [RTO/RPO] Add Phi Accrual for Node failure detection [iotdb]
Posted by "CRZbulabula (via GitHub)" <gi...@apache.org>.
CRZbulabula commented on code in PR #14866:
URL: https://github.com/apache/iotdb/pull/14866#discussion_r1963442503
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/cache/detector/PhiAccrualDetector.java:
##########
@@ -0,0 +1,160 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.confignode.manager.load.cache.detector;
+
+import org.apache.iotdb.confignode.manager.load.cache.AbstractHeartbeatSample;
+import org.apache.iotdb.confignode.manager.load.cache.IFailureDetector;
+import org.apache.iotdb.confignode.manager.load.cache.node.NodeHeartbeatSample;
+
+import org.apache.commons.lang3.ArrayUtils;
+import org.apache.commons.math3.stat.descriptive.DescriptiveStatistics;
+import org.apache.tsfile.utils.Preconditions;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public class PhiAccrualDetector implements IFailureDetector {
+
+ private final long threshold;
+ private final long acceptableHeartbeatPauseNs;
+ private final long firstHeartbeatEstimateNs;
+ private final long minHeartbeatStdNs;
+
+ public PhiAccrualDetector(
+ long threshold,
+ long acceptableHeartbeatPauseNs,
+ long firstHeartbeatEstimateNs,
+ long minHeartbeatStdNs) {
+ this.threshold = threshold;
+ this.acceptableHeartbeatPauseNs = acceptableHeartbeatPauseNs;
+ this.firstHeartbeatEstimateNs = firstHeartbeatEstimateNs;
+ this.minHeartbeatStdNs = minHeartbeatStdNs;
+ }
+
+ @Override
+ public boolean isAvailable(List<AbstractHeartbeatSample> history) {
+ if (history.isEmpty()) {
+ /* We haven't received the first heartbeat reply. We cannot decide the node availability. */
+ return true;
Review Comment:
Exactly as u said, my mistake
--
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
Re: [PR] [RTO/RPO] Add Phi Accrual for Node failure detection [iotdb]
Posted by "SzyWilliam (via GitHub)" <gi...@apache.org>.
SzyWilliam commented on code in PR #14866:
URL: https://github.com/apache/iotdb/pull/14866#discussion_r1962771893
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/load/cache/detector/PhiAccrualDetector.java:
##########
@@ -0,0 +1,160 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.confignode.manager.load.cache.detector;
+
+import org.apache.iotdb.confignode.manager.load.cache.AbstractHeartbeatSample;
+import org.apache.iotdb.confignode.manager.load.cache.IFailureDetector;
+import org.apache.iotdb.confignode.manager.load.cache.node.NodeHeartbeatSample;
+
+import org.apache.commons.lang3.ArrayUtils;
+import org.apache.commons.math3.stat.descriptive.DescriptiveStatistics;
+import org.apache.tsfile.utils.Preconditions;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public class PhiAccrualDetector implements IFailureDetector {
+
+ private final long threshold;
+ private final long acceptableHeartbeatPauseNs;
+ private final long firstHeartbeatEstimateNs;
+ private final long minHeartbeatStdNs;
+
+ public PhiAccrualDetector(
+ long threshold,
+ long acceptableHeartbeatPauseNs,
+ long firstHeartbeatEstimateNs,
+ long minHeartbeatStdNs) {
+ this.threshold = threshold;
+ this.acceptableHeartbeatPauseNs = acceptableHeartbeatPauseNs;
+ this.firstHeartbeatEstimateNs = firstHeartbeatEstimateNs;
+ this.minHeartbeatStdNs = minHeartbeatStdNs;
+ }
+
+ @Override
+ public boolean isAvailable(List<AbstractHeartbeatSample> history) {
+ if (history.isEmpty()) {
+ /* We haven't received the first heartbeat reply. We cannot decide the node availability. */
+ return true;
+ }
+ final PhiAccrual phiAccrual = create(history);
+ return phiAccrual.phi() < (double) this.threshold;
+ }
+
+ PhiAccrual create(List<AbstractHeartbeatSample> history) {
+ final int size = history.size();
+ Preconditions.checkArgument(size >= 1);
Review Comment:
You are right, we can remove this line.
--
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