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

Spooling Directory Source cannot handle channel exceptions

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.5.0
    • Component/s: None
    • Labels:
      None
    1. FLUME-2255.patch
      5 kB
      Hari Shreedharan
    2. FLUME-2255-1.patch
      7 kB
      Hari Shreedharan
    3. FLUME-2255-2.patch
      9 kB
      Hari Shreedharan
    4. FLUME-2255-3.patch
      9 kB
      Hari Shreedharan
    5. FLUME-2255-4.patch
      8 kB
      Hari Shreedharan
    6. FLUME-2255-5.patch
      8 kB
      Hari Shreedharan

      Activity

      Hide
      hshreedharan Hari Shreedharan added a comment -

      Patch that handles that fixes this.

      Show
      hshreedharan Hari Shreedharan added a comment - Patch that handles that fixes this.
      Hide
      mpercy Mike Percy added a comment -

      LGTM, my one suggestion is to make maxBackoffMillis a user-configurable parameter.

      Show
      mpercy Mike Percy added a comment - LGTM, my one suggestion is to make maxBackoffMillis a user-configurable parameter.
      Hide
      roshan_naik Roshan Naik added a comment -

      Hari Shreedharan
      The JIRA title seems very broad. it would be nice to add some more specifics in the description.

      Would be nice to understand .... which exceptions we are talking about (channel/ file reading / other?) , what is the undesired behavior being exhibited etc.

      Show
      roshan_naik Roshan Naik added a comment - Hari Shreedharan The JIRA title seems very broad. it would be nice to add some more specifics in the description. Would be nice to understand .... which exceptions we are talking about (channel/ file reading / other?) , what is the undesired behavior being exhibited etc.
      Hide
      hshreedharan Hari Shreedharan added a comment -

      Making the maxBackoff parameter configurable

      Show
      hshreedharan Hari Shreedharan added a comment - Making the maxBackoff parameter configurable
      Hide
      hshreedharan Hari Shreedharan added a comment -

      Roshan Naik - As mentioned in the title, I am talking about channel exceptions.

      Show
      hshreedharan Hari Shreedharan added a comment - Roshan Naik - As mentioned in the title, I am talking about channel exceptions.
      Hide
      hshreedharan Hari Shreedharan added a comment -

      Test based on the source counter that is deterministic.

      Show
      hshreedharan Hari Shreedharan added a comment - Test based on the source counter that is deterministic.
      Hide
      roshan_naik Roshan Naik added a comment -

      what is the undesired behavior being exhibited ?

      Show
      roshan_naik Roshan Naik added a comment - what is the undesired behavior being exhibited ?
      Hide
      hshreedharan Hari Shreedharan added a comment -

      The source dies if the channel is full and cannot recover. You need to restart the agent to fix this issue

      Show
      hshreedharan Hari Shreedharan added a comment - The source dies if the channel is full and cannot recover. You need to restart the agent to fix this issue
      Hide
      roshan_naik Roshan Naik added a comment -

      when the channel getting full triggers an exception... will the SpoolDir source ...

      • retry with the same data later the channel frees up (no data loss) ?
      • or would it essentially keep dropping events (data loss)?
      Show
      roshan_naik Roshan Naik added a comment - when the channel getting full triggers an exception... will the SpoolDir source ... retry with the same data later the channel frees up (no data loss) ? or would it essentially keep dropping events (data loss)?
      Hide
      hshreedharan Hari Shreedharan added a comment -

      Minor test fix

      Show
      hshreedharan Hari Shreedharan added a comment - Minor test fix
      Hide
      hshreedharan Hari Shreedharan added a comment -

      Roshan Naik - If you look at the patch, it is quite self-explanatory. The source will not commit the read with the event reader, so no data is lost. The same events are retried.

      Show
      hshreedharan Hari Shreedharan added a comment - Roshan Naik - If you look at the patch, it is quite self-explanatory. The source will not commit the read with the event reader, so no data is lost. The same events are retried.
      Hide
      mpercy Mike Percy added a comment -

      Hari, looks pretty good but can you take out the first sleep? If we need an arbitrary sleep() for anything other than burning too much CPU while in a loop, then it's a flaky test.

      The test is also a little complicated, I wonder if instead we could do something like:

      source.start();
      
      // Make sure the Channel fills and the Source fails a put-transaction before continuing.
      while (!source.hitChannelException()) {
        Thread.sleep(50);
      }
      
      // Now, make sure the source still works. Read all of the events.
      while (numEventsTaken < numEventsPut) {
        // Clear the channel and increment our counters
        Thread.sleep(50);
      }
      
      Show
      mpercy Mike Percy added a comment - Hari, looks pretty good but can you take out the first sleep? If we need an arbitrary sleep() for anything other than burning too much CPU while in a loop, then it's a flaky test. The test is also a little complicated, I wonder if instead we could do something like: source.start(); // Make sure the Channel fills and the Source fails a put-transaction before continuing. while (!source.hitChannelException()) { Thread.sleep(50); } // Now, make sure the source still works. Read all of the events. while (numEventsTaken < numEventsPut) { // Clear the channel and increment our counters Thread.sleep(50); }
      Hide
      hshreedharan Hari Shreedharan added a comment -

      Updated test based on Mike's suggestion.

      Show
      hshreedharan Hari Shreedharan added a comment - Updated test based on Mike's suggestion.
      Hide
      hshreedharan Hari Shreedharan added a comment -

      Moving the while loop outside the main loop in the test.

      Show
      hshreedharan Hari Shreedharan added a comment - Moving the while loop outside the main loop in the test.
      Hide
      mpercy Mike Percy added a comment -

      +1 LGTM, thanks for updating the test. I plan on committing this later today.

      Show
      mpercy Mike Percy added a comment - +1 LGTM, thanks for updating the test. I plan on committing this later today.
      Hide
      mpercy Mike Percy added a comment -

      Pushed to trunk and flume-1.5 branches. Thanks for the patch Hari!

      Show
      mpercy Mike Percy added a comment - Pushed to trunk and flume-1.5 branches. Thanks for the patch Hari!
      Hide
      hudson Hudson added a comment -

      FAILURE: Integrated in flume-trunk #523 (See https://builds.apache.org/job/flume-trunk/523/)
      FLUME-2255. Correctly handle ChannelExceptions in SpoolingDirectorySource (mpercy: http://git-wip-us.apache.org/repos/asf/flume/repo?p=flume.git&a=commit&h=c23448fc959844eece5a8ab2dbf091c2c4973a26)

      • flume-ng-doc/sphinx/FlumeUserGuide.rst
      • flume-ng-core/src/test/java/org/apache/flume/source/TestSpoolDirectorySource.java
      • flume-ng-core/src/main/java/org/apache/flume/source/SpoolDirectorySourceConfigurationConstants.java
      • flume-ng-core/src/main/java/org/apache/flume/source/SpoolDirectorySource.java
      Show
      hudson Hudson added a comment - FAILURE: Integrated in flume-trunk #523 (See https://builds.apache.org/job/flume-trunk/523/ ) FLUME-2255 . Correctly handle ChannelExceptions in SpoolingDirectorySource (mpercy: http://git-wip-us.apache.org/repos/asf/flume/repo?p=flume.git&a=commit&h=c23448fc959844eece5a8ab2dbf091c2c4973a26 ) flume-ng-doc/sphinx/FlumeUserGuide.rst flume-ng-core/src/test/java/org/apache/flume/source/TestSpoolDirectorySource.java flume-ng-core/src/main/java/org/apache/flume/source/SpoolDirectorySourceConfigurationConstants.java flume-ng-core/src/main/java/org/apache/flume/source/SpoolDirectorySource.java

        People

        • Assignee:
          hshreedharan Hari Shreedharan
          Reporter:
          hshreedharan Hari Shreedharan
        • Votes:
          0 Vote for this issue
          Watchers:
          6 Start watching this issue

          Dates

          • Created:
            Updated:
            Resolved:

            Development