Uploaded image for project: 'Hadoop HDFS'
  1. Hadoop HDFS
  2. HDFS-12191

Provide option to not capture the accessTime change of a file to snapshot if no other modification has been done to this file

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 3.0.0-beta1
    • Fix Version/s: 2.9.0, 3.0.0-beta1
    • Component/s: hdfs, namenode
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Currently, if the accessTime of a file changed before a snapshot is taken, this accessTime will be captured in the snapshot, even if there is no other modifications made to this file.

      Because of this, when we calculate snapshotDiff, more work need to be done for this file, e,g,, metadataEquals method will be called, even if there is no modification is made (thus not recorded to snapshotDiff). This can cause snapshotDiff to slow down quite a lot when there are a lot of files to be examined.

      This jira is to provide an option to skip capturing accessTime only change to snapshot. Thus snapshotDiff can be done faster.

      When accessTime of a file changed, if there is other modification to the file, the access time will still be captured in snapshot.

      Sometimes we want accessTime be captured to snapshot, such that when restoring from the snapshot, we know the accessTime of this snapshot. So this new feature is optional, and is controlled by a config property.

      Worth to mention is, how accurately the acessTime is captured is dependent on the following config that has default value of 1 hour, which means new access within an hour of previous access will not be captured.

      public static final String  DFS_NAMENODE_ACCESSTIME_PRECISION_KEY =
            HdfsClientConfigKeys.DeprecatedKeys.DFS_NAMENODE_ACCESSTIME_PRECISION_KEY;
      public static final long    DFS_NAMENODE_ACCESSTIME_PRECISION_DEFAULT = 3600000;
      

      .

      1. HDFS-12191.001.patch
        15 kB
        Yongjun Zhang
      2. HDFS-12191.002.patch
        15 kB
        Yongjun Zhang
      3. HDFS-12191.003.patch
        19 kB
        Yongjun Zhang
      4. HDFS-12191.004.patch
        20 kB
        Yongjun Zhang
      5. HDFS-12191.005.patch
        20 kB
        Yongjun Zhang

        Activity

        Hide
        yzhangal Yongjun Zhang added a comment -

        Submitted patch 001.

        Show
        yzhangal Yongjun Zhang added a comment - Submitted patch 001.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 16s Docker mode activated.
              Prechecks
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
              trunk Compile Tests
        +1 mvninstall 13m 24s trunk passed
        +1 compile 0m 54s trunk passed
        +1 checkstyle 0m 43s trunk passed
        +1 mvnsite 0m 53s trunk passed
        -1 findbugs 1m 40s hadoop-hdfs-project/hadoop-hdfs in trunk has 10 extant Findbugs warnings.
        +1 javadoc 0m 38s trunk passed
              Patch Compile Tests
        +1 mvninstall 0m 48s the patch passed
        +1 compile 0m 46s the patch passed
        +1 javac 0m 46s the patch passed
        -0 checkstyle 0m 40s hadoop-hdfs-project/hadoop-hdfs: The patch generated 6 new + 568 unchanged - 0 fixed = 574 total (was 568)
        +1 mvnsite 0m 51s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 xml 0m 2s The patch has no ill-formed XML file.
        +1 findbugs 1m 47s the patch passed
        +1 javadoc 0m 38s the patch passed
              Other Tests
        +1 unit 64m 36s hadoop-hdfs in the patch passed.
        +1 asflicense 0m 19s The patch does not generate ASF License warnings.
        90m 13s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:14b5c93
        JIRA Issue HDFS-12191
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12878561/HDFS-12191.001.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
        uname Linux 6d550cea3de0 3.13.0-116-generic #163-Ubuntu SMP Fri Mar 31 14:13:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 2054324
        Default Java 1.8.0_131
        findbugs v3.1.0-RC1
        findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/20393/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-warnings.html
        checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/20393/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/20393/testReport/
        modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/20393/console
        Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 16s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.       trunk Compile Tests +1 mvninstall 13m 24s trunk passed +1 compile 0m 54s trunk passed +1 checkstyle 0m 43s trunk passed +1 mvnsite 0m 53s trunk passed -1 findbugs 1m 40s hadoop-hdfs-project/hadoop-hdfs in trunk has 10 extant Findbugs warnings. +1 javadoc 0m 38s trunk passed       Patch Compile Tests +1 mvninstall 0m 48s the patch passed +1 compile 0m 46s the patch passed +1 javac 0m 46s the patch passed -0 checkstyle 0m 40s hadoop-hdfs-project/hadoop-hdfs: The patch generated 6 new + 568 unchanged - 0 fixed = 574 total (was 568) +1 mvnsite 0m 51s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 2s The patch has no ill-formed XML file. +1 findbugs 1m 47s the patch passed +1 javadoc 0m 38s the patch passed       Other Tests +1 unit 64m 36s hadoop-hdfs in the patch passed. +1 asflicense 0m 19s The patch does not generate ASF License warnings. 90m 13s Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue HDFS-12191 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12878561/HDFS-12191.001.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 6d550cea3de0 3.13.0-116-generic #163-Ubuntu SMP Fri Mar 31 14:13:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 2054324 Default Java 1.8.0_131 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/20393/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-warnings.html checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/20393/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/20393/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/20393/console Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        manojg Manoj Govindassamy added a comment - - edited

        Jing Zhao, Zhe Zhang, Nicholas,
        Would like to understand the rational behind "atime" only or "mtime" only change being part of recordModification(). Technically speaking, it makes sense to record modification for any meta data modification, and since setTime() is changing the meta data, it wants to do record modification. So, would like to know what we would be missing if we are going to skip recoding modification for "atime" changes. Your thoughts please?

        Also, if we want to skip the "atime" only changes to be recorded, the already existing access precision time configuration sounds like a good approach. Having one more configuration to skip atime in the snap diffs, which will override the existing access precision time doesn't gel well.

        Show
        manojg Manoj Govindassamy added a comment - - edited Jing Zhao , Zhe Zhang , Nicholas, Would like to understand the rational behind "atime" only or "mtime" only change being part of recordModification(). Technically speaking, it makes sense to record modification for any meta data modification, and since setTime() is changing the meta data, it wants to do record modification. So, would like to know what we would be missing if we are going to skip recoding modification for "atime" changes. Your thoughts please? Also, if we want to skip the "atime" only changes to be recorded, the already existing access precision time configuration sounds like a good approach. Having one more configuration to skip atime in the snap diffs, which will override the existing access precision time doesn't gel well.
        Hide
        yzhangal Yongjun Zhang added a comment -

        Thanks much Manoj Govindassamy for the comment.

        Please notice that this jira is about about providing a config to enable skipping recording atime only change (no other modification than atime), not mtime. So to make snapshotDiff calculation much faster. My understanding is, the original implementation is trying to be complete, that is, any modification to a file including access time is recorded in a snapshot.

        The reason this change can potentially make snapshotDiff calc much faster is, not capturing atime only change in snapshot can prevent this kind of files get to metadataEquals method (which is expensive, especially when external auth provider is turned on). But of course we may lose the accurate access time in snapshot (only if access time is the only thing changed about the file).

        Notice that snapshotDiff will not report atime only change as diff.

        Hi Jing Zhao, Tsz Wo Nicholas Sze and Zhe Zhang,

        Would appreciate if you guys can help taking a look and comment.

        Thanks a lot.

        Show
        yzhangal Yongjun Zhang added a comment - Thanks much Manoj Govindassamy for the comment. Please notice that this jira is about about providing a config to enable skipping recording atime only change (no other modification than atime), not mtime. So to make snapshotDiff calculation much faster. My understanding is, the original implementation is trying to be complete, that is, any modification to a file including access time is recorded in a snapshot. The reason this change can potentially make snapshotDiff calc much faster is, not capturing atime only change in snapshot can prevent this kind of files get to metadataEquals method (which is expensive, especially when external auth provider is turned on). But of course we may lose the accurate access time in snapshot (only if access time is the only thing changed about the file). Notice that snapshotDiff will not report atime only change as diff. Hi Jing Zhao , Tsz Wo Nicholas Sze and Zhe Zhang , Would appreciate if you guys can help taking a look and comment. Thanks a lot.
        Hide
        manojg Manoj Govindassamy added a comment -

        If the atime recorded in the snapshots for the files are not useful, I would prefer to avoid recording them them without even having a config which would be always turned on.

        Show
        manojg Manoj Govindassamy added a comment - If the atime recorded in the snapshots for the files are not useful, I would prefer to avoid recording them them without even having a config which would be always turned on.
        Hide
        yzhangal Yongjun Zhang added a comment - - edited

        Thanks Manoj Govindassamy.

        • We don't want to break backward compatibility, so I made the solution only enabled with config, and disabled by default.
        • Though it'd be uncommon, some user may still need the atime in snapshot.
        Show
        yzhangal Yongjun Zhang added a comment - - edited Thanks Manoj Govindassamy . We don't want to break backward compatibility, so I made the solution only enabled with config, and disabled by default. Though it'd be uncommon, some user may still need the atime in snapshot.
        Hide
        manojg Manoj Govindassamy added a comment -

        Thanks for working on this Yongjun Zhang. My comments below.

        1. DFSConfigKeys

        DFS_NAMENODE_DONT_CAPTURE_ACCESSTIME_ONLY_CHANGE_IN_SNAPSHOT = "dfs.namenode.dont.capture.accesstime.only.change.in.snapshot"
        

        1.1 Any other better name for the above new config key? We have one other snapshot related config with the prefix "dfs.namenode.snapshot...". May be we should have all snapshot related one under the same prefix for grouping and consistency. How about "dfs.namenode.snapshot.skip.accesstime-only-diff. Your thoughts?
        1.2 Config and the default value lines have checkstyle issue. can you please take care of this?

        2. DirectoryWithSnapshotFeature
        2.1 new blank line added to line 50 is not needed
        2.2

                } else {
                  if (!dirCopy.metadataEquals(sdiff.snapshotINode)) {
                    dirMetadataChanged = true;
                  }
                }
        

        Is this different compared to the existing "else if" block. Doesn't look so. Can be reverted.

        3. hdfs-default.xml
        3.1 Once the config key is changed, need to be incorporated here
        3.2 "..it will not be captured in next snapshot." sounds ambiguous. The access time change history is not preserved. The file is still part of snapshot. Can we reword this? Also, there is typo in "lastest"

        4. class INode

          private static boolean dontCaptureAccessTimeOnlyChangeInSnapshot = false;
        
          public static void setDontCaptureAccessTimeOnlyChangeInSnapshot(boolean s) {
            LOG.info("Setting dontCaptureAccessTimeOnlyChangeInSnapshot to " + s);
            dontCaptureAccessTimeOnlyChangeInSnapshot = s;
          }
        

        4.1 Any other better ways of doing this instead of adding a static member to the core INode class. Snapshot manager can be the one to read all the configuration related to snapshots and it can take the decision accordingly instead of having the logic at the INode level

          public static boolean getDontCaptureAccessTimeOnlyChangeInSnapshot() {
            return dontCaptureAccessTimeOnlyChangeInSnapshot;
          }
        

        4.2 Above method is never used.

        5. FSDirAttrOp

          static boolean unprotectedSetTimes(
              FSDirectory fsd, INodesInPath iip, long mtime, long atime, boolean force)
                  throws QuotaExceededException {
            ....
            if (mtime != -1) {
              inode = inode.setModificationTime(mtime, latest);
              status = true;
            }
            ...
            if (atime != -1 && (status || force
                || atime > inode.getAccessTime() + fsd.getAccessTimePrecision())) {
              inode.setAccessTime(atime, latest);
              status = true;
            }
        }
        

        >> "if there is other modification made to the file, the latest access
        time will be captured together with the modification in next snapshot."

        Looks like accessTime will always be skipped with the config turned on. Please correct me if my understanding from the code is wrong.

        Show
        manojg Manoj Govindassamy added a comment - Thanks for working on this Yongjun Zhang . My comments below. 1. DFSConfigKeys DFS_NAMENODE_DONT_CAPTURE_ACCESSTIME_ONLY_CHANGE_IN_SNAPSHOT = "dfs.namenode.dont.capture.accesstime.only.change.in.snapshot" 1.1 Any other better name for the above new config key? We have one other snapshot related config with the prefix "dfs.namenode.snapshot...". May be we should have all snapshot related one under the same prefix for grouping and consistency. How about "dfs.namenode.snapshot.skip.accesstime-only-diff. Your thoughts? 1.2 Config and the default value lines have checkstyle issue. can you please take care of this? 2. DirectoryWithSnapshotFeature 2.1 new blank line added to line 50 is not needed 2.2 } else { if (!dirCopy.metadataEquals(sdiff.snapshotINode)) { dirMetadataChanged = true; } } Is this different compared to the existing "else if" block. Doesn't look so. Can be reverted. 3. hdfs-default.xml 3.1 Once the config key is changed, need to be incorporated here 3.2 "..it will not be captured in next snapshot." sounds ambiguous. The access time change history is not preserved. The file is still part of snapshot. Can we reword this? Also, there is typo in "lastest" 4. class INode private static boolean dontCaptureAccessTimeOnlyChangeInSnapshot = false; public static void setDontCaptureAccessTimeOnlyChangeInSnapshot(boolean s) { LOG.info("Setting dontCaptureAccessTimeOnlyChangeInSnapshot to " + s); dontCaptureAccessTimeOnlyChangeInSnapshot = s; } 4.1 Any other better ways of doing this instead of adding a static member to the core INode class. Snapshot manager can be the one to read all the configuration related to snapshots and it can take the decision accordingly instead of having the logic at the INode level public static boolean getDontCaptureAccessTimeOnlyChangeInSnapshot() { return dontCaptureAccessTimeOnlyChangeInSnapshot; } 4.2 Above method is never used. 5. FSDirAttrOp static boolean unprotectedSetTimes( FSDirectory fsd, INodesInPath iip, long mtime, long atime, boolean force) throws QuotaExceededException { .... if (mtime != -1) { inode = inode.setModificationTime(mtime, latest); status = true; } ... if (atime != -1 && (status || force || atime > inode.getAccessTime() + fsd.getAccessTimePrecision())) { inode.setAccessTime(atime, latest); status = true; } } >> "if there is other modification made to the file, the latest access time will be captured together with the modification in next snapshot." Looks like accessTime will always be skipped with the config turned on. Please correct me if my understanding from the code is wrong.
        Hide
        yzhangal Yongjun Zhang added a comment -

        Thanks a lot Manoj Govindassamy for the detailed review. I'm uploading new rev 002 to address them.

        • 1.1 changed to "dfs.namenode.snapshot.skip.accesstime-only-change"
        • 1.2 exceeding 80 chars is consistent in this file, so kept as is. Hard to avoid because of the long string
        • 2. reverted. This is the eft over from adding/removing debug messages.
        • 3. addressed
        • 4. This seems the best place to me to have the change. Otherwise we need to change the interface to communicate the value to here somehow. If you have thoughts, would be happy to hear.
        • 5. added a test to verify that access time is captured when there is other modification.

        Would you please take a look again?

        Thanks.

        Show
        yzhangal Yongjun Zhang added a comment - Thanks a lot Manoj Govindassamy for the detailed review. I'm uploading new rev 002 to address them. 1.1 changed to "dfs.namenode.snapshot.skip.accesstime-only-change" 1.2 exceeding 80 chars is consistent in this file, so kept as is. Hard to avoid because of the long string 2. reverted. This is the eft over from adding/removing debug messages. 3. addressed 4. This seems the best place to me to have the change. Otherwise we need to change the interface to communicate the value to here somehow. If you have thoughts, would be happy to hear. 5. added a test to verify that access time is captured when there is other modification. Would you please take a look again? Thanks.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 21s Docker mode activated.
              Prechecks
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
              trunk Compile Tests
        +1 mvninstall 13m 53s trunk passed
        +1 compile 0m 48s trunk passed
        +1 checkstyle 0m 43s trunk passed
        +1 mvnsite 0m 57s trunk passed
        +1 findbugs 1m 44s trunk passed
        +1 javadoc 0m 43s trunk passed
              Patch Compile Tests
        +1 mvninstall 0m 53s the patch passed
        +1 compile 0m 48s the patch passed
        +1 javac 0m 48s the patch passed
        -0 checkstyle 0m 41s hadoop-hdfs-project/hadoop-hdfs: The patch generated 4 new + 555 unchanged - 0 fixed = 559 total (was 555)
        +1 mvnsite 0m 56s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 xml 0m 1s The patch has no ill-formed XML file.
        +1 findbugs 1m 48s the patch passed
        +1 javadoc 0m 39s the patch passed
              Other Tests
        -1 unit 66m 32s hadoop-hdfs in the patch failed.
        +1 asflicense 0m 15s The patch does not generate ASF License warnings.
        93m 5s



        Reason Tests
        Failed junit tests hadoop.hdfs.TestDFSStripedInputStreamWithRandomECPolicy
          hadoop.hdfs.TestDFSStripedOutputStreamWithFailure140
          hadoop.hdfs.server.blockmanagement.TestUnderReplicatedBlocks



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:14b5c93
        JIRA Issue HDFS-12191
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12883446/HDFS-12191.002.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
        uname Linux ccecd0f39527 3.13.0-117-generic #164-Ubuntu SMP Fri Apr 7 11:05:26 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 7e6463d
        Default Java 1.8.0_144
        findbugs v3.1.0-RC1
        checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/20830/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt
        unit https://builds.apache.org/job/PreCommit-HDFS-Build/20830/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/20830/testReport/
        modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/20830/console
        Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 21s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.       trunk Compile Tests +1 mvninstall 13m 53s trunk passed +1 compile 0m 48s trunk passed +1 checkstyle 0m 43s trunk passed +1 mvnsite 0m 57s trunk passed +1 findbugs 1m 44s trunk passed +1 javadoc 0m 43s trunk passed       Patch Compile Tests +1 mvninstall 0m 53s the patch passed +1 compile 0m 48s the patch passed +1 javac 0m 48s the patch passed -0 checkstyle 0m 41s hadoop-hdfs-project/hadoop-hdfs: The patch generated 4 new + 555 unchanged - 0 fixed = 559 total (was 555) +1 mvnsite 0m 56s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 1s The patch has no ill-formed XML file. +1 findbugs 1m 48s the patch passed +1 javadoc 0m 39s the patch passed       Other Tests -1 unit 66m 32s hadoop-hdfs in the patch failed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 93m 5s Reason Tests Failed junit tests hadoop.hdfs.TestDFSStripedInputStreamWithRandomECPolicy   hadoop.hdfs.TestDFSStripedOutputStreamWithFailure140   hadoop.hdfs.server.blockmanagement.TestUnderReplicatedBlocks Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue HDFS-12191 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12883446/HDFS-12191.002.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux ccecd0f39527 3.13.0-117-generic #164-Ubuntu SMP Fri Apr 7 11:05:26 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 7e6463d Default Java 1.8.0_144 findbugs v3.1.0-RC1 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/20830/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/20830/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/20830/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/20830/console Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        manojg Manoj Govindassamy added a comment - - edited

        Thanks for working on the patch revision Yongjun Zhang. Few comments below:

        1. INode

        public abstract class INode {
        .. 
           private static boolean dontCaptureAccessTimeOnlyChangeInSnapshot = false;
           
           public static void setDontCaptureAccessTimeOnlyChangeInSnapshot(boolean s) {
               LOG.info("Setting dontCaptureAccessTimeOnlyChangeInSnapshot to " + s);
               dontCaptureAccessTimeOnlyChangeInSnapshot = s;
           }
        
           public static boolean getDontCaptureAccessTimeOnlyChangeInSnapshot() {
               return dontCaptureAccessTimeOnlyChangeInSnapshot;
           }
        
        • Abstract class INode doesn't look a right place for placing the snapshot logic. The callers of INode#setAccessTime() can pass in the needed details to skip recording the modifications
        • FSNamesystem#setAccessTimes() has all the needed details to make the decision whether to record the accesstime changes in the snapshots or not. So, shall we pass in the details from here?

        2. Config

        • typos in DFS_NAMENODE_SNAPTHOT_SKIP_ACCESSTIME_ONLY_CHANG_DEFAULT

        3.

        <description>
        4228	  <name>dfs.namenode.snapshot.skip.accesstime-only-change</name>
        4229	  <value>false</value>
        4231	    If accessTime of a file changed but there is no other modification
        4232	    made to the file, the changed accesstime will not be captured in next
        4233	    snapshot. However, if there is other modification made to the file,
        4234	    the latest access time will be captured together with the modification
        4235	    in next snapshot.
        4236	  </description>
        
        • How about something on the below lines?
          "When enabled, for access time only modification operations, HDFS snapshots will skip capturing pre modification copy of files/directories metadata. However, for all other metadata modification operations, files/directories latest access time will be captured along with the other metadata."
        Show
        manojg Manoj Govindassamy added a comment - - edited Thanks for working on the patch revision Yongjun Zhang . Few comments below: 1. INode public abstract class INode { .. private static boolean dontCaptureAccessTimeOnlyChangeInSnapshot = false; public static void setDontCaptureAccessTimeOnlyChangeInSnapshot(boolean s) { LOG.info("Setting dontCaptureAccessTimeOnlyChangeInSnapshot to " + s); dontCaptureAccessTimeOnlyChangeInSnapshot = s; } public static boolean getDontCaptureAccessTimeOnlyChangeInSnapshot() { return dontCaptureAccessTimeOnlyChangeInSnapshot; } Abstract class INode doesn't look a right place for placing the snapshot logic. The callers of INode#setAccessTime() can pass in the needed details to skip recording the modifications FSNamesystem#setAccessTimes() has all the needed details to make the decision whether to record the accesstime changes in the snapshots or not. So, shall we pass in the details from here? 2. Config typos in DFS_NAMENODE_SNAPTHOT_SKIP_ACCESSTIME_ONLY_CHANG_DEFAULT 3. <description> 4228 <name>dfs.namenode.snapshot.skip.accesstime-only-change</name> 4229 <value>false</value> 4231 If accessTime of a file changed but there is no other modification 4232 made to the file, the changed accesstime will not be captured in next 4233 snapshot. However, if there is other modification made to the file, 4234 the latest access time will be captured together with the modification 4235 in next snapshot. 4236 </description> How about something on the below lines? "When enabled, for access time only modification operations, HDFS snapshots will skip capturing pre modification copy of files/directories metadata. However, for all other metadata modification operations, files/directories latest access time will be captured along with the other metadata."
        Hide
        yzhangal Yongjun Zhang added a comment -

        Thanks a lot for the review Manoj Govindassamy.

        Uploaded rev3 to address all.

        For item 3, per our discussion, I kept my original wording, but changed file to file/directory as you pointed out.

        Would you please take a look again? thanks.

        Show
        yzhangal Yongjun Zhang added a comment - Thanks a lot for the review Manoj Govindassamy . Uploaded rev3 to address all. For item 3, per our discussion, I kept my original wording, but changed file to file/directory as you pointed out. Would you please take a look again? thanks.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 15s Docker mode activated.
              Prechecks
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
              trunk Compile Tests
        +1 mvninstall 16m 39s trunk passed
        +1 compile 0m 54s trunk passed
        +1 checkstyle 0m 43s trunk passed
        +1 mvnsite 0m 57s trunk passed
        +1 findbugs 1m 44s trunk passed
        +1 javadoc 0m 42s trunk passed
              Patch Compile Tests
        +1 mvninstall 0m 51s the patch passed
        +1 compile 0m 47s the patch passed
        +1 javac 0m 47s the patch passed
        -0 checkstyle 0m 41s hadoop-hdfs-project/hadoop-hdfs: The patch generated 3 new + 567 unchanged - 0 fixed = 570 total (was 567)
        +1 mvnsite 0m 53s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 xml 0m 1s The patch has no ill-formed XML file.
        +1 findbugs 1m 48s the patch passed
        +1 javadoc 0m 36s the patch passed
              Other Tests
        -1 unit 66m 18s hadoop-hdfs in the patch failed.
        +1 asflicense 0m 16s The patch does not generate ASF License warnings.
        95m 26s



        Reason Tests
        Failed junit tests hadoop.hdfs.TestDFSStripedOutputStreamWithFailure160
          hadoop.hdfs.TestDFSStripedOutputStreamWithFailure070
          hadoop.hdfs.TestDFSStripedOutputStreamWithFailure080
          hadoop.hdfs.server.namenode.TestFSDirAttrOp
          hadoop.hdfs.server.datanode.TestDataNodeVolumeFailureReporting



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:14b5c93
        JIRA Issue HDFS-12191
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12883638/HDFS-12191.003.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
        uname Linux d961167b8177 3.13.0-123-generic #172-Ubuntu SMP Mon Jun 26 18:04:35 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 9e2699a
        Default Java 1.8.0_144
        findbugs v3.1.0-RC1
        checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/20855/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt
        unit https://builds.apache.org/job/PreCommit-HDFS-Build/20855/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/20855/testReport/
        modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/20855/console
        Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 15s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.       trunk Compile Tests +1 mvninstall 16m 39s trunk passed +1 compile 0m 54s trunk passed +1 checkstyle 0m 43s trunk passed +1 mvnsite 0m 57s trunk passed +1 findbugs 1m 44s trunk passed +1 javadoc 0m 42s trunk passed       Patch Compile Tests +1 mvninstall 0m 51s the patch passed +1 compile 0m 47s the patch passed +1 javac 0m 47s the patch passed -0 checkstyle 0m 41s hadoop-hdfs-project/hadoop-hdfs: The patch generated 3 new + 567 unchanged - 0 fixed = 570 total (was 567) +1 mvnsite 0m 53s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 1s The patch has no ill-formed XML file. +1 findbugs 1m 48s the patch passed +1 javadoc 0m 36s the patch passed       Other Tests -1 unit 66m 18s hadoop-hdfs in the patch failed. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 95m 26s Reason Tests Failed junit tests hadoop.hdfs.TestDFSStripedOutputStreamWithFailure160   hadoop.hdfs.TestDFSStripedOutputStreamWithFailure070   hadoop.hdfs.TestDFSStripedOutputStreamWithFailure080   hadoop.hdfs.server.namenode.TestFSDirAttrOp   hadoop.hdfs.server.datanode.TestDataNodeVolumeFailureReporting Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue HDFS-12191 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12883638/HDFS-12191.003.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux d961167b8177 3.13.0-123-generic #172-Ubuntu SMP Mon Jun 26 18:04:35 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 9e2699a Default Java 1.8.0_144 findbugs v3.1.0-RC1 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/20855/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/20855/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/20855/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/20855/console Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        manojg Manoj Govindassamy added a comment -

        Thanks for working on the patch revision Yongjun Zhang. Overall looks good to me. +1 pending following nits:

        1. DFSConfigKeys
        Typo in DFS_NAMENODE_SNAPTHOT_SKIP_CAPTURE_ACCESSTIME_ONLY_CHANGE_DEFAULT

        2. Unit test TestFSDirAttrOp.testUnprotectedSetTimes() is failing with NPE. Probably the test needs more mock up to avoid NPE.

        2. SnapshotManager

          public boolean skipCaptureAccessTimeOnlyChange() {
            return skipCaptureAccessTimeOnlyChange;
          }
        

        How about a different method name getSkipCaptureAccessTimeOnlyChange() or isSkipCaptureAccessTimeOnlyChange()?

        Show
        manojg Manoj Govindassamy added a comment - Thanks for working on the patch revision Yongjun Zhang . Overall looks good to me. +1 pending following nits: 1. DFSConfigKeys Typo in DFS_NAMENODE_SNAPTHOT_SKIP_CAPTURE_ACCESSTIME_ONLY_CHANGE_DEFAULT 2. Unit test TestFSDirAttrOp.testUnprotectedSetTimes() is failing with NPE. Probably the test needs more mock up to avoid NPE. 2. SnapshotManager public boolean skipCaptureAccessTimeOnlyChange() { return skipCaptureAccessTimeOnlyChange; } How about a different method name getSkipCaptureAccessTimeOnlyChange() or isSkipCaptureAccessTimeOnlyChange()?
        Hide
        yzhangal Yongjun Zhang added a comment -

        Thanks much for the review Manoj Govindassamy.

        Uploaded rev004 to address your comments. Good catch of the typo.

        Show
        yzhangal Yongjun Zhang added a comment - Thanks much for the review Manoj Govindassamy . Uploaded rev004 to address your comments. Good catch of the typo.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 15s Docker mode activated.
              Prechecks
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
              trunk Compile Tests
        +1 mvninstall 14m 23s trunk passed
        +1 compile 0m 48s trunk passed
        +1 checkstyle 0m 44s trunk passed
        +1 mvnsite 0m 53s trunk passed
        +1 findbugs 1m 41s trunk passed
        +1 javadoc 0m 41s trunk passed
              Patch Compile Tests
        +1 mvninstall 0m 49s the patch passed
        +1 compile 0m 46s the patch passed
        +1 javac 0m 46s the patch passed
        -0 checkstyle 0m 41s hadoop-hdfs-project/hadoop-hdfs: The patch generated 4 new + 567 unchanged - 0 fixed = 571 total (was 567)
        +1 mvnsite 0m 51s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 xml 0m 1s The patch has no ill-formed XML file.
        +1 findbugs 1m 45s the patch passed
        +1 javadoc 0m 37s the patch passed
              Other Tests
        -1 unit 98m 8s hadoop-hdfs in the patch failed.
        +1 asflicense 0m 16s The patch does not generate ASF License warnings.
        124m 36s



        Reason Tests
        Failed junit tests hadoop.hdfs.TestDFSStripedOutputStreamWithFailure140
          hadoop.hdfs.TestDFSStripedOutputStreamWithFailure100
          hadoop.hdfs.TestDFSStripedOutputStreamWithFailure060
          hadoop.hdfs.TestDFSStripedOutputStreamWithFailure090
          hadoop.hdfs.TestDFSStripedOutputStreamWithFailure210
          hadoop.hdfs.TestFileAppendRestart
          hadoop.hdfs.TestReadStripedFileWithMissingBlocks
          hadoop.hdfs.TestDFSStripedOutputStreamWithFailure170
          hadoop.hdfs.TestDFSStripedOutputStreamWithFailure040
          hadoop.hdfs.TestLeaseRecoveryStriped
          hadoop.hdfs.TestDFSStripedOutputStreamWithFailure070
          hadoop.hdfs.TestDFSStripedOutputStreamWithFailure120
        Timed out junit tests org.apache.hadoop.hdfs.TestWriteReadStripedFile
          org.apache.hadoop.hdfs.TestReadStripedFileWithDecoding



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:14b5c93
        JIRA Issue HDFS-12191
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12883856/HDFS-12191.004.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
        uname Linux 9892b18f8413 3.13.0-116-generic #163-Ubuntu SMP Fri Mar 31 14:13:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / b89ffcf
        Default Java 1.8.0_144
        findbugs v3.1.0-RC1
        checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/20878/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt
        unit https://builds.apache.org/job/PreCommit-HDFS-Build/20878/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/20878/testReport/
        modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/20878/console
        Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 15s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.       trunk Compile Tests +1 mvninstall 14m 23s trunk passed +1 compile 0m 48s trunk passed +1 checkstyle 0m 44s trunk passed +1 mvnsite 0m 53s trunk passed +1 findbugs 1m 41s trunk passed +1 javadoc 0m 41s trunk passed       Patch Compile Tests +1 mvninstall 0m 49s the patch passed +1 compile 0m 46s the patch passed +1 javac 0m 46s the patch passed -0 checkstyle 0m 41s hadoop-hdfs-project/hadoop-hdfs: The patch generated 4 new + 567 unchanged - 0 fixed = 571 total (was 567) +1 mvnsite 0m 51s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 1s The patch has no ill-formed XML file. +1 findbugs 1m 45s the patch passed +1 javadoc 0m 37s the patch passed       Other Tests -1 unit 98m 8s hadoop-hdfs in the patch failed. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 124m 36s Reason Tests Failed junit tests hadoop.hdfs.TestDFSStripedOutputStreamWithFailure140   hadoop.hdfs.TestDFSStripedOutputStreamWithFailure100   hadoop.hdfs.TestDFSStripedOutputStreamWithFailure060   hadoop.hdfs.TestDFSStripedOutputStreamWithFailure090   hadoop.hdfs.TestDFSStripedOutputStreamWithFailure210   hadoop.hdfs.TestFileAppendRestart   hadoop.hdfs.TestReadStripedFileWithMissingBlocks   hadoop.hdfs.TestDFSStripedOutputStreamWithFailure170   hadoop.hdfs.TestDFSStripedOutputStreamWithFailure040   hadoop.hdfs.TestLeaseRecoveryStriped   hadoop.hdfs.TestDFSStripedOutputStreamWithFailure070   hadoop.hdfs.TestDFSStripedOutputStreamWithFailure120 Timed out junit tests org.apache.hadoop.hdfs.TestWriteReadStripedFile   org.apache.hadoop.hdfs.TestReadStripedFileWithDecoding Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue HDFS-12191 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12883856/HDFS-12191.004.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 9892b18f8413 3.13.0-116-generic #163-Ubuntu SMP Fri Mar 31 14:13:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / b89ffcf Default Java 1.8.0_144 findbugs v3.1.0-RC1 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/20878/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/20878/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/20878/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/20878/console Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 15s Docker mode activated.
              Prechecks
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
              trunk Compile Tests
        +1 mvninstall 14m 15s trunk passed
        +1 compile 0m 49s trunk passed
        +1 checkstyle 0m 44s trunk passed
        +1 mvnsite 0m 54s trunk passed
        +1 findbugs 1m 42s trunk passed
        +1 javadoc 0m 39s trunk passed
              Patch Compile Tests
        +1 mvninstall 0m 51s the patch passed
        +1 compile 0m 46s the patch passed
        +1 javac 0m 46s the patch passed
        -0 checkstyle 0m 40s hadoop-hdfs-project/hadoop-hdfs: The patch generated 4 new + 567 unchanged - 0 fixed = 571 total (was 567)
        +1 mvnsite 0m 57s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 xml 0m 2s The patch has no ill-formed XML file.
        +1 findbugs 1m 52s the patch passed
        +1 javadoc 0m 36s the patch passed
              Other Tests
        -1 unit 85m 53s hadoop-hdfs in the patch failed.
        +1 asflicense 0m 16s The patch does not generate ASF License warnings.
        113m 0s



        Reason Tests
        Failed junit tests hadoop.hdfs.TestDFSStripedOutputStreamWithFailure130
          hadoop.hdfs.TestDFSStripedInputStreamWithRandomECPolicy
          hadoop.hdfs.TestReadStripedFileWithDecoding
          hadoop.hdfs.TestLeaseRecoveryStriped
          hadoop.hdfs.server.datanode.TestDirectoryScanner
        Timed out junit tests org.apache.hadoop.hdfs.TestWriteReadStripedFile



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:14b5c93
        JIRA Issue HDFS-12191
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12883856/HDFS-12191.004.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
        uname Linux 6726b4986195 3.13.0-119-generic #166-Ubuntu SMP Wed May 3 12:18:55 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / bb6a3c8
        Default Java 1.8.0_144
        findbugs v3.1.0-RC1
        checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/20880/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt
        unit https://builds.apache.org/job/PreCommit-HDFS-Build/20880/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/20880/testReport/
        modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/20880/console
        Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 15s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.       trunk Compile Tests +1 mvninstall 14m 15s trunk passed +1 compile 0m 49s trunk passed +1 checkstyle 0m 44s trunk passed +1 mvnsite 0m 54s trunk passed +1 findbugs 1m 42s trunk passed +1 javadoc 0m 39s trunk passed       Patch Compile Tests +1 mvninstall 0m 51s the patch passed +1 compile 0m 46s the patch passed +1 javac 0m 46s the patch passed -0 checkstyle 0m 40s hadoop-hdfs-project/hadoop-hdfs: The patch generated 4 new + 567 unchanged - 0 fixed = 571 total (was 567) +1 mvnsite 0m 57s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 2s The patch has no ill-formed XML file. +1 findbugs 1m 52s the patch passed +1 javadoc 0m 36s the patch passed       Other Tests -1 unit 85m 53s hadoop-hdfs in the patch failed. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 113m 0s Reason Tests Failed junit tests hadoop.hdfs.TestDFSStripedOutputStreamWithFailure130   hadoop.hdfs.TestDFSStripedInputStreamWithRandomECPolicy   hadoop.hdfs.TestReadStripedFileWithDecoding   hadoop.hdfs.TestLeaseRecoveryStriped   hadoop.hdfs.server.datanode.TestDirectoryScanner Timed out junit tests org.apache.hadoop.hdfs.TestWriteReadStripedFile Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue HDFS-12191 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12883856/HDFS-12191.004.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 6726b4986195 3.13.0-119-generic #166-Ubuntu SMP Wed May 3 12:18:55 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / bb6a3c8 Default Java 1.8.0_144 findbugs v3.1.0-RC1 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/20880/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/20880/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/20880/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/20880/console Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        yzhangal Yongjun Zhang added a comment -

        The failed doesn't seem related. Rerun them manually, all succeeded except TestLeaseRecoveryStriped.testLeaseRecovery, created HDFS-12360.

        Show
        yzhangal Yongjun Zhang added a comment - The failed doesn't seem related. Rerun them manually, all succeeded except TestLeaseRecoveryStriped.testLeaseRecovery, created HDFS-12360 .
        Hide
        manojg Manoj Govindassamy added a comment -

        LGTM, +1. Thanks Yongjun Zhang.

        Nits:

        • TestSnapshotDiffReport:732 still refers to the the old property name
        • There are quite a few thread sleep for 3sec. You can always call the setTimes() with a constructed time instead of the actual waiting.
        Show
        manojg Manoj Govindassamy added a comment - LGTM, +1. Thanks Yongjun Zhang . Nits: TestSnapshotDiffReport:732 still refers to the the old property name There are quite a few thread sleep for 3sec. You can always call the setTimes() with a constructed time instead of the actual waiting.
        Hide
        yzhangal Yongjun Zhang added a comment -

        Thanks Manoj Govindassamy.

        There are quite a few thread sleep for 3sec. You can always call the setTimes() with a constructed time instead of the actual waiting.

        The sleep is there because the new operation takes the real time. If we use setTimes() to set the time to a later time, the real operation that records real time could move the time back which would be wrong.

        Show
        yzhangal Yongjun Zhang added a comment - Thanks Manoj Govindassamy . There are quite a few thread sleep for 3sec. You can always call the setTimes() with a constructed time instead of the actual waiting. The sleep is there because the new operation takes the real time. If we use setTimes() to set the time to a later time, the real operation that records real time could move the time back which would be wrong.
        Hide
        yzhangal Yongjun Zhang added a comment -

        Uploaded rev5 to correct the config name in javadoc of the test code:

        TestSnapshotDiffReport:732 still refers to the the old property name

        Show
        yzhangal Yongjun Zhang added a comment - Uploaded rev5 to correct the config name in javadoc of the test code: TestSnapshotDiffReport:732 still refers to the the old property name
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 15s Docker mode activated.
              Prechecks
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
              trunk Compile Tests
        +1 mvninstall 13m 51s trunk passed
        +1 compile 0m 49s trunk passed
        +1 checkstyle 0m 43s trunk passed
        +1 mvnsite 0m 55s trunk passed
        +1 findbugs 1m 41s trunk passed
        +1 javadoc 0m 39s trunk passed
              Patch Compile Tests
        +1 mvninstall 0m 48s the patch passed
        +1 compile 0m 46s the patch passed
        +1 javac 0m 46s the patch passed
        -0 checkstyle 0m 40s hadoop-hdfs-project/hadoop-hdfs: The patch generated 3 new + 567 unchanged - 0 fixed = 570 total (was 567)
        +1 mvnsite 0m 51s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 xml 0m 1s The patch has no ill-formed XML file.
        +1 findbugs 1m 45s the patch passed
        +1 javadoc 0m 37s the patch passed
              Other Tests
        -1 unit 90m 35s hadoop-hdfs in the patch failed.
        +1 asflicense 0m 17s The patch does not generate ASF License warnings.
        116m 31s



        Reason Tests
        Failed junit tests hadoop.hdfs.server.datanode.TestDataNodeVolumeFailureReporting
          hadoop.hdfs.server.blockmanagement.TestUnderReplicatedBlocks
          hadoop.hdfs.TestReadStripedFileWithDecoding
          hadoop.hdfs.TestLeaseRecoveryStriped
          hadoop.hdfs.TestDFSStripedOutputStreamWithFailure060
          hadoop.hdfs.TestDFSStripedOutputStreamWithFailure010
          hadoop.hdfs.TestClientProtocolForPipelineRecovery
          hadoop.hdfs.TestDFSStripedOutputStreamWithFailure160
          hadoop.hdfs.TestListFilesInFileContext
        Timed out junit tests org.apache.hadoop.hdfs.TestReplication
          org.apache.hadoop.hdfs.TestWriteReadStripedFile



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:14b5c93
        JIRA Issue HDFS-12191
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12884182/HDFS-12191.005.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
        uname Linux 686e2c70db53 3.13.0-119-generic #166-Ubuntu SMP Wed May 3 12:18:55 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 312b1fd
        Default Java 1.8.0_144
        findbugs v3.1.0-RC1
        checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/20906/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt
        unit https://builds.apache.org/job/PreCommit-HDFS-Build/20906/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/20906/testReport/
        modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/20906/console
        Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 15s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.       trunk Compile Tests +1 mvninstall 13m 51s trunk passed +1 compile 0m 49s trunk passed +1 checkstyle 0m 43s trunk passed +1 mvnsite 0m 55s trunk passed +1 findbugs 1m 41s trunk passed +1 javadoc 0m 39s trunk passed       Patch Compile Tests +1 mvninstall 0m 48s the patch passed +1 compile 0m 46s the patch passed +1 javac 0m 46s the patch passed -0 checkstyle 0m 40s hadoop-hdfs-project/hadoop-hdfs: The patch generated 3 new + 567 unchanged - 0 fixed = 570 total (was 567) +1 mvnsite 0m 51s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 1s The patch has no ill-formed XML file. +1 findbugs 1m 45s the patch passed +1 javadoc 0m 37s the patch passed       Other Tests -1 unit 90m 35s hadoop-hdfs in the patch failed. +1 asflicense 0m 17s The patch does not generate ASF License warnings. 116m 31s Reason Tests Failed junit tests hadoop.hdfs.server.datanode.TestDataNodeVolumeFailureReporting   hadoop.hdfs.server.blockmanagement.TestUnderReplicatedBlocks   hadoop.hdfs.TestReadStripedFileWithDecoding   hadoop.hdfs.TestLeaseRecoveryStriped   hadoop.hdfs.TestDFSStripedOutputStreamWithFailure060   hadoop.hdfs.TestDFSStripedOutputStreamWithFailure010   hadoop.hdfs.TestClientProtocolForPipelineRecovery   hadoop.hdfs.TestDFSStripedOutputStreamWithFailure160   hadoop.hdfs.TestListFilesInFileContext Timed out junit tests org.apache.hadoop.hdfs.TestReplication   org.apache.hadoop.hdfs.TestWriteReadStripedFile Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue HDFS-12191 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12884182/HDFS-12191.005.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 686e2c70db53 3.13.0-119-generic #166-Ubuntu SMP Wed May 3 12:18:55 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 312b1fd Default Java 1.8.0_144 findbugs v3.1.0-RC1 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/20906/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/20906/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/20906/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/20906/console Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        manojg Manoj Govindassamy added a comment -

        LGTM, +1. Thanks Yongjun Zhang.

        Show
        manojg Manoj Govindassamy added a comment - LGTM, +1. Thanks Yongjun Zhang .
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #12267 (See https://builds.apache.org/job/Hadoop-trunk-Commit/12267/)
        HDFS-12191. Provide option to not capture the accessTime change of a (yzhang: rev cf93d60d3f032000e5b78a08d320793d78799f3d)

        • (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAttrOp.java
        • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSDirAttrOp.java
        • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotDiffReport.java
        • (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java
        • (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/SnapshotManager.java
        • (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java
        • (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java
        • (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #12267 (See https://builds.apache.org/job/Hadoop-trunk-Commit/12267/ ) HDFS-12191 . Provide option to not capture the accessTime change of a (yzhang: rev cf93d60d3f032000e5b78a08d320793d78799f3d) (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAttrOp.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSDirAttrOp.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotDiffReport.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/SnapshotManager.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml
        Hide
        yzhangal Yongjun Zhang added a comment -

        Many thanks to Manoj Govindassamy for the review, committed to trunk and branch-2.

        Show
        yzhangal Yongjun Zhang added a comment - Many thanks to Manoj Govindassamy for the review, committed to trunk and branch-2.

          People

          • Assignee:
            yzhangal Yongjun Zhang
            Reporter:
            yzhangal Yongjun Zhang
          • Votes:
            0 Vote for this issue
            Watchers:
            7 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development