Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-504

HDFS updates the modification time of a file when the file is closed.

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 0.20.1
    • Fix Version/s: 0.21.0
    • Component/s: namenode
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Current HDFS updates the modification time of a file when the file is created. We would like to update the modification time of the file when the file is closed after being written to. This helps HDFS Raid to detect file changes more aggressively.

      Solution includes:
      1. Made changes to closeFile@/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java;
      2. Added unit test to testTimesAtClose@/org/apache/hadoop/hdfs/TestSetTimes.java.

      1. pathfile.txt
        17 kB
        Chun Zhang
      2. pathfile.txt
        7 kB
        Chun Zhang
      3. pathfile.txt
        4 kB
        Chun Zhang
      4. pathfile.txt
        4 kB
        Chun Zhang

        Activity

        Hide
        Chun Zhang added a comment -

        Solution includes:
        1. Made changes to closeFile@/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java;
        2. Added unit test to testTimesAtClose@/org/apache/hadoop/hdfs/TestSetTimes.java.

        Show
        Chun Zhang added a comment - Solution includes: 1. Made changes to closeFile@/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java; 2. Added unit test to testTimesAtClose@/org/apache/hadoop/hdfs/TestSetTimes.java.
        Hide
        dhruba borthakur added a comment -

        If I look at the patch fle, I see that there are lots of extraneous changes because of chages to indentation. Hadoop uses two spaces for indentation. Can you pl ensure that your patch has only the lines that need to be changed for this JIRA? Thanks.

        Show
        dhruba borthakur added a comment - If I look at the patch fle, I see that there are lots of extraneous changes because of chages to indentation. Hadoop uses two spaces for indentation. Can you pl ensure that your patch has only the lines that need to be changed for this JIRA? Thanks.
        Hide
        Chun Zhang added a comment -

        Oops. Set up the right indentation — two spaces, and try again.

        Show
        Chun Zhang added a comment - Oops. Set up the right indentation — two spaces, and try again.
        Hide
        dhruba borthakur added a comment -

        The new patch looks better. There are still a few empty change lines in TestSetModTimes.java, Can you pl remove these empty lines?

        The unit test added a section that verifies that access times and modification times persist after a cluster restart. Maybe this is not needed for a unit test that is testing that the mod-time of a file gets updated when the file is closed?

        Show
        dhruba borthakur added a comment - The new patch looks better. There are still a few empty change lines in TestSetModTimes.java, Can you pl remove these empty lines? The unit test added a section that verifies that access times and modification times persist after a cluster restart. Maybe this is not needed for a unit test that is testing that the mod-time of a file gets updated when the file is closed?
        Hide
        Konstantin Shvachko added a comment -

        To second Dhruba:

        1. Need to remove empty line changes.
        2. Still see tabs or wrong indentation, say around sleep statements.
        3. Please avoid using sleeps in unit tests. This has been discussed previously. It is better to verify the condition you are waiting for than sleep hoping it will be satisfied when you wake up. This can not be uniformly tuned for all machines and environments and usually leads to non-deterministic test failures.
        4. private byte[] fileContents should be a local variable of testTimesAtClose().
        5. Also you use currently unsupported functionality in your test turned on by setting "dfs.support.append" to true. Unsupported means that it may fail, and this has nothing to do with the modTime. Same as Dhruba I think the test should be more focused on the introduced functionality.
        Show
        Konstantin Shvachko added a comment - To second Dhruba: Need to remove empty line changes. Still see tabs or wrong indentation, say around sleep statements. Please avoid using sleeps in unit tests. This has been discussed previously. It is better to verify the condition you are waiting for than sleep hoping it will be satisfied when you wake up. This can not be uniformly tuned for all machines and environments and usually leads to non-deterministic test failures. private byte[] fileContents should be a local variable of testTimesAtClose() . Also you use currently unsupported functionality in your test turned on by setting "dfs.support.append" to true. Unsupported means that it may fail, and this has nothing to do with the modTime. Same as Dhruba I think the test should be more focused on the introduced functionality.
        Hide
        Chun Zhang added a comment -

        1. Removed empty lines;
        2. Corrected indentation;
        3. Removed non-relevant check (e.g., time persist after restart) and non-supported check (e.g., modification time change after append);
        4. Removed unnecessary sleep;
        5. Removed unnecessary fileContents variable.

        Show
        Chun Zhang added a comment - 1. Removed empty lines; 2. Corrected indentation; 3. Removed non-relevant check (e.g., time persist after restart) and non-supported check (e.g., modification time change after append); 4. Removed unnecessary sleep; 5. Removed unnecessary fileContents variable.
        Hide
        Konstantin Shvachko added a comment -

        +1

        Show
        Konstantin Shvachko added a comment - +1
        Hide
        Konstantin Shvachko added a comment -

        Need to see test results and test-patch warnings before it can be committed.

        Show
        Konstantin Shvachko added a comment - Need to see test results and test-patch warnings before it can be committed.
        Show
        Chun Zhang added a comment - Still queued at http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-Admin/lastSuccessfulBuild/artifact/HDFS_PatchQueue.html .
        Hide
        dhruba borthakur added a comment -

        You can run the tests in your own workspace via running something similar to :

        ant -Dpatch.file=pathfile.txt -Dforrest.home=/home/dhruba/forrest/ -Dfindbugs.home=/home/dhruba/findbugs -Dscratch.dir=$RESULTS -Dsvn.cmd=/usr/local/bin/svn -Dgrep.cmd=/bin/grep -Dpatch.cmd=/usr/bin/patch -Djava5.home=/usr/local/jdk1.5.0_07/ test-patch

        Show
        dhruba borthakur added a comment - You can run the tests in your own workspace via running something similar to : ant -Dpatch.file=pathfile.txt -Dforrest.home=/home/dhruba/forrest/ -Dfindbugs.home=/home/dhruba/findbugs -Dscratch.dir=$RESULTS -Dsvn.cmd=/usr/local/bin/svn -Dgrep.cmd=/bin/grep -Dpatch.cmd=/usr/bin/patch -Djava5.home=/usr/local/jdk1.5.0_07/ test-patch
        Hide
        Chun Zhang added a comment -

        [exec] +1 overall.
        [exec]
        [exec] +1 @author. The patch does not contain any @author tags.
        [exec]
        [exec] +1 tests included. The patch appears to include 3 new or modified tests.
        [exec]
        [exec] +1 javadoc. The javadoc tool did not generate any warning messages.
        [exec]
        [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings.
        [exec]
        [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings.
        [exec]
        [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings.

        Show
        Chun Zhang added a comment - [exec] +1 overall. [exec] [exec] +1 @author. The patch does not contain any @author tags. [exec] [exec] +1 tests included. The patch appears to include 3 new or modified tests. [exec] [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings.
        Hide
        Chun Zhang added a comment -

        [junit] Running org.apache.hadoop.hdfs.TestSetTimes
        [junit] Tests run: 2, Failures: 0, Errors: 0, Time elapsed: 9.048 sec

        Show
        Chun Zhang added a comment - [junit] Running org.apache.hadoop.hdfs.TestSetTimes [junit] Tests run: 2, Failures: 0, Errors: 0, Time elapsed: 9.048 sec
        Hide
        Konstantin Shvachko added a comment -

        Could you please run at least the "run-commit-test" target, which is a 10-minute test, rather than just one.
        In general you should run "run-test-hdfs" in order to have higher confidence. This is what Hudson runs for you, ... when it does.

        Show
        Konstantin Shvachko added a comment - Could you please run at least the "run-commit-test" target, which is a 10-minute test, rather than just one. In general you should run "run-test-hdfs" in order to have higher confidence. This is what Hudson runs for you, ... when it does.
        Hide
        Chun Zhang added a comment -

        tested using "ant test" with and without patch;
        tested using "ant run-commit-test" with and without patch.
        "ant test" generated following identical errors with and without patch:

        [junit] Running org.apache.hadoop.hdfs.TestHDFSServerPorts
        [junit] Tests run: 4, Failures: 1, Errors: 0, Time elapsed: 3.845 sec
        [junit] Running org.apache.hadoop.hdfs.server.namenode.TestBackupNode
        [junit] Tests run: 2, Failures: 1, Errors: 0, Time elapsed: 37.159 sec

        "ant run-commit-test" generated following identical errors with and without patch:

        [junit] Running org.apache.hadoop.hdfs.TestHDFSServerPorts
        [junit] Tests run: 4, Failures: 1, Errors: 0, Time elapsed: 35.492 sec
        [junit] Running org.apache.hadoop.hdfs.server.namenode.TestBackupNode
        [junit] Tests run: 1, Failures: 0, Errors: 1, Time elapsed: 0 sec

        Since #Failures/Errors is the same with and without patch, it should be OK to check in.

        However, to have even higher confidence, let wait for Hudson's result.

        Show
        Chun Zhang added a comment - tested using "ant test" with and without patch; tested using "ant run-commit-test" with and without patch. "ant test" generated following identical errors with and without patch: [junit] Running org.apache.hadoop.hdfs.TestHDFSServerPorts [junit] Tests run: 4, Failures: 1, Errors: 0, Time elapsed: 3.845 sec [junit] Running org.apache.hadoop.hdfs.server.namenode.TestBackupNode [junit] Tests run: 2, Failures: 1, Errors: 0, Time elapsed: 37.159 sec "ant run-commit-test" generated following identical errors with and without patch: [junit] Running org.apache.hadoop.hdfs.TestHDFSServerPorts [junit] Tests run: 4, Failures: 1, Errors: 0, Time elapsed: 35.492 sec [junit] Running org.apache.hadoop.hdfs.server.namenode.TestBackupNode [junit] Tests run: 1, Failures: 0, Errors: 1, Time elapsed: 0 sec Since #Failures/Errors is the same with and without patch, it should be OK to check in. However, to have even higher confidence, let wait for Hudson's result.
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12414676/pathfile.txt
        against trunk revision 799695.

        +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/Hdfs-Patch-vesta.apache.org/37/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-vesta.apache.org/37/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-vesta.apache.org/37/artifact/trunk/build/test/checkstyle-errors.html
        Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-vesta.apache.org/37/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/12414676/pathfile.txt against trunk revision 799695. +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/Hdfs-Patch-vesta.apache.org/37/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-vesta.apache.org/37/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-vesta.apache.org/37/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-vesta.apache.org/37/console This message is automatically generated.
        Hide
        dhruba borthakur added a comment -

        I just committed this. Thanks Chun!

        Show
        dhruba borthakur added a comment - I just committed this. Thanks Chun!
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk #38 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk/38/)
        . Update the modification time of a file when the file
        is closed. (Chun Zhang via dhruba)

        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #38 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk/38/ ) . Update the modification time of a file when the file is closed. (Chun Zhang via dhruba)
        Hide
        dhruba borthakur added a comment -

        This was erroneously marked as fixed in 0.20.1. Actually, it has been fixed in 0.21.

        Show
        dhruba borthakur added a comment - This was erroneously marked as fixed in 0.20.1. Actually, it has been fixed in 0.21.

          People

          • Assignee:
            Chun Zhang
            Reporter:
            Chun Zhang
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - 2h
              2h
              Remaining:
              Remaining Estimate - 2h
              2h
              Logged:
              Time Spent - Not Specified
              Not Specified

                Development