Uploaded image for project: 'Flume'
  1. Flume
  2. FLUME-2897

AsyncHBase sink NPE when Channel.getTransaction() fails

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.6.0
    • Fix Version/s: 1.7.0
    • Component/s: Sinks+Sources
    • Labels:
      None

      Description

      There is a possibility for a NPE in the AsyncHBaseSink when a channel getTransaction() call fails. This is possible when the FileChannel is out of disk space.

      Patch forthcoming.

        Activity

        Hide
        mpercy Mike Percy added a comment -

        The reason this patch is correct is because it would be incorrect to call txn.close() in the finally block if txn is null. Same goes for what we're doing in the catch block: txn.rollback(). This is the correct idiom.

        Show
        mpercy Mike Percy added a comment - The reason this patch is correct is because it would be incorrect to call txn.close() in the finally block if txn is null. Same goes for what we're doing in the catch block: txn.rollback(). This is the correct idiom.
        Hide
        jarcec Jarek Jarcec Cecho added a comment -

        Thanks Mike Percy. I've quickly looked into other Sinks to see how they are handling this and both AvroSink and HdfsSink are using the idiom that you've mentioned. Some of the newer sinks (KafkaSink) are getting the transaction inside the try-catch, but then they are properly guarding the transaction object against being null. This seems a bit simpler, so I'm +1.

        Show
        jarcec Jarek Jarcec Cecho added a comment - Thanks Mike Percy . I've quickly looked into other Sinks to see how they are handling this and both AvroSink and HdfsSink are using the idiom that you've mentioned. Some of the newer sinks ( KafkaSink ) are getting the transaction inside the try-catch , but then they are properly guarding the transaction object against being null . This seems a bit simpler, so I'm +1.
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 5293eba9a418180b42b3138c0c0b5aac38361f7f in flume's branch refs/heads/trunk from Jarek Jarcec Cecho
        [ https://git-wip-us.apache.org/repos/asf?p=flume.git;h=5293eba ]

        FLUME-2897: AsyncHBase sink NPE when Channel.getTransaction() fails

        (Mike Percy via Jarek Jarcec Cecho)

        Show
        jira-bot ASF subversion and git services added a comment - Commit 5293eba9a418180b42b3138c0c0b5aac38361f7f in flume's branch refs/heads/trunk from Jarek Jarcec Cecho [ https://git-wip-us.apache.org/repos/asf?p=flume.git;h=5293eba ] FLUME-2897 : AsyncHBase sink NPE when Channel.getTransaction() fails (Mike Percy via Jarek Jarcec Cecho)
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 9efbe92f6f00e626d34847b4f3f40378fdb6a1f3 in flume's branch refs/heads/flume-1.7 from Jarek Jarcec Cecho
        [ https://git-wip-us.apache.org/repos/asf?p=flume.git;h=9efbe92 ]

        FLUME-2897: AsyncHBase sink NPE when Channel.getTransaction() fails

        (Mike Percy via Jarek Jarcec Cecho)

        Show
        jira-bot ASF subversion and git services added a comment - Commit 9efbe92f6f00e626d34847b4f3f40378fdb6a1f3 in flume's branch refs/heads/flume-1.7 from Jarek Jarcec Cecho [ https://git-wip-us.apache.org/repos/asf?p=flume.git;h=9efbe92 ] FLUME-2897 : AsyncHBase sink NPE when Channel.getTransaction() fails (Mike Percy via Jarek Jarcec Cecho)
        Hide
        jarcec Jarek Jarcec Cecho added a comment -

        Thank you for your contribution Mike Percy!

        Show
        jarcec Jarek Jarcec Cecho added a comment - Thank you for your contribution Mike Percy !
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Flume-trunk-hbase-1 #153 (See https://builds.apache.org/job/Flume-trunk-hbase-1/153/)
        FLUME-2897: AsyncHBase sink NPE when Channel.getTransaction() fails (jarcec: http://git-wip-us.apache.org/repos/asf/flume/repo?p=flume.git&a=commit&h=5293eba9a418180b42b3138c0c0b5aac38361f7f)

        • flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/AsyncHBaseSink.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Flume-trunk-hbase-1 #153 (See https://builds.apache.org/job/Flume-trunk-hbase-1/153/ ) FLUME-2897 : AsyncHBase sink NPE when Channel.getTransaction() fails (jarcec: http://git-wip-us.apache.org/repos/asf/flume/repo?p=flume.git&a=commit&h=5293eba9a418180b42b3138c0c0b5aac38361f7f ) flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/AsyncHBaseSink.java
        Hide
        mpercy Mike Percy added a comment -

        Thanks for the commit Jarek Jarcec Cecho!

        Show
        mpercy Mike Percy added a comment - Thanks for the commit Jarek Jarcec Cecho !
        Hide
        mpercy Mike Percy added a comment -

        Hi Lior Zeno, just wondering... what is the purpose of closing the JIRAs? Seems like needless noise. We don't have a formal QA process in Apache Flume so Resolved == Closed.

        Show
        mpercy Mike Percy added a comment - Hi Lior Zeno , just wondering... what is the purpose of closing the JIRAs? Seems like needless noise. We don't have a formal QA process in Apache Flume so Resolved == Closed.
        Hide
        liorze Lior Zeno added a comment -

        The JIRA workflow is open for interpretations and usually there are many different views, one of them is as you state - testers (or QA) close issues after they verified that the patch satisfies the issue. However, another view is that a closed issue is an issue that has been resolved and there are no further actions that need to be taken. One of the benefits of closing an issue is disallowing comments. Since a resolved issue is committed to the repo, in case there is a bug with the provided patch, a new issue should be created.

        After having a look at the reports, the majority of issues are resolved but not closed, so I guess we will continue in that path. Sorry for the noise.

        Show
        liorze Lior Zeno added a comment - The JIRA workflow is open for interpretations and usually there are many different views, one of them is as you state - testers (or QA) close issues after they verified that the patch satisfies the issue. However, another view is that a closed issue is an issue that has been resolved and there are no further actions that need to be taken. One of the benefits of closing an issue is disallowing comments. Since a resolved issue is committed to the repo, in case there is a bug with the provided patch, a new issue should be created. After having a look at the reports, the majority of issues are resolved but not closed, so I guess we will continue in that path. Sorry for the noise.
        Hide
        mpercy Mike Percy added a comment -

        It's alright. Thanks for your efforts and for working on the 1.7 release.

        Show
        mpercy Mike Percy added a comment - It's alright. Thanks for your efforts and for working on the 1.7 release.

          People

          • Assignee:
            mpercy Mike Percy
            Reporter:
            mpercy Mike Percy
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development