[
https://issues.apache.org/jira/browse/FLUME-1085?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13257763#comment-13257763
]
[email protected] commented on FLUME-1085:
------------------------------------------------------
bq. On 2012-04-19 09:12:40, Arvind Prabhakar wrote:
bq. > Thanks for the patch Brock. The implementation looks great to me! Some
feedback follows:
bq. >
bq. > Some convention nits:
bq. > * Please make sure the line lengths are under 80 chars.
bq. > * Please use all caps for variables that are static final.
bq. >
bq. > One high-level consideration: Since it is possible to have multiple file
channels within the same agent, we should either make file channel multi-tenant
capable like the jdbc channel, or instead have the ability to associate
directory locks for the file channel instance for checkpoint and data
directories. Doing this will ensure that there is no corruption and the system
exits cleanly in case of misconfiguration.
bq. >
bq. > If you chose to implement the non-multitenant system, it will be
preferable to use the channel name within the default paths for checkpoint and
data directories. This will ensure minimum configuration necessary even when
using multiple file channels within the same agent.
bq. >
bq. >
I added locks to ensure only one channel has access to the directories at a
given time.
This is a concern I have a about JDBC Channel. I am not sold on using the
channel name from the configuration as that is a symbolic name, users will
likely think they can change it. However, if there are events stored in that
channel and they change the name, all their events are gone. If we used the
name this same thing would occur. Today with JDBC Channel and this
implementation of FileChannel, it can be multi-tenant if configured to use
different directories for each Channel. I feel like this is a better approach.
Thoughts?
bq. On 2012-04-19 09:12:40, Arvind Prabhakar wrote:
bq. >
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FlumeEventQueue.java,
line 214
bq. > <https://reviews.apache.org/r/4661/diff/6/?file=102330#file102330line214>
bq. >
bq. > From cursory analysis it seems that this will likely not be able to
handle the wrap-around logic correctly. For example, if the capacity is 10,
next is 9 and size/index is 1: the calculated index will be 10, when it should
be 0.
bq. >
bq. > One way to address this would be modulo the converted value before
return:
bq. >
bq. > return (next + index % elements.capacity()) % elements.capacity();
bq. >
bq.
bq. Brock Noland wrote:
bq. I am working on the rest of the feedback, but I just wanted to say
"big thank you" for finding this bug. I hit it last night while stress testing
and was planning on spending much of my day finding it.
I have never written a data structure like this one, so I encourage a thorough
review of it.
bq. On 2012-04-19 09:12:40, Arvind Prabhakar wrote:
bq. >
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FlumeEventQueue.java,
line 46
bq. > <https://reviews.apache.org/r/4661/diff/6/?file=102330#file102330line46>
bq. >
bq. > Not a request, but something to keep an eye on: this implementation
uses the modulo operation heavily to calculate the queue positions. The
alternative way of doing this would be to maintain head, size and optionally
tail pointers. That will reduce the number of overall modulo operations needed.
bq. >
bq. > The only reason I point this out is that modulo is CPU intensive for
large numbers and therefore may become a concern for performance.
I added a note, if we find this is troublesome we do a later JIRA. Since we
will be writing to disk every time we do modulus I think it's unlikely to be a
performance problem.
- Brock
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4661/#review6786
-----------------------------------------------------------
On 2012-04-19 20:10:53, Brock Noland wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/4661/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2012-04-19 20:10:53)
bq.
bq.
bq. Review request for Flume.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. This patch implements a durable file channel. It does by writing all
transaction events to disk and syncing to disk when a commit occurs. It does
have a memory component in that pointers to the event on disk are kept in
memory. This will consume 8 bytes of direct memory (non-heap) per event. Some
basic calculations are in the FileChannel java docs.
bq.
bq.
bq. This addresses bug FLUME-1085.
bq. https://issues.apache.org/jira/browse/FLUME-1085
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. flume-ng-channels/flume-file-channel/pom.xml e8155be
bq.
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Checkpoint.java
PRE-CREATION
bq.
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Commit.java
PRE-CREATION
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/FileChannelConfiguration.java
PRE-CREATION
bq.
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FlumeEvent.java
PRE-CREATION
bq.
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FlumeEventPointer.java
PRE-CREATION
bq.
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FlumeEventQueue.java
PRE-CREATION
bq.
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Log.java
PRE-CREATION
bq.
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFile.java
PRE-CREATION
bq.
flume-ng-sdk/src/test/java/org/apache/flume/api/TestNettyAvroRpcClient.java
0b8a2c0
bq.
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogUtils.java
PRE-CREATION
bq.
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Pair.java
PRE-CREATION
bq.
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Put.java
PRE-CREATION
bq.
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/ReplayHandler.java
PRE-CREATION
bq.
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Rollback.java
PRE-CREATION
bq.
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Take.java
PRE-CREATION
bq.
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/TransactionEventRecord.java
PRE-CREATION
bq.
flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/CountingSinkRunner.java
PRE-CREATION
bq.
flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/CountingSourceRunner.java
PRE-CREATION
bq.
flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestCheckpoint.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/TestFlumeEvent.java
PRE-CREATION
bq.
flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestFlumeEventPointer.java
PRE-CREATION
bq.
flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestFlumeEventQueue.java
PRE-CREATION
bq.
flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestLog.java
PRE-CREATION
bq.
flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestLogFile.java
PRE-CREATION
bq.
flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestTransactionEventRecord.java
PRE-CREATION
bq.
flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestUtils.java
PRE-CREATION
bq. flume-ng-channels/flume-file-channel/src/test/resources/log4j.properties
739ecc8
bq.
flume-ng-channels/flume-recoverable-memory-channel/src/main/java/org/apache/flume/channel/recoverable/memory/RecoverableMemoryChannel.java
0622f27
bq.
flume-ng-channels/flume-recoverable-memory-channel/src/main/java/org/apache/flume/channel/recoverable/memory/wal/SequenceIDBuffer.java
fa63b73
bq.
flume-ng-core/src/main/java/org/apache/flume/channel/ChannelProcessor.java
eb6460e
bq. flume-ng-core/src/main/java/org/apache/flume/channel/ChannelType.java
d8419e8
bq. flume-ng-core/src/main/java/org/apache/flume/sink/NullSink.java c812851
bq. flume-ng-core/src/main/java/org/apache/flume/source/StressSource.java
PRE-CREATION
bq.
flume-ng-core/src/main/java/org/apache/flume/tools/DirectMemoryUtils.java
PRE-CREATION
bq.
flume-ng-sdk/src/test/java/org/apache/flume/api/TestFailoverRpcClient.java
225cd34
bq.
bq. Diff: https://reviews.apache.org/r/4661/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. Unit tests and integration tests added to cover obvious cases.
bq.
bq.
bq. Thanks,
bq.
bq. Brock
bq.
bq.
> Implement a durable FileChannel
> -------------------------------
>
> Key: FLUME-1085
> URL: https://issues.apache.org/jira/browse/FLUME-1085
> Project: Flume
> Issue Type: New Feature
> Components: Channel
> Affects Versions: v1.2.0
> Reporter: Brock Noland
> Assignee: Brock Noland
>
> FLUME-896 turned into a durable Memory Channel, we need a durable File Channel
--
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
