[
https://issues.apache.org/jira/browse/FLUME-896?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13234041#comment-13234041
]
[email protected] commented on FLUME-896:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4325/#review6126
-----------------------------------------------------------
Thanks for the patch Brock. The changes look good. Some feedback follows:
General:
* There are a few places where whitespace has creeped in - highlighted as red
in the review. Please remove it in the final patch.
* Also, there are a few places where the line length exceeds 80 chars, will be
nice to take care of that as well in the final patch.
Design/Impl:
* The FileBackedTransaction.doCommit() only calls the
fileChannel.commitSequenceID(lastSequenceId) if the lastSequenceId is greater
than 0. This will not be the case if the channel has no sinks for it. As a
result everything will be replayed every time the channel starts up, causing
FileChannel.configure() to fail with OOM after a certain threshold is breached.
Ideally, the sequeceId commit should happen on every commit call.
* On the same lines, the implementation should override the stop() method to
perform cleanup and close any of the WAL resources that may be actively in use.
* The take implementation seems to be limited to the contents of MemoryChannel
only. This will not have any of the events that were persisted in the previous
run of the FileChannel and therefore those events will get locked out of any
sink's reach.
* Finally, it is not clear to me from the implementation how the concurrent
semantic will work. Can you please explain (or better still javadoc) the
expected workings of the WAL implementation?
flume-ng-channels/flume-file-channel/pom.xml
<https://reviews.apache.org/r/4325/#comment13164>
This change seems to be unrelated to the file channel. If that is the case,
we should factor it out.
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FileChannel.java
<https://reviews.apache.org/r/4325/#comment13181>
Please override the initialize() method to do any initialization for the
system.
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FileChannel.java
<https://reviews.apache.org/r/4325/#comment13180>
The JDBC channel defaults to ${user.home}/.flume/jdbc-channel. I suggest
defaulting the file channel working directory to something similar like
${user.home}/.flume/file-channel
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FileChannel.java
<https://reviews.apache.org/r/4325/#comment13214>
It will be great if this memory channel can also be configured correctly
via the system configuration. That way, the end user will be able to specify
the cap on the number of events that can live in all the active transactions
combined.
This will help ensure that this channel does not run into OOM issues.
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FileChannel.java
<https://reviews.apache.org/r/4325/#comment13182>
This constructor does not allow the specification of roll size, roll
required interval, max log size, min log retention period and worker interval.
All of these properties should be extracted from the context and passed into
the WAL constructor.
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FileChannel.java
<https://reviews.apache.org/r/4325/#comment13222>
This logic is probably better done in the start() method than in the
configure method.
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/wal/SequenceIDBuffer.java
<https://reviews.apache.org/r/4325/#comment13162>
Missing license header.
flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/wal/TestSequenceIDBuffer.java
<https://reviews.apache.org/r/4325/#comment13163>
Missing license header.
- Arvind
On 2012-03-14 19:04:52, Brock Noland wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/4325/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2012-03-14 19:04:52)
bq.
bq.
bq. Review request for Flume.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. Attached patch not meant for commit. Just posting here for easy review.
bq.
bq.
bq. This addresses bug FLUME-896.
bq. https://issues.apache.org/jira/browse/FLUME-896
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. flume-ng-channels/flume-file-channel/pom.xml ee2d20f
bq.
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FileChannel.java
a279453
bq.
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FileChannelEvent.java
PRE-CREATION
bq.
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/wal/SequenceIDBuffer.java
PRE-CREATION
bq.
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/wal/WAL.java
PRE-CREATION
bq.
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/wal/WALDataFile.java
PRE-CREATION
bq.
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/wal/WALEntry.java
PRE-CREATION
bq.
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/wal/WALReplayResult.java
PRE-CREATION
bq.
flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestFileChannel.java
ab66998
bq.
flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/wal/TestSequenceIDBuffer.java
PRE-CREATION
bq.
flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/wal/TestWAL.java
PRE-CREATION
bq.
bq. Diff: https://reviews.apache.org/r/4325/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq.
bq. Thanks,
bq.
bq. Brock
bq.
bq.
> Implement file write ahead log channel
> --------------------------------------
>
> Key: FLUME-896
> URL: https://issues.apache.org/jira/browse/FLUME-896
> Project: Flume
> Issue Type: New Feature
> Components: Channel
> Affects Versions: NG alpha 1
> Reporter: E. Sammer
> Assignee: Brock Noland
> Fix For: v1.2.0
>
> Attachments: FLUME-896-1.patch, FLUME-896-4.patch
>
>
> Implement a channel that uses a regular file system and a write ahead log for
> durable event delivery.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
