[ 
https://issues.apache.org/jira/browse/HBASE-8699?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13702721#comment-13702721
 ] 

stack commented on HBASE-8699:
------------------------------

I think you need test to prove that regex finds the right versions.  Just do a 
little method in here in your test.

Why we do it like this:

{code}
+  @Test
+  public void testExistenceOfIsFileClosed() throws IOException {
+    String hadoopVer = VersionInfo.getVersion();
+    // known versions where DistributedFileSystem supports IsFileClosed(Path)
+    final Pattern knownVers[] = {
+        Pattern.compile("1\\.2\\."),
+        Pattern.compile("2\\.1\\.")
+    };
+    for (Pattern pat : knownVers) {
+      if (pat.matcher(hadoopVer).matches()) {
+        try {
+          DistributedFileSystem.class.getMethod("isFileClosed", new Class[]{ 
Path.class });
+          break;
+        } catch (NoSuchMethodException nsme) {
+          throw new IOException(nsme);
+        }
+      }
+    }
+  }
{code}

Why not just do reflection to see if method is there?  Skip the test if it is 
not present?

What about hadoop 3.x?

What is this test doing?  Testing that a hadoop version has isFileClosed?  Do 
we care?  Either it is present or it is not.  If it is not present when we 
expect it in a particular hadoop, what will we do?  Why not add a unit test for 
all methods we expect in hadoop?  Remove it I'd say?

We will do this reflection (and fail most of the time, at least currently) 
whenever we make this call?

{code}
+    
+    Method isFileClosedMeth = null;
+    try {
+      isFileClosedMeth = dfs.getClass().getMethod("isFileClosed", new Class[]{ 
Path.class });
+    } catch (NoSuchMethodException nsme) {
+      LOG.debug("isFileClosed not available");
+    }
{code}

Why not do it on instantiation once?
                
> Parameter to DistributedFileSystem#isFileClosed should be of type Path
> ----------------------------------------------------------------------
>
>                 Key: HBASE-8699
>                 URL: https://issues.apache.org/jira/browse/HBASE-8699
>             Project: HBase
>          Issue Type: Bug
>          Components: wal
>            Reporter: Ted Yu
>            Assignee: Ted Yu
>         Attachments: 8699-v1.txt, 8699-v3.txt, 8699-v4.txt
>
>
> Here is current code of FSHDFSUtils#isFileClosed():
> {code}
>   boolean isFileClosed(final DistributedFileSystem dfs, final Path p) {
>     try {
>       Method m = dfs.getClass().getMethod("isFileClosed", new Class<?>[] 
> {String.class});
>       return (Boolean) m.invoke(dfs, p.toString());
> {code}
> We look for isFileClosed method with parameter type of String.
> However, from 
> hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java
>  (branch-2):
> {code}
>   public boolean isFileClosed(Path src) throws IOException {
> {code}
> The parameter type is of Path.
> This means we would get NoSuchMethodException.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Reply via email to