Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-2225

HDFS-2018 Part 1 : Refactor file management so its not in classes which should be generic

    Details

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

      Description

      This is the first part of HDFS-2018 changes, to refactor some of the file management classes so they're in file specific places rather than in the generic classes.

      1. HDFS-2225.diff
        44 kB
        Ivan Kelly
      2. HDFS-2225.diff
        46 kB
        Ivan Kelly
      3. HDFS-2225.diff
        45 kB
        Ivan Kelly

        Issue Links

          Activity

          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk #740 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/740/)
          Edit change log entry for HDFS-2225 by request of Nicholas

          todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1154414
          Files :

          • /hadoop/common/trunk/hdfs/CHANGES.txt
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #740 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/740/ ) Edit change log entry for HDFS-2225 by request of Nicholas todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1154414 Files : /hadoop/common/trunk/hdfs/CHANGES.txt
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk-Commit #814 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/814/)
          Edit change log entry for HDFS-2225 by request of Nicholas

          todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1154414
          Files :

          • /hadoop/common/trunk/hdfs/CHANGES.txt
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #814 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/814/ ) Edit change log entry for HDFS-2225 by request of Nicholas todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1154414 Files : /hadoop/common/trunk/hdfs/CHANGES.txt
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk-Commit #707 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/707/)
          Edit change log entry for HDFS-2225 by request of Nicholas

          todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1154414
          Files :

          • /hadoop/common/trunk/hdfs/CHANGES.txt
          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #707 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/707/ ) Edit change log entry for HDFS-2225 by request of Nicholas todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1154414 Files : /hadoop/common/trunk/hdfs/CHANGES.txt
          Hide
          Tsz Wo Nicholas Sze added a comment -

          Thanks Todd.

          Show
          Tsz Wo Nicholas Sze added a comment - Thanks Todd.
          Hide
          Todd Lipcon added a comment -

          Good point. I will change it to:

          HDFS-2225. Refactor edit log file management so it's not in classes
          which should be generic to the type of edit log storage. (Ivan Kelly
          via todd)

          Show
          Todd Lipcon added a comment - Good point. I will change it to: HDFS-2225 . Refactor edit log file management so it's not in classes which should be generic to the type of edit log storage. (Ivan Kelly via todd)
          Hide
          Tsz Wo Nicholas Sze added a comment -

          Hi Todd, the entry in CHANGES.txt is not very clear. Could we emphasize that it is edit/image related?

          Show
          Tsz Wo Nicholas Sze added a comment - Hi Todd, the entry in CHANGES.txt is not very clear. Could we emphasize that it is edit/image related?
          Hide
          Todd Lipcon added a comment -

          I committed this yesterday, but apparently forgot to resolve the JIRA. Thanks Ivan!

          Show
          Todd Lipcon added a comment - I committed this yesterday, but apparently forgot to resolve the JIRA. Thanks Ivan!
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk-Commit #812 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/812/)
          HDFS-2225. Refactor file management so it's not in classes which should be generic. Contributed by Ivan Kelly.

          todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1154029
          Files :

          • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/BackupJournalManager.java
          • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/FSImageTestUtil.java
          • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSImageTransactionalStorageInspector.java
          • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/JournalManager.java
          • /hadoop/common/trunk/hdfs/CHANGES.txt
          • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FileJournalManager.java
          • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestBackupNode.java
          • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestCheckPointForSecurityTokens.java
          • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestNNStorageRetentionManager.java
          • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestFSImageStorageInspector.java
          • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSEditLog.java
          • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/NNStorageRetentionManager.java
          • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSImageStorageInspector.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #812 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/812/ ) HDFS-2225 . Refactor file management so it's not in classes which should be generic. Contributed by Ivan Kelly. todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1154029 Files : /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/BackupJournalManager.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/FSImageTestUtil.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSImageTransactionalStorageInspector.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/JournalManager.java /hadoop/common/trunk/hdfs/CHANGES.txt /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FileJournalManager.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestBackupNode.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestCheckPointForSecurityTokens.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestNNStorageRetentionManager.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestFSImageStorageInspector.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSEditLog.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/NNStorageRetentionManager.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSImageStorageInspector.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk #738 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/738/)
          HDFS-2225. Refactor file management so it's not in classes which should be generic. Contributed by Ivan Kelly.

          todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1154029
          Files :

          • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/BackupJournalManager.java
          • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/FSImageTestUtil.java
          • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSImageTransactionalStorageInspector.java
          • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/JournalManager.java
          • /hadoop/common/trunk/hdfs/CHANGES.txt
          • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FileJournalManager.java
          • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestBackupNode.java
          • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestCheckPointForSecurityTokens.java
          • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestNNStorageRetentionManager.java
          • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestFSImageStorageInspector.java
          • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSEditLog.java
          • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/NNStorageRetentionManager.java
          • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSImageStorageInspector.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #738 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/738/ ) HDFS-2225 . Refactor file management so it's not in classes which should be generic. Contributed by Ivan Kelly. todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1154029 Files : /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/BackupJournalManager.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/FSImageTestUtil.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSImageTransactionalStorageInspector.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/JournalManager.java /hadoop/common/trunk/hdfs/CHANGES.txt /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FileJournalManager.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestBackupNode.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestCheckPointForSecurityTokens.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestNNStorageRetentionManager.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestFSImageStorageInspector.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSEditLog.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/NNStorageRetentionManager.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSImageStorageInspector.java
          Hide
          Todd Lipcon added a comment -

          +1. test-patch:

          [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 15 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 (version 1.3.8) warnings.
          [exec]
          [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings.
          [exec]
          [exec] +1 system test framework. The patch passed system test framework compile.

          and I ran all the unit tests that contain the word EditLog or were modified by this patch.

          Will commit momentarily

          Show
          Todd Lipcon added a comment - +1. test-patch: [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 15 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 (version 1.3.8) warnings. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings. [exec] [exec] +1 system test framework. The patch passed system test framework compile. and I ran all the unit tests that contain the word EditLog or were modified by this patch. Will commit momentarily
          Hide
          Ivan Kelly added a comment -

          Changed the purger interface back.

          Show
          Ivan Kelly added a comment - Changed the purger interface back.
          Hide
          Todd Lipcon added a comment -

          Ivan and I just chatted on IRC about this. The dependency is only cyclic in that EditLogFile is a static inner class of FileJournalManager. The dependency graph wouldn't actually be a cycle between the actual classes. So, he's going to change the interface back to taking the classes.

          Show
          Todd Lipcon added a comment - Ivan and I just chatted on IRC about this. The dependency is only cyclic in that EditLogFile is a static inner class of FileJournalManager. The dependency graph wouldn't actually be a cycle between the actual classes. So, he's going to change the interface back to taking the classes.
          Hide
          Ivan Kelly added a comment -

          I did it to keep the dependencies acyclic. If we use EditLogFile, then NNStorageRetentionManager has a dependency on FileJournalManager which has a dependency on NNStorageRetentionManager.

          Show
          Ivan Kelly added a comment - I did it to keep the dependencies acyclic. If we use EditLogFile, then NNStorageRetentionManager has a dependency on FileJournalManager which has a dependency on NNStorageRetentionManager.
          Hide
          Todd Lipcon added a comment -

          Looks pretty good. Only question: why not have StoragePurger continue to use EditLogFile and FSImageFile? ie why break out the arguments to File and transaction IDs? It seems like passing the classes allows us more room to add extra data about the stuff to be purged without having to change the interface.

          Show
          Todd Lipcon added a comment - Looks pretty good. Only question: why not have StoragePurger continue to use EditLogFile and FSImageFile? ie why break out the arguments to File and transaction IDs? It seems like passing the classes allows us more room to add extra data about the stuff to be purged without having to change the interface.
          Hide
          Ivan Kelly added a comment -

          rebased on trunk

          Show
          Ivan Kelly added a comment - rebased on trunk
          Hide
          Ivan Kelly added a comment -

          There are 3 parts to this patch:

          FoundEditLog becomes EditLogFile in FileJournalManager.
          FoundFSImage becomes FSImageFile in FSImageStorageInspector.
          StoragePurger is changed to take File rather than FoundEditLog and FoundFSImage.

          Show
          Ivan Kelly added a comment - There are 3 parts to this patch: FoundEditLog becomes EditLogFile in FileJournalManager. FoundFSImage becomes FSImageFile in FSImageStorageInspector. StoragePurger is changed to take File rather than FoundEditLog and FoundFSImage.

            People

            • Assignee:
              Ivan Kelly
              Reporter:
              Ivan Kelly
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development