ptlrs commented on code in PR #8843:
URL: https://github.com/apache/ozone/pull/8843#discussion_r2985324862
##########
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestStorageVolumeHealthChecks.java:
##########
@@ -304,13 +308,23 @@ private void
testCheckIOUntilFailure(StorageVolume.Builder<?> builder,
DatanodeConfiguration dnConf = CONF.getObject(DatanodeConfiguration.class);
dnConf.setVolumeIOTestCount(ioTestCount);
dnConf.setVolumeIOFailureTolerance(ioFailureTolerance);
+ dnConf.setDiskCheckSlidingWindowTimeout(Duration.ofMillis(ioTestCount));
CONF.setFromObject(dnConf);
builder.conf(CONF);
StorageVolume volume = builder.build();
volume.format(CLUSTER_ID);
volume.createTmpDirs(CLUSTER_ID);
+ // Sliding window protocol transitioned from count-based to a time-based
system
+ // Update the default failure duration of the window from 60 minutes to a
shorter duration for the test
+ long eventRate = 1L;
+ TestClock testClock = TestClock.newInstance();
+ Field clock = SlidingWindow.class.getDeclaredField("clock");
+ clock.setAccessible(true);
+ clock.set(volume.getIoTestSlidingWindow(), testClock);
Review Comment:
Done
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/StorageVolume.java:
##########
@@ -717,39 +718,25 @@ public synchronized VolumeCheckResult check(@Nullable
Boolean unused)
// We can check again if disk is full. If it is full,
// in this case keep volume as healthy so that READ can still be served
if (!diskChecksPassed && getCurrentUsage().getAvailable() <
minimumDiskSpace) {
- ioTestSlidingWindow.add(true);
return VolumeCheckResult.HEALTHY;
}
- // Move the sliding window of IO test results forward 1 by adding the
- // latest entry and removing the oldest entry from the window.
- // Update the failure counter for the new window.
- ioTestSlidingWindow.add(diskChecksPassed);
if (!diskChecksPassed) {
- currentIOFailureCount.incrementAndGet();
- }
- if (ioTestSlidingWindow.size() > ioTestCount &&
- Objects.equals(ioTestSlidingWindow.poll(), Boolean.FALSE)) {
- currentIOFailureCount.decrementAndGet();
+ ioTestSlidingWindow.add();
}
- // If the failure threshold has been crossed, fail the volume without
- // further scans.
+ // If the failure threshold has been crossed, fail the volume without
further scans.
// Once the volume is failed, it will not be checked anymore.
// The failure counts can be left as is.
- if (currentIOFailureCount.get() > ioFailureTolerance) {
- LOG.error("Failed IO test for volume {}: the last {} runs " +
- "encountered {} out of {} tolerated failures.", this,
- ioTestSlidingWindow.size(), currentIOFailureCount,
- ioFailureTolerance);
+ if (ioTestSlidingWindow.isExceeded()) {
+ LOG.error("Failed IO test for volume {}: encountered more than the {}
tolerated failures within the past {} ms.",
+ this, ioTestSlidingWindow.getWindowSize(),
ioTestSlidingWindow.getExpiryDurationMillis());
return VolumeCheckResult.FAILED;
- } else if (LOG.isDebugEnabled()) {
- LOG.debug("IO test results for volume {}: the last {} runs encountered "
+
- "{} out of {} tolerated failures", this,
- ioTestSlidingWindow.size(),
- currentIOFailureCount, ioFailureTolerance);
}
+ LOG.debug("IO test results for volume {}: encountered {} out of {}
tolerated failures",
+ this, ioTestSlidingWindow.getNumEventsInWindow(),
ioTestSlidingWindow.getWindowSize());
Review Comment:
`getNumEventsInWindow` is correct here. It will show how many failures are
in the actual window. The window size is the number of failures that are
allowed.
This log line is logged every time in debug mode. Since our queue is larger
than the window, logging `getNumEvents` may give us the false impression that
we got more failures as it will include the errors outside the window as well.
We are using `getNumEvents` only for logging in tests.
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeConfiguration.java:
##########
@@ -404,6 +407,16 @@ public class DatanodeConfiguration extends
ReconfigurableConfig {
)
private Duration diskCheckTimeout = DISK_CHECK_TIMEOUT_DEFAULT;
+ @Config(key = "hdds.datanode.disk.check.sliding.window.timeout",
+ defaultValue = "60m",
+ type = ConfigType.TIME,
+ tags = {ConfigTag.DATANODE},
+ description = "Time interval after which a disk check"
+ + " failure result stored in the sliding window will expire."
+ + " Unit could be defined with postfix (ns,ms,s,m,h,d)."
+ )
+ private Duration diskCheckSlidingWindowTimeout =
DISK_CHECK_SLIDING_WINDOW_TIMEOUT_DEFAULT;
Review Comment:
This is a good question.
The period disk check currently runs every 1 hour and the sliding window
coverage is also for 1 hour.
The window should likely cover the period between and inclusive of the two
periodic checks such that if two periodic checks fail, then they get counted in
the same window.
Otherwise the only opportunity for a failure will be by a combination of
periodic and on-demand checks and never due to two periodic checks.
I have updated the sliding window to be as long as the periodic disk check
interval plus the timeout value for the disk check.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
