[GitHub] [hadoop] taklwu commented on a diff in pull request #5553: HADOOP-18671 Add recoverLease(), setSafeMode(), isFileClosed() as interfaces to hadoop-common

["taklwu (via GitHub)" <gi...@apache.org>]

taklwu commented on code in PR #5553:
URL: https://github.com/apache/hadoop/pull/5553#discussion_r1182853373


##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestViewDistributedFileSystem.java:
##########
@@ -195,18 +202,103 @@ public void testQuota() throws IOException {
 
   @Test
   public void testPathCapabilities() throws IOException {
-    Configuration conf = getTestConfiguration();
-    try (MiniDFSCluster cluster = new MiniDFSCluster.Builder(conf).numDataNodes(0).build()) {
-      URI defaultUri = URI.create(conf.get(CommonConfigurationKeys.FS_DEFAULT_NAME_KEY));
-      conf.set("fs.viewfs.mounttable." + defaultUri.getHost() + ".linkFallback",
-          defaultUri.toString());
-      try (ViewDistributedFileSystem fileSystem = (ViewDistributedFileSystem) FileSystem.get(
-          conf)) {
-        final Path testFile = new Path("/test");
-        assertTrue("ViewDfs supports truncate",
-            fileSystem.hasPathCapability(testFile, CommonPathCapabilities.FS_TRUNCATE));
-      }
+    try (MiniDFSCluster cluster = new MiniDFSCluster.Builder(getViewFsConfiguration())
+        .numDataNodes(0).build();
+        ViewDistributedFileSystem fileSystem = (ViewDistributedFileSystem) FileSystem.get(
+            cluster.getConfiguration(0))) {
+      final Path testFile = new Path("/test");
+      assertTrue("ViewDfs supports truncate",
+          fileSystem.hasPathCapability(testFile, CommonPathCapabilities.FS_TRUNCATE));
+      final boolean isLeaseRecoverable = fileSystem.hasPathCapability(testFile, LEASE_RECOVERABLE);
+      assertThat(isLeaseRecoverable).describedAs("path capabilities %s=%s in %s",
+          LEASE_RECOVERABLE, fileSystem.hasPathCapability(testFile, LEASE_RECOVERABLE),
+          fileSystem).isTrue();
+      assertThat(fileSystem).describedAs("filesystem %s", fileSystem)
+          .isInstanceOf(LeaseRecoverable.class);
+      assertThat(fileSystem).describedAs("filesystem %s", fileSystem).isInstanceOf(SafeMode.class);
+    }
+  }
+
+  @Test
+  public void testSafeMode() throws IOException {
+    testSafeMode(this::executeAssertionsWithSafeMode);
+  }
+
+  @Test
+  public void testSafeModeWithDeprecatedAPIs() throws IOException {
+    testSafeMode(this::executeAssertionsWithDeprecatedAPIs);
+  }
+
+  private void testSafeMode(Consumer<ViewDistributedFileSystem> executeAssertionsFunction)
+      throws IOException {
+    try (MiniDFSCluster cluster = new MiniDFSCluster.Builder(getViewFsConfiguration())
+        .numDataNodes(0).build();
+        ViewDistributedFileSystem fileSystem = (ViewDistributedFileSystem) FileSystem.get(
+            cluster.getConfiguration(0))) {
+      executeAssertionsFunction.accept(fileSystem);
     }
   }
 
+  private SafeMode verifyAndGetSafeModeInstance(FileSystem fs) {
+    Assertions.assertThat(fs)
+        .describedAs("File system %s must be an instance of %s", fs, SafeMode.class.getClass())
+        .isInstanceOf(SafeMode.class);
+    return (SafeMode) fs;
+  }
+
+  private void executeAssertionsWithSafeMode(ViewDistributedFileSystem fileSystem) {
+    SafeMode fsWithSafeMode = verifyAndGetSafeModeInstance(fileSystem);
+    assertSafeModeStatus(fsWithSafeMode, SafeModeAction.GET, false,
+        "Getting the status of safe mode before entering should be off.");
+    assertSafeModeStatus(fsWithSafeMode, SafeModeAction.ENTER, true,
+        "Entering Safe mode and safe mode turns on.");
+    assertSafeModeStatus(fsWithSafeMode, SafeModeAction.GET, true,
+        "Getting the status of safe mode after entering, safe mode should be on.");
+    assertSafeModeStatus(fsWithSafeMode, SafeModeAction.LEAVE, false,
+        "Leaving safe mode, and safe mode switches off.");
+    assertSafeModeStatus(fsWithSafeMode, SafeModeAction.FORCE_EXIT, false,
+        "Force exist safe mode at any time, safe mode should always switches off.");
+  }
+
+  private void executeAssertionsWithDeprecatedAPIs(ViewDistributedFileSystem fileSystem) {
+    assertSafeModeStatus(fileSystem, HdfsConstants.SafeModeAction.SAFEMODE_GET, false,
+        "Getting the status of safe mode before entering should be off.");
+    assertSafeModeStatus(fileSystem, HdfsConstants.SafeModeAction.SAFEMODE_ENTER, true,
+        "Entering Safe mode and safe mode turns on.");
+    assertSafeModeStatus(fileSystem, HdfsConstants.SafeModeAction.SAFEMODE_GET, true,
+        "Getting the status of safe mode after entering, safe mode should be on.");
+    assertSafeModeStatus(fileSystem, HdfsConstants.SafeModeAction.SAFEMODE_LEAVE, false,
+        "Leaving safe mode, and safe mode switches off.");
+    assertSafeModeStatus(fileSystem, HdfsConstants.SafeModeAction.SAFEMODE_FORCE_EXIT, false,
+        "Force exist safe mode at any time, safe mode should always switches off.");
+  }
+
+  private void assertSafeModeStatus(SafeMode fsWithSafeMode, SafeModeAction action,
+      boolean expectedStatus, String message) {
+    try {
+      Assertions.assertThat(fsWithSafeMode.setSafeMode(action)).describedAs(message)
+          .isEqualTo(expectedStatus);
+    } catch (Exception e) {

Review Comment:
   yup, and thanks that `ConsumerRaisingIOE` really helps in this case



-- 
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: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org