Hadoop Common
  1. Hadoop Common
  2. HADOOP-6690

FilterFileSystem doesn't overwrite setTimes

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.22.0
    • Fix Version/s: 0.21.0
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      FilterFileSystem seems to be a little outdated and it doesn't implement a few methods (setTimes being the most important one):

      • setTimes(Path, long, long)
      • copyFromLocalFile(boolean,boolean, Path, Path)
      • copyFromLocalFile(boolean, boolean, Path[], Path)
      • getUsed()
      • deleteOnExit()

      I'm not sure if all of these methods should be wrapped in FilterFileSystem, but given its purpose, I would say the more the better. It would be great to have other people's opinions about this.

      1. HADOOP-6690.2.patch
        8 kB
        Rodrigo Schmidt
      2. HADOOP-6690.1.patch
        9 kB
        Rodrigo Schmidt
      3. HADOOP-6690.0.patch
        9 kB
        Rodrigo Schmidt

        Issue Links

          Activity

          Hide
          Eli Collins added a comment -

          Can you use FilterFs? It has setTimes.

          Show
          Eli Collins added a comment - Can you use FilterFs? It has setTimes.
          Hide
          Rodrigo Schmidt added a comment -

          Is FilterFs supposed to suppress FilterFileSystem? When was it introduced?

          Show
          Rodrigo Schmidt added a comment - Is FilterFs supposed to suppress FilterFileSystem? When was it introduced?
          Hide
          Eli Collins added a comment -

          FilterFs is the AbstractFileSystem version of FilterFileSystem. If you're doing work on trunk you should be writing against the new APIs (or at least make sure they stay on par), if you're making a change against branch 20 then you'll want FilterFileSystem. I'm assuming you want setTimes to work on LocalFileSystem?

          Show
          Eli Collins added a comment - FilterFs is the AbstractFileSystem version of FilterFileSystem. If you're doing work on trunk you should be writing against the new APIs (or at least make sure they stay on par), if you're making a change against branch 20 then you'll want FilterFileSystem. I'm assuming you want setTimes to work on LocalFileSystem?
          Hide
          Rodrigo Schmidt added a comment -

          I noticed the problem when using contrib/raid, which currently relies on FilterFileSystem (because it was originally implemented on hadoop 0.20). Maybe we should change the trunk version to work with FilterFs.

          However, this workaround doesn't make FilterFileSystem more correct. We should either fix it or deprecate it so that people don't feel compelled to use it in the future. What do you think?

          Show
          Rodrigo Schmidt added a comment - I noticed the problem when using contrib/raid, which currently relies on FilterFileSystem (because it was originally implemented on hadoop 0.20). Maybe we should change the trunk version to work with FilterFs. However, this workaround doesn't make FilterFileSystem more correct. We should either fix it or deprecate it so that people don't feel compelled to use it in the future. What do you think?
          Hide
          Eli Collins added a comment -

          Having FilterFileSystem override setTimes (and other APIs) is very reasonable. Just pointing out the new APIs in case you were doing trunk work. We'll want to move contrib/raid off FileSystem as part of HADOOP-6446.

          Show
          Eli Collins added a comment - Having FilterFileSystem override setTimes (and other APIs) is very reasonable. Just pointing out the new APIs in case you were doing trunk work. We'll want to move contrib/raid off FileSystem as part of HADOOP-6446 .
          Hide
          Rodrigo Schmidt added a comment -

          Sure! Thanks for the hint, BTW.

          With respect to HADOOP-6446, I should be able to help with Raid/Har/DistCp as soon as I'm done with some critical raid patches.

          Show
          Rodrigo Schmidt added a comment - Sure! Thanks for the hint, BTW. With respect to HADOOP-6446 , I should be able to help with Raid/Har/DistCp as soon as I'm done with some critical raid patches.
          Hide
          Rodrigo Schmidt added a comment -

          Uploading patch

          Show
          Rodrigo Schmidt added a comment - Uploading patch
          Hide
          Eli Collins added a comment -

          +1 Patch looks good. Mind adding TestFilterFs that's a FilterFs equivalent to TestFilterFileSystem? Should be an pretty easy port.

          Show
          Eli Collins added a comment - +1 Patch looks good. Mind adding TestFilterFs that's a FilterFs equivalent to TestFilterFileSystem? Should be an pretty easy port.
          Hide
          Rodrigo Schmidt added a comment -

          Sure! I can do that. I'll update the patch later today.

          Show
          Rodrigo Schmidt added a comment - Sure! I can do that. I'll update the patch later today.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12442315/HADOOP-6690.0.patch
          against trunk revision 934619.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 3 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          -1 javac. The applied patch generated 1028 javac compiler warnings (more than the trunk's current 1024 warnings).

          +1 findbugs. The patch does not introduce any new Findbugs warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/466/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/466/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/466/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/466/console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12442315/HADOOP-6690.0.patch against trunk revision 934619. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. -1 javac. The applied patch generated 1028 javac compiler warnings (more than the trunk's current 1024 warnings). +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/466/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/466/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/466/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/466/console This message is automatically generated.
          Hide
          Rodrigo Schmidt added a comment -

          The warnings are generated because the methods of the DontCheck class are used by reflection. I'll silence these warnings in my next patch, but I'm not sure if Hadoop QA won't complain about that.

          Show
          Rodrigo Schmidt added a comment - The warnings are generated because the methods of the DontCheck class are used by reflection. I'll silence these warnings in my next patch, but I'm not sure if Hadoop QA won't complain about that.
          Hide
          Rodrigo Schmidt added a comment -

          Eli, while I was creating TestFilterFs, I realized there was a number of methods that were not being implemented. Some of them are quite suspicious like getUri(), getStatistics(), getHomeDirectory(), ... Can you please double check if FilterFs is fine without these methods. If it's not the case, it would be better to create a separate JIRA for that.

          Here is the complete list:

          {
              public FSDataInputStream open(final Path f) { return null; }
              public void checkPath(Path path) { }
              public Statistics getStatistics() { return null; }
              public URI getUri() { return null; }
              public Path getHomeDirectory() { return null; }
              public void checkScheme(URI uri, String supportedScheme) { }
              public String getUriPath(final Path p) { return null; }
              public void renameInternal(final Path src, final Path dst, boolean overwrite) { }
              public FsStatus getFsStatus(final Path f) { return null; }
          }
          
          Show
          Rodrigo Schmidt added a comment - Eli, while I was creating TestFilterFs, I realized there was a number of methods that were not being implemented. Some of them are quite suspicious like getUri(), getStatistics(), getHomeDirectory(), ... Can you please double check if FilterFs is fine without these methods. If it's not the case, it would be better to create a separate JIRA for that. Here is the complete list: { public FSDataInputStream open( final Path f) { return null ; } public void checkPath(Path path) { } public Statistics getStatistics() { return null ; } public URI getUri() { return null ; } public Path getHomeDirectory() { return null ; } public void checkScheme(URI uri, String supportedScheme) { } public String getUriPath( final Path p) { return null ; } public void renameInternal( final Path src, final Path dst, boolean overwrite) { } public FsStatus getFsStatus( final Path f) { return null ; } }
          Hide
          Rodrigo Schmidt added a comment -

          New patch without any warnings.

          Show
          Rodrigo Schmidt added a comment - New patch without any warnings.
          Hide
          Rodrigo Schmidt added a comment -

          New patch without any warnings.

          Show
          Rodrigo Schmidt added a comment - New patch without any warnings.
          Hide
          Rodrigo Schmidt added a comment -

          I was talking to Dhruba and we thought it would be better to create a new issue to discuss the methods missing on FilterFs() and its unit test. I just created HADOOP-6719 for that.

          Show
          Rodrigo Schmidt added a comment - I was talking to Dhruba and we thought it would be better to create a new issue to discuss the methods missing on FilterFs() and its unit test. I just created HADOOP-6719 for that.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12442377/HADOOP-6690.1.patch
          against trunk revision 934619.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 3 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          -1 javac. The applied patch generated 1028 javac compiler warnings (more than the trunk's current 1024 warnings).

          +1 findbugs. The patch does not introduce any new Findbugs warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/467/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/467/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/467/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/467/console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12442377/HADOOP-6690.1.patch against trunk revision 934619. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. -1 javac. The applied patch generated 1028 javac compiler warnings (more than the trunk's current 1024 warnings). +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/467/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/467/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/467/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/467/console This message is automatically generated.
          Hide
          Rodrigo Schmidt added a comment -

          Found a couple of more warnings.

          Show
          Rodrigo Schmidt added a comment - Found a couple of more warnings.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12442393/HADOOP-6690.2.patch
          against trunk revision 934619.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 3 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h1.grid.sp2.yahoo.net/46/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h1.grid.sp2.yahoo.net/46/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h1.grid.sp2.yahoo.net/46/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h1.grid.sp2.yahoo.net/46/console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12442393/HADOOP-6690.2.patch against trunk revision 934619. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h1.grid.sp2.yahoo.net/46/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h1.grid.sp2.yahoo.net/46/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h1.grid.sp2.yahoo.net/46/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h1.grid.sp2.yahoo.net/46/console This message is automatically generated.
          Hide
          dhruba borthakur added a comment -

          +1 thanks, I will commit this patch.

          Show
          dhruba borthakur added a comment - +1 thanks, I will commit this patch.
          Hide
          dhruba borthakur added a comment -

          I just committed this. Thanks Rodrigo!

          Show
          dhruba borthakur added a comment - I just committed this. Thanks Rodrigo!

            People

            • Assignee:
              Rodrigo Schmidt
              Reporter:
              Rodrigo Schmidt
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development