HBase
  1. HBase
  2. HBASE-10181

HBaseObjectWritable.readObject catches DoNotRetryIOException and wraps it back in a regular IOException

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.94.12
    • Fix Version/s: 0.94.15
    • Component/s: IPC/RPC
    • Labels:
      None
    • Hadoop Flags:
      Reviewed
    • Tags:
      Phoenix

      Description

      Exception handling inside HbaseObjectWritable needs to be reworked, IMHO.

      For example:
      At several places inside HbaseObjectWritable.readObject, exceptions are caught and rethrown as I/O Exception (including ClassNotFoundException!).

      So, if an implementation of readFields method throws a DoNotRetryIOException, HBase still ends up retrying.

      This problem exists at least in 0.94.12 version of HBase.

      1. 10181.txt
        1 kB
        Lars Hofhansl
      2. 10181-alternate.txt
        2 kB
        Lars Hofhansl

        Issue Links

          Activity

          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Open Open Resolved Resolved
          19h 40m 1 Lars Hofhansl 17/Dec/13 19:10
          Resolved Resolved Closed Closed
          12d 9h 1m 1 Lars Hofhansl 30/Dec/13 04:11
          Lars Hofhansl made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Hide
          Hudson added a comment -

          FAILURE: Integrated in HBase-0.94 #1230 (See https://builds.apache.org/job/HBase-0.94/1230/)
          HBASE-10181 HBaseObjectWritable.readObject catches DoNotRetryIOException and wraps it back in a regular IOException (larsh: rev 1551657)

          • /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/io/HbaseObjectWritable.java
          Show
          Hudson added a comment - FAILURE: Integrated in HBase-0.94 #1230 (See https://builds.apache.org/job/HBase-0.94/1230/ ) HBASE-10181 HBaseObjectWritable.readObject catches DoNotRetryIOException and wraps it back in a regular IOException (larsh: rev 1551657) /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/io/HbaseObjectWritable.java
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in HBase-0.94-security #363 (See https://builds.apache.org/job/HBase-0.94-security/363/)
          HBASE-10181 HBaseObjectWritable.readObject catches DoNotRetryIOException and wraps it back in a regular IOException (larsh: rev 1551657)

          • /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/io/HbaseObjectWritable.java
          Show
          Hudson added a comment - SUCCESS: Integrated in HBase-0.94-security #363 (See https://builds.apache.org/job/HBase-0.94-security/363/ ) HBASE-10181 HBaseObjectWritable.readObject catches DoNotRetryIOException and wraps it back in a regular IOException (larsh: rev 1551657) /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/io/HbaseObjectWritable.java
          Lars Hofhansl made changes -
          Status Open [ 1 ] Resolved [ 5 ]
          Hadoop Flags Reviewed [ 10343 ]
          Resolution Fixed [ 1 ]
          Hide
          Lars Hofhansl added a comment -

          Committed to 0.94 only.

          Show
          Lars Hofhansl added a comment - Committed to 0.94 only.
          Lars Hofhansl made changes -
          Assignee Lars Hofhansl [ lhofhansl ]
          Hide
          Lars Hofhansl added a comment -

          TestFromClientSide, TestHbaseObjectWritable, and TestSerialization pass. Going to commit.

          Show
          Lars Hofhansl added a comment - TestFromClientSide, TestHbaseObjectWritable, and TestSerialization pass. Going to commit.
          Hide
          Ted Yu added a comment -

          +1 on 10181-alternate.txt, if tests pass.

          Show
          Ted Yu added a comment - +1 on 10181-alternate.txt, if tests pass.
          Lars Hofhansl made changes -
          Link This issue is related to HBASE-10185 [ HBASE-10185 ]
          Lars Hofhansl made changes -
          Attachment 10181-alternate.txt [ 12619138 ]
          Hide
          Lars Hofhansl added a comment -

          More "radical" change. Seems pointless to retry after a ClassNotFoundException. If the de-serialization encountered an IOException, don't wrap again - this obviously covers the DNRE scenario.

          I'm fine with either changes. Would like to commit today, so that I can roll an RC.

          Show
          Lars Hofhansl added a comment - More "radical" change. Seems pointless to retry after a ClassNotFoundException. If the de-serialization encountered an IOException, don't wrap again - this obviously covers the DNRE scenario. I'm fine with either changes. Would like to commit today, so that I can roll an RC.
          Hide
          Samarth Jain added a comment - - edited

          Filed HBASE-10185 for tracking the retry bug in write(dataoutput).

          Show
          Samarth Jain added a comment - - edited Filed HBASE-10185 for tracking the retry bug in write(dataoutput).
          Hide
          Samarth Jain added a comment -

          Looks good to me, Lars. I also looked into throwing a DoNotRetryIOException inside write(DataOutput) method and looks like there is another bug within HBaseClient which is causing HBase to still retry the write call. I will log another JIRA issue for that.

          Show
          Samarth Jain added a comment - Looks good to me, Lars. I also looked into throwing a DoNotRetryIOException inside write(DataOutput) method and looks like there is another bug within HBaseClient which is causing HBase to still retry the write call. I will log another JIRA issue for that.
          Hide
          Anoop Sam John added a comment -

          I could also handle IOExceptions special and just rethrow them instead of wrapping inside a new IOException

          +1 for doing that also..

          The description tells abt (including ClassNotFoundException).. So you are not doing those changes to make the changes less Lars?
          Yes this change looks to be good enough IMO

          Show
          Anoop Sam John added a comment - I could also handle IOExceptions special and just rethrow them instead of wrapping inside a new IOException +1 for doing that also.. The description tells abt (including ClassNotFoundException).. So you are not doing those changes to make the changes less Lars? Yes this change looks to be good enough IMO
          Lars Hofhansl made changes -
          Attachment 10181.txt [ 12619049 ]
          Hide
          Lars Hofhansl added a comment -

          Is this good enough?
          Special casing DNRE directly should have the least impact on existing code.
          (I could also handle IOExceptions special and just rethrow them instead of wrapping inside a new IOException).

          Show
          Lars Hofhansl added a comment - Is this good enough? Special casing DNRE directly should have the least impact on existing code. (I could also handle IOExceptions special and just rethrow them instead of wrapping inside a new IOException).
          Lars Hofhansl made changes -
          Fix Version/s 0.94.15 [ 12325559 ]
          James Taylor made changes -
          Field Original Value New Value
          Tags Phoenix
          Hide
          Lars Hofhansl added a comment -

          I'll take a look.

          Show
          Lars Hofhansl added a comment - I'll take a look.
          Samarth Jain created issue -

            People

            • Assignee:
              Lars Hofhansl
              Reporter:
              Samarth Jain
            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development