Uploaded image for project: 'Hadoop Common'
  1. Hadoop Common
  2. HADOOP-2078

Name-node should be able to close empty files.

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.15.0
    • Fix Version/s: 0.16.0
    • Component/s: None
    • Labels:
      None

      Description

      When I try to close an empty file, the name-node throws an exception "Could not complete write to file"
      and issues a warning "NameSystem.completeFile: failed to complete".
      I don't see any reason why empty files should not be allowed.

      1. emptyClose1.patch
        1 kB
        Konstantin Shvachko

        Issue Links

          Activity

          Hide
          hudson Hudson added a comment -
          Show
          hudson Hudson added a comment - Integrated in Hadoop-Nightly #305 (See http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Nightly/305/ )
          Hide
          dhruba dhruba borthakur added a comment -

          I just committed this. Thanks Konstantin!

          Show
          dhruba dhruba borthakur added a comment - I just committed this. Thanks Konstantin!
          Hide
          hadoopqa Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12368449/emptyClose1.patch
          against trunk revision r588341.

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

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

          javac +1. The applied patch does not generate any new compiler warnings.

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

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

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

          Test results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1000/testReport/
          Findbugs warnings: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1000/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1000/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1000/console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12368449/emptyClose1.patch against trunk revision r588341. @author +1. The patch does not contain any @author tags. javadoc +1. The javadoc tool did not generate any warning messages. javac +1. The applied patch does not generate any new compiler warnings. findbugs +1. The patch does not introduce any new Findbugs warnings. core tests +1. The patch passed core unit tests. contrib tests +1. The patch passed contrib unit tests. Test results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1000/testReport/ Findbugs warnings: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1000/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1000/artifact/trunk/build/test/checkstyle-errors.html Console output: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1000/console This message is automatically generated.
          Hide
          shv Konstantin Shvachko added a comment -

          Simplifying the log message is findBugs recommends.

          Show
          shv Konstantin Shvachko added a comment - Simplifying the log message is findBugs recommends.
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12367987/emptyClose.patch
          against trunk revision r588341.

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

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

          javac +1. The applied patch does not generate any new compiler warnings.

          findbugs -1. The patch appears to introduce 2 new Findbugs warnings.

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

          contrib tests -1. The patch failed contrib unit tests.

          Test results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/991/testReport/
          Findbugs warnings: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/991/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/991/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/991/console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12367987/emptyClose.patch against trunk revision r588341. @author +1. The patch does not contain any @author tags. javadoc +1. The javadoc tool did not generate any warning messages. javac +1. The applied patch does not generate any new compiler warnings. findbugs -1. The patch appears to introduce 2 new Findbugs warnings. core tests +1. The patch passed core unit tests. contrib tests -1. The patch failed contrib unit tests. Test results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/991/testReport/ Findbugs warnings: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/991/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/991/artifact/trunk/build/test/checkstyle-errors.html Console output: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/991/console This message is automatically generated.
          Hide
          dhruba dhruba borthakur added a comment -

          Resubmitting patch so that the automatic tests run can occur.

          Show
          dhruba dhruba borthakur added a comment - Resubmitting patch so that the automatic tests run can occur.
          Hide
          shv Konstantin Shvachko added a comment -

          I don't think it is critical for 0.15

          Show
          shv Konstantin Shvachko added a comment - I don't think it is critical for 0.15
          Hide
          dhruba dhruba borthakur added a comment -

          Ok got it. Thanks for the explanation. Would it make sense to enhance the unit test in such a way that this condition is detected?

          Also, do u think that we should put this into the 0.15 release?

          Show
          dhruba dhruba borthakur added a comment - Ok got it. Thanks for the explanation. Would it make sense to enhance the unit test in such a way that this condition is detected? Also, do u think that we should put this into the 0.15 release?
          Hide
          shv Konstantin Shvachko added a comment -

          testZeroSizeFile() does not fail because DFSClient writes 0 bytes into the file before closing.
          When the output stream buffer is empty DFSClient .endBlock() allocates one block and sends 0 bytes to 2 data-nodes.
          The data-nodes create one empty data-file, and one not empty meta-file each and then report to the name-node that the block have been received.
          So empty file is represented by one block of size 0.

          In the case you describe fileBlocks == null and pendingFile != null. So condition

          fileBlocks == null || pendingFile == null
          

          is equivalent to

          fileBlocks == null
          

          They are not equivalent only if fileBlocks != null and pendingFile == null, which never happens. Because no file means no blocks.

          Show
          shv Konstantin Shvachko added a comment - testZeroSizeFile() does not fail because DFSClient writes 0 bytes into the file before closing. When the output stream buffer is empty DFSClient .endBlock() allocates one block and sends 0 bytes to 2 data-nodes. The data-nodes create one empty data-file, and one not empty meta-file each and then report to the name-node that the block have been received. So empty file is represented by one block of size 0. In the case you describe fileBlocks == null and pendingFile != null. So condition fileBlocks == null || pendingFile == null is equivalent to fileBlocks == null They are not equivalent only if fileBlocks != null and pendingFile == null, which never happens. Because no file means no blocks.
          Hide
          dhruba dhruba borthakur added a comment -

          There is a test called TestDFSShell.testZeroSizeFile() that cheks for zero-size files I wonder why it cound not detect this error earlier. Can it be enhanced to catch this problem in future?

          I think it is theoretically possible that a FileUnderConstruction exists, in which case pendingFile will be non-null. But the list of blocks associated with it is null, in which case fileBlocks will be null.

          Show
          dhruba dhruba borthakur added a comment - There is a test called TestDFSShell.testZeroSizeFile() that cheks for zero-size files I wonder why it cound not detect this error earlier. Can it be enhanced to catch this problem in future? I think it is theoretically possible that a FileUnderConstruction exists, in which case pendingFile will be non-null. But the list of blocks associated with it is null, in which case fileBlocks will be null.
          Hide
          shv Konstantin Shvachko added a comment -

          I don't see any tests with empty files, do you? We always write something into it before closing.
          Checking pendingFile is redundant, since if fileBlocks is null then pendingFile is always null as well.

          Show
          shv Konstantin Shvachko added a comment - I don't see any tests with empty files, do you? We always write something into it before closing. Checking pendingFile is redundant, since if fileBlocks is null then pendingFile is always null as well.
          Hide
          dhruba dhruba borthakur added a comment -

          Good catch. I wonder why our unit test does not catch this one. There is a test that checks zero-length files.

          Shouldn't the test be if (fileBlocks == null || pendingFile == null)

          { throw exception() }

          ?

          Show
          dhruba dhruba borthakur added a comment - Good catch. I wonder why our unit test does not catch this one. There is a test that checks zero-length files. Shouldn't the test be if (fileBlocks == null || pendingFile == null) { throw exception() } ?
          Hide
          shv Konstantin Shvachko added a comment -

          This patch solves the problem.

          Show
          shv Konstantin Shvachko added a comment - This patch solves the problem.

            People

            • Assignee:
              shv Konstantin Shvachko
              Reporter:
              shv Konstantin Shvachko
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development