Flume
  1. Flume
  2. FLUME-1075

HDFSEventSink begin is called when transaction opened due to other error

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: v1.1.0
    • Fix Version/s: v1.2.0
    • Component/s: Channel
    • Labels:
      None

      Description

      If you HDFSEvenSink is unable to close a transaction, then the next time process is called, begin is called on that same transaction and another error is thrown.

      2012-03-28 17:47:36,011 (SinkRunner-PollingRunner-DefaultSinkProcessor) [ERROR - org.apache.flume.SinkRunner$PollingRunner.run(SinkRunner.java:165)] Unhandled exception, logging and sleeping for 5000ms
      java.lang.IllegalStateException: close() called when transaction is OPEN - you must either commit or rollback first
      	at com.google.common.base.Preconditions.checkState(Preconditions.java:172)
      	at org.apache.flume.channel.BasicTransactionSemantics.close(BasicTransactionSemantics.java:179)
      	at org.apache.flume.sink.hdfs.HDFSEventSink.process(HDFSEventSink.java:381)
      	at org.apache.flume.sink.DefaultSinkProcessor.process(DefaultSinkProcessor.java:65)
      	at org.apache.flume.SinkRunner$PollingRunner.run(SinkRunner.java:148)
      	at java.lang.Thread.run(Thread.java:662)
      2012-03-28 17:47:41,013 (SinkRunner-PollingRunner-DefaultSinkProcessor) [ERROR - org.apache.flume.sink.hdfs.HDFSEventSink.process(HDFSEventSink.java:374)] process failed
      java.lang.IllegalStateException: begin() called when transaction is OPEN!
      	at com.google.common.base.Preconditions.checkState(Preconditions.java:145)
      	at org.apache.flume.channel.BasicTransactionSemantics.begin(BasicTransactionSemantics.java:131)
      	at org.apache.flume.sink.hdfs.HDFSEventSink.process(HDFSEventSink.java:323)
      	at org.apache.flume.sink.DefaultSinkProcessor.process(DefaultSinkProcessor.java:65)
      	at org.apache.flume.SinkRunner$PollingRunner.run(SinkRunner.java:148)
      	at java.lang.Thread.run(Thread.java:662)
      
      1. FLUME-1075.patch.2
        1 kB
        Prasad Mujumdar

        Activity

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

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

        Review request for Flume, Arvind Prabhakar and Hari Shreedharan.

        Summary
        -------

        Unhandled thorwable in HDFS sink can leave transaction open.

        This addresses bug FLUME-1075.
        https://issues.apache.org/jira/browse/FLUME-1075

        Diffs


        flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java eee9221

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

        Testing
        -------

        regression tests

        Thanks,

        Prasad

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4569/ ----------------------------------------------------------- Review request for Flume, Arvind Prabhakar and Hari Shreedharan. Summary ------- Unhandled thorwable in HDFS sink can leave transaction open. This addresses bug FLUME-1075 . https://issues.apache.org/jira/browse/FLUME-1075 Diffs flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java eee9221 Diff: https://reviews.apache.org/r/4569/diff Testing ------- regression tests Thanks, Prasad
        Hide
        jiraposter@reviews.apache.org added a comment -

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

        Thanks for the patch Prasad. I have one suggestion noted below for your consideration.

        flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java
        <https://reviews.apache.org/r/4569/#comment14248>

        Since the code is now catching Throwable, please modify the throw logic to test and rethrow any Error types. For example:

        } catch (Throwable th) {
        ...
        if (th instance of Error)

        { throw (Error) th; }

        else

        { throw new EventDeliveryException(th); }

        }

        • Arvind

        On 2012-03-30 05:38:44, Prasad Mujumdar wrote:

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

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

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

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

        (Updated 2012-03-30 05:38:44)

        Review request for Flume, Arvind Prabhakar and Hari Shreedharan.

        Summary

        -------

        Unhandled thorwable in HDFS sink can leave transaction open.

        This addresses bug FLUME-1075.

        https://issues.apache.org/jira/browse/FLUME-1075

        Diffs

        -----

        flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java eee9221

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

        Testing

        -------

        regression tests

        Thanks,

        Prasad

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4569/#review6567 ----------------------------------------------------------- Thanks for the patch Prasad. I have one suggestion noted below for your consideration. flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java < https://reviews.apache.org/r/4569/#comment14248 > Since the code is now catching Throwable, please modify the throw logic to test and rethrow any Error types. For example: } catch (Throwable th) { ... if (th instance of Error) { throw (Error) th; } else { throw new EventDeliveryException(th); } } Arvind On 2012-03-30 05:38:44, Prasad Mujumdar wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4569/ ----------------------------------------------------------- (Updated 2012-03-30 05:38:44) Review request for Flume, Arvind Prabhakar and Hari Shreedharan. Summary ------- Unhandled thorwable in HDFS sink can leave transaction open. This addresses bug FLUME-1075 . https://issues.apache.org/jira/browse/FLUME-1075 Diffs ----- flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java eee9221 Diff: https://reviews.apache.org/r/4569/diff Testing ------- regression tests Thanks, Prasad
        Hide
        jiraposter@reviews.apache.org added a comment -

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

        Since we are catching Throwable here, we may want to do it for FLUME-1074 too. Shouldn't we do this in all places where Transaction.commit() is called, if for nothing else than consistency?

        • Mike

        On 2012-03-30 05:38:44, Prasad Mujumdar wrote:

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

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

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

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

        (Updated 2012-03-30 05:38:44)

        Review request for Flume, Arvind Prabhakar and Hari Shreedharan.

        Summary

        -------

        Unhandled thorwable in HDFS sink can leave transaction open.

        This addresses bug FLUME-1075.

        https://issues.apache.org/jira/browse/FLUME-1075

        Diffs

        -----

        flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java eee9221

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

        Testing

        -------

        regression tests

        Thanks,

        Prasad

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4569/#review6570 ----------------------------------------------------------- Since we are catching Throwable here, we may want to do it for FLUME-1074 too. Shouldn't we do this in all places where Transaction.commit() is called, if for nothing else than consistency? Mike On 2012-03-30 05:38:44, Prasad Mujumdar wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4569/ ----------------------------------------------------------- (Updated 2012-03-30 05:38:44) Review request for Flume, Arvind Prabhakar and Hari Shreedharan. Summary ------- Unhandled thorwable in HDFS sink can leave transaction open. This addresses bug FLUME-1075 . https://issues.apache.org/jira/browse/FLUME-1075 Diffs ----- flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java eee9221 Diff: https://reviews.apache.org/r/4569/diff Testing ------- regression tests Thanks, Prasad
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2012-03-30 08:33:50, Mike Percy wrote:

        > Since we are catching Throwable here, we may want to do it for FLUME-1074 too. Shouldn't we do this in all places where Transaction.commit() is called, if for nothing else than consistency?

        +1

        • Brock

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

        On 2012-03-30 05:38:44, Prasad Mujumdar wrote:

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

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

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

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

        (Updated 2012-03-30 05:38:44)

        Review request for Flume, Arvind Prabhakar and Hari Shreedharan.

        Summary

        -------

        Unhandled thorwable in HDFS sink can leave transaction open.

        This addresses bug FLUME-1075.

        https://issues.apache.org/jira/browse/FLUME-1075

        Diffs

        -----

        flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java eee9221

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

        Testing

        -------

        regression tests

        Thanks,

        Prasad

        Show
        jiraposter@reviews.apache.org added a comment - On 2012-03-30 08:33:50, Mike Percy wrote: > Since we are catching Throwable here, we may want to do it for FLUME-1074 too. Shouldn't we do this in all places where Transaction.commit() is called, if for nothing else than consistency? +1 Brock ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4569/#review6570 ----------------------------------------------------------- On 2012-03-30 05:38:44, Prasad Mujumdar wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4569/ ----------------------------------------------------------- (Updated 2012-03-30 05:38:44) Review request for Flume, Arvind Prabhakar and Hari Shreedharan. Summary ------- Unhandled thorwable in HDFS sink can leave transaction open. This addresses bug FLUME-1075 . https://issues.apache.org/jira/browse/FLUME-1075 Diffs ----- flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java eee9221 Diff: https://reviews.apache.org/r/4569/diff Testing ------- regression tests Thanks, Prasad
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2012-03-30 05:50:30, Arvind Prabhakar wrote:

        > flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java, line 398

        > <https://reviews.apache.org/r/4569/diff/1/?file=97555#file97555line398>

        >

        > Since the code is now catching Throwable, please modify the throw logic to test and rethrow any Error types. For example:

        >

        > } catch (Throwable th) {

        > ...

        > if (th instance of Error) { bq. > throw (Error) th; bq. > } else { bq. > throw new EventDeliveryException(th); bq. > }

        > }

        +1

        • Brock

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

        On 2012-03-30 05:38:44, Prasad Mujumdar wrote:

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

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

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

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

        (Updated 2012-03-30 05:38:44)

        Review request for Flume, Arvind Prabhakar and Hari Shreedharan.

        Summary

        -------

        Unhandled thorwable in HDFS sink can leave transaction open.

        This addresses bug FLUME-1075.

        https://issues.apache.org/jira/browse/FLUME-1075

        Diffs

        -----

        flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java eee9221

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

        Testing

        -------

        regression tests

        Thanks,

        Prasad

        Show
        jiraposter@reviews.apache.org added a comment - On 2012-03-30 05:50:30, Arvind Prabhakar wrote: > flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java, line 398 > < https://reviews.apache.org/r/4569/diff/1/?file=97555#file97555line398 > > > Since the code is now catching Throwable, please modify the throw logic to test and rethrow any Error types. For example: > > } catch (Throwable th) { > ... > if (th instance of Error) { bq. > throw (Error) th; bq. > } else { bq. > throw new EventDeliveryException(th); bq. > } > } +1 Brock ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4569/#review6567 ----------------------------------------------------------- On 2012-03-30 05:38:44, Prasad Mujumdar wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4569/ ----------------------------------------------------------- (Updated 2012-03-30 05:38:44) Review request for Flume, Arvind Prabhakar and Hari Shreedharan. Summary ------- Unhandled thorwable in HDFS sink can leave transaction open. This addresses bug FLUME-1075 . https://issues.apache.org/jira/browse/FLUME-1075 Diffs ----- flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java eee9221 Diff: https://reviews.apache.org/r/4569/diff Testing ------- regression tests Thanks, Prasad
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2012-03-30 08:33:50, Mike Percy wrote:

        > Since we are catching Throwable here, we may want to do it for FLUME-1074 too. Shouldn't we do this in all places where Transaction.commit() is called, if for nothing else than consistency?

        Brock Noland wrote:

        +1

        Perhaps we should have a cleanup method in transaction interface that rolls back the transaction if its still open and then close it. This way we can consolidate the transaction error handling in a single finally block instead of multiple catch blocks.

        • Prasad

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

        On 2012-03-30 05:38:44, Prasad Mujumdar wrote:

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

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

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

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

        (Updated 2012-03-30 05:38:44)

        Review request for Flume, Arvind Prabhakar and Hari Shreedharan.

        Summary

        -------

        Unhandled thorwable in HDFS sink can leave transaction open.

        This addresses bug FLUME-1075.

        https://issues.apache.org/jira/browse/FLUME-1075

        Diffs

        -----

        flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java eee9221

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

        Testing

        -------

        regression tests

        Thanks,

        Prasad

        Show
        jiraposter@reviews.apache.org added a comment - On 2012-03-30 08:33:50, Mike Percy wrote: > Since we are catching Throwable here, we may want to do it for FLUME-1074 too. Shouldn't we do this in all places where Transaction.commit() is called, if for nothing else than consistency? Brock Noland wrote: +1 Perhaps we should have a cleanup method in transaction interface that rolls back the transaction if its still open and then close it. This way we can consolidate the transaction error handling in a single finally block instead of multiple catch blocks. Prasad ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4569/#review6570 ----------------------------------------------------------- On 2012-03-30 05:38:44, Prasad Mujumdar wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4569/ ----------------------------------------------------------- (Updated 2012-03-30 05:38:44) Review request for Flume, Arvind Prabhakar and Hari Shreedharan. Summary ------- Unhandled thorwable in HDFS sink can leave transaction open. This addresses bug FLUME-1075 . https://issues.apache.org/jira/browse/FLUME-1075 Diffs ----- flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java eee9221 Diff: https://reviews.apache.org/r/4569/diff Testing ------- regression tests Thanks, Prasad
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2012-03-30 08:33:50, Mike Percy wrote:

        > Since we are catching Throwable here, we may want to do it for FLUME-1074 too. Shouldn't we do this in all places where Transaction.commit() is called, if for nothing else than consistency?

        Brock Noland wrote:

        +1

        Prasad Mujumdar wrote:

        Perhaps we should have a cleanup method in transaction interface that rolls back the transaction if its still open and then close it. This way we can consolidate the transaction error handling in a single finally block instead of multiple catch blocks.

        I mailed flume-dev on a similar issue "Channel/Transaction States"

        • Brock

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

        On 2012-03-30 05:38:44, Prasad Mujumdar wrote:

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

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

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

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

        (Updated 2012-03-30 05:38:44)

        Review request for Flume, Arvind Prabhakar and Hari Shreedharan.

        Summary

        -------

        Unhandled thorwable in HDFS sink can leave transaction open.

        This addresses bug FLUME-1075.

        https://issues.apache.org/jira/browse/FLUME-1075

        Diffs

        -----

        flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java eee9221

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

        Testing

        -------

        regression tests

        Thanks,

        Prasad

        Show
        jiraposter@reviews.apache.org added a comment - On 2012-03-30 08:33:50, Mike Percy wrote: > Since we are catching Throwable here, we may want to do it for FLUME-1074 too. Shouldn't we do this in all places where Transaction.commit() is called, if for nothing else than consistency? Brock Noland wrote: +1 Prasad Mujumdar wrote: Perhaps we should have a cleanup method in transaction interface that rolls back the transaction if its still open and then close it. This way we can consolidate the transaction error handling in a single finally block instead of multiple catch blocks. I mailed flume-dev on a similar issue "Channel/Transaction States" Brock ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4569/#review6570 ----------------------------------------------------------- On 2012-03-30 05:38:44, Prasad Mujumdar wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4569/ ----------------------------------------------------------- (Updated 2012-03-30 05:38:44) Review request for Flume, Arvind Prabhakar and Hari Shreedharan. Summary ------- Unhandled thorwable in HDFS sink can leave transaction open. This addresses bug FLUME-1075 . https://issues.apache.org/jira/browse/FLUME-1075 Diffs ----- flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java eee9221 Diff: https://reviews.apache.org/r/4569/diff Testing ------- regression tests Thanks, Prasad
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2012-03-30 08:33:50, Mike Percy wrote:

        > Since we are catching Throwable here, we may want to do it for FLUME-1074 too. Shouldn't we do this in all places where Transaction.commit() is called, if for nothing else than consistency?

        Brock Noland wrote:

        +1

        Prasad Mujumdar wrote:

        Perhaps we should have a cleanup method in transaction interface that rolls back the transaction if its still open and then close it. This way we can consolidate the transaction error handling in a single finally block instead of multiple catch blocks.

        Brock Noland wrote:

        I mailed flume-dev on a similar issue "Channel/Transaction States"

        I would like to note that I think catching Throwable in 1074 and or creating this idea of a cleanup shouldn't hold up either 1074 or 1075. These are both good incremental changes.

        • Brock

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

        On 2012-03-30 05:38:44, Prasad Mujumdar wrote:

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

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

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

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

        (Updated 2012-03-30 05:38:44)

        Review request for Flume, Arvind Prabhakar and Hari Shreedharan.

        Summary

        -------

        Unhandled thorwable in HDFS sink can leave transaction open.

        This addresses bug FLUME-1075.

        https://issues.apache.org/jira/browse/FLUME-1075

        Diffs

        -----

        flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java eee9221

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

        Testing

        -------

        regression tests

        Thanks,

        Prasad

        Show
        jiraposter@reviews.apache.org added a comment - On 2012-03-30 08:33:50, Mike Percy wrote: > Since we are catching Throwable here, we may want to do it for FLUME-1074 too. Shouldn't we do this in all places where Transaction.commit() is called, if for nothing else than consistency? Brock Noland wrote: +1 Prasad Mujumdar wrote: Perhaps we should have a cleanup method in transaction interface that rolls back the transaction if its still open and then close it. This way we can consolidate the transaction error handling in a single finally block instead of multiple catch blocks. Brock Noland wrote: I mailed flume-dev on a similar issue "Channel/Transaction States" I would like to note that I think catching Throwable in 1074 and or creating this idea of a cleanup shouldn't hold up either 1074 or 1075. These are both good incremental changes. Brock ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4569/#review6570 ----------------------------------------------------------- On 2012-03-30 05:38:44, Prasad Mujumdar wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4569/ ----------------------------------------------------------- (Updated 2012-03-30 05:38:44) Review request for Flume, Arvind Prabhakar and Hari Shreedharan. Summary ------- Unhandled thorwable in HDFS sink can leave transaction open. This addresses bug FLUME-1075 . https://issues.apache.org/jira/browse/FLUME-1075 Diffs ----- flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java eee9221 Diff: https://reviews.apache.org/r/4569/diff Testing ------- regression tests Thanks, Prasad
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2012-03-30 08:33:50, Mike Percy wrote:

        > Since we are catching Throwable here, we may want to do it for FLUME-1074 too. Shouldn't we do this in all places where Transaction.commit() is called, if for nothing else than consistency?

        Brock Noland wrote:

        +1

        Prasad Mujumdar wrote:

        Perhaps we should have a cleanup method in transaction interface that rolls back the transaction if its still open and then close it. This way we can consolidate the transaction error handling in a single finally block instead of multiple catch blocks.

        Brock Noland wrote:

        I mailed flume-dev on a similar issue "Channel/Transaction States"

        Brock Noland wrote:

        I would like to note that I think catching Throwable in 1074 and or creating this idea of a cleanup shouldn't hold up either 1074 or 1075. These are both good incremental changes.

        @Brock, sorry I didn't see the thread earlier. I will log a new ticket once we have a consensus on the dev list thread.
        @Mike, if we are going to change the close() logic, then we probably won't need to change all the exception handling code.

        • Prasad

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

        On 2012-03-30 05:38:44, Prasad Mujumdar wrote:

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

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

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

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

        (Updated 2012-03-30 05:38:44)

        Review request for Flume, Arvind Prabhakar and Hari Shreedharan.

        Summary

        -------

        Unhandled thorwable in HDFS sink can leave transaction open.

        This addresses bug FLUME-1075.

        https://issues.apache.org/jira/browse/FLUME-1075

        Diffs

        -----

        flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java eee9221

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

        Testing

        -------

        regression tests

        Thanks,

        Prasad

        Show
        jiraposter@reviews.apache.org added a comment - On 2012-03-30 08:33:50, Mike Percy wrote: > Since we are catching Throwable here, we may want to do it for FLUME-1074 too. Shouldn't we do this in all places where Transaction.commit() is called, if for nothing else than consistency? Brock Noland wrote: +1 Prasad Mujumdar wrote: Perhaps we should have a cleanup method in transaction interface that rolls back the transaction if its still open and then close it. This way we can consolidate the transaction error handling in a single finally block instead of multiple catch blocks. Brock Noland wrote: I mailed flume-dev on a similar issue "Channel/Transaction States" Brock Noland wrote: I would like to note that I think catching Throwable in 1074 and or creating this idea of a cleanup shouldn't hold up either 1074 or 1075. These are both good incremental changes. @Brock, sorry I didn't see the thread earlier. I will log a new ticket once we have a consensus on the dev list thread. @Mike, if we are going to change the close() logic, then we probably won't need to change all the exception handling code. Prasad ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4569/#review6570 ----------------------------------------------------------- On 2012-03-30 05:38:44, Prasad Mujumdar wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4569/ ----------------------------------------------------------- (Updated 2012-03-30 05:38:44) Review request for Flume, Arvind Prabhakar and Hari Shreedharan. Summary ------- Unhandled thorwable in HDFS sink can leave transaction open. This addresses bug FLUME-1075 . https://issues.apache.org/jira/browse/FLUME-1075 Diffs ----- flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java eee9221 Diff: https://reviews.apache.org/r/4569/diff Testing ------- regression tests Thanks, Prasad
        Hide
        jiraposter@reviews.apache.org added a comment -

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

        (Updated 2012-03-30 19:18:19.682350)

        Review request for Flume, Arvind Prabhakar and Hari Shreedharan.

        Summary
        -------

        Unhandled thorwable in HDFS sink can leave transaction open.

        This addresses bug FLUME-1075.
        https://issues.apache.org/jira/browse/FLUME-1075

        Diffs (updated)


        flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java eee9221

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

        Testing
        -------

        regression tests

        Thanks,

        Prasad

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4569/ ----------------------------------------------------------- (Updated 2012-03-30 19:18:19.682350) Review request for Flume, Arvind Prabhakar and Hari Shreedharan. Summary ------- Unhandled thorwable in HDFS sink can leave transaction open. This addresses bug FLUME-1075 . https://issues.apache.org/jira/browse/FLUME-1075 Diffs (updated) flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java eee9221 Diff: https://reviews.apache.org/r/4569/diff Testing ------- regression tests Thanks, Prasad
        Hide
        jiraposter@reviews.apache.org added a comment -

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

        +1 from me, I agree that we should get this in right away. Thanks!

        • Mike

        On 2012-03-30 19:18:19, Prasad Mujumdar wrote:

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

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

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

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

        (Updated 2012-03-30 19:18:19)

        Review request for Flume, Arvind Prabhakar and Hari Shreedharan.

        Summary

        -------

        Unhandled thorwable in HDFS sink can leave transaction open.

        This addresses bug FLUME-1075.

        https://issues.apache.org/jira/browse/FLUME-1075

        Diffs

        -----

        flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java eee9221

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

        Testing

        -------

        regression tests

        Thanks,

        Prasad

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4569/#review6587 ----------------------------------------------------------- +1 from me, I agree that we should get this in right away. Thanks! Mike On 2012-03-30 19:18:19, Prasad Mujumdar wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4569/ ----------------------------------------------------------- (Updated 2012-03-30 19:18:19) Review request for Flume, Arvind Prabhakar and Hari Shreedharan. Summary ------- Unhandled thorwable in HDFS sink can leave transaction open. This addresses bug FLUME-1075 . https://issues.apache.org/jira/browse/FLUME-1075 Diffs ----- flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java eee9221 Diff: https://reviews.apache.org/r/4569/diff Testing ------- regression tests Thanks, Prasad
        Hide
        jiraposter@reviews.apache.org added a comment -

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

        Ship it!

        +1.

        • Hari

        On 2012-03-30 19:18:19, Prasad Mujumdar wrote:

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

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

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

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

        (Updated 2012-03-30 19:18:19)

        Review request for Flume, Arvind Prabhakar and Hari Shreedharan.

        Summary

        -------

        Unhandled thorwable in HDFS sink can leave transaction open.

        This addresses bug FLUME-1075.

        https://issues.apache.org/jira/browse/FLUME-1075

        Diffs

        -----

        flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java eee9221

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

        Testing

        -------

        regression tests

        Thanks,

        Prasad

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4569/#review6592 ----------------------------------------------------------- Ship it! +1. Hari On 2012-03-30 19:18:19, Prasad Mujumdar wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4569/ ----------------------------------------------------------- (Updated 2012-03-30 19:18:19) Review request for Flume, Arvind Prabhakar and Hari Shreedharan. Summary ------- Unhandled thorwable in HDFS sink can leave transaction open. This addresses bug FLUME-1075 . https://issues.apache.org/jira/browse/FLUME-1075 Diffs ----- flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java eee9221 Diff: https://reviews.apache.org/r/4569/diff Testing ------- regression tests Thanks, Prasad
        Hide
        Arvind Prabhakar added a comment -

        Patch committed. Thanks Prasad!

        Show
        Arvind Prabhakar added a comment - Patch committed. Thanks Prasad!
        Hide
        Hudson added a comment -

        Integrated in flume-trunk #154 (See https://builds.apache.org/job/flume-trunk/154/)
        FLUME-1075. HDFSEventSink begin is called when transaction opened due to other error.

        (Prasad Mujumdar via Arvind Prabhakar) (Revision 1308171)

        Result = SUCCESS
        arvind : http://svn.apache.org/viewvc/?view=rev&rev=1308171
        Files :

        • /incubator/flume/trunk/flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java
        Show
        Hudson added a comment - Integrated in flume-trunk #154 (See https://builds.apache.org/job/flume-trunk/154/ ) FLUME-1075 . HDFSEventSink begin is called when transaction opened due to other error. (Prasad Mujumdar via Arvind Prabhakar) (Revision 1308171) Result = SUCCESS arvind : http://svn.apache.org/viewvc/?view=rev&rev=1308171 Files : /incubator/flume/trunk/flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java

          People

          • Assignee:
            Prasad Mujumdar
            Reporter:
            Brock Noland
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development