HBase
  1. HBase
  2. HBASE-3872

Hole in split transaction rollback; edits to .META. need to be rolled back even if it seems like they didn't make it

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 0.90.3
    • Fix Version/s: 0.90.4
    • Component/s: regionserver
    • Labels:
      None

      Description

      Saw this interesting one on a cluster of ours. The cluster was configured with too few handlers so lots of the phenomeneon where actions were queued but then by the time they got into the server and tried respond to the client, the client had disconnected because of the timeout of 60 seconds. Well, the meta edits for a split were queued at the regionserver carrying .META. and by the time it went to write back, the client had gone (the first insert of parent offline with daughter regions added as info:splitA and info:splitB). The client presumed the edits failed and 'successfully' rolled back the transaction (failing to undo .META. edits thinking they didn't go through).

      A few minutes later the .META. scanner on master runs. It sees 'no references' in daughters – the daughters had been cleaned up as part of the split transaction rollback – so it thinks its safe to delete the parent.

      Two things:

      + Tighten up check in master... need to check daughter region at least exists and possibly the daughter region has an entry in .META.
      + Dependent on the edit that fails, schedule rollback edits though it will seem like they didn't go through.

      This is pretty critical one.

      1. 3872.txt
        13 kB
        stack
      2. 3872-v2.txt
        13 kB
        stack

        Activity

        Hide
        bluedavy added a comment -

        I created the HBASE-4562,HBASE-4563.

        Show
        bluedavy added a comment - I created the HBASE-4562 , HBASE-4563 .
        Hide
        Ted Yu added a comment -

        I think the above code should be improved.
        If testing is true, JournalEntry.PONR wouldn't be set.
        It seems JournalEntry.PONR should be set before the check for testing.

        A new JIRA should be opened.

        Show
        Ted Yu added a comment - I think the above code should be improved. If testing is true, JournalEntry.PONR wouldn't be set. It seems JournalEntry.PONR should be set before the check for testing. A new JIRA should be opened.
        Hide
        bluedavy added a comment -
        Bar.java
            if (!testing) {
              this.journal.add(JournalEntry.PONR); 
              MetaEditor.offlineParentInMeta(server.getCatalogTracker(),
                this.parent.getRegionInfo(), a.getRegionInfo(), b.getRegionInfo());
            }
            // this.journal.add(JournalEntry.PONR); 
        
        Show
        bluedavy added a comment - Bar.java if (!testing) { this .journal.add(JournalEntry.PONR); MetaEditor.offlineParentInMeta(server.getCatalogTracker(), this .parent.getRegionInfo(), a.getRegionInfo(), b.getRegionInfo()); } // this .journal.add(JournalEntry.PONR);
        Hide
        bluedavy added a comment -

        We fix the bug using below code:
        if (!testing)

        { + this.journal.add(JournalEntry.PONR); MetaEditor.offlineParentInMeta(server.getCatalogTracker(),this.parent.getRegionInfo(), a.getRegionInfo(), b.getRegionInfo()); }
        • this.journal.add(JournalEntry.PONR);
        Show
        bluedavy added a comment - We fix the bug using below code: if (!testing) { + this.journal.add(JournalEntry.PONR); MetaEditor.offlineParentInMeta(server.getCatalogTracker(),this.parent.getRegionInfo(), a.getRegionInfo(), b.getRegionInfo()); } this.journal.add(JournalEntry.PONR);
        Hide
        bluedavy added a comment -

        @stack
        current patch will cause data-loss in this situation:
        1. change the SplitTransaction just like @mingjian said;
        2. then create a table & put some data in hbase shell;
        3. split the table in hbase shell;
        4. kill the region server hosted the table;
        5. after master do servershutdownhandler,then the table can be wrote again,but the data previous wrote to the table lost.

        and in above code,if we don't kill the region server,then the parent region cann't be wrote,even if restart the cluster.

        Show
        bluedavy added a comment - @stack current patch will cause data-loss in this situation: 1. change the SplitTransaction just like @mingjian said; 2. then create a table & put some data in hbase shell; 3. split the table in hbase shell; 4. kill the region server hosted the table; 5. after master do servershutdownhandler,then the table can be wrote again,but the data previous wrote to the table lost. and in above code,if we don't kill the region server,then the parent region cann't be wrote,even if restart the cluster.
        Hide
        mingjian added a comment -

        @stack: Do you think delete daughter regions when rollbacking will cause data loss?
        We can replay this bug while using the follow code:

        SplitTransaction.java
            if (!testing) {
              MetaEditor.offlineParentInMeta(server.getCatalogTracker(),
                this.parent.getRegionInfo(), a.getRegionInfo(), b.getRegionInfo());
        throw new IOException("for test!");
            }
        

        we think we could move "this.journal.add(JournalEntry.PONR);" before MetaEditor.offlineParentInMeta, then rollback would cause RS exit, not delete Daughter regions.
        Do you think so?

        Show
        mingjian added a comment - @stack: Do you think delete daughter regions when rollbacking will cause data loss? We can replay this bug while using the follow code: SplitTransaction.java if (!testing) { MetaEditor.offlineParentInMeta(server.getCatalogTracker(), this .parent.getRegionInfo(), a.getRegionInfo(), b.getRegionInfo()); throw new IOException( " for test!" ); } we think we could move "this.journal.add(JournalEntry.PONR);" before MetaEditor.offlineParentInMeta, then rollback would cause RS exit, not delete Daughter regions. Do you think so?
        Hide
        stack added a comment -

        This patch plugged this hole two ways:

        1. not removing parent region unless the daughter region existed
        2. on split transaction failure, do NOT remove daughter regions; crash out the regionserver and let fix up of the shutdown server do the fixup in meta adding daughters.

        #1 in the above created HBASE-4238 where we won't delete parents if daughters that have themselves split get cleaned up before their parent has all its references let go. So #1 is being undone over in the fix for HBASE-4238 since its possible daughter regions may not exist in the fs legitimately.

        Show
        stack added a comment - This patch plugged this hole two ways: 1. not removing parent region unless the daughter region existed 2. on split transaction failure, do NOT remove daughter regions; crash out the regionserver and let fix up of the shutdown server do the fixup in meta adding daughters. #1 in the above created HBASE-4238 where we won't delete parents if daughters that have themselves split get cleaned up before their parent has all its references let go. So #1 is being undone over in the fix for HBASE-4238 since its possible daughter regions may not exist in the fs legitimately.
        Hide
        Hudson added a comment -

        Integrated in HBase-TRUNK #2049 (See https://builds.apache.org/job/HBase-TRUNK/2049/)
        HBASE-4129 hbase-3872 added a warn message 'CatalogJanitor: Daughter regiondir does not exist' that is triggered though its often legit that daughter is not present

        Show
        Hudson added a comment - Integrated in HBase-TRUNK #2049 (See https://builds.apache.org/job/HBase-TRUNK/2049/ ) HBASE-4129 hbase-3872 added a warn message 'CatalogJanitor: Daughter regiondir does not exist' that is triggered though its often legit that daughter is not present
        Hide
        stack added a comment -

        I committed this to branch a while back (and earlier to trunk)

        Show
        stack added a comment - I committed this to branch a while back (and earlier to trunk)
        Hide
        Hudson added a comment -

        Integrated in HBase-TRUNK #2029 (See https://builds.apache.org/job/HBase-TRUNK/2029/)
        HBASE-3872 Hole in split transaction rollback; edits to .META. need to be rolled back even if it seems like they didn't make it; fix failing HRegion test – server passed is null, allow for this in tests when running splits

        stack :
        Files :

        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java
        Show
        Hudson added a comment - Integrated in HBase-TRUNK #2029 (See https://builds.apache.org/job/HBase-TRUNK/2029/ ) HBASE-3872 Hole in split transaction rollback; edits to .META. need to be rolled back even if it seems like they didn't make it; fix failing HRegion test – server passed is null, allow for this in tests when running splits stack : Files : /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java
        Hide
        Hudson added a comment -

        Integrated in HBase-TRUNK #2028 (See https://builds.apache.org/job/HBase-TRUNK/2028/)
        HBASE-3872 Hole in split transaction rollback; edits to .META. need to be rolled back even if it seems like they didn't make it
        HBASE-3872 Hole in split transaction rollback; edits to .META. need to be rolled back even if it seems like they didn't make it

        stack :
        Files :

        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java

        stack :
        Files :

        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java
        • /hbase/trunk/CHANGES.txt
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransaction.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/SplitRequest.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java
        Show
        Hudson added a comment - Integrated in HBase-TRUNK #2028 (See https://builds.apache.org/job/HBase-TRUNK/2028/ ) HBASE-3872 Hole in split transaction rollback; edits to .META. need to be rolled back even if it seems like they didn't make it HBASE-3872 Hole in split transaction rollback; edits to .META. need to be rolled back even if it seems like they didn't make it stack : Files : /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java stack : Files : /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java /hbase/trunk/CHANGES.txt /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransaction.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/SplitRequest.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java
        Hide
        stack added a comment -

        I applied patch to TRUNK. Working on version for 0.90.0 now.

        Show
        stack added a comment - I applied patch to TRUNK. Working on version for 0.90.0 now.
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/1097/#review1051
        -----------------------------------------------------------

        Ship it!

        • Ted

        On 2011-07-13 04:06:54, Michael Stack wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/1097/

        -----------------------------------------------------------

        (Updated 2011-07-13 04:06:54)

        Review request for hbase.

        Summary

        -------

        Fix is two-fold.

        1. Fix catalogjanitor so that it does not remove parent if daughter regions are not present.

        2. Fix the SplitTransaction so that if we go past the point of no return, DO NOT REMOVE daughter regions as part of cleanup; leave them in place. Because we went past PONR, we are going to abort and server shutdown processing has what it needs to do fixup.

        This addresses bug hbase-3872.

        https://issues.apache.org/jira/browse/hbase-3872

        Diffs

        -----

        src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java 7082342

        src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 977115d

        src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java c402b87

        src/main/java/org/apache/hadoop/hbase/regionserver/SplitRequest.java 16e970e

        src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java ab88766

        src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java 49333ac

        src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java 56fc0b8

        src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransaction.java 98a7ee7

        src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java 4a35d09

        Diff: https://reviews.apache.org/r/1097/diff

        Testing

        -------

        Added two new unit tests. One to test we do not remove parent if no daughter region in fs and another to test that the right answer pops out of the call to rollback if we go past PONR (and that the daughter regions are still in place after rollback).

        Thanks,

        Michael

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1097/#review1051 ----------------------------------------------------------- Ship it! Ted On 2011-07-13 04:06:54, Michael Stack wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1097/ ----------------------------------------------------------- (Updated 2011-07-13 04:06:54) Review request for hbase. Summary ------- Fix is two-fold. 1. Fix catalogjanitor so that it does not remove parent if daughter regions are not present. 2. Fix the SplitTransaction so that if we go past the point of no return, DO NOT REMOVE daughter regions as part of cleanup; leave them in place. Because we went past PONR, we are going to abort and server shutdown processing has what it needs to do fixup. This addresses bug hbase-3872. https://issues.apache.org/jira/browse/hbase-3872 Diffs ----- src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java 7082342 src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 977115d src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java c402b87 src/main/java/org/apache/hadoop/hbase/regionserver/SplitRequest.java 16e970e src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java ab88766 src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java 49333ac src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java 56fc0b8 src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransaction.java 98a7ee7 src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java 4a35d09 Diff: https://reviews.apache.org/r/1097/diff Testing ------- Added two new unit tests. One to test we do not remove parent if no daughter region in fs and another to test that the right answer pops out of the call to rollback if we go past PONR (and that the daughter regions are still in place after rollback). Thanks, Michael
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/1097/#review1050
        -----------------------------------------------------------

        src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java
        <https://reviews.apache.org/r/1097/#comment2123>

        Good one. You +1 on committing?

        • Michael

        On 2011-07-13 04:06:54, Michael Stack wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/1097/

        -----------------------------------------------------------

        (Updated 2011-07-13 04:06:54)

        Review request for hbase.

        Summary

        -------

        Fix is two-fold.

        1. Fix catalogjanitor so that it does not remove parent if daughter regions are not present.

        2. Fix the SplitTransaction so that if we go past the point of no return, DO NOT REMOVE daughter regions as part of cleanup; leave them in place. Because we went past PONR, we are going to abort and server shutdown processing has what it needs to do fixup.

        This addresses bug hbase-3872.

        https://issues.apache.org/jira/browse/hbase-3872

        Diffs

        -----

        src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java 7082342

        src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 977115d

        src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java c402b87

        src/main/java/org/apache/hadoop/hbase/regionserver/SplitRequest.java 16e970e

        src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java ab88766

        src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java 49333ac

        src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java 56fc0b8

        src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransaction.java 98a7ee7

        src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java 4a35d09

        Diff: https://reviews.apache.org/r/1097/diff

        Testing

        -------

        Added two new unit tests. One to test we do not remove parent if no daughter region in fs and another to test that the right answer pops out of the call to rollback if we go past PONR (and that the daughter regions are still in place after rollback).

        Thanks,

        Michael

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1097/#review1050 ----------------------------------------------------------- src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java < https://reviews.apache.org/r/1097/#comment2123 > Good one. You +1 on committing? Michael On 2011-07-13 04:06:54, Michael Stack wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1097/ ----------------------------------------------------------- (Updated 2011-07-13 04:06:54) Review request for hbase. Summary ------- Fix is two-fold. 1. Fix catalogjanitor so that it does not remove parent if daughter regions are not present. 2. Fix the SplitTransaction so that if we go past the point of no return, DO NOT REMOVE daughter regions as part of cleanup; leave them in place. Because we went past PONR, we are going to abort and server shutdown processing has what it needs to do fixup. This addresses bug hbase-3872. https://issues.apache.org/jira/browse/hbase-3872 Diffs ----- src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java 7082342 src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 977115d src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java c402b87 src/main/java/org/apache/hadoop/hbase/regionserver/SplitRequest.java 16e970e src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java ab88766 src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java 49333ac src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java 56fc0b8 src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransaction.java 98a7ee7 src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java 4a35d09 Diff: https://reviews.apache.org/r/1097/diff Testing ------- Added two new unit tests. One to test we do not remove parent if no daughter region in fs and another to test that the right answer pops out of the call to rollback if we go past PONR (and that the daughter regions are still in place after rollback). Thanks, Michael
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/1097/#review1049
        -----------------------------------------------------------

        There are a few white spaces.

        src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java
        <https://reviews.apache.org/r/1097/#comment2122>

        I assume this is a copy-and-paste issue.

        • Ted

        On 2011-07-13 04:06:54, Michael Stack wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/1097/

        -----------------------------------------------------------

        (Updated 2011-07-13 04:06:54)

        Review request for hbase.

        Summary

        -------

        Fix is two-fold.

        1. Fix catalogjanitor so that it does not remove parent if daughter regions are not present.

        2. Fix the SplitTransaction so that if we go past the point of no return, DO NOT REMOVE daughter regions as part of cleanup; leave them in place. Because we went past PONR, we are going to abort and server shutdown processing has what it needs to do fixup.

        This addresses bug hbase-3872.

        https://issues.apache.org/jira/browse/hbase-3872

        Diffs

        -----

        src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java 7082342

        src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 977115d

        src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java c402b87

        src/main/java/org/apache/hadoop/hbase/regionserver/SplitRequest.java 16e970e

        src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java ab88766

        src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java 49333ac

        src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java 56fc0b8

        src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransaction.java 98a7ee7

        src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java 4a35d09

        Diff: https://reviews.apache.org/r/1097/diff

        Testing

        -------

        Added two new unit tests. One to test we do not remove parent if no daughter region in fs and another to test that the right answer pops out of the call to rollback if we go past PONR (and that the daughter regions are still in place after rollback).

        Thanks,

        Michael

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1097/#review1049 ----------------------------------------------------------- There are a few white spaces. src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java < https://reviews.apache.org/r/1097/#comment2122 > I assume this is a copy-and-paste issue. Ted On 2011-07-13 04:06:54, Michael Stack wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1097/ ----------------------------------------------------------- (Updated 2011-07-13 04:06:54) Review request for hbase. Summary ------- Fix is two-fold. 1. Fix catalogjanitor so that it does not remove parent if daughter regions are not present. 2. Fix the SplitTransaction so that if we go past the point of no return, DO NOT REMOVE daughter regions as part of cleanup; leave them in place. Because we went past PONR, we are going to abort and server shutdown processing has what it needs to do fixup. This addresses bug hbase-3872. https://issues.apache.org/jira/browse/hbase-3872 Diffs ----- src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java 7082342 src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 977115d src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java c402b87 src/main/java/org/apache/hadoop/hbase/regionserver/SplitRequest.java 16e970e src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java ab88766 src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java 49333ac src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java 56fc0b8 src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransaction.java 98a7ee7 src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java 4a35d09 Diff: https://reviews.apache.org/r/1097/diff Testing ------- Added two new unit tests. One to test we do not remove parent if no daughter region in fs and another to test that the right answer pops out of the call to rollback if we go past PONR (and that the daughter regions are still in place after rollback). Thanks, Michael
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/1097/
        -----------------------------------------------------------

        Review request for hbase.

        Summary
        -------

        Fix is two-fold.

        1. Fix catalogjanitor so that it does not remove parent if daughter regions are not present.
        2. Fix the SplitTransaction so that if we go past the point of no return, DO NOT REMOVE daughter regions as part of cleanup; leave them in place. Because we went past PONR, we are going to abort and server shutdown processing has what it needs to do fixup.

        This addresses bug hbase-3872.
        https://issues.apache.org/jira/browse/hbase-3872

        Diffs


        src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java 7082342
        src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 977115d
        src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java c402b87
        src/main/java/org/apache/hadoop/hbase/regionserver/SplitRequest.java 16e970e
        src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java ab88766
        src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java 49333ac
        src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java 56fc0b8
        src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransaction.java 98a7ee7
        src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java 4a35d09

        Diff: https://reviews.apache.org/r/1097/diff

        Testing
        -------

        Added two new unit tests. One to test we do not remove parent if no daughter region in fs and another to test that the right answer pops out of the call to rollback if we go past PONR (and that the daughter regions are still in place after rollback).

        Thanks,

        Michael

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1097/ ----------------------------------------------------------- Review request for hbase. Summary ------- Fix is two-fold. 1. Fix catalogjanitor so that it does not remove parent if daughter regions are not present. 2. Fix the SplitTransaction so that if we go past the point of no return, DO NOT REMOVE daughter regions as part of cleanup; leave them in place. Because we went past PONR, we are going to abort and server shutdown processing has what it needs to do fixup. This addresses bug hbase-3872. https://issues.apache.org/jira/browse/hbase-3872 Diffs src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java 7082342 src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 977115d src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java c402b87 src/main/java/org/apache/hadoop/hbase/regionserver/SplitRequest.java 16e970e src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java ab88766 src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java 49333ac src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java 56fc0b8 src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransaction.java 98a7ee7 src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java 4a35d09 Diff: https://reviews.apache.org/r/1097/diff Testing ------- Added two new unit tests. One to test we do not remove parent if no daughter region in fs and another to test that the right answer pops out of the call to rollback if we go past PONR (and that the daughter regions are still in place after rollback). Thanks, Michael
        Hide
        Ted Yu added a comment -

        A separate patch for 0.90 is needed.

        Show
        Ted Yu added a comment - A separate patch for 0.90 is needed.
        Hide
        stack added a comment -

        Update patch to apply to trunk.

        Show
        stack added a comment - Update patch to apply to trunk.
        Hide
        stack added a comment -

        Yeah, what Ted says. If region is gone from filesystem then there is likely little of data up in the RS memory. Any chance of more log from around the 'event' Aaron.

        Meantime I'll work more on the attached patch.

        Show
        stack added a comment - Yeah, what Ted says. If region is gone from filesystem then there is likely little of data up in the RS memory. Any chance of more log from around the 'event' Aaron. Meantime I'll work more on the attached patch.
        Hide
        Ted Yu added a comment -

        From HBaseFsck.WorkItemRegion.run():

                List<HRegionInfo> regions = server.getOnlineRegions();
        

        I think the above error log just means that UNKNOWN_REGION is in the memory of regionserver-redacted.

        Show
        Ted Yu added a comment - From HBaseFsck.WorkItemRegion.run(): List<HRegionInfo> regions = server.getOnlineRegions(); I think the above error log just means that UNKNOWN_REGION is in the memory of regionserver-redacted.
        Hide
        Aaron Kimball added a comment -

        Stack,

        I'm still working on getting more log data available. Hopefully I can get you more info. Here's something interesting: I believe the parent regions are still hanging around in HBase regionservers.

        hbck reports:

        ERROR: Region UNKNOWN_REGION on <regionserver-redacted>:60020, key=4f424608930f7b3ae7c05c49e2bac2c1, not on HDFS or in META but deployed on <regionserver-redacted>:60020
        ERROR: Region UNKNOWN_REGION on <regionserver-redacted>:60020, key=81c5fb35e10f8ef61da78bbba28db7f9, not on HDFS or in META but deployed on <regionserver-redacted>:60020
        

        The region keys match those of the split parents which were abandoned when the "successful" rollback didn't restore the parent entries in .META.. Is there a way to force these back to storefiles on disk, and then manually add them to .META.?

        Show
        Aaron Kimball added a comment - Stack, I'm still working on getting more log data available. Hopefully I can get you more info. Here's something interesting: I believe the parent regions are still hanging around in HBase regionservers. hbck reports: ERROR: Region UNKNOWN_REGION on <regionserver-redacted>:60020, key=4f424608930f7b3ae7c05c49e2bac2c1, not on HDFS or in META but deployed on <regionserver-redacted>:60020 ERROR: Region UNKNOWN_REGION on <regionserver-redacted>:60020, key=81c5fb35e10f8ef61da78bbba28db7f9, not on HDFS or in META but deployed on <regionserver-redacted>:60020 The region keys match those of the split parents which were abandoned when the "successful" rollback didn't restore the parent entries in .META. . Is there a way to force these back to storefiles on disk, and then manually add them to .META. ?
        Hide
        Ted Yu added a comment -

        For the case Aaron described, should HRegionServer.splitRegion() inform LogRoller that a split is pending so that Hlog roll can be delayed ?

        Show
        Ted Yu added a comment - For the case Aaron described, should HRegionServer.splitRegion() inform LogRoller that a split is pending so that Hlog roll can be delayed ?
        Hide
        Aaron Kimball added a comment -

        The parent is not in .META. There is a "hole" in the list of regions, as seen by running a scan on .META. from the hbase shell and/or by looking at table.jsp on the master server's website.

        Also, running hbase hbck identified one of the missing regions ("chain of regions in table ... is broken; edges does not contain <rowkey>"). It did not notice the second missing region. Is that because the process that checks the region chain gives up after the first error? Or could that be unrelated?

        Show
        Aaron Kimball added a comment - The parent is not in .META. There is a "hole" in the list of regions, as seen by running a scan on .META. from the hbase shell and/or by looking at table.jsp on the master server's website. Also, running hbase hbck identified one of the missing regions ("chain of regions in table ... is broken; edges does not contain <rowkey>"). It did not notice the second missing region. Is that because the process that checks the region chain gives up after the first error? Or could that be unrelated?
        Hide
        stack added a comment -

        Stick in more log from around the fail

        Show
        stack added a comment - Stick in more log from around the fail
        Hide
        stack added a comment -

        @Aaron That might be different to what I saw at head of this issue. Is the parent still in .META.?

        Show
        stack added a comment - @Aaron That might be different to what I saw at head of this issue. Is the parent still in .META.?
        Hide
        Aaron Kimball added a comment -

        A further observation: this seems to have occurred when splitting multiple regions within the same table (during a day of large bulk loads). The logs show the parent region being offlined, then both daughter regions being instantiated. The following sequence of log messages appeared both times:

        2011-06-21 21:51:17,594 DEBUG org.apache.hadoop.hbase.regionserver.HRegion: Instantiated (redacted-a-daughter).
        2011-06-21 21:51:17,630 DEBUG org.apache.hadoop.hbase.regionserver.HRegion: Instantiated (redacted-b-daughter).
        2011-06-21 21:52:05,412 DEBUG org.apache.hadoop.hbase.regionserver.LogRoller: Hlog roll period 3600000ms elapsed
        2011-06-21 21:52:17,666 INFO org.apache.hadoop.hbase.regionserver.CompactSplitThread: Running rollback of failed split of (redacted-parent-region); Call to (redacted-server-address):60020 failed on socket timeout exception: java.net.SocketTimeoutException: 60000 millis timeout while waiting for channel to be ready for read. ch : java.nio.channels.SocketChannel[connected local=(redacted-server-ip):54054 remote=(redacted-server-address):60020]
        

        I find it noteworthy that the "Hlog roll period elapsed" message occurred between the "B" daughter instantiation and the socket timeout in both cases of missing regions I am aware of in my table.

        Show
        Aaron Kimball added a comment - A further observation: this seems to have occurred when splitting multiple regions within the same table (during a day of large bulk loads). The logs show the parent region being offlined, then both daughter regions being instantiated. The following sequence of log messages appeared both times: 2011-06-21 21:51:17,594 DEBUG org.apache.hadoop.hbase.regionserver.HRegion: Instantiated (redacted-a-daughter). 2011-06-21 21:51:17,630 DEBUG org.apache.hadoop.hbase.regionserver.HRegion: Instantiated (redacted-b-daughter). 2011-06-21 21:52:05,412 DEBUG org.apache.hadoop.hbase.regionserver.LogRoller: Hlog roll period 3600000ms elapsed 2011-06-21 21:52:17,666 INFO org.apache.hadoop.hbase.regionserver.CompactSplitThread: Running rollback of failed split of (redacted-parent-region); Call to (redacted-server-address):60020 failed on socket timeout exception: java.net.SocketTimeoutException: 60000 millis timeout while waiting for channel to be ready for read. ch : java.nio.channels.SocketChannel[connected local=(redacted-server-ip):54054 remote=(redacted-server-address):60020] I find it noteworthy that the "Hlog roll period elapsed" message occurred between the "B" daughter instantiation and the socket timeout in both cases of missing regions I am aware of in my table.
        Hide
        stack added a comment -

        @Aaron If same issue as I saw, the split parent was deleted. The region is gone.

        Show
        stack added a comment - @Aaron If same issue as I saw, the split parent was deleted. The region is gone.
        Hide
        Aaron Kimball added a comment -

        (This was on HBase 0.90.1-cdh3u0)

        Show
        Aaron Kimball added a comment - (This was on HBase 0.90.1-cdh3u0)
        Hide
        Aaron Kimball added a comment -

        We were bit by a bug that I believe is the same as this one. A regionserver started performing a split, but got a SocketTimeoutException connecting to the regionserver with .META. on it, triggering a rollback; the rollback was reported as successful, but the region has now gone completely missing from the table.

        Would definitely like to see this get in. Would also like to know if there's a way to retrieve the pre-split storefile and get that region back online?

        Show
        Aaron Kimball added a comment - We were bit by a bug that I believe is the same as this one. A regionserver started performing a split, but got a SocketTimeoutException connecting to the regionserver with .META. on it, triggering a rollback; the rollback was reported as successful, but the region has now gone completely missing from the table. Would definitely like to see this get in. Would also like to know if there's a way to retrieve the pre-split storefile and get that region back online?
        Hide
        stack added a comment -

        Not finished but I think best tactic is moving meta edit beyond point-of-no-return splitting; if it fails at all, crash out the regionserver and let servershutdownhandler do the fixup. Fixup will now have this second case to deal with. Do the fixup in one place only. Need to also amend CatalogJanitor so it does not mistakenly cleanup the parent region.

        Show
        stack added a comment - Not finished but I think best tactic is moving meta edit beyond point-of-no-return splitting; if it fails at all, crash out the regionserver and let servershutdownhandler do the fixup. Fixup will now have this second case to deal with. Do the fixup in one place only. Need to also amend CatalogJanitor so it does not mistakenly cleanup the parent region.
        Hide
        stack added a comment -

        offlineParentInMeta doesn't have an entry in the journal (OFFLINED_PARENT is something else) but I think that is ok

        Show
        stack added a comment - offlineParentInMeta doesn't have an entry in the journal (OFFLINED_PARENT is something else) but I think that is ok
        Hide
        stack added a comment -

        This bug caused hbase to delete a whole region. Critical.

        Show
        stack added a comment - This bug caused hbase to delete a whole region. Critical.
        Hide
        stack added a comment -

        Look at OFFLINED_PARENT state in split transaction too. It looks like we do not record it in journal as we run execution.

        Show
        stack added a comment - Look at OFFLINED_PARENT state in split transaction too. It looks like we do not record it in journal as we run execution.

          People

          • Assignee:
            stack
            Reporter:
            stack
          • Votes:
            0 Vote for this issue
            Watchers:
            8 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development