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

Spooldir source does not log channel exceptions

    Details

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

      Description

      Spooldir assumes that any ChannelException means that the channel is full and it does not log the exception message.

      1. FLUME-2619.patch
        4 kB
        Johny Rufus
      2. FLUME-2619-1.patch
        4 kB
        Johny Rufus

        Issue Links

          Activity

          Hide
          jrufus Johny Rufus added a comment -

          Nice catch, SpoolDir should specifically catch ChannelFullException instead of catching ChannelException and deciding that the channel is Full.
          Had to make one more change in ChannelProcessor, to not over wrap a ChannelException with itself

          Show
          jrufus Johny Rufus added a comment - Nice catch, SpoolDir should specifically catch ChannelFullException instead of catching ChannelException and deciding that the channel is Full. Had to make one more change in ChannelProcessor, to not over wrap a ChannelException with itself
          Hide
          jrufus Johny Rufus added a comment -

          There is one more place in ChannelProcessor, where this should be handled, including that change too

          Show
          jrufus Johny Rufus added a comment - There is one more place in ChannelProcessor, where this should be handled, including that change too
          Hide
          hshreedharan Hari Shreedharan added a comment - - edited

          Why do you need this change:

          if (t instanceof ChannelException) {
            throw (ChannelException)t;
          }
          

          ChannelException is unchecked, so you don't need that cast. I don't see any benefit of doing this, since a catch for ChannelException will catch it even without the cast here. Am I missing something? You can just throw it as is no?

          Show
          hshreedharan Hari Shreedharan added a comment - - edited Why do you need this change: if (t instanceof ChannelException) { throw (ChannelException)t; } ChannelException is unchecked, so you don't need that cast. I don't see any benefit of doing this, since a catch for ChannelException will catch it even without the cast here. Am I missing something? You can just throw it as is no?
          Hide
          jrufus Johny Rufus added a comment -

          t is of type Throwable, so if we just use - throw t, then @ compile time it would expect Throwable to be caught or declared as thrown, hence the cast

          Show
          jrufus Johny Rufus added a comment - t is of type Throwable, so if we just use - throw t, then @ compile time it would expect Throwable to be caught or declared as thrown, hence the cast
          Hide
          bessbd Bessenyei Balázs Donát added a comment -

          Johny Rufus: as there has been no updates for about a year, I'm assigning this ticket to me. If you disagree, please do tell.

          Show
          bessbd Bessenyei Balázs Donát added a comment - Johny Rufus : as there has been no updates for about a year, I'm assigning this ticket to me. If you disagree, please do tell.
          Hide
          bessbd Bessenyei Balázs Donát added a comment -

          I have rebased this patch on trunk. Please, review it at https://reviews.apache.org/r/50232/

          Show
          bessbd Bessenyei Balázs Donát added a comment - I have rebased this patch on trunk. Please, review it at https://reviews.apache.org/r/50232/
          Hide
          bessbd Bessenyei Balázs Donát added a comment -

          The patch for this issue has been available for a while, but it needs some reviews to be committed to trunk.

          Is there anyone who could help with this?

          Show
          bessbd Bessenyei Balázs Donát added a comment - The patch for this issue has been available for a while, but it needs some reviews to be committed to trunk. Is there anyone who could help with this?
          Hide
          mpercy Mike Percy added a comment -

          +1. I am about to commit this.

          Show
          mpercy Mike Percy added a comment - +1. I am about to commit this.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 1422f733007dbb78caae7e5135bc33470e88502a in flume's branch refs/heads/trunk from Bessenyei Balázs Donát
          [ https://git-wip-us.apache.org/repos/asf?p=flume.git;h=1422f73 ]

          FLUME-2619. Spooldir source should log channel exceptions

          Reviewed by Denes Arvay and Mike Percy.

          (Bessenyei Balázs Donát via Mike Percy)

          Show
          jira-bot ASF subversion and git services added a comment - Commit 1422f733007dbb78caae7e5135bc33470e88502a in flume's branch refs/heads/trunk from Bessenyei Balázs Donát [ https://git-wip-us.apache.org/repos/asf?p=flume.git;h=1422f73 ] FLUME-2619 . Spooldir source should log channel exceptions Reviewed by Denes Arvay and Mike Percy. (Bessenyei Balázs Donát via Mike Percy)
          Hide
          mpercy Mike Percy added a comment -

          Pushed to trunk. Thanks for the patch Donat!

          Show
          mpercy Mike Percy added a comment - Pushed to trunk. Thanks for the patch Donat!
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Flume-trunk-hbase-1 #192 (See https://builds.apache.org/job/Flume-trunk-hbase-1/192/)
          FLUME-2619. Spooldir source should log channel exceptions (mpercy: http://git-wip-us.apache.org/repos/asf/flume/repo?p=flume.git&a=commit&h=1422f733007dbb78caae7e5135bc33470e88502a)

          • flume-ng-core/src/main/java/org/apache/flume/channel/ChannelProcessor.java
          • flume-ng-core/src/main/java/org/apache/flume/source/SpoolDirectorySource.java
          • flume-ng-core/src/test/java/org/apache/flume/source/TestSpoolDirectorySource.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Flume-trunk-hbase-1 #192 (See https://builds.apache.org/job/Flume-trunk-hbase-1/192/ ) FLUME-2619 . Spooldir source should log channel exceptions (mpercy: http://git-wip-us.apache.org/repos/asf/flume/repo?p=flume.git&a=commit&h=1422f733007dbb78caae7e5135bc33470e88502a ) flume-ng-core/src/main/java/org/apache/flume/channel/ChannelProcessor.java flume-ng-core/src/main/java/org/apache/flume/source/SpoolDirectorySource.java flume-ng-core/src/test/java/org/apache/flume/source/TestSpoolDirectorySource.java

            People

            • Assignee:
              bessbd Bessenyei Balázs Donát
              Reporter:
              smolav Santiago M. Mola
            • Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development