Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ public abstract class StorageVolume
// The name of the directory where temporary files used to check disk
// health are written to. This will go inside the tmp directory.
public static final String TMP_DISK_CHECK_DIR_NAME = "disk-check";
public static final int HUNDRED_MB = 100 * 1024 * 1024;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we end up using this value, it would be better represented as 100 * OzoneConsts.MB.


/**
* Type for StorageVolume.
Expand Down Expand Up @@ -614,6 +615,14 @@ public synchronized VolumeCheckResult check(@Nullable Boolean unused)
return VolumeCheckResult.HEALTHY;
}

// At least some space required to check disk read/write
// If there are not enough space remaining,
// to avoid volume failure we can ignore checking disk read/write
int minimumDiskSpace = Math.max(healthCheckFileSize * 10, HUNDRED_MB);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks a little arbitrary, what's the reasoning behind these number choices? Something simple like a 2x multiplier on the file size would probably suffice.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kept 100MB to make sure if any other WRITES are happening on volume it should not go out of space during this check and actual disk write check(still cannot guarantee though).
I have changed it to 2x now.

if (volumeInfo.get().getCurrentUsage().getAvailable() < minimumDiskSpace) {
return VolumeCheckResult.HEALTHY;
Comment thread
errose28 marked this conversation as resolved.
}

// Since IO errors may be intermittent, volume remains healthy until the
// threshold of failures is crossed.
boolean diskChecksPassed = DiskCheckUtil.checkReadWrite(storageDir,
Expand All @@ -625,6 +634,13 @@ public synchronized VolumeCheckResult check(@Nullable Boolean unused)
" interrupted.");
}

// As WRITE keeps happening there is probability, disk has become full during above check.
// 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 && volumeInfo.get().getCurrentUsage().getAvailable() < minimumDiskSpace) {
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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

package org.apache.hadoop.ozone.container.common.volume;

import static org.apache.hadoop.ozone.container.common.volume.StorageVolume.HUNDRED_MB;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;

Expand Down Expand Up @@ -110,6 +111,38 @@ public boolean checkExistence(File storageDir) {
assertEquals(VolumeCheckResult.FAILED, result);
}

@ParameterizedTest
@MethodSource("volumeBuilders")
public void testVolumeFullHealth(StorageVolume.Builder<?> builder)
throws Exception {
StorageVolume volume = builder.build();

VolumeUsage usage = volume.getVolumeInfo().get().getUsageForTesting();
// Keep remaining space as just less than 100 MB
usage.incrementUsedSpace(usage.getCurrentUsage().getAvailable() - HUNDRED_MB + 1);
usage.realUsage();
DiskCheckUtil.DiskChecks ioFailure = new DiskCheckUtil.DiskChecks() {
@Override
public boolean checkReadWrite(File storageDir, File testFileDir,
int numBytesToWrite) {
return false;
}
};
DiskCheckUtil.setTestImpl(ioFailure);
// Volume will remain healthy as volume don't have enough space to check READ/WRITE
assertEquals(VolumeCheckResult.HEALTHY, volume.check(false));
// Even in second try volume will remain HEALTHY
assertEquals(VolumeCheckResult.HEALTHY, volume.check(false));

// Now keep enough space for read/write check to go through
usage.decrementUsedSpace(HUNDRED_MB + 1);

// volumeIOFailureTolerance is 1, so first time it will be HEALTHY
assertEquals(VolumeCheckResult.HEALTHY, volume.check(false));
// second time volume will fail as READ/WRITE check has failed
assertEquals(VolumeCheckResult.FAILED, volume.check(false));
}

@ParameterizedTest
@MethodSource("volumeBuilders")
public void testCheckPermissions(StorageVolume.Builder<?> builder)
Expand Down