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


##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java:
##########
@@ -169,7 +169,8 @@
 @InterfaceAudience.Public
 @InterfaceStability.Stable
 public abstract class FileSystem extends Configured
-    implements Closeable, DelegationTokenIssuer, PathCapabilities {
+  implements Closeable, DelegationTokenIssuer, PathCapabilities, PathStatus, 
LeaseRecoverable,

Review Comment:
   move to hdfs/ozone/webhdfs only



##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/SafeMode.java:
##########
@@ -0,0 +1,47 @@
+/*
+ * 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.hadoop.fs;
+
+import java.io.IOException;
+
+/**
+ * Whether the given filesystem is in any status of safe mode.
+ */
+public interface SafeMode {
+
+  /**
+   * Enter, leave, or get safe mode.
+   *
+   * @param action One of {@link SafeModeAction} LEAVE, ENTER, GET, FORCE_EXIT
+   */
+  default boolean setSafeMode(SafeModeAction action) throws IOException {
+    return setSafeMode(action, false);
+  }
+
+  /**
+   * Enter, leave, or get safe mode.
+   *
+   * @param action One of {@link SafeModeAction} LEAVE, ENTER, GET, FORCE_EXIT
+   * @param isChecked If true check only for Active metadata node / NameNode's 
status,
+   *                  else check first metadata node / NameNode's status
+   */
+  default boolean setSafeMode(SafeModeAction action, boolean isChecked) throws 
IOException {
+    throw new IOException("This file system does not support safe mode");

Review Comment:
   throw UnsupportedOperationException();



##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/SafeModeAction.java:
##########
@@ -0,0 +1,26 @@
+/*
+ * 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.hadoop.fs;
+
+/**
+ * An identical copy from 
org.apache.hadoop.hdfs.protocol.HdfsConstants.SafeModeAction, that helps
+ * the other file system implementation to define {@link SafeMode}.
+ */
+public enum SafeModeAction {
+  SAFEMODE_LEAVE, SAFEMODE_ENTER, SAFEMODE_GET, SAFEMODE_FORCE_EXIT;

Review Comment:
   1. javadocs?
   2. does having the duplicate wit h the same name complicate life?



##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestFileSystemInitialization.java:
##########
@@ -105,5 +108,27 @@ public void testFileSystemCapabilities() throws Throwable {
             ETAGS_PRESERVED_IN_RENAME, etagsAcrossRename,
             FS_ACLS, acls, fs)
         .isEqualTo(acls);
+
+    final boolean leaseRecovery = fs.hasPathCapability(p, LEASE_RECOVERABLE);

Review Comment:
   same as for s3a



##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/PathStatus.java:
##########
@@ -0,0 +1,37 @@
+/*
+ * 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.hadoop.fs;
+
+import java.io.IOException;
+
+/**
+ * Interface to check the status of the given Path

Review Comment:
   nit, add a . so javadoc is happy



##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/LeaseRecoverable.java:
##########
@@ -0,0 +1,39 @@
+/*
+ * 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.hadoop.fs;
+
+import java.io.IOException;
+
+/**
+ * Whether the given Path of the FileSystem has the capability to performance
+ * lease recovery.
+ */
+public interface LeaseRecoverable {
+
+  /**
+   * Start the lease recovery of a file
+   *
+   * @param file path to a file
+   * @return true if the file is already closed, and it does not require lease 
recovery
+   * @throws IOException if an error occurs during lease recovery
+   */
+  default boolean recoverLease(Path file) throws IOException {
+    throw new IOException("This file system does not support lease recovery 
for Path " + file);

Review Comment:
   throw UnsupportedOperationException() 



##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AFileSystemContract.java:
##########
@@ -137,4 +142,30 @@ public void testRenameNonExistentPath() throws Exception {
         () -> super.testRenameNonExistentPath());
 
   }
+
+  @Test
+  public void testFileSystemCapabilities() throws Exception {

Review Comment:
   if the methods are on an interface only implemented by a few stores, there's 
no need to worry about s3a, abfs etc



##########
hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java:
##########
@@ -1633,6 +1634,53 @@ public DatanodeInfo[] getDataNodeStats(final 
DatanodeReportType type)
    * @see org.apache.hadoop.hdfs.protocol.ClientProtocol#setSafeMode(
    *    HdfsConstants.SafeModeAction,boolean)
    */
+  @Override
+  public boolean setSafeMode(SafeModeAction action)
+    throws IOException {
+    return setSafeMode(action, false);
+  }
+
+  /**
+   * Enter, leave or get safe mode.
+   *
+   * @param action
+   *          One of SafeModeAction.ENTER, SafeModeAction.LEAVE and
+   *          SafeModeAction.GET
+   * @param isChecked
+   *          If true check only for Active NNs status, else check first NN's
+   *          status
+   */
+  @Override
+  public boolean setSafeMode(SafeModeAction action, boolean isChecked)
+    throws IOException {
+    return dfs.setSafeMode(convertToClientProtocolSafeModeAction(action), 
isChecked);
+  }
+
+  private HdfsConstants.SafeModeAction convertToClientProtocolSafeModeAction(

Review Comment:
   does webhdfs export this api too? and if so, should this be static/public



##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/PathStatus.java:
##########
@@ -0,0 +1,37 @@
+/*
+ * 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.hadoop.fs;
+
+import java.io.IOException;
+
+/**
+ * Interface to check the status of the given Path
+ */
+public interface PathStatus {
+
+  /**
+   * Get the close status of a file.
+   * @param file The string representation of the path to the file
+   * @return return true if file is closed
+   * @throws IOException If an I/O error occurred
+   */
+  default boolean isFileClosed(Path file) throws IOException {
+    throw new IOException("This file system does not support isFileClosed for 
path " + file);

Review Comment:
   throw UnsupportedOperationException() 



-- 
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]

Reply via email to