HBase
  1. HBase
  2. HBASE-4269

Add tests and restore semantics to TableInputFormat/TableRecordReader

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.90.5, 0.92.0
    • Fix Version/s: 0.90.5
    • Component/s: mapreduce, test
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      HBASE-4196 Modified the semantics of failures in TableImportFormat/TableRecordReader, and had no tests cases. This patch restores semantics to rethrow when a DoNotRetryIOException is triggered and adds test cases.

        Issue Links

          Activity

          Hide
          jiraposter@reviews.apache.org added a comment -

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

          Review request for hbase, Ted Yu and Ming Ma.

          Summary
          -------

          commit d2bbee8696d54245b046158bacb5ab1a210e5df6
          Author: Jonathan Hsieh <jon@cloudera.com>
          Date: Sun Aug 28 19:10:57 2011 -0700

          HBASE-4269: Add tests and restore semantics to TableImportFormat/TableRecordReader

          Adding tests cases and restoring sematics from HBASE-4196 patch.

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

          Diffs


          src/main/java/org/apache/hadoop/hbase/mapred/TableRecordReaderImpl.java 54df2cf
          src/main/java/org/apache/hadoop/hbase/mapreduce/TableRecordReaderImpl.java c5fc496
          src/test/java/org/apache/hadoop/hbase/mapred/TestTableInputFormat.java PRE-CREATION

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

          Testing
          -------

          New unit tests pass, running other tests.

          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/1675/ ----------------------------------------------------------- Review request for hbase, Ted Yu and Ming Ma. Summary ------- commit d2bbee8696d54245b046158bacb5ab1a210e5df6 Author: Jonathan Hsieh <jon@cloudera.com> Date: Sun Aug 28 19:10:57 2011 -0700 HBASE-4269 : Add tests and restore semantics to TableImportFormat/TableRecordReader Adding tests cases and restoring sematics from HBASE-4196 patch. This addresses bug HBASE-4269 . https://issues.apache.org/jira/browse/HBASE-4269 Diffs src/main/java/org/apache/hadoop/hbase/mapred/TableRecordReaderImpl.java 54df2cf src/main/java/org/apache/hadoop/hbase/mapreduce/TableRecordReaderImpl.java c5fc496 src/test/java/org/apache/hadoop/hbase/mapred/TestTableInputFormat.java PRE-CREATION Diff: https://reviews.apache.org/r/1675/diff Testing ------- New unit tests pass, running other tests. 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/1675/#review1675
          -----------------------------------------------------------

          Nice work.

          src/test/java/org/apache/hadoop/hbase/mapred/TestTableInputFormat.java
          <https://reviews.apache.org/r/1675/#comment3823>

          Please add javadoc stating that this test covers org.apache.hadoop.hbase.mapred namespace.

          src/test/java/org/apache/hadoop/hbase/mapred/TestTableInputFormat.java
          <https://reviews.apache.org/r/1675/#comment3826>

          I think calling implementation class' ctor is better, following example of runTestMapreduce().

          src/test/java/org/apache/hadoop/hbase/mapred/TestTableInputFormat.java
          <https://reviews.apache.org/r/1675/#comment3824>

          Please add javadoc stating that this test covers org.apache.hadoop.hbase.mapreduce namespace.

          src/test/java/org/apache/hadoop/hbase/mapred/TestTableInputFormat.java
          <https://reviews.apache.org/r/1675/#comment3825>

          Please wrap long line.

          • Ted

          On 2011-08-29 04:20:16, jmhsieh wrote:

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

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

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

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

          (Updated 2011-08-29 04:20:16)

          Review request for hbase, Ted Yu and Ming Ma.

          Summary

          -------

          commit d2bbee8696d54245b046158bacb5ab1a210e5df6

          Author: Jonathan Hsieh <jon@cloudera.com>

          Date: Sun Aug 28 19:10:57 2011 -0700

          HBASE-4269: Add tests and restore semantics to TableImportFormat/TableRecordReader

          Adding tests cases and restoring sematics from HBASE-4196 patch.

          This addresses bug HBASE-4269.

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

          Diffs

          -----

          src/main/java/org/apache/hadoop/hbase/mapred/TableRecordReaderImpl.java 54df2cf

          src/main/java/org/apache/hadoop/hbase/mapreduce/TableRecordReaderImpl.java c5fc496

          src/test/java/org/apache/hadoop/hbase/mapred/TestTableInputFormat.java PRE-CREATION

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

          Testing

          -------

          New unit tests pass, running other tests.

          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/1675/#review1675 ----------------------------------------------------------- Nice work. src/test/java/org/apache/hadoop/hbase/mapred/TestTableInputFormat.java < https://reviews.apache.org/r/1675/#comment3823 > Please add javadoc stating that this test covers org.apache.hadoop.hbase.mapred namespace. src/test/java/org/apache/hadoop/hbase/mapred/TestTableInputFormat.java < https://reviews.apache.org/r/1675/#comment3826 > I think calling implementation class' ctor is better, following example of runTestMapreduce(). src/test/java/org/apache/hadoop/hbase/mapred/TestTableInputFormat.java < https://reviews.apache.org/r/1675/#comment3824 > Please add javadoc stating that this test covers org.apache.hadoop.hbase.mapreduce namespace. src/test/java/org/apache/hadoop/hbase/mapred/TestTableInputFormat.java < https://reviews.apache.org/r/1675/#comment3825 > Please wrap long line. Ted On 2011-08-29 04:20:16, jmhsieh wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1675/ ----------------------------------------------------------- (Updated 2011-08-29 04:20:16) Review request for hbase, Ted Yu and Ming Ma. Summary ------- commit d2bbee8696d54245b046158bacb5ab1a210e5df6 Author: Jonathan Hsieh <jon@cloudera.com> Date: Sun Aug 28 19:10:57 2011 -0700 HBASE-4269 : Add tests and restore semantics to TableImportFormat/TableRecordReader Adding tests cases and restoring sematics from HBASE-4196 patch. This addresses bug HBASE-4269 . https://issues.apache.org/jira/browse/HBASE-4269 Diffs ----- src/main/java/org/apache/hadoop/hbase/mapred/TableRecordReaderImpl.java 54df2cf src/main/java/org/apache/hadoop/hbase/mapreduce/TableRecordReaderImpl.java c5fc496 src/test/java/org/apache/hadoop/hbase/mapred/TestTableInputFormat.java PRE-CREATION Diff: https://reviews.apache.org/r/1675/diff Testing ------- New unit tests pass, running other tests. 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/1675/
          -----------------------------------------------------------

          (Updated 2011-08-29 12:10:51.509870)

          Review request for hbase, Ted Yu and Ming Ma.

          Changes
          -------

          Addressed comments

          Summary (updated)
          -------

          commit d2bbee8696d54245b046158bacb5ab1a210e5df6
          Author: Jonathan Hsieh <jon@cloudera.com>
          Date: Sun Aug 28 19:10:57 2011 -0700

          HBASE-4269: Add tests and restore semantics to TableInputFormat/TableRecordReader

          Adding tests cases and restoring sematics from HBASE-4196 patch.

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

          Diffs (updated)


          src/main/java/org/apache/hadoop/hbase/mapreduce/TableRecordReaderImpl.java c5fc496
          src/test/java/org/apache/hadoop/hbase/mapred/TestTableInputFormat.java PRE-CREATION
          src/main/java/org/apache/hadoop/hbase/mapred/TableRecordReaderImpl.java 54df2cf

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

          Testing
          -------

          New unit tests pass, running other tests.

          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/1675/ ----------------------------------------------------------- (Updated 2011-08-29 12:10:51.509870) Review request for hbase, Ted Yu and Ming Ma. Changes ------- Addressed comments Summary (updated) ------- commit d2bbee8696d54245b046158bacb5ab1a210e5df6 Author: Jonathan Hsieh <jon@cloudera.com> Date: Sun Aug 28 19:10:57 2011 -0700 HBASE-4269 : Add tests and restore semantics to TableInputFormat/TableRecordReader Adding tests cases and restoring sematics from HBASE-4196 patch. This addresses bug HBASE-4269 . https://issues.apache.org/jira/browse/HBASE-4269 Diffs (updated) src/main/java/org/apache/hadoop/hbase/mapreduce/TableRecordReaderImpl.java c5fc496 src/test/java/org/apache/hadoop/hbase/mapred/TestTableInputFormat.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/mapred/TableRecordReaderImpl.java 54df2cf Diff: https://reviews.apache.org/r/1675/diff Testing ------- New unit tests pass, running other tests. Thanks, jmhsieh
          Hide
          jiraposter@reviews.apache.org added a comment -

          On 2011-08-29 06:54:46, Ted Yu wrote:

          > src/test/java/org/apache/hadoop/hbase/mapred/TestTableInputFormat.java, line 107

          > <https://reviews.apache.org/r/1675/diff/1/?file=36176#file36176line107>

          >

          > Please add javadoc stating that this test covers org.apache.hadoop.hbase.mapred namespace.

          done

          On 2011-08-29 06:54:46, Ted Yu wrote:

          > src/test/java/org/apache/hadoop/hbase/mapred/TestTableInputFormat.java, line 137

          > <https://reviews.apache.org/r/1675/diff/1/?file=36176#file36176line137>

          >

          > Please add javadoc stating that this test covers org.apache.hadoop.hbase.mapreduce namespace.

          done

          On 2011-08-29 06:54:46, Ted Yu wrote:

          > src/test/java/org/apache/hadoop/hbase/mapred/TestTableInputFormat.java, line 145

          > <https://reviews.apache.org/r/1675/diff/1/?file=36176#file36176line145>

          >

          > Please wrap long line.

          done

          On 2011-08-29 06:54:46, Ted Yu wrote:

          > src/test/java/org/apache/hadoop/hbase/mapred/TestTableInputFormat.java, line 113

          > <https://reviews.apache.org/r/1675/diff/1/?file=36176#file36176line113>

          >

          > I think calling implementation class' ctor is better, following example of runTestMapreduce().

          i think you mean spelling out the class completely on the constructor like this right?

          org.apache.hadoop.hbase.mapred.TableRecordReader trr =
          new org.apache.hadoop.hbase.mapred.TableRecordReader();

          did you mean more than that?

          • jmhsieh

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

          On 2011-08-29 12:10:51, jmhsieh wrote:

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

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

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

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

          (Updated 2011-08-29 12:10:51)

          Review request for hbase, Ted Yu and Ming Ma.

          Summary

          -------

          commit d2bbee8696d54245b046158bacb5ab1a210e5df6

          Author: Jonathan Hsieh <jon@cloudera.com>

          Date: Sun Aug 28 19:10:57 2011 -0700

          HBASE-4269: Add tests and restore semantics to TableInputFormat/TableRecordReader

          Adding tests cases and restoring sematics from HBASE-4196 patch.

          This addresses bug HBASE-4269.

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

          Diffs

          -----

          src/main/java/org/apache/hadoop/hbase/mapreduce/TableRecordReaderImpl.java c5fc496

          src/test/java/org/apache/hadoop/hbase/mapred/TestTableInputFormat.java PRE-CREATION

          src/main/java/org/apache/hadoop/hbase/mapred/TableRecordReaderImpl.java 54df2cf

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

          Testing

          -------

          New unit tests pass, running other tests.

          Thanks,

          jmhsieh

          Show
          jiraposter@reviews.apache.org added a comment - On 2011-08-29 06:54:46, Ted Yu wrote: > src/test/java/org/apache/hadoop/hbase/mapred/TestTableInputFormat.java, line 107 > < https://reviews.apache.org/r/1675/diff/1/?file=36176#file36176line107 > > > Please add javadoc stating that this test covers org.apache.hadoop.hbase.mapred namespace. done On 2011-08-29 06:54:46, Ted Yu wrote: > src/test/java/org/apache/hadoop/hbase/mapred/TestTableInputFormat.java, line 137 > < https://reviews.apache.org/r/1675/diff/1/?file=36176#file36176line137 > > > Please add javadoc stating that this test covers org.apache.hadoop.hbase.mapreduce namespace. done On 2011-08-29 06:54:46, Ted Yu wrote: > src/test/java/org/apache/hadoop/hbase/mapred/TestTableInputFormat.java, line 145 > < https://reviews.apache.org/r/1675/diff/1/?file=36176#file36176line145 > > > Please wrap long line. done On 2011-08-29 06:54:46, Ted Yu wrote: > src/test/java/org/apache/hadoop/hbase/mapred/TestTableInputFormat.java, line 113 > < https://reviews.apache.org/r/1675/diff/1/?file=36176#file36176line113 > > > I think calling implementation class' ctor is better, following example of runTestMapreduce(). i think you mean spelling out the class completely on the constructor like this right? org.apache.hadoop.hbase.mapred.TableRecordReader trr = new org.apache.hadoop.hbase.mapred.TableRecordReader(); did you mean more than that? jmhsieh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1675/#review1675 ----------------------------------------------------------- On 2011-08-29 12:10:51, jmhsieh wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1675/ ----------------------------------------------------------- (Updated 2011-08-29 12:10:51) Review request for hbase, Ted Yu and Ming Ma. Summary ------- commit d2bbee8696d54245b046158bacb5ab1a210e5df6 Author: Jonathan Hsieh <jon@cloudera.com> Date: Sun Aug 28 19:10:57 2011 -0700 HBASE-4269 : Add tests and restore semantics to TableInputFormat/TableRecordReader Adding tests cases and restoring sematics from HBASE-4196 patch. This addresses bug HBASE-4269 . https://issues.apache.org/jira/browse/HBASE-4269 Diffs ----- src/main/java/org/apache/hadoop/hbase/mapreduce/TableRecordReaderImpl.java c5fc496 src/test/java/org/apache/hadoop/hbase/mapred/TestTableInputFormat.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/mapred/TableRecordReaderImpl.java 54df2cf Diff: https://reviews.apache.org/r/1675/diff Testing ------- New unit tests pass, running other tests. Thanks, jmhsieh
          Hide
          Jonathan Hsieh added a comment -

          Updated patch to address review comments.

          Show
          Jonathan Hsieh added a comment - Updated patch to address review comments.
          Hide
          jiraposter@reviews.apache.org added a comment -

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

          Ship it!

          • Ted

          On 2011-08-29 12:10:51, jmhsieh wrote:

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

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

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

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

          (Updated 2011-08-29 12:10:51)

          Review request for hbase, Ted Yu and Ming Ma.

          Summary

          -------

          commit d2bbee8696d54245b046158bacb5ab1a210e5df6

          Author: Jonathan Hsieh <jon@cloudera.com>

          Date: Sun Aug 28 19:10:57 2011 -0700

          HBASE-4269: Add tests and restore semantics to TableInputFormat/TableRecordReader

          Adding tests cases and restoring sematics from HBASE-4196 patch.

          This addresses bug HBASE-4269.

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

          Diffs

          -----

          src/main/java/org/apache/hadoop/hbase/mapreduce/TableRecordReaderImpl.java c5fc496

          src/test/java/org/apache/hadoop/hbase/mapred/TestTableInputFormat.java PRE-CREATION

          src/main/java/org/apache/hadoop/hbase/mapred/TableRecordReaderImpl.java 54df2cf

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

          Testing

          -------

          New unit tests pass, running other tests.

          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/1675/#review1678 ----------------------------------------------------------- Ship it! Ted On 2011-08-29 12:10:51, jmhsieh wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1675/ ----------------------------------------------------------- (Updated 2011-08-29 12:10:51) Review request for hbase, Ted Yu and Ming Ma. Summary ------- commit d2bbee8696d54245b046158bacb5ab1a210e5df6 Author: Jonathan Hsieh <jon@cloudera.com> Date: Sun Aug 28 19:10:57 2011 -0700 HBASE-4269 : Add tests and restore semantics to TableInputFormat/TableRecordReader Adding tests cases and restoring sematics from HBASE-4196 patch. This addresses bug HBASE-4269 . https://issues.apache.org/jira/browse/HBASE-4269 Diffs ----- src/main/java/org/apache/hadoop/hbase/mapreduce/TableRecordReaderImpl.java c5fc496 src/test/java/org/apache/hadoop/hbase/mapred/TestTableInputFormat.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/mapred/TableRecordReaderImpl.java 54df2cf Diff: https://reviews.apache.org/r/1675/diff Testing ------- New unit tests pass, running other tests. 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/1675/#review1682
          -----------------------------------------------------------

          Ship it!

          +1

          nit: test is excellent but looks like could have gone into TestTableInputFormatScan?

          Lets commit.

          • Michael

          On 2011-08-29 12:10:51, jmhsieh wrote:

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

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

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

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

          (Updated 2011-08-29 12:10:51)

          Review request for hbase, Ted Yu and Ming Ma.

          Summary

          -------

          commit d2bbee8696d54245b046158bacb5ab1a210e5df6

          Author: Jonathan Hsieh <jon@cloudera.com>

          Date: Sun Aug 28 19:10:57 2011 -0700

          HBASE-4269: Add tests and restore semantics to TableInputFormat/TableRecordReader

          Adding tests cases and restoring sematics from HBASE-4196 patch.

          This addresses bug HBASE-4269.

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

          Diffs

          -----

          src/main/java/org/apache/hadoop/hbase/mapreduce/TableRecordReaderImpl.java c5fc496

          src/test/java/org/apache/hadoop/hbase/mapred/TestTableInputFormat.java PRE-CREATION

          src/main/java/org/apache/hadoop/hbase/mapred/TableRecordReaderImpl.java 54df2cf

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

          Testing

          -------

          New unit tests pass, running other tests.

          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/1675/#review1682 ----------------------------------------------------------- Ship it! +1 nit: test is excellent but looks like could have gone into TestTableInputFormatScan? Lets commit. Michael On 2011-08-29 12:10:51, jmhsieh wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1675/ ----------------------------------------------------------- (Updated 2011-08-29 12:10:51) Review request for hbase, Ted Yu and Ming Ma. Summary ------- commit d2bbee8696d54245b046158bacb5ab1a210e5df6 Author: Jonathan Hsieh <jon@cloudera.com> Date: Sun Aug 28 19:10:57 2011 -0700 HBASE-4269 : Add tests and restore semantics to TableInputFormat/TableRecordReader Adding tests cases and restoring sematics from HBASE-4196 patch. This addresses bug HBASE-4269 . https://issues.apache.org/jira/browse/HBASE-4269 Diffs ----- src/main/java/org/apache/hadoop/hbase/mapreduce/TableRecordReaderImpl.java c5fc496 src/test/java/org/apache/hadoop/hbase/mapred/TestTableInputFormat.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/mapred/TableRecordReaderImpl.java 54df2cf Diff: https://reviews.apache.org/r/1675/diff Testing ------- New unit tests pass, running other tests. Thanks, jmhsieh
          Hide
          Ted Yu added a comment -

          Would src/test/java/org/apache/hadoop/hbase/ be a better place to host TestTableInputFormat ?
          The reason being that it tests both mapred and mapreduce input formats.

          Show
          Ted Yu added a comment - Would src/test/java/org/apache/hadoop/hbase/ be a better place to host TestTableInputFormat ? The reason being that it tests both mapred and mapreduce input formats.
          Hide
          Jonathan Hsieh added a comment -

          @Ted Most of the MR tests seem to live in o.a.h.hbase.mapreduce instead of mapred. Would it make more sense to move it there? IMO it just doesn't feel like it should be in o.a.h.hbase.

          (yeahm, I started working on the mapred package and then added it the mapreduce tests afterwards.)

          Show
          Jonathan Hsieh added a comment - @Ted Most of the MR tests seem to live in o.a.h.hbase.mapreduce instead of mapred. Would it make more sense to move it there? IMO it just doesn't feel like it should be in o.a.h.hbase. (yeahm, I started working on the mapred package and then added it the mapreduce tests afterwards.)
          Hide
          Jonathan Hsieh added a comment -

          @Stack: The test you mention sets up a lot more stuff (and takes a long time to run) so I opted not to. Maybe rename the test class to TestTableInputFormatScanFailures?

          Show
          Jonathan Hsieh added a comment - @Stack: The test you mention sets up a lot more stuff (and takes a long time to run) so I opted not to. Maybe rename the test class to TestTableInputFormatScanFailures?
          Hide
          stack added a comment -

          @Jon Don't worry about it; lets commit (I wanted to amorticize cluster startup – thats what takes the time in my experience)

          Show
          stack added a comment - @Jon Don't worry about it; lets commit (I wanted to amorticize cluster startup – thats what takes the time in my experience)
          Hide
          Ted Yu added a comment -

          We can keep the new test in o.a.h.hbase.mapred

          Show
          Ted Yu added a comment - We can keep the new test in o.a.h.hbase.mapred
          Hide
          Ted Yu added a comment -

          Integrated to branch and TRUNK.

          Thanks for the patch Jonathan.
          Thanks for the review Michael.

          Show
          Ted Yu added a comment - Integrated to branch and TRUNK. Thanks for the patch Jonathan. Thanks for the review Michael.
          Hide
          Hudson added a comment -

          Integrated in HBase-TRUNK #2159 (See https://builds.apache.org/job/HBase-TRUNK/2159/)
          HBASE-4269 Add tests and restore semantics to TableInputFormat/TableRecordReader
          (Jonathan Hsieh)

          tedyu :
          Files :

          • /hbase/trunk/CHANGES.txt
          • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/mapred/TableRecordReaderImpl.java
          • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/mapreduce/TableRecordReaderImpl.java
          • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/mapred/TestTableInputFormat.java
          Show
          Hudson added a comment - Integrated in HBase-TRUNK #2159 (See https://builds.apache.org/job/HBase-TRUNK/2159/ ) HBASE-4269 Add tests and restore semantics to TableInputFormat/TableRecordReader (Jonathan Hsieh) tedyu : Files : /hbase/trunk/CHANGES.txt /hbase/trunk/src/main/java/org/apache/hadoop/hbase/mapred/TableRecordReaderImpl.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/mapreduce/TableRecordReaderImpl.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/mapred/TestTableInputFormat.java
          Hide
          Jan Lukavsky added a comment -

          Hi,

          I think patch to this issue changed semantics for mapreduce API. In HBASE-4196 there was no change in semantics in org.apache.hadoop.hbase.mapreduce.TableRecordReaderImpl, the only change was in org.apache.hadoop.hbase.mapred.TableRecordReaderImpl (where the catch of UnknownScannerException was changed to IOException). Now the semantics of mapreduce API is different of the one before HBASE-4196, and I think this should be reverted. Is there any reason why to have different semantics for the two APIs? Wouldn't it be better to accept the change of semantics in HBASE-4196? Are there any negative side-effects of this change? I don't see any discussion of the type "do we need to change the semantics back"?

          Thanks for reply

          Jan

          Show
          Jan Lukavsky added a comment - Hi, I think patch to this issue changed semantics for mapreduce API. In HBASE-4196 there was no change in semantics in org.apache.hadoop.hbase.mapreduce.TableRecordReaderImpl, the only change was in org.apache.hadoop.hbase.mapred.TableRecordReaderImpl (where the catch of UnknownScannerException was changed to IOException). Now the semantics of mapreduce API is different of the one before HBASE-4196 , and I think this should be reverted. Is there any reason why to have different semantics for the two APIs? Wouldn't it be better to accept the change of semantics in HBASE-4196 ? Are there any negative side-effects of this change? I don't see any discussion of the type "do we need to change the semantics back"? Thanks for reply Jan
          Hide
          Jonathan Hsieh added a comment -

          Jan,

          When I did this patch, the two versions had different error recovery path and I made them similar. UnknownScannerException is a subclass DoNotRetryIOException so I chose that instead.

          I'm assuming this is causing some pain now – how is this affecting the job you are running? (is it catching and rethrowing other exceptions as well?)

          If there is something we need to change I'm fine with that. Let's file a new issue – this patch has been in included in a few releases now.

          Show
          Jonathan Hsieh added a comment - Jan, When I did this patch, the two versions had different error recovery path and I made them similar. UnknownScannerException is a subclass DoNotRetryIOException so I chose that instead. I'm assuming this is causing some pain now – how is this affecting the job you are running? (is it catching and rethrowing other exceptions as well?) If there is something we need to change I'm fine with that. Let's file a new issue – this patch has been in included in a few releases now.
          Hide
          Jan Lukavsky added a comment -

          Hi Jonathan,

          I will file a new issue with description of issues it causes us.

          Thanks.

          Show
          Jan Lukavsky added a comment - Hi Jonathan, I will file a new issue with description of issues it causes us. Thanks.

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development