HBase
  1. HBase
  2. HBASE-7467

CleanerChore checkAndDeleteDirectory not deleting empty directories

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.94.4, 0.95.0
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      CleanerChore checkAndDeleteDirectory is not deleting empty directories. As a result, some directories are kept in the FS but should have been removed.

      To reproduce, simply create an empty directory under /hbase/.archive/table_name/. If you place a file into this directory, it's not more empty and therefore it's correctly removed.

      1. 7465_7467-v5.txt
        7 kB
        Ted Yu
      2. 7465_7467-v5-0.94
        7 kB
        Ted Yu
      3. HBASE-7465_7467-v3.patch
        6 kB
        Jean-Marc Spaggiari
      4. HBASE-7465_7467-v4.patch
        7 kB
        Jean-Marc Spaggiari
      5. HBASE-7465_7467-v5-0.94.patch
        7 kB
        Jean-Marc Spaggiari
      6. HBASE-7465_7467-v6-0.94.patch
        7 kB
        Jean-Marc Spaggiari
      7. HBASE-7465_7467-v7-0.94.patch
        6 kB
        Jean-Marc Spaggiari
      8. HBASE-7467.patch
        0.6 kB
        Jean-Marc Spaggiari
      9. HBASE-7467-v2.patch
        1 kB
        Jean-Marc Spaggiari

        Activity

        Hide
        Jesse Yates added a comment -

        Jean-Marc, can you be a little more explicit? The way the cleaner works is that it doesn't delete non-empty directories unless all the files under the directory have been removed. So adding the file under the directory would ensure it doesn't get deleted.

        This is also covered in TestCleanerChore#testDeletesEmptyDirectories

        Show
        Jesse Yates added a comment - Jean-Marc, can you be a little more explicit? The way the cleaner works is that it doesn't delete non-empty directories unless all the files under the directory have been removed. So adding the file under the directory would ensure it doesn't get deleted. This is also covered in TestCleanerChore#testDeletesEmptyDirectories
        Hide
        Jean-Marc Spaggiari added a comment -

        Jesse,

        If an empty directory is created for any reason, which mine without any file into it, the current checkAndDeleteDirectory will not delete it.

        Current version of checkAndDeleteDirectory only delete directories where where is files which can be deleted too. But if the directory is empty, it's not deleted.

        The patch proposed here is to address that.

        Just let me know if I'm not clear.

        Show
        Jean-Marc Spaggiari added a comment - Jesse, If an empty directory is created for any reason, which mine without any file into it, the current checkAndDeleteDirectory will not delete it. Current version of checkAndDeleteDirectory only delete directories where where is files which can be deleted too. But if the directory is empty, it's not deleted. The patch proposed here is to address that. Just let me know if I'm not clear.
        Hide
        Hadoop QA added a comment -

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

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

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        -1 patch. The patch command could not apply the patch.

        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/3775//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/12562743/HBASE-7467.patch against trunk revision . +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/3775//console This message is automatically generated.
        Hide
        Jean-Marc Spaggiari added a comment -

        On more comment:
        Regarding TestCleanerChore#testDeletesEmptyDirectories if you comment the file creation (//fs.create(file).close() the test will fail since empty directories will be left behind.

        I think we should add some empty directories tests too on TestCleanerChore#testDeletesEmptyDirectories. I will add some and submit a patch for that too.

        Show
        Jean-Marc Spaggiari added a comment - On more comment: Regarding TestCleanerChore#testDeletesEmptyDirectories if you comment the file creation (//fs.create(file).close() the test will fail since empty directories will be left behind. I think we should add some empty directories tests too on TestCleanerChore#testDeletesEmptyDirectories. I will add some and submit a patch for that too.
        Hide
        Jesse Yates added a comment -

        Oh, yeah, that makes sense. This could still throw an exception though if we get a file added under that directory (see HBASE-7465). Make it something like:

           try {
              return fs.delete(toCheck, false);
            } catch (IOException e) {
              if (LOG.isTraceEnabled()) {
                LOG.trace("Couldn't delete directory: " + toCheck+ e);
              }
              return false;
            }
        

        for the delete and I'm +1

        Show
        Jesse Yates added a comment - Oh, yeah, that makes sense. This could still throw an exception though if we get a file added under that directory (see HBASE-7465 ). Make it something like: try { return fs.delete(toCheck, false ); } catch (IOException e) { if (LOG.isTraceEnabled()) { LOG.trace( "Couldn't delete directory: " + toCheck+ e); } return false ; } for the delete and I'm +1
        Hide
        Jesse Yates added a comment -

        And +1 on fixing the test. Good on ya Jean-Marc Spaggiari

        Show
        Jesse Yates added a comment - And +1 on fixing the test. Good on ya Jean-Marc Spaggiari
        Hide
        Jean-Marc Spaggiari added a comment -

        Ok, got it!

        Here is a patch which add the emptydirectory test and also fix it.

        It's still throwing the IOException, which will most probably be fixed when I will apply you patch from 7465.

        Show
        Jean-Marc Spaggiari added a comment - Ok, got it! Here is a patch which add the emptydirectory test and also fix it. It's still throwing the IOException, which will most probably be fixed when I will apply you patch from 7465.
        Hide
        Ted Yu added a comment -

        @Jean-Marc:
        Your finding was really good.

        @Jesse:
        Do you think that the log in the code snippet above should be at warn level ?

        Show
        Ted Yu added a comment - @Jean-Marc: Your finding was really good. @Jesse: Do you think that the log in the code snippet above should be at warn level ?
        Hide
        Jean-Marc Spaggiari added a comment -

        By applying HBASE-7465 over this patch, everything is working very well. No more issues displayed (Can still see them if required by setting the loglevel to trace), and all tests passing including the new one added by Jesse.

        Show
        Jean-Marc Spaggiari added a comment - By applying HBASE-7465 over this patch, everything is working very well. No more issues displayed (Can still see them if required by setting the loglevel to trace), and all tests passing including the new one added by Jesse.
        Hide
        Ted Yu added a comment -

        @Jean-Marc:
        Can you attach the combined patch ?

        Thanks for working over the weekend.

        Show
        Ted Yu added a comment - @Jean-Marc: Can you attach the combined patch ? Thanks for working over the weekend.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12562748/HBASE-7467-v2.patch
        against trunk revision .

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

        +1 tests included. The patch appears to include 3 new or modified tests.

        +1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

        +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 (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 lineLengths. The patch does not introduce lines longer than 100

        -1 core tests. The patch failed these unit tests:
        org.apache.hadoop.hbase.client.TestMultiParallel
        org.apache.hadoop.hbase.replication.TestReplicationWithCompression

        -1 core zombie tests. There are 3 zombie test(s): at org.apache.hadoop.hbase.io.encoding.TestUpgradeFromHFileV1ToEncoding.testUpgrade(TestUpgradeFromHFileV1ToEncoding.java:83)

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/3776//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3776//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3776//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3776//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3776//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3776//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3776//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3776//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/3776//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/12562748/HBASE-7467-v2.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 3 new or modified tests. +1 hadoop2.0 . The patch compiles against the hadoop 2.0 profile. +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 (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 lineLengths . The patch does not introduce lines longer than 100 -1 core tests . The patch failed these unit tests: org.apache.hadoop.hbase.client.TestMultiParallel org.apache.hadoop.hbase.replication.TestReplicationWithCompression -1 core zombie tests . There are 3 zombie test(s): at org.apache.hadoop.hbase.io.encoding.TestUpgradeFromHFileV1ToEncoding.testUpgrade(TestUpgradeFromHFileV1ToEncoding.java:83) Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/3776//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3776//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3776//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3776//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3776//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3776//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3776//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3776//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/3776//console This message is automatically generated.
        Hide
        Jean-Marc Spaggiari added a comment - - edited

        Attached is Jesse's patch + mine both in a single file. But they can still noth be applied separatly. I'm not sure what's the best way to go, so I will let you pickup the recommanded one.

        Also, I'm wondering if I should replace
        if (children == null) return fs.delete(toCheck, false);

        by

        if (children == null) {
          try {
            return fs.delete(toCheck, false);
          } catch (IOException e) {
            if (LOG.isTraceEnabled()) {
              LOG.trace("Couldn't delete directory: " + toCheck, e);
            }
          }
          // couldn't delete w/o exception, so we can't return success.
          return false;
        }
        

        Like Jesse did too...

        Show
        Jean-Marc Spaggiari added a comment - - edited Attached is Jesse's patch + mine both in a single file. But they can still noth be applied separatly. I'm not sure what's the best way to go, so I will let you pickup the recommanded one. Also, I'm wondering if I should replace if (children == null) return fs.delete(toCheck, false); by if (children == null ) { try { return fs.delete(toCheck, false ); } catch (IOException e) { if (LOG.isTraceEnabled()) { LOG.trace( "Couldn't delete directory: " + toCheck, e); } } // couldn't delete w/o exception, so we can't return success. return false ; } Like Jesse did too...
        Hide
        Ted Yu added a comment - - edited

        I understand Jesse's intention in HBASE-7465.
        I am fine with not leaking IOE. We should preserve the effect of IOE in that IOException would stop remaining checking / deletion from being executed in checkAndDeleteDirectory() calls. We can achieve this by introducing an enum one of whose values indicates fast fail.

        Show
        Ted Yu added a comment - - edited I understand Jesse's intention in HBASE-7465 . I am fine with not leaking IOE. We should preserve the effect of IOE in that IOException would stop remaining checking / deletion from being executed in checkAndDeleteDirectory() calls. We can achieve this by introducing an enum one of whose values indicates fast fail.
        Hide
        Jean-Marc Spaggiari added a comment -

        So. Another version is attached with the catch for IOException.

        FSUtils.listStatus(fs, toCheck); can still throw an IOException, so we are fine to keep the throws IOException declaration.

        Again, this is also including Jesse's fix for 7465.

        Show
        Jean-Marc Spaggiari added a comment - So. Another version is attached with the catch for IOException. FSUtils.listStatus(fs, toCheck); can still throw an IOException, so we are fine to keep the throws IOException declaration. Again, this is also including Jesse's fix for 7465.
        Hide
        Ted Yu added a comment -
        +      UTIL.cleanupTestDir();
        

        The above method isn't visible to TestCleanerChore.

        +  public void testNoExceptionFromDirectoryWithRaceyChildren() throws Exception {
        

        'Racey' -> 'Racy'

        Show
        Ted Yu added a comment - + UTIL.cleanupTestDir(); The above method isn't visible to TestCleanerChore. + public void testNoExceptionFromDirectoryWithRaceyChildren() throws Exception { 'Racey' -> 'Racy'
        Hide
        Ted Yu added a comment -

        Patch v5 fixed compilation error.

        Also corrected spelling.

        Show
        Ted Yu added a comment - Patch v5 fixed compilation error. Also corrected spelling.
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12562823/7465_7467-v5.txt
        against trunk revision .

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

        +1 tests included. The patch appears to include 6 new or modified tests.

        +1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

        +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 (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 lineLengths. The patch does not introduce lines longer than 100

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

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/3790//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3790//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3790//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3790//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3790//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3790//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3790//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3790//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/3790//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/12562823/7465_7467-v5.txt against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 6 new or modified tests. +1 hadoop2.0 . The patch compiles against the hadoop 2.0 profile. +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 (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 lineLengths . The patch does not introduce lines longer than 100 +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/3790//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3790//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3790//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3790//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3790//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3790//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3790//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3790//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/3790//console This message is automatically generated.
        Hide
        Ted Yu added a comment -

        Test suite is all green.

        Integrated to trunk.

        Thanks for the patch, Jean-Marc and Jesse.

        Thanks for the review, Jesse.

        Show
        Ted Yu added a comment - Test suite is all green. Integrated to trunk. Thanks for the patch, Jean-Marc and Jesse. Thanks for the review, Jesse.
        Hide
        Hudson added a comment -

        Integrated in HBase-TRUNK #3678 (See https://builds.apache.org/job/HBase-TRUNK/3678/)
        HBASE-7467 CleanerChore checkAndDeleteDirectory not deleting empty directories (Jean-Marc and Jesse Yates) (Revision 1427287)

        Result = FAILURE
        tedyu :
        Files :

        • /hbase/trunk/hbase-common/src/test/java/org/apache/hadoop/hbase/HBaseCommonTestingUtility.java
        • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java
        • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestCleanerChore.java
        Show
        Hudson added a comment - Integrated in HBase-TRUNK #3678 (See https://builds.apache.org/job/HBase-TRUNK/3678/ ) HBASE-7467 CleanerChore checkAndDeleteDirectory not deleting empty directories (Jean-Marc and Jesse Yates) (Revision 1427287) Result = FAILURE tedyu : Files : /hbase/trunk/hbase-common/src/test/java/org/apache/hadoop/hbase/HBaseCommonTestingUtility.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestCleanerChore.java
        Hide
        Hudson added a comment -

        Integrated in HBase-TRUNK-on-Hadoop-2.0.0 #323 (See https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-2.0.0/323/)
        HBASE-7467 CleanerChore checkAndDeleteDirectory not deleting empty directories (Jean-Marc and Jesse Yates) (Revision 1427287)

        Result = FAILURE
        tedyu :
        Files :

        • /hbase/trunk/hbase-common/src/test/java/org/apache/hadoop/hbase/HBaseCommonTestingUtility.java
        • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java
        • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestCleanerChore.java
        Show
        Hudson added a comment - Integrated in HBase-TRUNK-on-Hadoop-2.0.0 #323 (See https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-2.0.0/323/ ) HBASE-7467 CleanerChore checkAndDeleteDirectory not deleting empty directories (Jean-Marc and Jesse Yates) (Revision 1427287) Result = FAILURE tedyu : Files : /hbase/trunk/hbase-common/src/test/java/org/apache/hadoop/hbase/HBaseCommonTestingUtility.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestCleanerChore.java
        Hide
        Jesse Yates added a comment -

        Wow, good stuff guys. Thanks for working over New Years. Ted Yu do you want to commit to 0.94 as well?

        Show
        Jesse Yates added a comment - Wow, good stuff guys. Thanks for working over New Years. Ted Yu do you want to commit to 0.94 as well?
        Hide
        Ted Yu added a comment -

        I would wait for Lars' decision on inclusion for 0.94
        0.94 test suite validation is also needed.

        @Jean-Marc:
        Can you attach patch for 0.94 ?

        Show
        Ted Yu added a comment - I would wait for Lars' decision on inclusion for 0.94 0.94 test suite validation is also needed. @Jean-Marc: Can you attach patch for 0.94 ?
        Hide
        Lars Hofhansl added a comment -

        +1 on 0.94 of course.
        Can this leak (somehow) to large hierarchies of empty directories that will never be cleaned up?
        If so, I will consider sinking the current 0.94.4RC for this.

        Show
        Lars Hofhansl added a comment - +1 on 0.94 of course. Can this leak (somehow) to large hierarchies of empty directories that will never be cleaned up? If so, I will consider sinking the current 0.94.4RC for this.
        Hide
        Lars Hofhansl added a comment -

        +1 on patch. Personally I would have preferred for the these logs to be at debug level.

        Show
        Lars Hofhansl added a comment - +1 on patch. Personally I would have preferred for the these logs to be at debug level.
        Hide
        Ted Yu added a comment -

        Here was description Jean-Marc wrote in 'CleanerChore exception' email thread:

        I have a "IOException" /hbase/.archive/table_name is non empty
        exception every minute on my logs.

        There is 30 directories under this directory. the main directory is
        from yesterday, but all sub directories are from December 10th, all
        the same time.

        Show
        Ted Yu added a comment - Here was description Jean-Marc wrote in 'CleanerChore exception' email thread: I have a "IOException" /hbase/.archive/table_name is non empty exception every minute on my logs. There is 30 directories under this directory. the main directory is from yesterday, but all sub directories are from December 10th, all the same time.
        Hide
        Jean-Marc Spaggiari added a comment -

        I agree that we should push tht to 0.94.4RC too. I will submit the patch for that in the next 2 hours.

        Show
        Jean-Marc Spaggiari added a comment - I agree that we should push tht to 0.94.4RC too. I will submit the patch for that in the next 2 hours.
        Hide
        Jean-Marc Spaggiari added a comment -

        Patch for 0.94 attached.

        Show
        Jean-Marc Spaggiari added a comment - Patch for 0.94 attached.
        Hide
        Jean-Marc Spaggiari added a comment -

        So, here is the updated patch for 0.94.
        Few modifications.
        HBaseCommonTestingUtility doesn't exist on 0.94 so I removed this part.
        UTIL.cleanupTestDir(); is visible for TestCleanerChore in 0.94 so I kept it.

        Show
        Jean-Marc Spaggiari added a comment - So, here is the updated patch for 0.94. Few modifications. HBaseCommonTestingUtility doesn't exist on 0.94 so I removed this part. UTIL.cleanupTestDir(); is visible for TestCleanerChore in 0.94 so I kept it.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12562931/HBASE-7465_7467-v5-0.94.patch
        against trunk revision .

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

        +1 tests included. The patch appears to include 3 new or modified tests.

        -1 patch. The patch command could not apply the patch.

        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/3803//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/12562931/HBASE-7465_7467-v5-0.94.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 3 new or modified tests. -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/3803//console This message is automatically generated.
        Hide
        Ted Yu added a comment -

        The previous 0.94 patch was malformed.
        Attached is after some modification.
        However it still didn't apply cleanly:

        patching file src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java
        Reversed (or previously applied) patch detected! Assume -R? [n] ^C

        Show
        Ted Yu added a comment - The previous 0.94 patch was malformed. Attached is after some modification. However it still didn't apply cleanly: patching file src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java Reversed (or previously applied) patch detected! Assume -R? [n] ^C
        Hide
        Jean-Marc Spaggiari added a comment -

        Hum, strange. I have generated it with git different. I will re-clone and retry...

        Show
        Jean-Marc Spaggiari added a comment - Hum, strange. I have generated it with git different. I will re-clone and retry...
        Hide
        Jean-Marc Spaggiari added a comment -

        Another try Still learning the right way to build the patch...

        Show
        Jean-Marc Spaggiari added a comment - Another try Still learning the right way to build the patch...
        Hide
        Ted Yu added a comment -

        Looks like your 0.94 workspace is out of date - use the tip of 0.94, please:

        p1 7465_7467-v6-0.94.patch
        patching file src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java
        Reversed (or previously applied) patch detected! Assume -R? [n]
        Apply anyway? [n] y
        Hunk #1 FAILED at 142.
        Hunk #2 FAILED at 165.
        2 out of 2 hunks FAILED – saving rejects to file src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java.rej
        patching file src/test/java/org/apache/hadoop/hbase/master/cleaner/TestCleanerChore.java
        Reversed (or previously applied) patch detected! Assume -R? [n] ^C

        Show
        Ted Yu added a comment - Looks like your 0.94 workspace is out of date - use the tip of 0.94, please: p1 7465_7467-v6-0.94.patch patching file src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java Reversed (or previously applied) patch detected! Assume -R? [n] Apply anyway? [n] y Hunk #1 FAILED at 142. Hunk #2 FAILED at 165. 2 out of 2 hunks FAILED – saving rejects to file src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java.rej patching file src/test/java/org/apache/hadoop/hbase/master/cleaner/TestCleanerChore.java Reversed (or previously applied) patch detected! Assume -R? [n] ^C
        Hide
        Jean-Marc Spaggiari added a comment -

        Thanks to Ted for his help to build this patch correctly. Sorry for all those version.

        I hope this one is correct.

        Show
        Jean-Marc Spaggiari added a comment - Thanks to Ted for his help to build this patch correctly. Sorry for all those version. I hope this one is correct.
        Hide
        Ted Yu added a comment -

        I ran TestCleanerChore 6 times and they all passed.

        @Lars:
        Please let us know whether you want the backport in next 0.94.4 RC.

        Thanks

        Show
        Ted Yu added a comment - I ran TestCleanerChore 6 times and they all passed. @Lars: Please let us know whether you want the backport in next 0.94.4 RC. Thanks
        Hide
        Lars Hofhansl added a comment -

        +1 on v7. I'll sink the RC.

        Show
        Lars Hofhansl added a comment - +1 on v7. I'll sink the RC.
        Hide
        Ted Yu added a comment -

        Integrated to 0.94 branch.

        Thanks for the patch, Jean-Marc.

        Thanks for the review, Lars.

        I will watch 0.94 build closely.

        Show
        Ted Yu added a comment - Integrated to 0.94 branch. Thanks for the patch, Jean-Marc. Thanks for the review, Lars. I will watch 0.94 build closely.
        Hide
        Ted Yu added a comment -

        TestHRegionOnCluster failed in the 0.94 build.

        I ran TestHRegionOnCluster locally 5 times and didn't reproduce the failure.

        Show
        Ted Yu added a comment - TestHRegionOnCluster failed in the 0.94 build. I ran TestHRegionOnCluster locally 5 times and didn't reproduce the failure.
        Hide
        Lars Hofhansl added a comment -

        Yeah, that's one I have seen failing before. @#%$^

        Show
        Lars Hofhansl added a comment - Yeah, that's one I have seen failing before. @#%$^
        Hide
        Lars Hofhansl added a comment -

        And of course TestSplitTransactionOnCluster and TestReplication. (I don't know how much more time it takes to make these stable)

        Show
        Lars Hofhansl added a comment - And of course TestSplitTransactionOnCluster and TestReplication. (I don't know how much more time it takes to make these stable)
        Hide
        Hudson added a comment -

        Integrated in HBase-0.94 #692 (See https://builds.apache.org/job/HBase-0.94/692/)
        HBASE-7467 CleanerChore checkAndDeleteDirectory not deleting empty directories (Jean-Marc) (Revision 1428114)

        Result = FAILURE
        tedyu :
        Files :

        • /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java
        • /hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestCleanerChore.java
        Show
        Hudson added a comment - Integrated in HBase-0.94 #692 (See https://builds.apache.org/job/HBase-0.94/692/ ) HBASE-7467 CleanerChore checkAndDeleteDirectory not deleting empty directories (Jean-Marc) (Revision 1428114) Result = FAILURE tedyu : Files : /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java /hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestCleanerChore.java
        Hide
        Jean-Marc Spaggiari added a comment -

        All tests just finished and went well for me using dev-support/hbasetests.sh:
        ##########################
        315 tests executed successfully
        0 tests are in error
        0 tests didn't finish

        Tests in error are:
        Tests that didn't finish are:

        Execution time in minutes: 115
        ##########################

        I'm now running the tests with mvn command line.

        Show
        Jean-Marc Spaggiari added a comment - All tests just finished and went well for me using dev-support/hbasetests.sh: ########################## 315 tests executed successfully 0 tests are in error 0 tests didn't finish Tests in error are: Tests that didn't finish are: Execution time in minutes: 115 ########################## I'm now running the tests with mvn command line.
        Hide
        Hudson added a comment -

        Integrated in HBase-0.94-security #93 (See https://builds.apache.org/job/HBase-0.94-security/93/)
        HBASE-7467 CleanerChore checkAndDeleteDirectory not deleting empty directories (Jean-Marc) (Revision 1428114)

        Result = SUCCESS
        tedyu :
        Files :

        • /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java
        • /hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestCleanerChore.java
        Show
        Hudson added a comment - Integrated in HBase-0.94-security #93 (See https://builds.apache.org/job/HBase-0.94-security/93/ ) HBASE-7467 CleanerChore checkAndDeleteDirectory not deleting empty directories (Jean-Marc) (Revision 1428114) Result = SUCCESS tedyu : Files : /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java /hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestCleanerChore.java
        Hide
        Hudson added a comment -

        Integrated in HBase-0.94-security-on-Hadoop-23 #10 (See https://builds.apache.org/job/HBase-0.94-security-on-Hadoop-23/10/)
        HBASE-7467 CleanerChore checkAndDeleteDirectory not deleting empty directories (Jean-Marc) (Revision 1428114)

        Result = FAILURE
        tedyu :
        Files :

        • /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java
        • /hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestCleanerChore.java
        Show
        Hudson added a comment - Integrated in HBase-0.94-security-on-Hadoop-23 #10 (See https://builds.apache.org/job/HBase-0.94-security-on-Hadoop-23/10/ ) HBASE-7467 CleanerChore checkAndDeleteDirectory not deleting empty directories (Jean-Marc) (Revision 1428114) Result = FAILURE tedyu : Files : /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java /hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestCleanerChore.java

          People

          • Assignee:
            Jean-Marc Spaggiari
            Reporter:
            Jean-Marc Spaggiari
          • Votes:
            0 Vote for this issue
            Watchers:
            8 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development