Flume
  1. Flume
  2. FLUME-1285

FileChannel has a dependency on Hadoop IO classes

    Details

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

      Description

      The FileChannel has a dependency on Hadoop IO classes. This may not be necessary.

      1. FLUME-1285.20130429.patch
        24 kB
        Israel Ekpo
      2. FLUME-1285-1.patch
        20 kB
        Christopher Nagy
      3. FLUME-1285-3.patch
        9 kB
        Mike Percy
      4. FLUME-1285-4.patch
        15 kB
        Mike Percy

        Issue Links

          Activity

          Hide
          Hudson added a comment -

          Integrated in flume-trunk #454 (See https://builds.apache.org/job/flume-trunk/454/)
          FLUME-1285 - FileChannel has a dependency on Hadoop IO classes (Revision 97170aebf4240e1f7c628650b621f3f4c3178dc0)

          Result = FAILURE
          brock : http://git-wip-us.apache.org/repos/asf/flume/repo?p=flume.git&a=commit&h=97170aebf4240e1f7c628650b621f3f4c3178dc0
          Files :

          • flume-ng-channels/flume-file-channel/pom.xml
          • flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/WritableUtils.java
          • flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestUtils.java
          • flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/TransactionEventRecord.java
          • flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Writable.java
          • flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FlumeEvent.java
          Show
          Hudson added a comment - Integrated in flume-trunk #454 (See https://builds.apache.org/job/flume-trunk/454/ ) FLUME-1285 - FileChannel has a dependency on Hadoop IO classes (Revision 97170aebf4240e1f7c628650b621f3f4c3178dc0) Result = FAILURE brock : http://git-wip-us.apache.org/repos/asf/flume/repo?p=flume.git&a=commit&h=97170aebf4240e1f7c628650b621f3f4c3178dc0 Files : flume-ng-channels/flume-file-channel/pom.xml flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/WritableUtils.java flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestUtils.java flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/TransactionEventRecord.java flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Writable.java flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FlumeEvent.java
          Hide
          Brock Noland added a comment -

          Thank you Israel and Christopher!! Committed to trunk and 1.4!

          Show
          Brock Noland added a comment - Thank you Israel and Christopher!! Committed to trunk and 1.4!
          Hide
          Brock Noland added a comment -

          Running tests.

          Show
          Brock Noland added a comment - Running tests.
          Hide
          Mike Percy added a comment -

          I'd like to get this in for 1.4. Brock do you have time to review / commit this? Unit tests pass and it looks OK to me too.

          Christopher / Israel, apologies for just jumping in here, but I want to roll an RC of 1.4 today if possible and this is something that should go in if we can make it happen.

          Show
          Mike Percy added a comment - I'd like to get this in for 1.4. Brock do you have time to review / commit this? Unit tests pass and it looks OK to me too. Christopher / Israel, apologies for just jumping in here, but I want to roll an RC of 1.4 today if possible and this is something that should go in if we can make it happen.
          Hide
          Mike Percy added a comment -

          Oops, files were missing from the previous patch.

          Show
          Mike Percy added a comment - Oops, files were missing from the previous patch.
          Hide
          Brock Noland added a comment -

          This looks pretty good. Anyone think we shouldn't put it in 1.4?

          Show
          Brock Noland added a comment - This looks pretty good. Anyone think we shouldn't put it in 1.4?
          Hide
          Mike Percy added a comment -

          Attaching minimal patch against latest trunk

          Show
          Mike Percy added a comment - Attaching minimal patch against latest trunk
          Hide
          Hari Shreedharan added a comment -

          Israel Ekpo, I do realize that you picked it up since it looked like no one was working on this. But like Brock said, it is common practice to ask before you pick it up - it is possible that the assignee still plans to or has a partly completed patch which he may want to complete (or contribute).

          Show
          Hari Shreedharan added a comment - Israel Ekpo , I do realize that you picked it up since it looked like no one was working on this. But like Brock said, it is common practice to ask before you pick it up - it is possible that the assignee still plans to or has a partly completed patch which he may want to complete (or contribute).
          Hide
          Brock Noland added a comment -

          Hey,

          No worries. I think Hari is just saying before we re-assign a JIRA we typically say something like "hey are you still working on this, I'd like to take it up" and then given no response for a few days just re-assign... Not a huge deal, just a common practice.

          Thanks for taking this up!!

          Brock

          Show
          Brock Noland added a comment - Hey, No worries. I think Hari is just saying before we re-assign a JIRA we typically say something like "hey are you still working on this, I'd like to take it up" and then given no response for a few days just re-assign... Not a huge deal, just a common practice. Thanks for taking this up!! Brock
          Hide
          Israel Ekpo added a comment - - edited

          Hari Shreedharan,

          Thank you for the feedback. I will keep this in mind before attempting to work on future tasks/issues.

          While I agree that is not efficient to duplicate effort or waste any committers' or contributors' time on the same task, I honestly do not believe I was in violation of this etiquette when I took on this task.

          It seemed a lot of users were experiencing problems with the missing dependency and I decided to help out after the discussion below in the mailing list:

          http://bit.ly/10Or4G9

          The JIRA issue remained unassigned since it was created on June 2012 and Christopher Nagy submitted a patch about 7 months ago in September 2012.

          Brock Noland upgraded the priority to Critical 6 months ago since there was already a release with the Hadoop dependency.

          There was no further activity on the issue since then.

          I decided to assign it to myself and work on it since it appeared the other developers were busy at the moment, based on comments made during the discussion, and I had some time to contribute.

          Show
          Israel Ekpo added a comment - - edited Hari Shreedharan , Thank you for the feedback. I will keep this in mind before attempting to work on future tasks/issues. While I agree that is not efficient to duplicate effort or waste any committers' or contributors' time on the same task, I honestly do not believe I was in violation of this etiquette when I took on this task. It seemed a lot of users were experiencing problems with the missing dependency and I decided to help out after the discussion below in the mailing list: http://bit.ly/10Or4G9 The JIRA issue remained unassigned since it was created on June 2012 and Christopher Nagy submitted a patch about 7 months ago in September 2012. Brock Noland upgraded the priority to Critical 6 months ago since there was already a release with the Hadoop dependency. There was no further activity on the issue since then. I decided to assign it to myself and work on it since it appeared the other developers were busy at the moment, based on comments made during the discussion, and I had some time to contribute.
          Hide
          Hari Shreedharan added a comment -

          Israel Ekpo, thanks for updating the patch. It is usually good etiquette to ask the person who is working on the jira before you update the patch or submit a new one (or reassign the jira to yourself). The reason is that it is possible that person may be working on it and maybe planning to submit a patch. It is better not to duplicate work, else his time or yours would be wasted. Please see: https://cwiki.apache.org/FLUME/how-to-contribute.html ("Make sure there is a jira" section).

          Show
          Hari Shreedharan added a comment - Israel Ekpo , thanks for updating the patch. It is usually good etiquette to ask the person who is working on the jira before you update the patch or submit a new one (or reassign the jira to yourself). The reason is that it is possible that person may be working on it and maybe planning to submit a patch. It is better not to duplicate work, else his time or yours would be wasted. Please see: https://cwiki.apache.org/FLUME/how-to-contribute.html ("Make sure there is a jira" section).
          Hide
          Israel Ekpo added a comment - - edited
          Show
          Israel Ekpo added a comment - - edited Ready for review https://reviews.apache.org/r/10829/
          Hide
          Israel Ekpo added a comment -

          Modified and Applied Fix from Christopher Nagy to 1.4 branch

          Tested on Ubuntu 12.04 with 1.6.0_33 64-bit

          modified: flume-ng-channels/flume-file-channel/pom.xml
          modified: flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FlumeEvent.java
          modified: flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/TransactionEventRecord.java
          new file: flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Writable.java
          new file: flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/WritableUtils.java
          modified: flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestUtils.java

          • Unit Tests for Flume NG file-based channel were all successful with these changes.
          • Created and deployed local build successfully with the following settings

          agent.sources = seqGenSrc
          agent.channels = fileChannel
          agent.sinks = loggerSink

          agent.sources.seqGenSrc.type = seq
          agent.sources.seqGenSrc.channels = fileChannel

          agent.channels.fileChannel.type = file
          agent.channels.c1.checkpointDir = /flume-staging/file-channel/checkpoint
          agent.channels.c1.dataDirs = /flume-staging/file-channel/data

          agent.sinks.loggerSink.type = logger
          agent.sinks.loggerSink.channel = fileChannel

          Agent was started as follows

          $ bin/flume-ng agent --conf-file conf/flume.properties --conf conf --name agent

          Agent started up and loaded File Channel successfully.

          Agent was also able to shut down successfully.

          Show
          Israel Ekpo added a comment - Modified and Applied Fix from Christopher Nagy to 1.4 branch Tested on Ubuntu 12.04 with 1.6.0_33 64-bit modified: flume-ng-channels/flume-file-channel/pom.xml modified: flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FlumeEvent.java modified: flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/TransactionEventRecord.java new file: flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Writable.java new file: flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/WritableUtils.java modified: flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestUtils.java Unit Tests for Flume NG file-based channel were all successful with these changes. Created and deployed local build successfully with the following settings agent.sources = seqGenSrc agent.channels = fileChannel agent.sinks = loggerSink agent.sources.seqGenSrc.type = seq agent.sources.seqGenSrc.channels = fileChannel agent.channels.fileChannel.type = file agent.channels.c1.checkpointDir = /flume-staging/file-channel/checkpoint agent.channels.c1.dataDirs = /flume-staging/file-channel/data agent.sinks.loggerSink.type = logger agent.sinks.loggerSink.channel = fileChannel Agent was started as follows $ bin/flume-ng agent --conf-file conf/flume.properties --conf conf --name agent Agent started up and loaded File Channel successfully. Agent was also able to shut down successfully.
          Hide
          Israel Ekpo added a comment -

          Patch from Christopher Nagy is being applied to flume-1.4

          Will be uploaded soon and made available for review

          Show
          Israel Ekpo added a comment - Patch from Christopher Nagy is being applied to flume-1.4 Will be uploaded soon and made available for review
          Hide
          Brock Noland added a comment -

          Changed to critical since there is already a release with the hadoop class dependency.

          Show
          Brock Noland added a comment - Changed to critical since there is already a release with the hadoop class dependency.
          Hide
          Christopher Nagy added a comment -

          FLUME-1285-1.patch includes all the changes described above and there are two binary files that are used to test that the code reads FlumeEvent's serialized before this patch.

          Show
          Christopher Nagy added a comment - FLUME-1285 -1.patch includes all the changes described above and there are two binary files that are used to test that the code reads FlumeEvent's serialized before this patch.
          Hide
          Brock Noland added a comment -

          Yeah I thought about doing the same. Do you have a patch?

          Show
          Brock Noland added a comment - Yeah I thought about doing the same. Do you have a patch?
          Hide
          Christopher Nagy added a comment -

          I've made changes to flume-file-channel so that it doesn't depend on Hadoop IO classes.
          Here is a summary:

          • Created a new interface called 'FileChannelWritable' that has the same methods as 'org.apache.hadoop.io.Writable' and replaced all references of 'org.apache.hadoop.io.Writable'
          • Inlined the execution path of 'org.apache.hadoop.io.MapWritable' and 'org.apache.hadoop.io.Text' readFields and write methods into FlumeEvent readFields and write methods
          • Created a new utility class 'WritableUtils' that is a copy of readVInt and writeVInt methods from 'org.apache.hadoop.io.WritableUtils'
          Show
          Christopher Nagy added a comment - I've made changes to flume-file-channel so that it doesn't depend on Hadoop IO classes. Here is a summary: Created a new interface called 'FileChannelWritable' that has the same methods as 'org.apache.hadoop.io.Writable' and replaced all references of 'org.apache.hadoop.io.Writable' Inlined the execution path of 'org.apache.hadoop.io.MapWritable' and 'org.apache.hadoop.io.Text' readFields and write methods into FlumeEvent readFields and write methods Created a new utility class 'WritableUtils' that is a copy of readVInt and writeVInt methods from 'org.apache.hadoop.io.WritableUtils'
          Hide
          Mike Percy added a comment -

          Tentatively marking as v1.2.0 blocker per discussion on flume-dev

          Show
          Mike Percy added a comment - Tentatively marking as v1.2.0 blocker per discussion on flume-dev

            People

            • Assignee:
              Israel Ekpo
              Reporter:
              Mike Percy
            • Votes:
              0 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Due:
                Created:
                Updated:
                Resolved:

                Development