Flume
  1. Flume
  2. FLUME-1227

Introduce some sort of SpillableChannel

    Details

    • Type: New Feature New Feature
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: v1.4.0
    • Fix Version/s: v1.5.0
    • Component/s: Channel
    • Labels:
      None
    • Release Note:
      New Spillable Memory Channel. Functions like Memory channel, but uses disk as overflow if main memory capacity is reached.

      Description

      I would like to introduce new channel that would behave similarly as scribe (https://github.com/facebook/scribe). It would be something between memory and file channel. Input events would be saved directly to the memory (only) and would be served from there. In case that the memory would be full, we would outsource the events to file.

      Let me describe the use case behind this request. We have plenty of frontend servers that are generating events. We want to send all events to just limited number of machines from where we would send the data to HDFS (some sort of staging layer). Reason for this second layer is our need to decouple event aggregation and front end code to separate machines. Using memory channel is fully sufficient as we can survive lost of some portion of the events. However in order to sustain maintenance windows or networking issues we would have to end up with a lot of memory assigned to those "staging" machines. Referenced "scribe" is dealing with this problem by implementing following logic - events are saved in memory similarly as our MemoryChannel. However in case that the memory gets full (because of maintenance, networking issues, ...) it will spill data to disk where they will be sitting until everything start working again.

      I would like to introduce channel that would implement similar logic. It's durability guarantees would be same as MemoryChannel - in case that someone would remove power cord, this channel would lose data. Based on the discussion in FLUME-1201, I would propose to have the implementation completely independent on any other channel internal code.

      Jarcec

      1. FLUME-1227.v9.patch
        77 kB
        Roshan Naik
      2. FLUME-1227.v8.patch
        75 kB
        Roshan Naik
      3. FLUME-1227.v7.patch
        72 kB
        Roshan Naik
      4. FLUME-1227.v6.patch
        71 kB
        Roshan Naik
      5. FLUME-1227.v5.patch
        69 kB
        Roshan Naik
      6. FLUME-1227.v2.patch
        16 kB
        Roshan Naik
      7. SpillableMemory Channel Design 2.pdf
        277 kB
        Roshan Naik
      8. 1227.patch.1
        74 kB
        Roshan Naik
      9. SpillableMemory Channel Design.pdf
        252 kB
        Roshan Naik

        Issue Links

          Activity

          Hide
          Hari Shreedharan added a comment -

          Roshan Naik - When we roll 1.5, jiras with no fix versions will be updated.

          Show
          Hari Shreedharan added a comment - Roshan Naik - When we roll 1.5, jiras with no fix versions will be updated.
          Hide
          Hari Shreedharan added a comment -

          Yes sir. New components usually get marked as experimental for a release or two, until we know that it is stable enough for prod use. This is the path we followed with File Channel and other components too.

          Show
          Hari Shreedharan added a comment - Yes sir. New components usually get marked as experimental for a release or two, until we know that it is stable enough for prod use. This is the path we followed with File Channel and other components too.
          Hide
          Otis Gospodnetic added a comment -

          Thanks everyone!

          Since this is being marked as experimental

          How come? Because it's new and not tested in anything close to production or some other reason? Just curious!

          Show
          Otis Gospodnetic added a comment - Thanks everyone! Since this is being marked as experimental How come? Because it's new and not tested in anything close to production or some other reason? Just curious!
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in flume-trunk #553 (See https://builds.apache.org/job/flume-trunk/553/)
          FLUME-1227. Introduce Spillable Channel. (hshreedharan: http://git-wip-us.apache.org/repos/asf/flume/repo/?p=flume.git&a=commit&h=6a50ec2ad33b8cbd057907c67030d855520c5f13)

          • flume-ng-doc/sphinx/FlumeUserGuide.rst
          • flume-ng-channels/flume-spillable-memory-channel/pom.xml
          • flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FileChannel.java
          • flume-ng-core/src/main/java/org/apache/flume/ChannelFullException.java
          • flume-ng-channels/flume-spillable-memory-channel/src/test/java/org/apache/flume/channel/TestSpillableMemoryChannel.java
          • flume-ng-core/src/main/java/org/apache/flume/channel/AbstractChannel.java
          • flume-ng-node/pom.xml
          • flume-ng-embedded-agent/src/main/java/org/apache/flume/agent/embedded/EmbeddedAgentConfiguration.java
          • pom.xml
          • flume-ng-core/src/main/java/org/apache/flume/channel/MemoryChannel.java
          • flume-ng-dist/pom.xml
          • flume-ng-channels/flume-spillable-memory-channel/src/main/java/org/apache/flume/channel/SpillableMemoryChannel.java
          • flume-ng-configuration/src/main/java/org/apache/flume/conf/channel/ChannelConfiguration.java
          • flume-ng-configuration/src/main/java/org/apache/flume/conf/channel/ChannelType.java
          • flume-ng-channels/pom.xml
          Show
          Hudson added a comment - SUCCESS: Integrated in flume-trunk #553 (See https://builds.apache.org/job/flume-trunk/553/ ) FLUME-1227 . Introduce Spillable Channel. (hshreedharan: http://git-wip-us.apache.org/repos/asf/flume/repo/?p=flume.git&a=commit&h=6a50ec2ad33b8cbd057907c67030d855520c5f13 ) flume-ng-doc/sphinx/FlumeUserGuide.rst flume-ng-channels/flume-spillable-memory-channel/pom.xml flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FileChannel.java flume-ng-core/src/main/java/org/apache/flume/ChannelFullException.java flume-ng-channels/flume-spillable-memory-channel/src/test/java/org/apache/flume/channel/TestSpillableMemoryChannel.java flume-ng-core/src/main/java/org/apache/flume/channel/AbstractChannel.java flume-ng-node/pom.xml flume-ng-embedded-agent/src/main/java/org/apache/flume/agent/embedded/EmbeddedAgentConfiguration.java pom.xml flume-ng-core/src/main/java/org/apache/flume/channel/MemoryChannel.java flume-ng-dist/pom.xml flume-ng-channels/flume-spillable-memory-channel/src/main/java/org/apache/flume/channel/SpillableMemoryChannel.java flume-ng-configuration/src/main/java/org/apache/flume/conf/channel/ChannelConfiguration.java flume-ng-configuration/src/main/java/org/apache/flume/conf/channel/ChannelType.java flume-ng-channels/pom.xml
          Hide
          Roshan Naik added a comment -

          Should we set the 'fix version' to 1.5 ?

          Show
          Roshan Naik added a comment - Should we set the 'fix version' to 1.5 ?
          Hide
          Hari Shreedharan added a comment -

          Committed! Thanks Roshan for working through all of the discussion and getting this done!

          Show
          Hari Shreedharan added a comment - Committed! Thanks Roshan for working through all of the discussion and getting this done!
          Hide
          ASF subversion and git services added a comment -

          Commit d5805c8598be4eec85de8973b4c98ecdd7ffe6d3 in flume's branch refs/heads/flume-1.5 from Hari Shreedharan
          [ https://git-wip-us.apache.org/repos/asf?p=flume.git;h=d5805c8 ]

          FLUME-1227. Introduce Spillable Channel.

          (Roshan Naik via Hari Shreedharan)

          Show
          ASF subversion and git services added a comment - Commit d5805c8598be4eec85de8973b4c98ecdd7ffe6d3 in flume's branch refs/heads/flume-1.5 from Hari Shreedharan [ https://git-wip-us.apache.org/repos/asf?p=flume.git;h=d5805c8 ] FLUME-1227 . Introduce Spillable Channel. (Roshan Naik via Hari Shreedharan)
          Hide
          ASF subversion and git services added a comment -

          Commit 6a50ec2ad33b8cbd057907c67030d855520c5f13 in flume's branch refs/heads/trunk from Hari Shreedharan
          [ https://git-wip-us.apache.org/repos/asf?p=flume.git;h=6a50ec2 ]

          FLUME-1227. Introduce Spillable Channel.

          (Roshan Naik via Hari Shreedharan)

          Show
          ASF subversion and git services added a comment - Commit 6a50ec2ad33b8cbd057907c67030d855520c5f13 in flume's branch refs/heads/trunk from Hari Shreedharan [ https://git-wip-us.apache.org/repos/asf?p=flume.git;h=6a50ec2 ] FLUME-1227 . Introduce Spillable Channel. (Roshan Naik via Hari Shreedharan)
          Hide
          Hari Shreedharan added a comment -

          +1. I am going to run tests and commit this one. Since this is being marked as experimental, I made a change in the user guide to clarify it is not recommended for production use.

          I also made some minor indentation changes in SpillableMemoryChannel.java

          Show
          Hari Shreedharan added a comment - +1. I am going to run tests and commit this one. Since this is being marked as experimental, I made a change in the user guide to clarify it is not recommended for production use. I also made some minor indentation changes in SpillableMemoryChannel.java
          Hide
          Thilo Seidel added a comment -

          Guten Tag,
          Ich bin heute nicht im Büro. Ihre Mail wird bis zu meiner Rückkehr weder gelesen noch automatisch weitergeleitet.
          Viele Grüße
          Thilo Seidel

          Show
          Thilo Seidel added a comment - Guten Tag, Ich bin heute nicht im Büro. Ihre Mail wird bis zu meiner Rückkehr weder gelesen noch automatisch weitergeleitet. Viele Grüße Thilo Seidel
          Hide
          Otis Gospodnetic added a comment - - edited

          Was just about to write to the ML asking about this functionality. Looks like all known issues have been fixed, plus this is new functionality, so it should go in and get some real-world action, which we'd love to give it as soon as 1.5.0 is out!

          +10 for committing this. Any chances of this going in before 1.5.0 is cut? It's got 32 eyeballs watching it, so there is clear interest.

          Show
          Otis Gospodnetic added a comment - - edited Was just about to write to the ML asking about this functionality. Looks like all known issues have been fixed, plus this is new functionality, so it should go in and get some real-world action, which we'd love to give it as soon as 1.5.0 is out! +10 for committing this. Any chances of this going in before 1.5.0 is cut? It's got 32 eyeballs watching it, so there is clear interest.
          Hide
          Roshan Naik added a comment -

          Hari Shreedharan if there are no other comments.. could you look into committing this ?

          Show
          Roshan Naik added a comment - Hari Shreedharan if there are no other comments.. could you look into committing this ?
          Hide
          Roshan Naik added a comment -

          My bad. Updating the state.

          Show
          Roshan Naik added a comment - My bad. Updating the state.
          Hide
          Hari Shreedharan added a comment -

          Roshan Naik - Is this ready for review (since you have not hit "Submit Patch")?

          Show
          Hari Shreedharan added a comment - Roshan Naik - Is this ready for review (since you have not hit "Submit Patch")?
          Hide
          Brock Noland added a comment -

          Thank you for addressing the feedback! I am OK with your reasoning regarding adding dual checkpointing to the example. I haven't looked at this code and review in detail. It looks like Hari has, so I think he'll have to make the call of when to commit.

          Thank you for your hard work Roshan!

          Show
          Brock Noland added a comment - Thank you for addressing the feedback! I am OK with your reasoning regarding adding dual checkpointing to the example. I haven't looked at this code and review in detail. It looks like Hari has, so I think he'll have to make the call of when to commit. Thank you for your hard work Roshan!
          Hide
          Roshan Naik added a comment -

          Updated patch addresses comments from Brock Noland

          Show
          Roshan Naik added a comment - Updated patch addresses comments from Brock Noland
          Hide
          Roshan Naik added a comment -

          thanks for the feedback Brock Noland
          Will incorporate ur feedback and update the patch soon.

          WRT to the adding notes on file channel best practices into Spillable Channel section, i am not too hot on that unless it has specifically to do with its coupling with Spillable channel. In (FLUME-2239) recently I made a note about multiple data dirs helping file channel perf. Also the dual checkpoint feature is broken on Windows(FLUME-2224). Let me know if you feel otherwise.

          Show
          Roshan Naik added a comment - thanks for the feedback Brock Noland Will incorporate ur feedback and update the patch soon. WRT to the adding notes on file channel best practices into Spillable Channel section, i am not too hot on that unless it has specifically to do with its coupling with Spillable channel. In ( FLUME-2239 ) recently I made a note about multiple data dirs helping file channel perf. Also the dual checkpoint feature is broken on Windows( FLUME-2224 ). Let me know if you feel otherwise.
          Hide
          Brock Noland added a comment -

          Hey, I have not participated in the review til now so sorry about this...but I just noticed the following items which are mostly "nits" and improvements.

          SpillableMemoryChannel
          1. Static stuff should be at the top
          2. Constructor should be directly below fields
          3. String constants should be static final fields with javadoc description
          4. Stuff can be final:

          private Object queueLock = new Object();
          

          TestSpillableMemoryChannel
          1. Take null has a commented out assertion
          2. There are locations where we expect "Exception" that should be a specific type of exception.
          3. Let's not use e.printStackTrace();
          4. Places we assert boolean should have a message
          5. Many missing spaces such as:

          for (int i=0; i<count; ++i) {
          

          and

          nullsFound=count;
          

          Docs

          1. Please specify multiple data directories in the examples and add a note
          that file channel performance will increase dramatically with multiple disks.

          2. Add dual checkpoint to the examples as that is a good practice.

          Show
          Brock Noland added a comment - Hey, I have not participated in the review til now so sorry about this...but I just noticed the following items which are mostly "nits" and improvements. SpillableMemoryChannel 1. Static stuff should be at the top 2. Constructor should be directly below fields 3. String constants should be static final fields with javadoc description 4. Stuff can be final: private Object queueLock = new Object(); TestSpillableMemoryChannel 1. Take null has a commented out assertion 2. There are locations where we expect "Exception" that should be a specific type of exception. 3. Let's not use e.printStackTrace(); 4. Places we assert boolean should have a message 5. Many missing spaces such as: for (int i=0; i<count; ++i) { and nullsFound=count; Docs 1. Please specify multiple data directories in the examples and add a note that file channel performance will increase dramatically with multiple disks. 2. Add dual checkpoint to the examples as that is a good practice.
          Hide
          Roshan Naik added a comment -

          Thanks for the comments Hari. This patch fixes the issues with

          • the 80char line,
          • The counter issue you noted plus other counters also fixed.. additional unit tests added to check counters.
          • test Failure you pointed out.. it turned out to be a flaky test and has now been strengthened (and renamed)

          I agree with you on the 2 issues that it shares with other channels wrt transactions. Now that you point it out, it reminds me that I did noticed it early on when studying the mem channel. Its definitely worth revisiting sometime.

          Show
          Roshan Naik added a comment - Thanks for the comments Hari. This patch fixes the issues with the 80char line, The counter issue you noted plus other counters also fixed.. additional unit tests added to check counters. test Failure you pointed out.. it turned out to be a flaky test and has now been strengthened (and renamed) I agree with you on the 2 issues that it shares with other channels wrt transactions. Now that you point it out, it reminds me that I did noticed it early on when studying the mem channel. Its definitely worth revisiting sometime.
          Hide
          Roshan Naik added a comment -

          Hi Hari Shreedharan.. i have addressed most of your comments locally.. but will need another day to address your comments on incorrect counter & test issue. it needs some thinking through on my part.. thanks for catching them.

          Show
          Roshan Naik added a comment - Hi Hari Shreedharan .. i have addressed most of your comments locally.. but will need another day to address your comments on incorrect counter & test issue. it needs some thinking through on my part.. thanks for catching them.
          Hide
          Hari Shreedharan added a comment -

          Hey Roshan Naik - Any updates here?

          Show
          Hari Shreedharan added a comment - Hey Roshan Naik - Any updates here?
          Hide
          Hari Shreedharan added a comment -

          Yes, it was v7.

          Show
          Hari Shreedharan added a comment - Yes, it was v7.
          Hide
          Roshan Naik added a comment -
          • will fix the 80 character length issue you noted
          • I will need to review code wrt your other comments related to Txn correctness more closely. let me get back to you on them.
          • Hari Shreedharan could you please confirm that the test failure was noticed in in patch v7 ?
          Show
          Roshan Naik added a comment - will fix the 80 character length issue you noted I will need to review code wrt your other comments related to Txn correctness more closely. let me get back to you on them. Hari Shreedharan could you please confirm that the test failure was noticed in in patch v7 ?
          Hide
          Hari Shreedharan added a comment -

          The patch seems to be failing tests :

          -------------------------------------------------------
          Picked up _JAVA_OPTIONS: -Djava.awt.headless=true
          Running org.apache.flume.channel.TestSpillableMemoryChannel
          Tests run: 14, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 103.657 sec <<< FAILURE!
          testTotalStoredSemaphore(org.apache.flume.channel.TestSpillableMemoryChannel)  Time elapsed: 2923 sec  <<< FAILURE!
          java.lang.AssertionError: expected:<0> but was:<4500>
          	at org.junit.Assert.fail(Assert.java:93)
          	at org.junit.Assert.failNotEquals(Assert.java:647)
          	at org.junit.Assert.assertEquals(Assert.java:128)
          	at org.junit.Assert.assertEquals(Assert.java:472)
          	at org.junit.Assert.assertEquals(Assert.java:456)
          	at org.apache.flume.channel.TestSpillableMemoryChannel.testTotalStoredSemaphore(TestSpillableMemoryChannel.java:735)
          	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
          	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
          	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
          	at java.lang.reflect.Method.invoke(Method.java:597)
          	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:45)
          	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:15)
          	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:42)
          	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:20)
          	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:28)
          	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:30)
          	at org.junit.rules.ExternalResource$1.evaluate(ExternalResource.java:46)
          	at org.junit.rules.RunRules.evaluate(RunRules.java:18)
          	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:263)
          	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:68)
          	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:47)
          	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:231)
          	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:60)
          	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:229)
          	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:50)
          	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:222)
          	at org.junit.runners.ParentRunner.run(ParentRunner.java:300)
          	at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:252)
          	at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:141)
          	at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:112)
          	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
          	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
          	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
          	at java.lang.reflect.Method.invoke(Method.java:597)
          	at org.apache.maven.surefire.util.ReflectionUtils.invokeMethodWithArray(ReflectionUtils.java:189)
          	at org.apache.maven.surefire.booter.ProviderFactory$ProviderProxy.invoke(ProviderFactory.java:165)
          	at org.apache.maven.surefire.booter.ProviderFactory.invokeProvider(ProviderFactory.java:85)
          	at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:115)
          	at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:75)
          
          
          Results :
          
          Failed tests:   testTotalStoredSemaphore(org.apache.flume.channel.TestSpillableMemoryChannel): expected:<0> but was:<4500>
          
          
          Show
          Hari Shreedharan added a comment - The patch seems to be failing tests : ------------------------------------------------------- Picked up _JAVA_OPTIONS: -Djava.awt.headless= true Running org.apache.flume.channel.TestSpillableMemoryChannel Tests run: 14, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 103.657 sec <<< FAILURE! testTotalStoredSemaphore(org.apache.flume.channel.TestSpillableMemoryChannel) Time elapsed: 2923 sec <<< FAILURE! java.lang.AssertionError: expected:<0> but was:<4500> at org.junit.Assert.fail(Assert.java:93) at org.junit.Assert.failNotEquals(Assert.java:647) at org.junit.Assert.assertEquals(Assert.java:128) at org.junit.Assert.assertEquals(Assert.java:472) at org.junit.Assert.assertEquals(Assert.java:456) at org.apache.flume.channel.TestSpillableMemoryChannel.testTotalStoredSemaphore(TestSpillableMemoryChannel.java:735) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25) at java.lang.reflect.Method.invoke(Method.java:597) at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:45) at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:15) at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:42) at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:20) at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:28) at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:30) at org.junit.rules.ExternalResource$1.evaluate(ExternalResource.java:46) at org.junit.rules.RunRules.evaluate(RunRules.java:18) at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:263) at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:68) at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:47) at org.junit.runners.ParentRunner$3.run(ParentRunner.java:231) at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:60) at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:229) at org.junit.runners.ParentRunner.access$000(ParentRunner.java:50) at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:222) at org.junit.runners.ParentRunner.run(ParentRunner.java:300) at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:252) at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:141) at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:112) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25) at java.lang.reflect.Method.invoke(Method.java:597) at org.apache.maven.surefire.util.ReflectionUtils.invokeMethodWithArray(ReflectionUtils.java:189) at org.apache.maven.surefire.booter.ProviderFactory$ProviderProxy.invoke(ProviderFactory.java:165) at org.apache.maven.surefire.booter.ProviderFactory.invokeProvider(ProviderFactory.java:85) at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:115) at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:75) Results : Failed tests: testTotalStoredSemaphore(org.apache.flume.channel.TestSpillableMemoryChannel): expected:<0> but was:<4500>
          Hide
          Hari Shreedharan added a comment -

          Also, there are several lines > 80 characters. Can you make sure that you fix this too. For comments, please put the comments before the relevant line if they are expected to be long.

          Show
          Hari Shreedharan added a comment - Also, there are several lines > 80 characters. Can you make sure that you fix this too. For comments, please put the comments before the relevant line if they are expected to be long.
          Hide
          Hari Shreedharan added a comment -

          Hi Roshan,

          In the takePrimary and takeOverflow methods, there is a Preconditions.checkArgument method where like you mentioned in takePrimary method comments, there is an int->Integer->String conversion in a hot path (this is handled with an if in the takePrimary method, not in takeOverflow) - can you get rid of the the preconditions call, and just do:

          if (...)

          { throw IllegalStateException(..) }

          .

          This for one is cleaner, since the if already checks for the issue and we can avoid an unneeded method call.

          Is this because rolling back the overflow txn will ensure that the event goes back into the file channel and you don't need to handle it?

               if (!useOverflow) {
                    takeList.offer(event);  // takeList is thd pvt, so no need to do this in synchronized block
                  }
          

          If that is the case the counters are incorrect when the transaction committed is overflow transaction, since this is how they are updated:

          channelCounter.addToEventTakeSuccessCount(takeList.size());
          

          Even this is not accurate:

                if (takeList.size() > largestTakeTxSize)
                  largestTakeTxSize = takeList.size();
          

          There are also a couple issue with regards to failed transactions when writing to primary (granted it is a queue and it should not fail, but if a lock acquire gets interrupted, it can still fail). The memQueueRemaining semaphore has already been updated before pushing the events to the queue (that is definitely the right thing to do), but if a queue.offer fails the memQueueRemaining is not updated. This might be an issue with the current channels too - and is sufficiently rare to say we can revisit this later.

          Also there is a possibility of partially successful transactions right now (if the queue inserts fail - that I guess is true for all channels right now, so I guess we can live with it - just mentioning it to ensure that we know it is a possibility and we can revisit if needed).

          Show
          Hari Shreedharan added a comment - Hi Roshan, In the takePrimary and takeOverflow methods, there is a Preconditions.checkArgument method where like you mentioned in takePrimary method comments, there is an int->Integer->String conversion in a hot path (this is handled with an if in the takePrimary method, not in takeOverflow) - can you get rid of the the preconditions call, and just do: if (...) { throw IllegalStateException(..) } . This for one is cleaner, since the if already checks for the issue and we can avoid an unneeded method call. Is this because rolling back the overflow txn will ensure that the event goes back into the file channel and you don't need to handle it? if (!useOverflow) { takeList.offer(event); // takeList is thd pvt, so no need to do this in synchronized block } If that is the case the counters are incorrect when the transaction committed is overflow transaction, since this is how they are updated: channelCounter.addToEventTakeSuccessCount(takeList.size()); Even this is not accurate: if (takeList.size() > largestTakeTxSize) largestTakeTxSize = takeList.size(); There are also a couple issue with regards to failed transactions when writing to primary (granted it is a queue and it should not fail, but if a lock acquire gets interrupted, it can still fail). The memQueueRemaining semaphore has already been updated before pushing the events to the queue (that is definitely the right thing to do), but if a queue.offer fails the memQueueRemaining is not updated. This might be an issue with the current channels too - and is sufficiently rare to say we can revisit this later. Also there is a possibility of partially successful transactions right now (if the queue inserts fail - that I guess is true for all channels right now, so I guess we can live with it - just mentioning it to ensure that we know it is a possibility and we can revisit if needed).
          Hide
          Roshan Naik added a comment -

          Hari Shreedharan could you take a look into committing this patch if there are no further concerns.

          Show
          Roshan Naik added a comment - Hari Shreedharan could you take a look into committing this patch if there are no further concerns.
          Hide
          Roshan Naik added a comment -

          Updating patch with some fixes in tests related to sequence of steps used to reconfigure and restart of flume and also added another test.

          Show
          Roshan Naik added a comment - Updating patch with some fixes in tests related to sequence of steps used to reconfigure and restart of flume and also added another test.
          Hide
          Roshan Naik added a comment -

          plz hold off on this patch... noticed a bug. will update soon.

          Show
          Roshan Naik added a comment - plz hold off on this patch... noticed a bug. will update soon.
          Hide
          Roshan Naik added a comment -

          Hari Shreedharan , all the review comments should be addressed now. if there are no other concerns, could you commit this ?

          Show
          Roshan Naik added a comment - Hari Shreedharan , all the review comments should be addressed now. if there are no other concerns, could you commit this ?
          Hide
          Roshan Naik added a comment -

          Updating patch:

          • Adding unit test case to verify totalStored Semaphore usage is correct.
          • Bug fix: Updating channelCounter correctly on commit.
          Show
          Roshan Naik added a comment - Updating patch: Adding unit test case to verify totalStored Semaphore usage is correct. Bug fix: Updating channelCounter correctly on commit.
          Hide
          Roshan Naik added a comment -

          Hari Shreedharan just updated it.

          Show
          Roshan Naik added a comment - Hari Shreedharan just updated it.
          Hide
          Hari Shreedharan added a comment -

          Roshan Naik - Could you please update the patch on rb?

          Show
          Hari Shreedharan added a comment - Roshan Naik - Could you please update the patch on rb?
          Hide
          Hari Shreedharan added a comment -

          I will try and take a look at this sometime in the next week. I can't promise a full review, but I will try to fit it into my schedule.

          Show
          Hari Shreedharan added a comment - I will try and take a look at this sometime in the next week. I can't promise a full review, but I will try to fit it into my schedule.
          Hide
          Roshan Naik added a comment - - edited

          Hari Shreedharan, others interested.. could you take a stab at reviewing this code ?

          Show
          Roshan Naik added a comment - - edited Hari Shreedharan , others interested.. could you take a stab at reviewing this code ?
          Hide
          Roshan Naik added a comment -

          minor tweaks to patch.

          Show
          Roshan Naik added a comment - minor tweaks to patch.
          Hide
          Roshan Naik added a comment -

          adding info to design doc v2 on policy of switching between overflow and primary

          Show
          Roshan Naik added a comment - adding info to design doc v2 on policy of switching between overflow and primary
          Hide
          Roshan Naik added a comment -

          Appreciate your feedback Hari.

          HARI > It looks like channel can actually return fewer events than total available in the case where there are only "n" events in the primary queue and an "n+1"-th take would happen - since the events in a particular txn will always come from one queue. I think we should be able to pull events from the other store if it turns out to be required - else we expect the sink to come back and poll immediately - and also cause sink side transactions to be smaller than they have to be - which can cause Avro/HDFS batch sizes to be smaller than configured causing perf issues.

          Yes that is correct. The sink's transaction batch size would be smaller in that case. The case
          would only occur in when the take transaction transitions between overflow and primary.
          The alternative, as you sugest, is to pull from both overflow and primary, but that opens up some fundamental problems similar to distributed transactions. Essentially the sink needs to have
          two transactions open (one each on overflow and primary) which needs to be atomically committed/rolledback. Thoughts ?

          HARI > How the channel recovers from an "overflow" situation.

          I have updated the design doc (section 2.1.2) to elaborate on this. The short version is:

          New incoming events will go into primary if the sinks have drained older events from the primary
          even if overflow is not empty.

          Let me know if the description addresses your question sufficiently.

          Show
          Roshan Naik added a comment - Appreciate your feedback Hari. HARI > It looks like channel can actually return fewer events than total available in the case where there are only "n" events in the primary queue and an "n+1"-th take would happen - since the events in a particular txn will always come from one queue. I think we should be able to pull events from the other store if it turns out to be required - else we expect the sink to come back and poll immediately - and also cause sink side transactions to be smaller than they have to be - which can cause Avro/HDFS batch sizes to be smaller than configured causing perf issues. Yes that is correct. The sink's transaction batch size would be smaller in that case. The case would only occur in when the take transaction transitions between overflow and primary. The alternative, as you sugest, is to pull from both overflow and primary, but that opens up some fundamental problems similar to distributed transactions. Essentially the sink needs to have two transactions open (one each on overflow and primary) which needs to be atomically committed/rolledback. Thoughts ? HARI > How the channel recovers from an "overflow" situation. I have updated the design doc (section 2.1.2) to elaborate on this. The short version is: New incoming events will go into primary if the sinks have drained older events from the primary even if overflow is not empty. Let me know if the description addresses your question sufficiently.
          Hide
          Hari Shreedharan added a comment -

          Hi Roshan,

          Thanks for the updated design doc and patch. I looked at the design doc and this approach looks good. I like the fact that there are no dependencies (at least as mentioned in the doc) on the file channel's implicit behavior. I have on question though. The drain order queue seems to keep a count of how many events are written to which store each time a write happens (using the -ve and +ve numbers). It looks like channel can actually return fewer events than total available in the case where there are only "n" events in the primary queue and an "n+1"-th take would happen - since the events in a particular txn will always come from one queue. I think we should be able to pull events from the other store if it turns out to be required - else we expect the sink to come back and poll immediately - and also cause sink side transactions to be smaller than they have to be - which can cause Avro/HDFS batch sizes to be smaller than configured causing perf issues.

          Also, I am not clear on how the channel recovers from an "overflow" situation. Assume that the primary has capacity of "n" and we are currently overflowing. When do we decide to go back to the primary? Is it when all "n" from the primary have been removed, or we don't go back to it until restart (sorry I didn't look at the code yet - this does not seem to have gotten a mention in the design doc).

          Show
          Hari Shreedharan added a comment - Hi Roshan, Thanks for the updated design doc and patch. I looked at the design doc and this approach looks good. I like the fact that there are no dependencies (at least as mentioned in the doc) on the file channel's implicit behavior. I have on question though. The drain order queue seems to keep a count of how many events are written to which store each time a write happens (using the -ve and +ve numbers). It looks like channel can actually return fewer events than total available in the case where there are only "n" events in the primary queue and an "n+1"-th take would happen - since the events in a particular txn will always come from one queue. I think we should be able to pull events from the other store if it turns out to be required - else we expect the sink to come back and poll immediately - and also cause sink side transactions to be smaller than they have to be - which can cause Avro/HDFS batch sizes to be smaller than configured causing perf issues. Also, I am not clear on how the channel recovers from an "overflow" situation. Assume that the primary has capacity of "n" and we are currently overflowing. When do we decide to go back to the primary? Is it when all "n" from the primary have been removed, or we don't go back to it until restart (sorry I didn't look at the code yet - this does not seem to have gotten a mention in the design doc).
          Hide
          Roshan Naik added a comment -

          Patch ready for review

          Show
          Roshan Naik added a comment - Patch ready for review
          Hide
          Roshan Naik added a comment -

          minor updates to design doc

          Show
          Roshan Naik added a comment - minor updates to design doc
          Hide
          Roshan Naik added a comment - - edited

          Patch implements the revised design with all features mentioned in the design document.

          Show
          Roshan Naik added a comment - - edited Patch implements the revised design with all features mentioned in the design document.
          Hide
          Roshan Naik added a comment -

          Revised design doc

          Show
          Roshan Naik added a comment - Revised design doc
          Hide
          Roshan Naik added a comment - - edited

          Draft patch for the Spillable Channel on review board for quick review.

          Fully functional. Documentation and unit tests need to be finished up.

          Show
          Roshan Naik added a comment - - edited Draft patch for the Spillable Channel on review board for quick review. Fully functional. Documentation and unit tests need to be finished up.
          Hide
          Roshan Naik added a comment -

          rebasing the old patch just for reference. does not reflect the new design yet.

          Show
          Roshan Naik added a comment - rebasing the old patch just for reference. does not reflect the new design yet.
          Hide
          Roshan Naik added a comment -

          Thats a very interesting suggestion. Thanks. I shall play with that idea also.

          Show
          Roshan Naik added a comment - Thats a very interesting suggestion. Thanks. I shall play with that idea also.
          Hide
          Hari Shreedharan added a comment -

          Thanks for your patience with this Roshan.
          This approach seems fine. It is a good idea to explicitly do the instantiation inside the SC. You can go ahead with that for now I guess.

          But here is some food for thought - The fundamental difference between this channel and the File Channel is the way the transactions get written out. Have you considered inheriting the File Channel and then adding a 2nd data structure (your primary memory channel) and have the decision making happen in the transaction code? I am not sure how feasible it is or even how smart an idea it is, but it might be worth considering.

          Show
          Hari Shreedharan added a comment - Thanks for your patience with this Roshan. This approach seems fine. It is a good idea to explicitly do the instantiation inside the SC. You can go ahead with that for now I guess. But here is some food for thought - The fundamental difference between this channel and the File Channel is the way the transactions get written out. Have you considered inheriting the File Channel and then adding a 2nd data structure (your primary memory channel) and have the decision making happen in the transaction code? I am not sure how feasible it is or even how smart an idea it is, but it might be worth considering.
          Hide
          Juhani Connolly added a comment -

          Seems like a reasonable compromise to me. I think any approach will have issues. 3 would probably be preferable to 4 if it's doable

          Show
          Juhani Connolly added a comment - Seems like a reasonable compromise to me. I think any approach will have issues. 3 would probably be preferable to 4 if it's doable
          Hide
          Roshan Naik added a comment -

          Hari, Juhani, if there is no additional concerns then i shall proceed with this approach. Settling on the general approach now will help us avoid pouring efforts into an unacceptable direction. I shall wait for another day before proceeding.

          Show
          Roshan Naik added a comment - Hari, Juhani, if there is no additional concerns then i shall proceed with this approach. Settling on the general approach now will help us avoid pouring efforts into an unacceptable direction. I shall wait for another day before proceeding.
          Hide
          Mike Percy added a comment -

          Roshan, that sounds good to me. Hari, Juhani, do you guys have any additional feedback on this proposal?

          Thanks,
          Mike

          Show
          Mike Percy added a comment - Roshan, that sounds good to me. Hari, Juhani, do you guys have any additional feedback on this proposal? Thanks, Mike
          Hide
          Roshan Naik added a comment -

          After some reconsideration, I am thinking of pursuing option 3 or 4 noted above. Basically instantiate FC or a derived type inside SC. At the same time ensuring there are no implicit assumptions wrt unspecified guarantees.

          These options seem to strike a balance between the following two primary concerns have been expressed:

          • Avoid changes to config system to enable two channels to discover each other.
          • Avoid forking FC code base inside SC

          Mike/Hari/Juhani ... does that seem like a reasonable middle ground ?

          Show
          Roshan Naik added a comment - After some reconsideration, I am thinking of pursuing option 3 or 4 noted above. Basically instantiate FC or a derived type inside SC. At the same time ensuring there are no implicit assumptions wrt unspecified guarantees. These options seem to strike a balance between the following two primary concerns have been expressed: Avoid changes to config system to enable two channels to discover each other. Avoid forking FC code base inside SC Mike/Hari/Juhani ... does that seem like a reasonable middle ground ?
          Hide
          Roshan Naik added a comment - - edited

          I am not particularly wedded to the current approach. My first attempt based on your suggestion to inline the config of overflow channel in the SC itself. I discovered some serious issues with it and so I pursued the alternative that had been discussed (but w/o consensus). Intent was to get the less contentious core logic working and return quickly to this phase of getting feedback on these shaky parts.

          • Since you mention it, explicitly depending on FC ( i assume by invoking 'new FileChannel()' inside SC ) ... has not been discussed. It might be worth considering.
          • Forking FC / Creating yet another durable channel : This has been talked about and concerns have been with duplication of code (perhaps the most complex piece Flume code). I think Juhani also noted the same. I too am concerned about that. If forked.. each FC bug would have to fixed in 2 places. FC seems to keep evolving, and the for will likely become stale. I wonder, if it makes sense to derive a class from FC and use it as overflow instead.
          • Your unresolved code review Question: We spoke about this when we met at the Flume meetup. On restart the overflow is drained completely first. It is addressed in the design doc under 'recovery from failures' but perhaps not very clearly.
          • Yes, if SC does not have to guarantee strict ordering, then as long as counts in DOQ are correct, things will work fine. Ordering guarantees from overflow are needed only if SC is reqd to provide ordering guarantee. We already have a consensus that SC will not rely on any non-explicit FC guarantees.
          • I totally agree with Hari and yourself on transactionCapacity issue. It makes total sense to expose channel size and capacity at the channel interface. I didn't do it in the first patch as I was afraid it might become a big point of contention. Perhaps a misplaced fear. MemC,FC & JdbcC may need minor tweaks for it. If there are no objections i can go ahead and make this change.

          I think now the only remaining open issue is how to deal with Overflow. Let me list the options that have been put forward so far and some more :

          1) User specifies in config which channel to use as overflow : Current approach and has given me all the grief that i anticipated
          2) Fork FC / create yet another durable FC like store. Then embed it into SC. Some comments have been made on this already.
          3) Explicitly instantiate FC directly inside SC.
          4) Derive another class from FC and embed it into SC.
          5) Based on Mike comment about SinkProcessors... Does it make sense to experiment with the notion of ChannelProcessors ?
          6) Any other ideas ? Now would be THE time to speak.

          Show
          Roshan Naik added a comment - - edited I am not particularly wedded to the current approach. My first attempt based on your suggestion to inline the config of overflow channel in the SC itself. I discovered some serious issues with it and so I pursued the alternative that had been discussed (but w/o consensus). Intent was to get the less contentious core logic working and return quickly to this phase of getting feedback on these shaky parts. Since you mention it, explicitly depending on FC ( i assume by invoking 'new FileChannel()' inside SC ) ... has not been discussed. It might be worth considering. Forking FC / Creating yet another durable channel : This has been talked about and concerns have been with duplication of code (perhaps the most complex piece Flume code). I think Juhani also noted the same. I too am concerned about that. If forked.. each FC bug would have to fixed in 2 places. FC seems to keep evolving, and the for will likely become stale. I wonder, if it makes sense to derive a class from FC and use it as overflow instead. Your unresolved code review Question: We spoke about this when we met at the Flume meetup. On restart the overflow is drained completely first. It is addressed in the design doc under 'recovery from failures' but perhaps not very clearly. Yes, if SC does not have to guarantee strict ordering, then as long as counts in DOQ are correct, things will work fine. Ordering guarantees from overflow are needed only if SC is reqd to provide ordering guarantee. We already have a consensus that SC will not rely on any non-explicit FC guarantees. I totally agree with Hari and yourself on transactionCapacity issue. It makes total sense to expose channel size and capacity at the channel interface. I didn't do it in the first patch as I was afraid it might become a big point of contention. Perhaps a misplaced fear. MemC,FC & JdbcC may need minor tweaks for it. If there are no objections i can go ahead and make this change. I think now the only remaining open issue is how to deal with Overflow. Let me list the options that have been put forward so far and some more : 1) User specifies in config which channel to use as overflow : Current approach and has given me all the grief that i anticipated 2) Fork FC / create yet another durable FC like store. Then embed it into SC. Some comments have been made on this already. 3) Explicitly instantiate FC directly inside SC. 4) Derive another class from FC and embed it into SC. 5) Based on Mike comment about SinkProcessors... Does it make sense to experiment with the notion of ChannelProcessors ? 6) Any other ideas ? Now would be THE time to speak.
          Hide
          Mike Percy added a comment -

          Roshan, thanks a lot for this design documentation.

          Guys, based on my prior reviewboard comment one big problem I have with this implementation is the way that the channels are allowed to know about each other. I am completely against this because it violates separation of responsibilities and encourages unmaintainable spaghetti dependencies between components. What's next, sinks? That is why we have SinkProcessors (so sinks don't have to know about each other). We simply cannot afford to open that Pandora's box. Let the SpillableChannel instantiate its own dependencies and govern their lifecycle.

          If explicitly depending on the file channel is a problem, then let's talk about ways to mitigate that... either forking a copy of the FC code into SC so that FC can evolve separately, or explicitly not relying on ordering in SC, if that is the issue. Therefore SC would not have ordering guarantees. Can the Drain Order Queue survive that situation? It makes me a little nervous that DOQ even exists to be honest... I don't really like it. It seems like a somewhat complex and brittle mechanism for achieving this spill functionality. But I would not block this patch because I'm not in love with the DOQ. And I think if the SC doesn't have to guarantee order then as long as its counts are correct then it should still work. Correct me if I'm wrong.

          If specific non-explicit guarantees of the FC are being relied on then an alternative is to consider a different design that relies on different invariants than the DOQ does. I'm not necessarily advocating for that, I'm just throwing it out there as an option. But I'd be happy with forking the FC and getting this checked in without a total redesign to make progress if that addresses others' concerns.

          My other as-yet unresolved item of code review feedback involved what happens when the agent is stopped then restarted while the channel has events in both the primary and secondary channels. Can this please be addressed as well?

          Additionally, I agree with Hari on the use of transactionCapacity as a poor substitute for a reservation amount on the underlying channels. We need a better way, and if exposing channel size and capacity via an interface will help then I'm all for it.

          Regards,
          Mike

          Show
          Mike Percy added a comment - Roshan, thanks a lot for this design documentation. Guys, based on my prior reviewboard comment one big problem I have with this implementation is the way that the channels are allowed to know about each other. I am completely against this because it violates separation of responsibilities and encourages unmaintainable spaghetti dependencies between components. What's next, sinks? That is why we have SinkProcessors (so sinks don't have to know about each other). We simply cannot afford to open that Pandora's box. Let the SpillableChannel instantiate its own dependencies and govern their lifecycle. If explicitly depending on the file channel is a problem, then let's talk about ways to mitigate that... either forking a copy of the FC code into SC so that FC can evolve separately, or explicitly not relying on ordering in SC, if that is the issue. Therefore SC would not have ordering guarantees. Can the Drain Order Queue survive that situation? It makes me a little nervous that DOQ even exists to be honest... I don't really like it. It seems like a somewhat complex and brittle mechanism for achieving this spill functionality. But I would not block this patch because I'm not in love with the DOQ. And I think if the SC doesn't have to guarantee order then as long as its counts are correct then it should still work. Correct me if I'm wrong. If specific non-explicit guarantees of the FC are being relied on then an alternative is to consider a different design that relies on different invariants than the DOQ does. I'm not necessarily advocating for that, I'm just throwing it out there as an option. But I'd be happy with forking the FC and getting this checked in without a total redesign to make progress if that addresses others' concerns. My other as-yet unresolved item of code review feedback involved what happens when the agent is stopped then restarted while the channel has events in both the primary and secondary channels. Can this please be addressed as well? Additionally, I agree with Hari on the use of transactionCapacity as a poor substitute for a reservation amount on the underlying channels. We need a better way, and if exposing channel size and capacity via an interface will help then I'm all for it. Regards, Mike
          Hide
          Roshan Naik added a comment - - edited
          • I concur that unspecified guarantees should not be depended upon. I can drop that assumption from the tests.
          • I think its very important to not continue to leave the guarantees unspecified. But that's for another Jira.
          • WRT to deferring the decision to commit() time. Let me revisit that issue.

          Instantiationa & config:
          For discussion, I would like to treat instantiation (new up the object) separate from life cycle (start/stop). Since in some cases existing channel instance may get reused during reconfigure. so single instance can have multiple start/stop.

          Overflow does not need to be instantiated or configured before SC! Just like sources, sinks and channels can be instantiated and configured independently in any order. Only start/stop needs to co-ordinated between the two. Also we need to ensure that SC is not able to get a reference to overflow if overflow had configuration errors.

          All components (sinks/sources/channels) get introduced to each other after they are correctly configured. There is already a step to introduce configured sinks and sources to their channels. I have extended that step to introduce channels to each other. The current implementation is a bit permissive and could be tightened up so that SC is limited to obtaining a handle only its overflow (not other channels).

          Life cycle:
          Hari, Correct me if you think its not the case, but i think the current design is in tune with your desire that the SC owns the lifecycle (start/stop) of the overflow. Config subsystem merely instantiates, configures and introduces the two channels to each other. Thereafter it disowns the lifecycle of overflow and lets the SC manage overflow's lifecycle. It retains ownership of SC's lifecycle however. This is nice because we dont have to replicate solutions to some of the config related aspects in SC. We don not have to worry about the order in which channels are instantiated and configured, and at the same time gain control over the order in which the start/stop is called on the SC and its overflow.

          Scribe:
          Juhani, I think spilling policy can be definitely tweaked. Right now I spill into overflow only when primary is full. I like the idea that we can take a cue from the fact that takes() have begun to fail and start spilling early to minimize data loss. There is a throughput concern that I have with Scribe's operating mode where it switches exclusively to using either memory or disk. In SC's design we do not need to wait for the overflow to completely drain before resuming the use of the faster primary. I'll look more into scribe and see what we can leverage.

          • The fsync experiment is something i would like to defer and resolve other open items. It does not look like a blocker and more of a perf tuning thing. does that sound reasonable ?
          Show
          Roshan Naik added a comment - - edited I concur that unspecified guarantees should not be depended upon. I can drop that assumption from the tests. I think its very important to not continue to leave the guarantees unspecified. But that's for another Jira. WRT to deferring the decision to commit() time. Let me revisit that issue. Instantiationa & config : For discussion, I would like to treat instantiation (new up the object) separate from life cycle (start/stop). Since in some cases existing channel instance may get reused during reconfigure. so single instance can have multiple start/stop. Overflow does not need to be instantiated or configured before SC! Just like sources, sinks and channels can be instantiated and configured independently in any order. Only start/stop needs to co-ordinated between the two. Also we need to ensure that SC is not able to get a reference to overflow if overflow had configuration errors. All components (sinks/sources/channels) get introduced to each other after they are correctly configured. There is already a step to introduce configured sinks and sources to their channels. I have extended that step to introduce channels to each other. The current implementation is a bit permissive and could be tightened up so that SC is limited to obtaining a handle only its overflow (not other channels). Life cycle : Hari, Correct me if you think its not the case, but i think the current design is in tune with your desire that the SC owns the lifecycle (start/stop) of the overflow. Config subsystem merely instantiates, configures and introduces the two channels to each other. Thereafter it disowns the lifecycle of overflow and lets the SC manage overflow's lifecycle. It retains ownership of SC's lifecycle however. This is nice because we dont have to replicate solutions to some of the config related aspects in SC. We don not have to worry about the order in which channels are instantiated and configured, and at the same time gain control over the order in which the start/stop is called on the SC and its overflow. Scribe : Juhani, I think spilling policy can be definitely tweaked. Right now I spill into overflow only when primary is full. I like the idea that we can take a cue from the fact that takes() have begun to fail and start spilling early to minimize data loss. There is a throughput concern that I have with Scribe's operating mode where it switches exclusively to using either memory or disk. In SC's design we do not need to wait for the overflow to completely drain before resuming the use of the faster primary. I'll look more into scribe and see what we can leverage. The fsync experiment is something i would like to defer and resolve other open items. It does not look like a blocker and more of a perf tuning thing. does that sound reasonable ?
          Hide
          Juhani Connolly added a comment -

          I would personally prefer seeing a dependence on existing channels than another implementation of something like the file channel and something like the memory channel. The code-base is already getting pretty big, and the interfaces are fixed. The spillable channel shouldn't even know or care about what type the main/sub channel are, just feed them data. While it might not be the most optimal solution performance-wise, I think the cost would be small and it would give us less code to maintain overall. Either approach certainly has its merits.

          Show
          Juhani Connolly added a comment - I would personally prefer seeing a dependence on existing channels than another implementation of something like the file channel and something like the memory channel. The code-base is already getting pretty big, and the interfaces are fixed. The spillable channel shouldn't even know or care about what type the main/sub channel are, just feed them data. While it might not be the most optimal solution performance-wise, I think the cost would be small and it would give us less code to maintain overall. Either approach certainly has its merits.
          Hide
          Hari Shreedharan added a comment -

          Hi Juhani,

          Thanks for you comments. I agree with most of what you have mentioned.

          As to lifecycle management, I don't necessary feel that having a channel own it's sub-channels is a particularly good precedent. I think it would be preferable that we allow the lifecycle manager to return interfaces rather than having components creating other components explicitly. Configuration would have to have some grasp of dependencies though... Sub-channels would need to be instantiated before the "owner"

          I agree with your last statement. Configuration will also need to detect cycles etc so that you don't have a cycle of interdependent components. I don't particularly like the idea of passing references of existing channels to others to use as sub-channels - something that I don't like, but won't block since there seems to have been some consensus regarding this earlier. I frankly think 2 channels within the same one is overkill. I think this channel can be easily implemented by using a mmap-ed file which is never specifically fsync-ed. This might cause some page faults etc., but the page cache management is usually smart enough to not cause this to affect performance a whole lot - this implementation is likely to be faster too (in fact, this is very similar to the File Channel checkpoint class). Using this as a cyclic buffer would probably be as good, and gives the same guarantees as the memory channel (which is what we are targeting in this jira, I suppose?).

          Also, I like the implementation you have mentioned above, though this can be quite tricky to get right.

          Show
          Hari Shreedharan added a comment - Hi Juhani, Thanks for you comments. I agree with most of what you have mentioned. As to lifecycle management, I don't necessary feel that having a channel own it's sub-channels is a particularly good precedent. I think it would be preferable that we allow the lifecycle manager to return interfaces rather than having components creating other components explicitly. Configuration would have to have some grasp of dependencies though... Sub-channels would need to be instantiated before the "owner" I agree with your last statement. Configuration will also need to detect cycles etc so that you don't have a cycle of interdependent components. I don't particularly like the idea of passing references of existing channels to others to use as sub-channels - something that I don't like, but won't block since there seems to have been some consensus regarding this earlier. I frankly think 2 channels within the same one is overkill. I think this channel can be easily implemented by using a mmap-ed file which is never specifically fsync-ed. This might cause some page faults etc., but the page cache management is usually smart enough to not cause this to affect performance a whole lot - this implementation is likely to be faster too (in fact, this is very similar to the File Channel checkpoint class). Using this as a cyclic buffer would probably be as good, and gives the same guarantees as the memory channel (which is what we are targeting in this jira, I suppose?). Also, I like the implementation you have mentioned above, though this can be quite tricky to get right.
          Hide
          Juhani Connolly added a comment -

          I had a look at the design doc and comments so just thought I'd chip in.

          So long as we're only depending on the Channel interface for behaviors, I think we're good, I believe this was the intention in an earlier proposal of this feature.

          I agree with Hari about ordering. It's not a guarantee we enforce in flume, and while nice, I think that it over-complicates things.

          As to lifecycle management, I don't necessary feel that having a channel own it's sub-channels is a particularly good precedent. I think it would be preferable that we allow the lifecycle manager to return interfaces rather than having components creating other components explicitly. Configuration would have to have some grasp of dependencies though... Sub-channels would need to be instantiated before the "owner"

          As to the fsync thing: definitely should be an option. Separate issue though. Making it possible to disable it would be great. Since this depends on in memory data, durability really shouldn't be an issue. If you have data in memory, it doesn't really matter if it's in the memory channel or in the OS file buffer

          One thing you may want to consider is the approach taken by scribed(which has other problems, but the buffer store implementation is very nice):

          • Default to using the main channel
          • Upon a next hop failure(roll back of take transaction in our case), switch to a buffering mode. All data is sent to the buffer channel until recovery. One may want to move the contents of the primary channel to the buffer if maintaining ordering is an objective. This could also reduce loss of data.
          • During buffering mode, puts and takes go to the buffer channel, until it has been drained. Once it has been drained, return to "streaming" mode where operations are performed against the primary channel.
          Show
          Juhani Connolly added a comment - I had a look at the design doc and comments so just thought I'd chip in. So long as we're only depending on the Channel interface for behaviors, I think we're good, I believe this was the intention in an earlier proposal of this feature. I agree with Hari about ordering. It's not a guarantee we enforce in flume, and while nice, I think that it over-complicates things. As to lifecycle management, I don't necessary feel that having a channel own it's sub-channels is a particularly good precedent. I think it would be preferable that we allow the lifecycle manager to return interfaces rather than having components creating other components explicitly. Configuration would have to have some grasp of dependencies though... Sub-channels would need to be instantiated before the "owner" As to the fsync thing: definitely should be an option. Separate issue though. Making it possible to disable it would be great. Since this depends on in memory data, durability really shouldn't be an issue. If you have data in memory, it doesn't really matter if it's in the memory channel or in the OS file buffer One thing you may want to consider is the approach taken by scribed(which has other problems, but the buffer store implementation is very nice): Default to using the main channel Upon a next hop failure(roll back of take transaction in our case), switch to a buffering mode. All data is sent to the buffer channel until recovery. One may want to move the contents of the primary channel to the buffer if maintaining ordering is an objective. This could also reduce loss of data. During buffering mode, puts and takes go to the buffer channel, until it has been drained. Once it has been drained, return to "streaming" mode where operations are performed against the primary channel.
          Hide
          Hari Shreedharan added a comment -

          1) WRT the concern on not depending on another channel, i went down this path since it looked like there was some consensus when i started. What alternative design do you have in mind ?

          2) WRT change in memory/file channel breaking the Spillable channel: Could you expand a bit ? I am not familiar with replay order issue and how it can impact. I dont think there is any intrinsic assumption being made wrt to any specific channel's behavior. Just to be doubly sure, i made sure not to rely on a single type of overflow channel in all the tests. The only material dependency (as far as I can tell) that Spillable Channel has on the overflow is the interface level guarantee that is expected from all channels: that order is maintained in case of single source/sink.
          Do you see any other assumptions/dependencies hiding there ?

          I am sorry, I was not part of the initial discussions - so I was not aware of the consensus aspect. What I am saying is that being dependent on another channel creates an undesired strong coupling between this channel and the other channels. An if there are unit tests in this channel which can break if one of the other channels' behavior is changed, then it is not something that is acceptable. If you look at all our other components, none of them have a dependence on each other (except the RPCClients - that is because the sinks are just glorified RPCClients).

          The reason I would not agree with even the single source/sink replay order is that our interfaces do not really enforce this. This is not really even enforced anywhere in the documentation either. The FileChannel did not even conform to that single source/sink replay order until FLUME-1432. In fact, conforming to that order even in FLUME-1432 was a side-effect of fixing a race condition, and not specifically because it was meant to be handled. At some point, if it is decided this can change again to some other order (maybe a thread based ordering, or or an order in which events in a transaction will all get written out together on commit, rather than getting written out on put and fsynced on commit), then if this channels' tests break, the onus will be on the contributor who submitted the file channel change to fix it - which I do not agree with.

          In summary, I am ok with depending on other channels. What I am not ok with is depending on the behavior of those channels, which are not explicitly guaranteed through interfaces (or even documentation).

          3) WRT reserving capacity on both channels. If you mean that each txn should not reserve capacity on both channels. I agree. And the current implementation does not do that. Or were you by any chance referring to the issue of upfront reservation (at put() time) versus commit() time ?

          I am talking about put v/s commit time. In most cases, transaction capacity is often configured to be much higher than the the max expected in most cases. I would suggest doing a full implementation where there is a transaction outside, and a backing store inside. Once the transaction is about to get committed, then decide where the events go. (It is going to be tricky to do this and avoid doing all the writes at once - the File Channel fsyncs on commit, but writes to OS buffers on every write - so it is possible some data is flushed to disk before explicit fsyncs). This is not a blocker anyway, we can work on it later as well.

          4) WRT to testing with fsyncs removed, i have not pursued it since i felt that would be compromising the durability guarantees. Do you think its useful to do that ?

          I was wondering whether simply adding a config param to change the fsyncs (fsync all files before checkpoint in parallel or something) to optional will give comparable performance to what is being proposed in this jira. I have a feeling it might, since fsyncs are the most expensive part of the file channel, and removing the fsyncs just writes to the in-memory OS buffer and the fsyncs will be taken care of in the background.

          5) WRT "we should make the configuration change". Can you elaborate ? I am not certain which change specifically you are referring to. Or are you referring to the whole config approach ?
          6) WRT lifecycle management and dependencies : After configuration, any channel that is found to be not connected with a source/sink is automatically discarded from the list of Life cycle system managed components. Consequently the Spillable Channel becomes the sole life cycle manager of the overflow channel. Otherwise, yes there would be havoc.

          I just think we should not allow one component to pull a reference to another component in the system. This explicitly breaks the "interact via interfaces" idea. We could make sure the spillable channel own both the channels (and manages the lifecycle of these) - to avoid components which end up being able to access other components owned by the lifecycle manager.

          Hope I made myself clearer this time!

          Show
          Hari Shreedharan added a comment - 1) WRT the concern on not depending on another channel, i went down this path since it looked like there was some consensus when i started. What alternative design do you have in mind ? 2) WRT change in memory/file channel breaking the Spillable channel: Could you expand a bit ? I am not familiar with replay order issue and how it can impact. I dont think there is any intrinsic assumption being made wrt to any specific channel's behavior. Just to be doubly sure, i made sure not to rely on a single type of overflow channel in all the tests. The only material dependency (as far as I can tell) that Spillable Channel has on the overflow is the interface level guarantee that is expected from all channels: that order is maintained in case of single source/sink. Do you see any other assumptions/dependencies hiding there ? I am sorry, I was not part of the initial discussions - so I was not aware of the consensus aspect. What I am saying is that being dependent on another channel creates an undesired strong coupling between this channel and the other channels. An if there are unit tests in this channel which can break if one of the other channels' behavior is changed, then it is not something that is acceptable. If you look at all our other components, none of them have a dependence on each other (except the RPCClients - that is because the sinks are just glorified RPCClients). The reason I would not agree with even the single source/sink replay order is that our interfaces do not really enforce this. This is not really even enforced anywhere in the documentation either. The FileChannel did not even conform to that single source/sink replay order until FLUME-1432 . In fact, conforming to that order even in FLUME-1432 was a side-effect of fixing a race condition, and not specifically because it was meant to be handled. At some point, if it is decided this can change again to some other order (maybe a thread based ordering, or or an order in which events in a transaction will all get written out together on commit, rather than getting written out on put and fsynced on commit), then if this channels' tests break, the onus will be on the contributor who submitted the file channel change to fix it - which I do not agree with. In summary, I am ok with depending on other channels. What I am not ok with is depending on the behavior of those channels, which are not explicitly guaranteed through interfaces (or even documentation). 3) WRT reserving capacity on both channels. If you mean that each txn should not reserve capacity on both channels. I agree. And the current implementation does not do that. Or were you by any chance referring to the issue of upfront reservation (at put() time) versus commit() time ? I am talking about put v/s commit time. In most cases, transaction capacity is often configured to be much higher than the the max expected in most cases. I would suggest doing a full implementation where there is a transaction outside, and a backing store inside. Once the transaction is about to get committed, then decide where the events go. (It is going to be tricky to do this and avoid doing all the writes at once - the File Channel fsyncs on commit, but writes to OS buffers on every write - so it is possible some data is flushed to disk before explicit fsyncs). This is not a blocker anyway, we can work on it later as well. 4) WRT to testing with fsyncs removed, i have not pursued it since i felt that would be compromising the durability guarantees. Do you think its useful to do that ? I was wondering whether simply adding a config param to change the fsyncs (fsync all files before checkpoint in parallel or something) to optional will give comparable performance to what is being proposed in this jira. I have a feeling it might, since fsyncs are the most expensive part of the file channel, and removing the fsyncs just writes to the in-memory OS buffer and the fsyncs will be taken care of in the background. 5) WRT "we should make the configuration change". Can you elaborate ? I am not certain which change specifically you are referring to. Or are you referring to the whole config approach ? 6) WRT lifecycle management and dependencies : After configuration, any channel that is found to be not connected with a source/sink is automatically discarded from the list of Life cycle system managed components. Consequently the Spillable Channel becomes the sole life cycle manager of the overflow channel. Otherwise, yes there would be havoc. I just think we should not allow one component to pull a reference to another component in the system. This explicitly breaks the "interact via interfaces" idea. We could make sure the spillable channel own both the channels (and manages the lifecycle of these) - to avoid components which end up being able to access other components owned by the lifecycle manager. Hope I made myself clearer this time!
          Hide
          Roshan Naik added a comment -

          Thanks Hari.

          1) WRT the concern on not depending on another channel, i went down this path since it looked like there was some consensus when i started. What alternative design do you have in mind ?

          2) WRT change in memory/file channel breaking the Spillable channel: Could you expand a bit ? I am not familiar with replay order issue and how it can impact. I dont think there is any intrinsic assumption being made wrt to any specific channel's behavior. Just to be doubly sure, i made sure not to rely on a single type of overflow channel in all the tests. The only material dependency (as far as I can tell) that Spillable Channel has on the overflow is the interface level guarantee that is expected from all channels: that order is maintained in case of single source/sink.
          Do you see any other assumptions/dependencies hiding there ?

          3) WRT reserving capacity on both channels. If you mean that each txn should not reserve capacity on both channels. I agree. And the current implementation does not do that. Or were you by any chance referring to the issue of upfront reservation (at put() time) versus commit() time ?

          4) WRT to testing with fsyncs removed, i have not pursued it since i felt that would be compromising the durability guarantees. Do you think its useful to do that ?

          5) WRT "we should make the configuration change". Can you elaborate ? I am not certain which change specifically you are referring to. Or are you referring to the whole config approach ?

          6) WRT lifecycle management and dependencies : After configuration, any channel that is found to be not connected with a source/sink is automatically discarded from the list of Life cycle system managed components. Consequently the Spillable Channel becomes the sole life cycle manager of the overflow channel. Otherwise, yes there would be havoc.

          Show
          Roshan Naik added a comment - Thanks Hari. 1) WRT the concern on not depending on another channel, i went down this path since it looked like there was some consensus when i started. What alternative design do you have in mind ? 2) WRT change in memory/file channel breaking the Spillable channel: Could you expand a bit ? I am not familiar with replay order issue and how it can impact. I dont think there is any intrinsic assumption being made wrt to any specific channel's behavior. Just to be doubly sure, i made sure not to rely on a single type of overflow channel in all the tests. The only material dependency (as far as I can tell) that Spillable Channel has on the overflow is the interface level guarantee that is expected from all channels: that order is maintained in case of single source/sink. Do you see any other assumptions/dependencies hiding there ? 3) WRT reserving capacity on both channels. If you mean that each txn should not reserve capacity on both channels. I agree. And the current implementation does not do that. Or were you by any chance referring to the issue of upfront reservation (at put() time) versus commit() time ? 4) WRT to testing with fsyncs removed, i have not pursued it since i felt that would be compromising the durability guarantees. Do you think its useful to do that ? 5) WRT "we should make the configuration change". Can you elaborate ? I am not certain which change specifically you are referring to. Or are you referring to the whole config approach ? 6) WRT lifecycle management and dependencies : After configuration, any channel that is found to be not connected with a source/sink is automatically discarded from the list of Life cycle system managed components. Consequently the Spillable Channel becomes the sole life cycle manager of the overflow channel. Otherwise, yes there would be havoc.
          Hide
          Hari Shreedharan added a comment - - edited

          Roshan,

          Thanks for attaching the design document as well. Sorry it took me this long to get to this one. I reviewed the design document and I have a couple of relatively major concerns:

          1. This channel implicitly depends on the behavior of current channels - the File Channel and Memory Channel. As one of the people who maintain the file channel, I strongly feel this is not the correct thing to do. It is possible that behavior of the File Channel or the Memory Channel could change (This is not without precedent. In FLUME-1437, we did change the replay order). At that point, a change in the behavior of the File Channel or Memory Channel would break unit/integration tests for this channel - which could delay a commit.
          2. I don't think we should make the configuration change. The idea of the Lifecycle manager is to handle all the components and make them independent of each other. Dependencies on other components managed by the Lifecycle system is a bad idea. This also sets a bad precedent. This can lead to patches that make component inter-dependent and depend on the other component being a particular one (example a source using this hook to figure out if it is operating on Memory Channel or File Channel).

          I believe the current design is a bit more complex than it needs to be - due to the handling of more than one transaction. Also reserving transaction capacity on both channels is a bad indicator of where the txn should go. In my experience, people do set the transaction capacity to a value much higher than the average transaction.

          Also, have you tested this against a slightly modified File Channel with all of the fsyncs removed (or commented out)? I'd be interested in seeing the difference in performance at that point. Also, see FLUME-1423 where Denny removed the fsyncs for performance (the performance of the channel has improved even more since then though).

          Show
          Hari Shreedharan added a comment - - edited Roshan, Thanks for attaching the design document as well. Sorry it took me this long to get to this one. I reviewed the design document and I have a couple of relatively major concerns: This channel implicitly depends on the behavior of current channels - the File Channel and Memory Channel. As one of the people who maintain the file channel, I strongly feel this is not the correct thing to do. It is possible that behavior of the File Channel or the Memory Channel could change (This is not without precedent. In FLUME-1437 , we did change the replay order). At that point, a change in the behavior of the File Channel or Memory Channel would break unit/integration tests for this channel - which could delay a commit. I don't think we should make the configuration change. The idea of the Lifecycle manager is to handle all the components and make them independent of each other. Dependencies on other components managed by the Lifecycle system is a bad idea. This also sets a bad precedent. This can lead to patches that make component inter-dependent and depend on the other component being a particular one (example a source using this hook to figure out if it is operating on Memory Channel or File Channel). I believe the current design is a bit more complex than it needs to be - due to the handling of more than one transaction. Also reserving transaction capacity on both channels is a bad indicator of where the txn should go. In my experience, people do set the transaction capacity to a value much higher than the average transaction. Also, have you tested this against a slightly modified File Channel with all of the fsyncs removed (or commented out)? I'd be interested in seeing the difference in performance at that point. Also, see FLUME-1423 where Denny removed the fsyncs for performance (the performance of the channel has improved even more since then though).
          Hide
          Roshan Naik added a comment -

          Looking to revive attention on this one.

          Show
          Roshan Naik added a comment - Looking to revive attention on this one.
          Hide
          Roshan Naik added a comment -

          Adding design doc for current implementation

          Show
          Roshan Naik added a comment - Adding design doc for current implementation
          Hide
          Roshan Naik added a comment -

          Thought of pinging again to see if i find a committer to review this.

          Show
          Roshan Naik added a comment - Thought of pinging again to see if i find a committer to review this.
          Hide
          Hari Shreedharan added a comment -

          I can take a quick look later today, though I can't promise when I can do a full review.

          Show
          Hari Shreedharan added a comment - I can take a quick look later today, though I can't promise when I can do a full review.
          Hide
          Brock Noland added a comment -

          Same as Mike. Hari Shreedharan any time for a review?

          Show
          Brock Noland added a comment - Same as Mike. Hari Shreedharan any time for a review?
          Hide
          Mike Percy added a comment -

          I'm interested but slammed for the next couple days.

          Show
          Mike Percy added a comment - I'm interested but slammed for the next couple days.
          Hide
          Roshan Naik added a comment -

          Havent seen much feedback. Looking to work with a committer to work with on this jira. Any volunteers ?

          Show
          Roshan Naik added a comment - Havent seen much feedback. Looking to work with a committer to work with on this jira. Any volunteers ?
          Hide
          Roshan Naik added a comment -

          See review description for open issues and algorithm used.

          Show
          Roshan Naik added a comment - See review description for open issues and algorithm used.
          Hide
          Roshan Naik added a comment -

          Spillable channel initial version for preliminary feedback.

          Big Open Issue : Overflow channel is specified at config time. this has required a hack to work around the limitations of the config subsystem

          The basic design of how the overflow is maintained and how the order of elements is maintained is previously discussed in this Jira (search for DOQ).

          Show
          Roshan Naik added a comment - Spillable channel initial version for preliminary feedback. Big Open Issue : Overflow channel is specified at config time. this has required a hack to work around the limitations of the config subsystem The basic design of how the overflow is maintained and how the order of elements is maintained is previously discussed in this Jira (search for DOQ).
          Hide
          Roshan Naik added a comment -

          Seeking input ..

          The current configuration system does not look conducive to chaining channels. Here are the config techniques that has been previously talked about :

          1) Out-of-line:

          agent1.channels = channel1 channel2

          agent1.channels.channel1.type = SPILLABLE
          agent1.channels.channel1.overflow = channel2

          agent1.channels.channel2.type = FILE
          agent1.channels.channel2.checkpointDir = /path1
          ...

          The problem here is that ..

          • At the time channel1 is configured, channel2 may not have been instantiated yet. So it is not possible to latch on to an instance of channel2. So it may be better to defer obtaining a reference to the overflow channel at start time.
          • No mechanism to get a reference to one channel from another (in this case, at start time)

          2) Inline: (as suggested by Mike)

          agent1.channels = channel1

          agent1.channels.channel1.type = SPILLABLE
          agent1.channels.channel1.overflowChannel.type = FILE
          agent1.channels.channel1.overflowChannel.checkpointDir = /path1
          agent1.channels.channel1.overflowChannel.dataDirs = /path2
          ...

          The issue here is that the instantiation and configuration of the overflow channel will now have to reside inside SpillableChannel::configure(). This method is not a very conducive place for doing such things.

          3) Hard coding
          Basically hard code the file channel to be the overflow channel. this allows the file channel to be easily instantiated and configured. downside is that it still duplicates the channel instantiation/config logic from AbstractConfigurationProvider.loadChannels()

          Any thoughts ?

          Show
          Roshan Naik added a comment - Seeking input .. The current configuration system does not look conducive to chaining channels. Here are the config techniques that has been previously talked about : 1) Out-of-line: agent1.channels = channel1 channel2 agent1.channels.channel1.type = SPILLABLE agent1.channels.channel1.overflow = channel2 agent1.channels.channel2.type = FILE agent1.channels.channel2.checkpointDir = /path1 ... The problem here is that .. At the time channel1 is configured, channel2 may not have been instantiated yet. So it is not possible to latch on to an instance of channel2. So it may be better to defer obtaining a reference to the overflow channel at start time. No mechanism to get a reference to one channel from another (in this case, at start time) 2) Inline: (as suggested by Mike) agent1.channels = channel1 agent1.channels.channel1.type = SPILLABLE agent1.channels.channel1.overflowChannel.type = FILE agent1.channels.channel1.overflowChannel.checkpointDir = /path1 agent1.channels.channel1.overflowChannel.dataDirs = /path2 ... The issue here is that the instantiation and configuration of the overflow channel will now have to reside inside SpillableChannel::configure(). This method is not a very conducive place for doing such things. 3) Hard coding Basically hard code the file channel to be the overflow channel. this allows the file channel to be easily instantiated and configured. downside is that it still duplicates the channel instantiation/config logic from AbstractConfigurationProvider.loadChannels() Any thoughts ?
          Hide
          Mike Percy added a comment -

          Right. Or we could call it SpillableChannel I guess. I don't have a strong opinion on the name, personally.

          Show
          Mike Percy added a comment - Right. Or we could call it SpillableChannel I guess. I don't have a strong opinion on the name, personally.
          Hide
          Roshan Naik added a comment -

          You mean we conceptually create a new MemChannel++ ? where the ++ part is basically the overflow ability ?

          Show
          Roshan Naik added a comment - You mean we conceptually create a new MemChannel++ ? where the ++ part is basically the overflow ability ?
          Hide
          Mike Percy added a comment -

          Hey Roshan, sounds good to me except I'd recommend trying this out with a brand new channel that delegates to a memory channel, in order to minimize the risk of destabilizing what is a very solid and important core component.

          Show
          Mike Percy added a comment - Hey Roshan, sounds good to me except I'd recommend trying this out with a brand new channel that delegates to a memory channel, in order to minimize the risk of destabilizing what is a very solid and important core component.
          Hide
          Roshan Naik added a comment -

          After some deliberation of Mike's and other feedback, i too feel it may be best to keep this feature simple to start.

          To start with, just add this feature to the Memory Channel only. The spill over channel could be specified using the config technique that Mike suggests.

          The durability semantics are naturally very clear (non durable) since mem channel is being used. it would probably address the majority of the use cases where such a functionality would be needed. in the future a similar enhancement could be made to file channel if durability is necessary. the overflow channel should not really need any modification.

          For memory channel the overflow management algorithm could be the one i proposed above.. it would be able to transaction gurantees and maximize the throughput.

          Show
          Roshan Naik added a comment - After some deliberation of Mike's and other feedback, i too feel it may be best to keep this feature simple to start. To start with, just add this feature to the Memory Channel only. The spill over channel could be specified using the config technique that Mike suggests. The durability semantics are naturally very clear (non durable) since mem channel is being used. it would probably address the majority of the use cases where such a functionality would be needed. in the future a similar enhancement could be made to file channel if durability is necessary. the overflow channel should not really need any modification. For memory channel the overflow management algorithm could be the one i proposed above.. it would be able to transaction gurantees and maximize the throughput.
          Hide
          Roshan Naik added a comment -

          Hi Mike.. yes you are right.. i think it is a downside of that algorithm. i realized the same after posting that comment.

          Show
          Roshan Naik added a comment - Hi Mike.. yes you are right.. i think it is a downside of that algorithm. i realized the same after posting that comment.
          Hide
          Roshan Naik added a comment -

          Since we are talking composite channels, here is another related jira ...

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

          Show
          Roshan Naik added a comment - Since we are talking composite channels, here is another related jira ... https://issues.apache.org/jira/browse/FLUME-1176
          Hide
          Brock Noland added a comment -

          If we move forward with this proposal, I think it'd be great to see a design document.

          Show
          Brock Noland added a comment - If we move forward with this proposal, I think it'd be great to see a design document.
          Hide
          Mike Percy added a comment -

          Hi Roshan, no problem! By "mapping" I mean that the algorithm you describe above requires a queue data structure to be persisted in order to maintain ordering state. Something to consider when mulling over what happens in agent failure cases.

          Show
          Mike Percy added a comment - Hi Roshan, no problem! By "mapping" I mean that the algorithm you describe above requires a queue data structure to be persisted in order to maintain ordering state. Something to consider when mulling over what happens in agent failure cases.
          Hide
          Roshan Naik added a comment -

          Thanks for those valuable thoughts Mike.

          I have described an algorithm for puts/takes here. It should solve the ordering problem, handle transactions correctly and maximize throughput.

          Show
          Roshan Naik added a comment - Thanks for those valuable thoughts Mike. I have described an algorithm for puts/takes here . It should solve the ordering problem, handle transactions correctly and maximize throughput.
          Hide
          Mike Percy added a comment -

          Hi Roshan,
          At this time, we don't have any channels that know about other channels. Likewise for sources and sinks - we don't provide an API to get a handle to a different component. This decoupling is important for reasons related to predictability at deploy time, maintenance (scope of expertise), and debugging. I don't think we should break that.

          Personally I believe a Mem-FC specialized spillable channel would be easier for users to understand and for developers to debug if there were problems. That said, if you want to work on a compound channel for the reasons you mentioned we would need to keep all the configuration of the sub-channels within the compound channel config namespace. The compound channel would have to be responsible for the lifecycle of those underlying objects, etc. That way it will not need a handle into the global config namespace and we remain free to evolve the implementation of Flume's configuration system over time. Example of what I mean:

          agent1.channels.compoundChannel.type = compound
          agent1.channels.compoundChannel.channels = primary overflow
          agent1.channels.compoundChannel.channels.primary.type = MEMORY
          agent1.channels.compoundChannel.channels.primary.capacity = 100000
          agent1.channels.compoundChannel.channels.primary.transactionCapacity = 10000
          agent1.channels.compoundChannel.channels.overflow.type = FILE
          agent1.channels.compoundChannel.channels.overflow.capacity = 10000000
          agent1.channels.compoundChannel.channels.overflow.transactionCapacity = 10000
          

          Regarding how to keep track of which events are in which underlying channels, I'd recommend using an algorithm instead of an explicit mapping. Otherwise to maintain correct ordering in a failure scenario with durable channels, you must make your mapping durable, which will make the implementation more complex and likely slower. Such an algorithm could be something like:

          Puts:

          1. If overflow channel (OC) is empty, put() to primary channel (PC). Otherwise, put() to the OC.

          Takes:

          1. If PC is not empty: take() from the PC.
          2. If PC is empty and OC is not empty, take() as large a batch as possible from the OC and put() it onto the PC. Then go back to #1.

          The potentially tricky bit about this algorithm is dealing with the transactions. You have to make sure you don't do a put() and a take() in the same transaction (same thread) on the same underlying channel, since that's not supported by all of the channel implementations. So if you empty your primary with take()s then you have to just take() from the overflow until the end of that transaction. My idea about moving big batches from overflow to primary would have to happen at the beginning of the outer transaction only, and it's basically just an optimization...

          Finally, if we were going this route, for an initial implementation I'd recommend only supporting a primary and a secondary, not a whole chain of fallbacks, again to keep the design & testing surface area simple at first.

          Show
          Mike Percy added a comment - Hi Roshan, At this time, we don't have any channels that know about other channels. Likewise for sources and sinks - we don't provide an API to get a handle to a different component. This decoupling is important for reasons related to predictability at deploy time, maintenance (scope of expertise), and debugging. I don't think we should break that. Personally I believe a Mem-FC specialized spillable channel would be easier for users to understand and for developers to debug if there were problems. That said, if you want to work on a compound channel for the reasons you mentioned we would need to keep all the configuration of the sub-channels within the compound channel config namespace. The compound channel would have to be responsible for the lifecycle of those underlying objects, etc. That way it will not need a handle into the global config namespace and we remain free to evolve the implementation of Flume's configuration system over time. Example of what I mean: agent1.channels.compoundChannel.type = compound agent1.channels.compoundChannel.channels = primary overflow agent1.channels.compoundChannel.channels.primary.type = MEMORY agent1.channels.compoundChannel.channels.primary.capacity = 100000 agent1.channels.compoundChannel.channels.primary.transactionCapacity = 10000 agent1.channels.compoundChannel.channels.overflow.type = FILE agent1.channels.compoundChannel.channels.overflow.capacity = 10000000 agent1.channels.compoundChannel.channels.overflow.transactionCapacity = 10000 Regarding how to keep track of which events are in which underlying channels, I'd recommend using an algorithm instead of an explicit mapping. Otherwise to maintain correct ordering in a failure scenario with durable channels, you must make your mapping durable, which will make the implementation more complex and likely slower. Such an algorithm could be something like: Puts: If overflow channel (OC) is empty, put() to primary channel (PC). Otherwise, put() to the OC. Takes: If PC is not empty: take() from the PC. If PC is empty and OC is not empty, take() as large a batch as possible from the OC and put() it onto the PC. Then go back to #1. The potentially tricky bit about this algorithm is dealing with the transactions. You have to make sure you don't do a put() and a take() in the same transaction (same thread) on the same underlying channel, since that's not supported by all of the channel implementations. So if you empty your primary with take()s then you have to just take() from the overflow until the end of that transaction. My idea about moving big batches from overflow to primary would have to happen at the beginning of the outer transaction only, and it's basically just an optimization... Finally, if we were going this route, for an initial implementation I'd recommend only supporting a primary and a secondary, not a whole chain of fallbacks, again to keep the design & testing surface area simple at first.
          Hide
          Roshan Naik added a comment -

          Continuing the discussion...

          I spent some time studying the discussions in the jiras related to solving the problem of spilling over (and/or failover). I think failover and spillover should not be conflated to be the same problem ... even though it may be possible to address them both in the same solution.

          There is a consensus that the problem worth addressing. There are concerns hovering around these dimensions.

          1) complexity of implementation and configuration. also potentially enhancements to existing interfaces
          2) complexity of testing
          3) Ensuring transaction guarantees are preserved and its weakness/strength level
          4) Defining the durability level (durable or not) of the final solution .. this is simple IMHO
          5) Efficiency of the solution (batching requests during when spilling over)
          6) Flexibility

          So far the solutions discussed along with their concerns ..

          1) FailOver Sink processor - has issues with retaining transaction guarantees (Reference)

          2) Mechanisms for Composing Existing Channels (1201 and my proposal) - Flexible but has complexities in regards to testing (mixed opinions here), implementation & determining durability See

          3) Spillable Channel - Limited functionality but easier to test and determine transaction+durability semantics.

          My thoughts...
          The concerns related to mechanisms for composing channels is largely centered around complexities. I feel some of them are not true.

          Testing a composition mechanism is not as complex as it has been feared for reasons stated here.

          In a pluggable system (like rest of flume) we rely on guarantees from the interface itself. There is no need to test all combination of all possible channels for testing. Just like it does not make sense to test all combinations of sink/channel/source/interceptors/sink-processors in Flume.

          Implementation of a composite mechanisms would also be simpler. It would be focussed only around issues involved in stitching channels. Not in actually providing a robust backing store.

          Spillover channel (Mem + File) seems a little too specialized .. for instance it does not provide durability for users if needed. It is nice to allow the primary channel to be on a fast smaller durable store (like SSDs) and overflow into a another slower durable store (like hard disk /jdbc)

          the following general strategy for compounding channels seems worth discussing ..

          agent1.channels.compoundChannel.type = compound
          agent1.channels.compoundChannel.1 = memChannel1
          agent1.channels.compoundChannel.2 = fileChannel1
          agent1.channels.compoundChannel.3 = jdbcChannel1

          agent1.channels.compoundChannel.1.overflowBatchSize = 100 # batch size when spilling into fileChannel1
          agent1.channels.compoundChannel.2.overflowBatchSize = 1000 # batch size when spilling into jdbcChannel1

          Show
          Roshan Naik added a comment - Continuing the discussion... I spent some time studying the discussions in the jiras related to solving the problem of spilling over (and/or failover). I think failover and spillover should not be conflated to be the same problem ... even though it may be possible to address them both in the same solution. There is a consensus that the problem worth addressing. There are concerns hovering around these dimensions. 1) complexity of implementation and configuration. also potentially enhancements to existing interfaces 2) complexity of testing 3) Ensuring transaction guarantees are preserved and its weakness/strength level 4) Defining the durability level (durable or not) of the final solution .. this is simple IMHO 5) Efficiency of the solution (batching requests during when spilling over) 6) Flexibility So far the solutions discussed along with their concerns .. 1) FailOver Sink processor - has issues with retaining transaction guarantees ( Reference ) 2) Mechanisms for Composing Existing Channels ( 1201 and my proposal ) - Flexible but has complexities in regards to testing ( mixed opinions here ), implementation & determining durability See 3) Spillable Channel - Limited functionality but easier to test and determine transaction+durability semantics. My thoughts... The concerns related to mechanisms for composing channels is largely centered around complexities. I feel some of them are not true. Testing a composition mechanism is not as complex as it has been feared for reasons stated here . In a pluggable system (like rest of flume) we rely on guarantees from the interface itself. There is no need to test all combination of all possible channels for testing. Just like it does not make sense to test all combinations of sink/channel/source/interceptors/sink-processors in Flume. Implementation of a composite mechanisms would also be simpler. It would be focussed only around issues involved in stitching channels. Not in actually providing a robust backing store. Spillover channel (Mem + File) seems a little too specialized .. for instance it does not provide durability for users if needed. It is nice to allow the primary channel to be on a fast smaller durable store (like SSDs) and overflow into a another slower durable store (like hard disk /jdbc) the following general strategy for compounding channels seems worth discussing .. agent1.channels.compoundChannel.type = compound agent1.channels.compoundChannel.1 = memChannel1 agent1.channels.compoundChannel.2 = fileChannel1 agent1.channels.compoundChannel.3 = jdbcChannel1 agent1.channels.compoundChannel.1.overflowBatchSize = 100 # batch size when spilling into fileChannel1 agent1.channels.compoundChannel.2.overflowBatchSize = 1000 # batch size when spilling into jdbcChannel1
          Hide
          Jarek Jarcec Cecho added a comment -

          Hi Roshan,
          you're right that some higher general logic like channel overflow would be interesting. I'm just worried that such logic would be hard to test and even harder to debug on users boxes.

          I would recommend taking a look on linked linked issue FLUME-1045 where we've discussed similar concept and the "general" consensus seems to be that SpillableChannel is quite limited in functionality when compared to the general solutions, but it should solve most of the use cases and still be testable and debugable.

          Jarcec

          Show
          Jarek Jarcec Cecho added a comment - Hi Roshan, you're right that some higher general logic like channel overflow would be interesting. I'm just worried that such logic would be hard to test and even harder to debug on users boxes. I would recommend taking a look on linked linked issue FLUME-1045 where we've discussed similar concept and the "general" consensus seems to be that SpillableChannel is quite limited in functionality when compared to the general solutions, but it should solve most of the use cases and still be testable and debugable. Jarcec
          Hide
          Bernardo de Seabra added a comment -

          I like this approach (quite popular with Scribe) but my only concern is around performance. You would get unexpected/unpredictable performance impact on disk IO which could be (in our case it would be) impacting your application if flume and the app are sharing the same disk. It's a tradeoff.

          Show
          Bernardo de Seabra added a comment - I like this approach (quite popular with Scribe) but my only concern is around performance. You would get unexpected/unpredictable performance impact on disk IO which could be (in our case it would be) impacting your application if flume and the app are sharing the same disk. It's a tradeoff.
          Hide
          Hari Shreedharan added a comment -

          Roshan - that might be a good thing to do - but there was a discussion about a compound channel several months ago, and I believe the consensus was that it would be too complex to write and even more complex to test. But feel free to file a jira - I am sure there will be a healthy discussion.

          Show
          Hari Shreedharan added a comment - Roshan - that might be a good thing to do - but there was a discussion about a compound channel several months ago, and I believe the consensus was that it would be too complex to write and even more complex to test. But feel free to file a jira - I am sure there will be a healthy discussion.
          Hide
          Roshan Naik added a comment -

          actually i think.. this proposal, if acceptable, would have to be a different jira. since the current jira is about introducing a new channel.

          Show
          Roshan Naik added a comment - actually i think.. this proposal, if acceptable, would have to be a different jira. since the current jira is about introducing a new channel.
          Hide
          Roshan Naik added a comment -

          Looks like this jira is up for grabs ??
          If there is agreement that my proposal is a good way forward I would like to pick it up.
          Thoughts ?

          Show
          Roshan Naik added a comment - Looks like this jira is up for grabs ?? If there is agreement that my proposal is a good way forward I would like to pick it up. Thoughts ?
          Hide
          Roshan Naik added a comment -

          apologies for email storm created by multiple edits to my prev comment.

          Show
          Roshan Naik added a comment - apologies for email storm created by multiple edits to my prev comment.
          Hide
          Roshan Naik added a comment - - edited

          I agree Scribe's policy is possibly sub optimal. It is better to prioritize the parent channel whenever it has spare capacity and still maintain order. To achieve this I have a simple algorithm in mind...

          The parent channel maintains a 'drain order' queue of signed numbers which indicates at anytime the order in which the items in it and its overflow channel should be drained. For instance the following numbers in that queue [3,-2,6,-1] indicate the following drain order:

          • drain 3 from self
          • then drain 2 from overflow
          • then 6 from self
          • then 1 from overflow

          The channel's put() will update its drain order queue (DOQ) as follows:

           
            if(I have capacity) {
               + add event to my list
               + if last element in DOQ is +ve then increment it
               + else push +1 to DOQ
            } else {
               + Call put() on overflow
               + if last element in DOQ is -ve then decrement it
               + else push -1 to DOQ
            }
          

          I think the take() should be obvious.

          Obviously corner cases like empty self and empty overflow need to be handled appropriately.. but this is just capturing the idea.

          Show
          Roshan Naik added a comment - - edited I agree Scribe's policy is possibly sub optimal. It is better to prioritize the parent channel whenever it has spare capacity and still maintain order. To achieve this I have a simple algorithm in mind... The parent channel maintains a 'drain order' queue of signed numbers which indicates at anytime the order in which the items in it and its overflow channel should be drained. For instance the following numbers in that queue [3,-2,6,-1] indicate the following drain order: drain 3 from self then drain 2 from overflow then 6 from self then 1 from overflow The channel's put() will update its drain order queue (DOQ) as follows: if (I have capacity) { + add event to my list + if last element in DOQ is +ve then increment it + else push +1 to DOQ } else { + Call put() on overflow + if last element in DOQ is -ve then decrement it + else push -1 to DOQ } I think the take() should be obvious. Obviously corner cases like empty self and empty overflow need to be handled appropriately.. but this is just capturing the idea.
          Hide
          Juhani Connolly added a comment -

          yes, but when does the parent channel start reading entries from the backup? Simplest solution is once it's been emptied, but that could introduce a very large delay as it refills.

          The way scribe deals with this is by appending everything to the backup store until it has been emptied, then returning back to normal streaming operation off the primary store(this also has the advantage of retaining order)

          Show
          Juhani Connolly added a comment - yes, but when does the parent channel start reading entries from the backup? Simplest solution is once it's been emptied, but that could introduce a very large delay as it refills. The way scribe deals with this is by appending everything to the backup store until it has been emptied, then returning back to normal streaming operation off the primary store(this also has the advantage of retaining order)
          Hide
          Roshan Naik added a comment -

          The parent channel's put()/take() will be the source/sink for its overflow channel.

          For the special case of just supporting it in memory channel, I think it could easily employ whatever policy the SpillableChannel would have used.

          For the more general case of making this a cross-cutting feature available to all channels with the ability to chain, i would conjecture, it may be possible to use the same policy at each level of the chain. So this policy could be pushed into the common base class for channels.

          Show
          Roshan Naik added a comment - The parent channel's put()/take() will be the source/sink for its overflow channel. For the special case of just supporting it in memory channel, I think it could easily employ whatever policy the SpillableChannel would have used. For the more general case of making this a cross-cutting feature available to all channels with the ability to chain, i would conjecture, it may be possible to use the same policy at each level of the chain. So this policy could be pushed into the common base class for channels.
          Hide
          Juhani Connolly added a comment -

          Interesting suggestion... When would you suggest that the overflow channels contents be read, and by what component?

          Show
          Juhani Connolly added a comment - Interesting suggestion... When would you suggest that the overflow channels contents be read, and by what component?
          Hide
          Roshan Naik added a comment -

          of course it implies that ... the overflow channel could itself overflow into another channel.

          Show
          Roshan Naik added a comment - of course it implies that ... the overflow channel could itself overflow into another channel.
          Hide
          Roshan Naik added a comment -

          I dont see this option discussed but it seems interesting (and IMO avoids some of the issues in sink triggered spooling as discussed in FLUME-1045).

          Basically instead of adding another Spillable channel which is logically a composite of mem & file channels, we could add a config directive to Memory Channel such as:

          agent1.channels.memChannel1.overflow = fileChannel1

          Basically, there would be a preconfigured file channel (or jdbc or some custom channel) into which memory channel would simply spill over events into when capacity has been reached. There should be no other sources or sinks tied to an overflow channel.

          Ideally any channel should be able to use another channel for overflow.

          Show
          Roshan Naik added a comment - I dont see this option discussed but it seems interesting (and IMO avoids some of the issues in sink triggered spooling as discussed in FLUME-1045 ). Basically instead of adding another Spillable channel which is logically a composite of mem & file channels, we could add a config directive to Memory Channel such as: agent1.channels.memChannel1.overflow = fileChannel1 Basically, there would be a preconfigured file channel (or jdbc or some custom channel) into which memory channel would simply spill over events into when capacity has been reached. There should be no other sources or sinks tied to an overflow channel. Ideally any channel should be able to use another channel for overflow.
          Hide
          Jarek Jarcec Cecho added a comment -

          I'm still highly interested in this, but I do not have enough time to start working on it at moment. I would check with Patrick as he was considering to work on it.

          Show
          Jarek Jarcec Cecho added a comment - I'm still highly interested in this, but I do not have enough time to start working on it at moment. I would check with Patrick as he was considering to work on it.
          Hide
          Mike Percy added a comment -

          I don't know of anyone actively working on this...

          Show
          Mike Percy added a comment - I don't know of anyone actively working on this...
          Hide
          Rahul Ravindran added a comment -

          Is there a timeline on when this new channel would be out?

          Show
          Rahul Ravindran added a comment - Is there a timeline on when this new channel would be out?
          Hide
          Juhani Connolly added a comment -

          Since the channel is not aware of the state of sinks, I think Jareks proposed method sounds good.

          In another place, it was pointed out that we cannot just change the interface as it will break peoples custom components.

          However I think you can get away with a similar method to configurable now. Add a "CapacityPollable" interface or something, and check whether the channel implements it, polling if it exists. In the case of non-existence you will just have to rely on catching exceptions as an indicator of problems)

          Show
          Juhani Connolly added a comment - Since the channel is not aware of the state of sinks, I think Jareks proposed method sounds good. In another place, it was pointed out that we cannot just change the interface as it will break peoples custom components. However I think you can get away with a similar method to configurable now. Add a "CapacityPollable" interface or something, and check whether the channel implements it, polling if it exists. In the case of non-existence you will just have to rely on catching exceptions as an indicator of problems)
          Hide
          Jarek Jarcec Cecho added a comment -

          I see pretty active discussion in FLUME-1045, so let me jump in and share my thoughts. Please feel free to comment or make objections (or suggest completely something else):

          • I wanted to add new public methods to Channel interface to get current number of items and maximal number of items (if applicable)
          • I wanted to use those methods to create new SpillableChannel that would wrap both memory and file channel.
          • I wanted this channel to be based only on public interface of underlying channels. It definitely should not use any internal details (that's why I wanted to add those new methods in first note).
          • I was thinking about implementing the logic as Inder proposed - put all events into memory. When memory gets full, move all events in one transaction to disk (e.g. no flushing issues). On reads, serve firstly events from disk (if there are any) and then from memory.

          Couple of notes:

          • I believe that this channel will not introduce any significant issues as long as it will be based only on public interfaces of underlying channels.
          • This channel could lose data, however it would loose only events currently stored in memory (user might use this knowledge to set up the memory size appropriately).
          • Spilling data from memory to disk could be visible to a user. "Once upon a time" in case that the memory would get full, one transaction would be frozen until all events would be migrated from memory to disk. Please note that this issue could be solved by another thread that would do this on background, however for simplicity I wanted to avoid that in the first implementation.

          Jarcec

          Show
          Jarek Jarcec Cecho added a comment - I see pretty active discussion in FLUME-1045 , so let me jump in and share my thoughts. Please feel free to comment or make objections (or suggest completely something else): I wanted to add new public methods to Channel interface to get current number of items and maximal number of items (if applicable) I wanted to use those methods to create new SpillableChannel that would wrap both memory and file channel. I wanted this channel to be based only on public interface of underlying channels. It definitely should not use any internal details (that's why I wanted to add those new methods in first note). I was thinking about implementing the logic as Inder proposed - put all events into memory. When memory gets full, move all events in one transaction to disk (e.g. no flushing issues). On reads, serve firstly events from disk (if there are any) and then from memory. Couple of notes: I believe that this channel will not introduce any significant issues as long as it will be based only on public interfaces of underlying channels. This channel could lose data, however it would loose only events currently stored in memory (user might use this knowledge to set up the memory size appropriately). Spilling data from memory to disk could be visible to a user. "Once upon a time" in case that the memory would get full, one transaction would be frozen until all events would be migrated from memory to disk. Please note that this issue could be solved by another thread that would do this on background, however for simplicity I wanted to avoid that in the first implementation. Jarcec
          Hide
          Jarek Jarcec Cecho added a comment -

          I believe that we agreed in FLUME-1045 to create an separate channel that is actually described in this JIRA. So I believe that solving this issue will also solve FLUME-1045. Does that make sense?

          Jarcec

          Show
          Jarek Jarcec Cecho added a comment - I believe that we agreed in FLUME-1045 to create an separate channel that is actually described in this JIRA. So I believe that solving this issue will also solve FLUME-1045 . Does that make sense? Jarcec
          Hide
          Venkatesh Seetharam added a comment -

          Does this mean there is no effort going into FLUME-1045?

          Show
          Venkatesh Seetharam added a comment - Does this mean there is no effort going into FLUME-1045 ?
          Hide
          Jarek Jarcec Cecho added a comment -

          Good luck with coding Patrik - I'll happy to help with review!

          Jarcec

          Show
          Jarek Jarcec Cecho added a comment - Good luck with coding Patrik - I'll happy to help with review! Jarcec
          Hide
          NO NAME added a comment -

          I grabbed it, thanks! Will take a look at discussion in the related JIRA as well.

          Show
          NO NAME added a comment - I grabbed it, thanks! Will take a look at discussion in the related JIRA as well.
          Hide
          Jarek Jarcec Cecho added a comment - - edited

          Thanks Denny and Patrick for support!

          This ticket is currently assigned to me and I still do have intent to work on it. However I doubt that I'll have time for at least month and half. So please, if anyone have interest in implementing this feature, feel free to reassign this JIRA.

          Jarcec

          Show
          Jarek Jarcec Cecho added a comment - - edited Thanks Denny and Patrick for support! This ticket is currently assigned to me and I still do have intent to work on it. However I doubt that I'll have time for at least month and half. So please, if anyone have interest in implementing this feature, feel free to reassign this JIRA. Jarcec
          Hide
          Denny Ye added a comment -

          That's great and useful when Flume cannot reaches to HDFS or other destination. Also it's the same concept in Scribe with named 'primary store' and 'secondary store'. Wish any implementations.

          Show
          Denny Ye added a comment - That's great and useful when Flume cannot reaches to HDFS or other destination. Also it's the same concept in Scribe with named 'primary store' and 'secondary store'. Wish any implementations.
          Hide
          NO NAME added a comment -

          This seems like a very sensible proposal and I think it makes sense to spill to a disk when memory is full.

          We are very likely to see busty rate behaviors for various sinks (e.g. HDFS sink) and it makes sense to have more elastic buffering happening in intermediate agent nodes. I would imagine several deployments would want to use this if it were included.

          I almost think disk spilling should just be a feature of the existing memory channel. We could have an option of how much to spill to disk uf memory is saturated - with the default of 0. If you chose some larger amount it will use a bounded amount of disk space. Could go either way on this last point, but overall think it is a great idea.

          Show
          NO NAME added a comment - This seems like a very sensible proposal and I think it makes sense to spill to a disk when memory is full. We are very likely to see busty rate behaviors for various sinks (e.g. HDFS sink) and it makes sense to have more elastic buffering happening in intermediate agent nodes. I would imagine several deployments would want to use this if it were included. I almost think disk spilling should just be a feature of the existing memory channel. We could have an option of how much to spill to disk uf memory is saturated - with the default of 0. If you chose some larger amount it will use a bounded amount of disk space. Could go either way on this last point, but overall think it is a great idea.
          Hide
          Jarek Jarcec Cecho added a comment -

          Thank you Inder for your comment, I've missed that JIRA.

          Jarcec

          Show
          Jarek Jarcec Cecho added a comment - Thank you Inder for your comment, I've missed that JIRA. Jarcec
          Hide
          Inder SIngh added a comment -

          Jarek,

          we had filed a similar JIRA - https://issues.apache.org/jira/browse/FLUME-1045
          This has certain discussion.

          Show
          Inder SIngh added a comment - Jarek, we had filed a similar JIRA - https://issues.apache.org/jira/browse/FLUME-1045 This has certain discussion.
          Hide
          Jarek Jarcec Cecho added a comment -

          I've assigned the issue to me to demonstrate will to work on that.

          However I would like to firstly open discussion whether it make really sense to implement such channel and what other demands/use cases would be.

          Show
          Jarek Jarcec Cecho added a comment - I've assigned the issue to me to demonstrate will to work on that. However I would like to firstly open discussion whether it make really sense to implement such channel and what other demands/use cases would be.

            People

            • Assignee:
              Roshan Naik
              Reporter:
              Jarek Jarcec Cecho
            • Votes:
              1 Vote for this issue
              Watchers:
              18 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development