HDDS-15623. Disk check may not completely read test file (#10552)

Author: adoroszlai

hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/DiskCheckUtil.java

@@ -30,9 +30,11 @@
 import java.io.SyncFailedException;
 import java.nio.file.Files;
 import java.nio.file.NoSuchFileException;
+import java.nio.file.Path;
 import java.util.Arrays;
 import java.util.Random;
 import java.util.UUID;
+import org.apache.commons.io.IOUtils;
 import org.apache.ratis.util.FileUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -141,9 +143,10 @@ public boolean checkPermissions(File storageDir) {
     public boolean checkReadWrite(File storageDir,
         File testFileDir, int numBytesToWrite) {
       File testFile = new File(testFileDir, "disk-check-" + UUID.randomUUID());
+      Path testPath = testFile.toPath();
       byte[] writtenBytes = new byte[numBytesToWrite];
       RANDOM.nextBytes(writtenBytes);
-      try (OutputStream fos = FileUtils.newOutputStreamForceAtClose(testFile, CREATE, TRUNCATE_EXISTING, WRITE)) {
+      try (OutputStream fos = FileUtils.newOutputStreamForceAtClose(testPath, CREATE, TRUNCATE_EXISTING, WRITE)) {
         fos.write(writtenBytes);
       } catch (FileNotFoundException | NoSuchFileException notFoundEx) {
         logError(storageDir, String.format("Could not find file %s for " +
@@ -152,35 +155,41 @@ public boolean checkReadWrite(File storageDir,
       } catch (SyncFailedException syncEx) {
         logError(storageDir, String.format("Could not sync file %s to disk.",
             testFile.getAbsolutePath()), syncEx);
+        FileUtils.deletePathQuietly(testPath);
         return false;
       } catch (IOException ioEx) {
         String msg = ioEx.getMessage();
         if (msg != null && msg.contains(LINUX_DISK_FULL_MESSAGE)) {
           LOG.warn("Could not write file {} for volume check", testFile.getAbsolutePath(), ioEx);
+          FileUtils.deletePathQuietly(testPath);
           return true;
         }
         logError(storageDir, String.format("Could not write file %s " +
             "for volume check.", testFile.getAbsolutePath()), ioEx);
+        FileUtils.deletePathQuietly(testPath);
         return false;
       }
 
       // Read data back from the test file.
       byte[] readBytes = new byte[numBytesToWrite];
-      try (InputStream fis = Files.newInputStream(testFile.toPath())) {
-        int numBytesRead = fis.read(readBytes);
+      try (InputStream fis = Files.newInputStream(testPath)) {
+        int numBytesRead = IOUtils.read(fis, readBytes);
         if (numBytesRead != numBytesToWrite) {
           logError(storageDir, String.format("%d bytes written to file %s " +
                   "but %d bytes were read back.", numBytesToWrite,
               testFile.getAbsolutePath(), numBytesRead));
+          FileUtils.deletePathQuietly(testPath);
           return false;
         }
       } catch (FileNotFoundException | NoSuchFileException notFoundEx) {
         logError(storageDir, String.format("Could not find file %s " +
             "for volume check.", testFile.getAbsolutePath()), notFoundEx);
+        FileUtils.deletePathQuietly(testPath);
         return false;
       } catch (IOException ioEx) {
         logError(storageDir, String.format("Could not read file %s " +
             "for volume check.", testFile.getAbsolutePath()), ioEx);
+        FileUtils.deletePathQuietly(testPath);
         return false;
       }
 
@@ -189,6 +198,7 @@ public boolean checkReadWrite(File storageDir,
         logError(storageDir, String.format("%d Bytes read from file " +
                 "%s do not match the %d bytes that were written.",
             writtenBytes.length, testFile.getAbsolutePath(), readBytes.length));
+        FileUtils.deletePathQuietly(testPath);
         return false;
       }
 

hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/utils/TestDiskCheckUtil.java

@@ -28,6 +28,7 @@
 import java.io.File;
 import java.nio.file.FileSystemException;
 import java.nio.file.OpenOption;
+import java.nio.file.Path;
 import org.apache.ratis.util.FileUtils;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
@@ -84,7 +85,10 @@ public void testExistence() {
   @Test
   public void testReadWrite() {
     assertTrue(DiskCheckUtil.checkReadWrite(testDir, testDir, 10));
+    assertTestFileDeleted();
+  }
 
+  private void assertTestFileDeleted() {
     // Test file should have been deleted.
     File[] children = testDir.listFiles();
     assertNotNull(children);
@@ -95,16 +99,16 @@ public void testReadWrite() {
   public void testCheckReadWriteDiskFull() {
     try (MockedStatic<FileUtils> mockService = mockStatic(FileUtils.class)) {
       // fos.write(writtenBytes) also through FileSystemException with the message
-      mockService.when(() -> FileUtils.newOutputStreamForceAtClose(any(File.class), any(OpenOption[].class)))
+      mockService.when(() -> FileUtils.newOutputStreamForceAtClose(any(Path.class), any(OpenOption[].class)))
           .thenThrow(new FileSystemException("No space left on device"));
 
-      String path = testDir.getAbsolutePath();
       assertThrows(FileSystemException.class,
-          () -> FileUtils.newOutputStreamForceAtClose(new File(path), new OpenOption[2]));
+          () -> FileUtils.newOutputStreamForceAtClose(testDir.toPath(), new OpenOption[2]));
 
       // Test that checkReadWrite returns true for the disk full case
       boolean result = DiskCheckUtil.checkReadWrite(testDir, testDir, 1024);
       assertTrue(result, "checkReadWrite should return true when disk is full");
+      assertTestFileDeleted();
     }
   }
 }