Uploaded image for project: 'HBase'
  1. HBase
  2. HBASE-9019

Port HBASE-8690: Reduce unnecessary getFileStatus hdfs calls in TTL hfile and hlog cleanners to 0.94

    Details

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

      Description

      For each in file in archive dir, the TimeToLiveHFileCleaner need call getFileStatus to get the modify time of file. Actually the CleanerChore have had the file status when listing its parent dir.

      When we set the TTL to 7 days in our cluster for data security, the number of files left in archive dir is up to 65 thousands. In each clean period, TimeToLiveHFileCleaner will generate ten thousand getFileStatus call in a short time, which is very heavy for hdfs namenode.

      Fix is to change the path param to FileStatus in isFileDeletable method and reduce unnecessary getFileStatus hdfs calls in TTL cleaners.

      This issue is to backport this improvement to 0.94

        Issue Links

          Activity

          Hide
          stack stack added a comment -

          Resolving issue ill-specified. When I click on the link it takes me to the end of the issue; I should not have to read a whole issue to figure out what a new issue is about.

          If you don't know how to make an issue, don't do it Ted Yu

          Show
          stack stack added a comment - Resolving issue ill-specified. When I click on the link it takes me to the end of the issue; I should not have to read a whole issue to figure out what a new issue is about. If you don't know how to make an issue, don't do it Ted Yu
          Hide
          lhofhansl Lars Hofhansl added a comment -

          Ted fixed the description. Reopening for the backport.

          Show
          lhofhansl Lars Hofhansl added a comment - Ted fixed the description. Reopening for the backport.
          Hide
          yuzhihong@gmail.com Ted Yu added a comment -

          Patch for 0.94

          Tests touched by the patch pass.

          Running test suite now.

          Show
          yuzhihong@gmail.com Ted Yu added a comment - Patch for 0.94 Tests touched by the patch pass. Running test suite now.
          Hide
          lhofhansl Lars Hofhansl added a comment -

          Could you name the patches with .txt or .patch? Otherwise I have download the patch first in order to open it.

          Show
          lhofhansl Lars Hofhansl added a comment - Could you name the patches with .txt or .patch? Otherwise I have download the patch first in order to open it.
          Hide
          lhofhansl Lars Hofhansl added a comment -

          Is this correct?

          -    assertFalse(cleaner.isFileDeletable(new Path(hfile)));
          +    assertFalse(cleaner.isFileDeletable(fs.getFileStatus(refFile)));
          

          Skimmed the patch, otherwise looks good.

          Show
          lhofhansl Lars Hofhansl added a comment - Is this correct? - assertFalse(cleaner.isFileDeletable( new Path(hfile))); + assertFalse(cleaner.isFileDeletable(fs.getFileStatus(refFile))); Skimmed the patch, otherwise looks good.
          Hide
          yuzhihong@gmail.com Ted Yu added a comment -

          Was the above code from TestSnapshotHFileCleaner.java ?

          Looking at https://issues.apache.org/jira/secure/attachment/12586615/8690-trunk-v4.patch, I see same code.

          Show
          yuzhihong@gmail.com Ted Yu added a comment - Was the above code from TestSnapshotHFileCleaner.java ? Looking at https://issues.apache.org/jira/secure/attachment/12586615/8690-trunk-v4.patch , I see same code.
          Hide
          yuzhihong@gmail.com Ted Yu added a comment -

          Same patch, renamed.

          Show
          yuzhihong@gmail.com Ted Yu added a comment - Same patch, renamed.
          Hide
          lhofhansl Lars Hofhansl added a comment -

          I suppose if the test passes, it should be OK.

          Show
          lhofhansl Lars Hofhansl added a comment - I suppose if the test passes, it should be OK.
          Hide
          yuzhihong@gmail.com Ted Yu added a comment -

          Tests run: 1372, Failures: 0, Errors: 0, Skipped: 13

          [INFO] ------------------------------------------------------------------------
          [INFO] BUILD SUCCESS
          [INFO] ------------------------------------------------------------------------
          [INFO] Total time: 1:08:48.999s
          [INFO] Finished at: Tue Jul 23 04:57:38 UTC 2013
          [INFO] Final Memory: 36M/640M

          Show
          yuzhihong@gmail.com Ted Yu added a comment - Tests run: 1372, Failures: 0, Errors: 0, Skipped: 13 [INFO] ------------------------------------------------------------------------ [INFO] BUILD SUCCESS [INFO] ------------------------------------------------------------------------ [INFO] Total time: 1:08:48.999s [INFO] Finished at: Tue Jul 23 04:57:38 UTC 2013 [INFO] Final Memory: 36M/640M
          Hide
          lhofhansl Lars Hofhansl added a comment -

          +1 Thanks Ted.

          Show
          lhofhansl Lars Hofhansl added a comment - +1 Thanks Ted.
          Hide
          yuzhihong@gmail.com Ted Yu added a comment -

          Integrated to 0.94.

          Thanks for the review, Lars.

          Show
          yuzhihong@gmail.com Ted Yu added a comment - Integrated to 0.94. Thanks for the review, Lars.

            People

            • Assignee:
              yuzhihong@gmail.com Ted Yu
              Reporter:
              yuzhihong@gmail.com Ted Yu
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development