Flume
  1. Flume
  2. FLUME-2255

Spooling Directory Source cannot handle channel exceptions

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: v1.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
      Hari Shreedharan added a comment -

      Patch that handles that fixes this.

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

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

      Show
      Mike Percy added a comment - LGTM, my one suggestion is to make maxBackoffMillis a user-configurable parameter.
      Hide
      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 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
      Hari Shreedharan added a comment -

      Making the maxBackoff parameter configurable

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

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

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

      Test based on the source counter that is deterministic.

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

      what is the undesired behavior being exhibited ?

      Show
      Roshan Naik added a comment - what is the undesired behavior being exhibited ?
      Hide
      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
      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 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 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
      Hari Shreedharan added a comment -

      Minor test fix

      Show
      Hari Shreedharan added a comment - Minor test fix
      Hide
      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
      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
      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
      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
      Hari Shreedharan added a comment -

      Updated test based on Mike's suggestion.

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

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

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

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

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

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

      Show
      Mike Percy added a comment - Pushed to trunk and flume-1.5 branches. Thanks for the patch Hari!
      Hide
      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 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:
          Hari Shreedharan
          Reporter:
          Hari Shreedharan
        • Votes:
          0 Vote for this issue
          Watchers:
          6 Start watching this issue

          Dates

          • Created:
            Updated:
            Resolved:

            Development