HBase
  1. HBase
  2. HBASE-5712

Parallelize load of .regioninfo files in diagnostic/repair portion of hbck.

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.90.7, 0.92.2, 0.94.0, 0.95.2
    • Fix Version/s: 0.94.0, 0.95.0
    • Component/s: hbck
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      On heavily loaded hdfs's some dfs nodes may not respond quickly and backs off for 60s before attempting to read data from another datanode. Portions of the information gathered from hdfs (.regioninfo files) are loaded serially. With HBase with clusters with 100's, or 1000's, or 10000's regions encountering these 60s delay blocks progress and can be very painful.

      There is already some parallelization of portions of the hdfs information load operations and the goal here is move the reading of .regioninfos into the parallelized sections..

      1. hbase-5712.patch
        10 kB
        Jonathan Hsieh
      2. hbase-5712-90.patch
        9 kB
        Jonathan Hsieh
      3. hbase-5712-90-v2.patch
        9 kB
        Jonathan Hsieh
      4. hbase-5712-v2.patch
        10 kB
        Jonathan Hsieh

        Issue Links

          Activity

          Hide
          Hudson added a comment -

          Integrated in HBase-0.92-security #106 (See https://builds.apache.org/job/HBase-0.92-security/106/)
          HBASE-5712 Parallelize load of .regioninfo files in diagnostic/repair portion of hbck (Revision 1332070)

          Result = SUCCESS
          jmhsieh :
          Files :

          • /hbase/branches/0.92/CHANGES.txt
          • /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java
          • /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java
          Show
          Hudson added a comment - Integrated in HBase-0.92-security #106 (See https://builds.apache.org/job/HBase-0.92-security/106/ ) HBASE-5712 Parallelize load of .regioninfo files in diagnostic/repair portion of hbck (Revision 1332070) Result = SUCCESS jmhsieh : Files : /hbase/branches/0.92/CHANGES.txt /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java
          Hide
          Hudson added a comment -

          Integrated in HBase-0.94-security #25 (See https://builds.apache.org/job/HBase-0.94-security/25/)
          HBASE-5712 Parallelize load of .regioninfo files in diagnostic/repair portion of hbck (Revision 1332071)

          Result = SUCCESS
          jmhsieh :
          Files :

          • /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java
          • /hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java
          Show
          Hudson added a comment - Integrated in HBase-0.94-security #25 (See https://builds.apache.org/job/HBase-0.94-security/25/ ) HBASE-5712 Parallelize load of .regioninfo files in diagnostic/repair portion of hbck (Revision 1332071) Result = SUCCESS jmhsieh : Files : /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java /hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java
          Hide
          Hudson added a comment -

          Integrated in HBase-TRUNK-security #188 (See https://builds.apache.org/job/HBase-TRUNK-security/188/)
          HBASE-5712 Parallelize load of .regioninfo files in diagnostic/repair portion of hbck (Revision 1332072)

          Result = FAILURE
          jmhsieh :
          Files :

          • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java
          • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java
          Show
          Hudson added a comment - Integrated in HBase-TRUNK-security #188 (See https://builds.apache.org/job/HBase-TRUNK-security/188/ ) HBASE-5712 Parallelize load of .regioninfo files in diagnostic/repair portion of hbck (Revision 1332072) Result = FAILURE jmhsieh : Files : /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java
          Hide
          Hudson added a comment -

          Integrated in HBase-0.92 #393 (See https://builds.apache.org/job/HBase-0.92/393/)
          HBASE-5712 Parallelize load of .regioninfo files in diagnostic/repair portion of hbck (Revision 1332070)

          Result = FAILURE
          jmhsieh :
          Files :

          • /hbase/branches/0.92/CHANGES.txt
          • /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java
          • /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java
          Show
          Hudson added a comment - Integrated in HBase-0.92 #393 (See https://builds.apache.org/job/HBase-0.92/393/ ) HBASE-5712 Parallelize load of .regioninfo files in diagnostic/repair portion of hbck (Revision 1332070) Result = FAILURE jmhsieh : Files : /hbase/branches/0.92/CHANGES.txt /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java
          Hide
          Hudson added a comment -

          Integrated in HBase-0.94 #160 (See https://builds.apache.org/job/HBase-0.94/160/)
          HBASE-5712 Parallelize load of .regioninfo files in diagnostic/repair portion of hbck (Revision 1332071)

          Result = FAILURE
          jmhsieh :
          Files :

          • /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java
          • /hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java
          Show
          Hudson added a comment - Integrated in HBase-0.94 #160 (See https://builds.apache.org/job/HBase-0.94/160/ ) HBASE-5712 Parallelize load of .regioninfo files in diagnostic/repair portion of hbck (Revision 1332071) Result = FAILURE jmhsieh : Files : /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java /hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java
          Hide
          Hudson added a comment -

          Integrated in HBase-TRUNK #2825 (See https://builds.apache.org/job/HBase-TRUNK/2825/)
          HBASE-5712 Parallelize load of .regioninfo files in diagnostic/repair portion of hbck (Revision 1332072)

          Result = SUCCESS
          jmhsieh :
          Files :

          • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java
          • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java
          Show
          Hudson added a comment - Integrated in HBase-TRUNK #2825 (See https://builds.apache.org/job/HBase-TRUNK/2825/ ) HBASE-5712 Parallelize load of .regioninfo files in diagnostic/repair portion of hbck (Revision 1332072) Result = SUCCESS jmhsieh : Files : /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java
          Hide
          jiraposter@reviews.apache.org added a comment -

          On 2012-04-27 23:27:20, Michael Stack wrote:

          > src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java, line 204

          > <https://reviews.apache.org/r/4883/diff/1/?file=104442#file104442line204>

          >

          > This'll work but why not ConcurrentSkipListMap?

          Sure, changed.

          On 2012-04-27 23:27:20, Michael Stack wrote:

          > src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java, line 418

          > <https://reviews.apache.org/r/4883/diff/1/?file=104442#file104442line418>

          >

          > +1 on suggested change

          done.

          On 2012-04-27 23:27:20, Michael Stack wrote:

          > src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java, line 2835

          > <https://reviews.apache.org/r/4883/diff/1/?file=104442#file104442line2835>

          >

          > Is this flag needed? Why not just check thread is alive? I see we can return with an error. What happens if the return on 2816 happens? Will the wait at #643 above be for ever?

          This is not a thread but actually fed to an executor (thread pool) at line 637. If the return happens on 2816, this is in a finally which will always mark the workitem as done.

          There are two other instances of this pattern that were originally in this code before I got to it – I'd have used Futures (and have filed a follow on issue for it) but it works.

          • jmhsieh

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

          On 2012-04-26 01:42:01, jmhsieh wrote:

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

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

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

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

          (Updated 2012-04-26 01:42:01)

          Review request for hbase, Ted Yu and Jimmy Xiang.

          Summary

          -------

          * Parallelized load of .regioninfo files

          * changed TreeMap to SortedMap in method signatures

          * renamed a test's name.

          This addresses bug HBASE-5712.

          https://issues.apache.org/jira/browse/HBASE-5712

          Diffs

          -----

          src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 66156c2

          src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java 6b64f10

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

          Testing

          -------

          Ran patch 10x on trunk, passes. Ran 1x on 0.92 and 0.94.

          Ther 0.90 version that is nearly identical except for ignoring changes near lines HBaseFsck lines 671-680.

          Thanks,

          jmhsieh

          Show
          jiraposter@reviews.apache.org added a comment - On 2012-04-27 23:27:20, Michael Stack wrote: > src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java, line 204 > < https://reviews.apache.org/r/4883/diff/1/?file=104442#file104442line204 > > > This'll work but why not ConcurrentSkipListMap? Sure, changed. On 2012-04-27 23:27:20, Michael Stack wrote: > src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java, line 418 > < https://reviews.apache.org/r/4883/diff/1/?file=104442#file104442line418 > > > +1 on suggested change done. On 2012-04-27 23:27:20, Michael Stack wrote: > src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java, line 2835 > < https://reviews.apache.org/r/4883/diff/1/?file=104442#file104442line2835 > > > Is this flag needed? Why not just check thread is alive? I see we can return with an error. What happens if the return on 2816 happens? Will the wait at #643 above be for ever? This is not a thread but actually fed to an executor (thread pool) at line 637. If the return happens on 2816, this is in a finally which will always mark the workitem as done. There are two other instances of this pattern that were originally in this code before I got to it – I'd have used Futures (and have filed a follow on issue for it) but it works. jmhsieh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4883/#review7337 ----------------------------------------------------------- On 2012-04-26 01:42:01, jmhsieh wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4883/ ----------------------------------------------------------- (Updated 2012-04-26 01:42:01) Review request for hbase, Ted Yu and Jimmy Xiang. Summary ------- * Parallelized load of .regioninfo files * changed TreeMap to SortedMap in method signatures * renamed a test's name. This addresses bug HBASE-5712 . https://issues.apache.org/jira/browse/HBASE-5712 Diffs ----- src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 66156c2 src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java 6b64f10 Diff: https://reviews.apache.org/r/4883/diff Testing ------- Ran patch 10x on trunk, passes. Ran 1x on 0.92 and 0.94. Ther 0.90 version that is nearly identical except for ignoring changes near lines HBaseFsck lines 671-680. Thanks, jmhsieh
          Hide
          Jonathan Hsieh added a comment -

          v2 is the versions I committed.

          Show
          Jonathan Hsieh added a comment - v2 is the versions I committed.
          Hide
          Jonathan Hsieh added a comment -

          Committed to 0.90/0.92/0.94/0.96-trunk

          Show
          Jonathan Hsieh added a comment - Committed to 0.90/0.92/0.94/0.96-trunk
          Hide
          jiraposter@reviews.apache.org added a comment -

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

          Ship it!

          +1 Minor comments below if interested

          src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java
          <https://reviews.apache.org/r/4883/#comment16190>

          This'll work but why not ConcurrentSkipListMap?

          src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java
          <https://reviews.apache.org/r/4883/#comment16191>

          +1 on suggested change

          src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java
          <https://reviews.apache.org/r/4883/#comment16192>

          Is this flag needed? Why not just check thread is alive? I see we can return with an error. What happens if the return on 2816 happens? Will the wait at #643 above be for ever?

          • Michael

          On 2012-04-26 01:42:01, jmhsieh wrote:

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

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

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

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

          (Updated 2012-04-26 01:42:01)

          Review request for hbase, Ted Yu and Jimmy Xiang.

          Summary

          -------

          * Parallelized load of .regioninfo files

          * changed TreeMap to SortedMap in method signatures

          * renamed a test's name.

          This addresses bug HBASE-5712.

          https://issues.apache.org/jira/browse/HBASE-5712

          Diffs

          -----

          src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 66156c2

          src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java 6b64f10

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

          Testing

          -------

          Ran patch 10x on trunk, passes. Ran 1x on 0.92 and 0.94.

          Ther 0.90 version that is nearly identical except for ignoring changes near lines HBaseFsck lines 671-680.

          Thanks,

          jmhsieh

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4883/#review7337 ----------------------------------------------------------- Ship it! +1 Minor comments below if interested src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java < https://reviews.apache.org/r/4883/#comment16190 > This'll work but why not ConcurrentSkipListMap? src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java < https://reviews.apache.org/r/4883/#comment16191 > +1 on suggested change src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java < https://reviews.apache.org/r/4883/#comment16192 > Is this flag needed? Why not just check thread is alive? I see we can return with an error. What happens if the return on 2816 happens? Will the wait at #643 above be for ever? Michael On 2012-04-26 01:42:01, jmhsieh wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4883/ ----------------------------------------------------------- (Updated 2012-04-26 01:42:01) Review request for hbase, Ted Yu and Jimmy Xiang. Summary ------- * Parallelized load of .regioninfo files * changed TreeMap to SortedMap in method signatures * renamed a test's name. This addresses bug HBASE-5712 . https://issues.apache.org/jira/browse/HBASE-5712 Diffs ----- src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 66156c2 src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java 6b64f10 Diff: https://reviews.apache.org/r/4883/diff Testing ------- Ran patch 10x on trunk, passes. Ran 1x on 0.92 and 0.94. Ther 0.90 version that is nearly identical except for ignoring changes near lines HBaseFsck lines 671-680. Thanks, jmhsieh
          Hide
          Ted Yu added a comment -

          The new sentence is better.
          +1.

          Show
          Ted Yu added a comment - The new sentence is better. +1.
          Hide
          jiraposter@reviews.apache.org added a comment -

          On 2012-04-27 21:33:19, Ted Yu wrote:

          > src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java, line 418

          > <https://reviews.apache.org/r/4883/diff/1/?file=104442#file104442line418>

          >

          > What does 'Adopting' mean ?

          > The grammar doesn't seem to be right.

          "Adopting" is a term that I use for taking an "orphaned" hdfs region dir (a region in hdfs missing .regioninfo file, and likely not in META/deployed) and adding it to meta, adding a .regioninfo file, and potentially deploying the region.

          How about I change it to this?
          "Attempt to adopt orphan region skipped because no files present in " ...

          • jmhsieh

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

          On 2012-04-26 01:42:01, jmhsieh wrote:

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

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

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

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

          (Updated 2012-04-26 01:42:01)

          Review request for hbase, Ted Yu and Jimmy Xiang.

          Summary

          -------

          * Parallelized load of .regioninfo files

          * changed TreeMap to SortedMap in method signatures

          * renamed a test's name.

          This addresses bug HBASE-5712.

          https://issues.apache.org/jira/browse/HBASE-5712

          Diffs

          -----

          src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 66156c2

          src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java 6b64f10

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

          Testing

          -------

          Ran patch 10x on trunk, passes. Ran 1x on 0.92 and 0.94.

          Ther 0.90 version that is nearly identical except for ignoring changes near lines HBaseFsck lines 671-680.

          Thanks,

          jmhsieh

          Show
          jiraposter@reviews.apache.org added a comment - On 2012-04-27 21:33:19, Ted Yu wrote: > src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java, line 418 > < https://reviews.apache.org/r/4883/diff/1/?file=104442#file104442line418 > > > What does 'Adopting' mean ? > The grammar doesn't seem to be right. "Adopting" is a term that I use for taking an "orphaned" hdfs region dir (a region in hdfs missing .regioninfo file, and likely not in META/deployed) and adding it to meta, adding a .regioninfo file, and potentially deploying the region. How about I change it to this? "Attempt to adopt orphan region skipped because no files present in " ... jmhsieh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4883/#review7325 ----------------------------------------------------------- On 2012-04-26 01:42:01, jmhsieh wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4883/ ----------------------------------------------------------- (Updated 2012-04-26 01:42:01) Review request for hbase, Ted Yu and Jimmy Xiang. Summary ------- * Parallelized load of .regioninfo files * changed TreeMap to SortedMap in method signatures * renamed a test's name. This addresses bug HBASE-5712 . https://issues.apache.org/jira/browse/HBASE-5712 Diffs ----- src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 66156c2 src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java 6b64f10 Diff: https://reviews.apache.org/r/4883/diff Testing ------- Ran patch 10x on trunk, passes. Ran 1x on 0.92 and 0.94. Ther 0.90 version that is nearly identical except for ignoring changes near lines HBaseFsck lines 671-680. Thanks, jmhsieh
          Hide
          jiraposter@reviews.apache.org added a comment -

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

          src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java
          <https://reviews.apache.org/r/4883/#comment16179>

          What does 'Adopting' mean ?
          The grammar doesn't seem to be right.

          • Ted

          On 2012-04-26 01:42:01, jmhsieh wrote:

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

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

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

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

          (Updated 2012-04-26 01:42:01)

          Review request for hbase, Ted Yu and Jimmy Xiang.

          Summary

          -------

          * Parallelized load of .regioninfo files

          * changed TreeMap to SortedMap in method signatures

          * renamed a test's name.

          This addresses bug HBASE-5712.

          https://issues.apache.org/jira/browse/HBASE-5712

          Diffs

          -----

          src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 66156c2

          src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java 6b64f10

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

          Testing

          -------

          Ran patch 10x on trunk, passes. Ran 1x on 0.92 and 0.94.

          Ther 0.90 version that is nearly identical except for ignoring changes near lines HBaseFsck lines 671-680.

          Thanks,

          jmhsieh

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4883/#review7325 ----------------------------------------------------------- src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java < https://reviews.apache.org/r/4883/#comment16179 > What does 'Adopting' mean ? The grammar doesn't seem to be right. Ted On 2012-04-26 01:42:01, jmhsieh wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4883/ ----------------------------------------------------------- (Updated 2012-04-26 01:42:01) Review request for hbase, Ted Yu and Jimmy Xiang. Summary ------- * Parallelized load of .regioninfo files * changed TreeMap to SortedMap in method signatures * renamed a test's name. This addresses bug HBASE-5712 . https://issues.apache.org/jira/browse/HBASE-5712 Diffs ----- src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 66156c2 src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java 6b64f10 Diff: https://reviews.apache.org/r/4883/diff Testing ------- Ran patch 10x on trunk, passes. Ran 1x on 0.92 and 0.94. Ther 0.90 version that is nearly identical except for ignoring changes near lines HBaseFsck lines 671-680. Thanks, jmhsieh
          Hide
          jiraposter@reviews.apache.org added a comment -

          On 2012-04-26 02:25:57, Jimmy Xiang wrote:

          > src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java, line 644

          > <https://reviews.apache.org/r/4883/diff/1/?file=104442#file104442line644>

          >

          > This is ok. However, I think it is better to use a timed wait in case it hangs.

          There is another section of code that uses the same logic and has been around for a while. I'll file a follow on issue to convert this homebrew fork join stuff in to a Futures.

          • jmhsieh

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

          On 2012-04-26 01:42:01, jmhsieh wrote:

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

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

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

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

          (Updated 2012-04-26 01:42:01)

          Review request for hbase, Ted Yu and Jimmy Xiang.

          Summary

          -------

          * Parallelized load of .regioninfo files

          * changed TreeMap to SortedMap in method signatures

          * renamed a test's name.

          This addresses bug HBASE-5712.

          https://issues.apache.org/jira/browse/HBASE-5712

          Diffs

          -----

          src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 66156c2

          src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java 6b64f10

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

          Testing

          -------

          Ran patch 10x on trunk, passes. Ran 1x on 0.92 and 0.94.

          Ther 0.90 version that is nearly identical except for ignoring changes near lines HBaseFsck lines 671-680.

          Thanks,

          jmhsieh

          Show
          jiraposter@reviews.apache.org added a comment - On 2012-04-26 02:25:57, Jimmy Xiang wrote: > src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java, line 644 > < https://reviews.apache.org/r/4883/diff/1/?file=104442#file104442line644 > > > This is ok. However, I think it is better to use a timed wait in case it hangs. There is another section of code that uses the same logic and has been around for a while. I'll file a follow on issue to convert this homebrew fork join stuff in to a Futures. jmhsieh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4883/#review7248 ----------------------------------------------------------- On 2012-04-26 01:42:01, jmhsieh wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4883/ ----------------------------------------------------------- (Updated 2012-04-26 01:42:01) Review request for hbase, Ted Yu and Jimmy Xiang. Summary ------- * Parallelized load of .regioninfo files * changed TreeMap to SortedMap in method signatures * renamed a test's name. This addresses bug HBASE-5712 . https://issues.apache.org/jira/browse/HBASE-5712 Diffs ----- src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 66156c2 src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java 6b64f10 Diff: https://reviews.apache.org/r/4883/diff Testing ------- Ran patch 10x on trunk, passes. Ran 1x on 0.92 and 0.94. Ther 0.90 version that is nearly identical except for ignoring changes near lines HBaseFsck lines 671-680. Thanks, jmhsieh
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12524377/hbase-5712.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 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 appears to introduce 4 new Findbugs (version 1.3.9) warnings.

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

          -1 core tests. The patch failed these unit tests:
          org.apache.hadoop.hbase.master.TestSplitLogManager

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1653//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1653//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1653//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/12524377/hbase-5712.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 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 appears to introduce 4 new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests: org.apache.hadoop.hbase.master.TestSplitLogManager Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1653//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1653//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1653//console This message is automatically generated.
          Hide
          jiraposter@reviews.apache.org added a comment -

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

          Looks very good.

          src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java
          <https://reviews.apache.org/r/4883/#comment16018>

          This is ok. However, I think it is better to use a timed wait in case it hangs.

          • Jimmy

          On 2012-04-26 01:42:01, jmhsieh wrote:

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

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

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

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

          (Updated 2012-04-26 01:42:01)

          Review request for hbase, Ted Yu and Jimmy Xiang.

          Summary

          -------

          * Parallelized load of .regioninfo files

          * changed TreeMap to SortedMap in method signatures

          * renamed a test's name.

          This addresses bug HBASE-5712.

          https://issues.apache.org/jira/browse/HBASE-5712

          Diffs

          -----

          src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 66156c2

          src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java 6b64f10

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

          Testing

          -------

          Ran patch 10x on trunk, passes. Ran 1x on 0.92 and 0.94.

          Ther 0.90 version that is nearly identical except for ignoring changes near lines HBaseFsck lines 671-680.

          Thanks,

          jmhsieh

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4883/#review7248 ----------------------------------------------------------- Looks very good. src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java < https://reviews.apache.org/r/4883/#comment16018 > This is ok. However, I think it is better to use a timed wait in case it hangs. Jimmy On 2012-04-26 01:42:01, jmhsieh wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4883/ ----------------------------------------------------------- (Updated 2012-04-26 01:42:01) Review request for hbase, Ted Yu and Jimmy Xiang. Summary ------- * Parallelized load of .regioninfo files * changed TreeMap to SortedMap in method signatures * renamed a test's name. This addresses bug HBASE-5712 . https://issues.apache.org/jira/browse/HBASE-5712 Diffs ----- src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 66156c2 src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java 6b64f10 Diff: https://reviews.apache.org/r/4883/diff Testing ------- Ran patch 10x on trunk, passes. Ran 1x on 0.92 and 0.94. Ther 0.90 version that is nearly identical except for ignoring changes near lines HBaseFsck lines 671-680. Thanks, jmhsieh
          Hide
          jiraposter@reviews.apache.org added a comment -

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

          Review request for hbase, Ted Yu and Jimmy Xiang.

          Summary
          -------

          • Parallelized load of .regioninfo files
          • changed TreeMap to SortedMap in method signatures
          • renamed a test's name.

          This addresses bug HBASE-5712.
          https://issues.apache.org/jira/browse/HBASE-5712

          Diffs


          src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 66156c2
          src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java 6b64f10

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

          Testing
          -------

          Ran patch 10x on trunk, passes. Ran 1x on 0.92 and 0.94.

          Ther 0.90 version that is nearly identical except for ignoring changes near lines HBaseFsck lines 671-680.

          Thanks,

          jmhsieh

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4883/ ----------------------------------------------------------- Review request for hbase, Ted Yu and Jimmy Xiang. Summary ------- Parallelized load of .regioninfo files changed TreeMap to SortedMap in method signatures renamed a test's name. This addresses bug HBASE-5712 . https://issues.apache.org/jira/browse/HBASE-5712 Diffs src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 66156c2 src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java 6b64f10 Diff: https://reviews.apache.org/r/4883/diff Testing ------- Ran patch 10x on trunk, passes. Ran 1x on 0.92 and 0.94. Ther 0.90 version that is nearly identical except for ignoring changes near lines HBaseFsck lines 671-680. Thanks, jmhsieh

            People

            • Assignee:
              Jonathan Hsieh
              Reporter:
              Jonathan Hsieh
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development