[
https://issues.apache.org/jira/browse/FLUME-1184?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13269919#comment-13269919
]
[email protected] commented on FLUME-1184:
------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5054/#review7652
-----------------------------------------------------------
Ship it!
Thanks for the patch Brock. One comment noted below. If you agree, please
address it and update the patch on the Jira for commit.
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Log.java
<https://reviews.apache.org/r/5054/#comment16863>
potential thread safety issue: the loop is over the keySet view of the
idLogFileMap and may not be synchronized. Suggest enclosing it in
synchronized(idLogFileMap) block since it is also accessed by the background
thread.
- Arvind
On 2012-05-07 18:39:04, Brock Noland wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/5054/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2012-05-07 18:39:04)
bq.
bq.
bq. Review request for Flume and Arvind Prabhakar.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. TestFileChannel.testThreaded has a race condition due to
FileChannel.FileBackedTransaction not blocking. Sometimes the take threads will
find no events on the queue and quit. This patch addresses this issue and
additionally addresses a few issues found in the Log class:
bq.
bq. 1) We are not closing files open for gets()
bq. 2) removeOldLogs could be called after the log as been closed by the
background thread (identified while fixing #1).
bq.
bq.
bq. This addresses bug FLUME-1184.
bq. https://issues.apache.org/jira/browse/FLUME-1184
bq.
bq.
bq. Diffs
bq. -----
bq.
bq.
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Log.java
a777cd6
bq.
flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestFileChannel.java
d20e68c
bq.
bq. Diff: https://reviews.apache.org/r/5054/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. All unit tests pass and the unit test in question passed 1000 times in a
row which it had previously failed to do.
bq.
bq.
bq. Thanks,
bq.
bq. Brock
bq.
bq.
> TestFileChannel.testThreaded fails sometimes
> --------------------------------------------
>
> Key: FLUME-1184
> URL: https://issues.apache.org/jira/browse/FLUME-1184
> Project: Flume
> Issue Type: Bug
> Components: Test
> Affects Versions: v1.2.0
> Reporter: Brock Noland
> Assignee: Brock Noland
> Attachments: FLUME-1184-1.patch
>
>
> {noformat}
> java.lang.AssertionError: expected:<[0-0-0, 0-1-0, 0-2-0, 0-3-0, 0-4-0,
> 1-0-0, 1-0-1, 1-0-2, 1-0-3, 1-0-4, 2-0-0, 2-1-0, 2-2-0, 2-3-0, 2-4-0, 3-0-0,
> 3-0-1, 3-0-2, 3-0-3, 3-0-4, 4-0-0, 4-1-0, 4-2-0, 4-3-0, 4-4-0, 5-0-0, 5-0-1,
> 5-0-2, 5-0-3, 5-0-4, 6-0-0, 6-1-0, 6-2-0, 6-3-0, 6-4-0, 7-0-0, 7-0-1, 7-0-2,
> 7-0-3, 7-0-4, 8-0-0, 8-1-0, 8-2-0, 8-3-0, 8-4-0, 9-0-0, 9-0-1, 9-0-2, 9-0-3,
> 9-0-4]> but was:<[0-0-0, 0-1-0, 0-2-0, 0-3-0, 0-4-0, 1-0-0, 1-0-1, 1-0-2,
> 1-0-3, 1-0-4, 2-0-0, 2-1-0, 2-2-0, 2-3-0, 2-4-0, 3-0-0, 3-0-1, 3-0-2, 3-0-3,
> 3-0-4, 4-0-0, 4-1-0, 4-2-0, 4-3-0, 4-4-0, 5-0-0, 5-0-1, 5-0-2, 5-0-3, 5-0-4,
> 6-0-0, 6-1-0, 6-2-0, 6-3-0, 7-0-0, 7-0-1, 7-0-2, 7-0-3, 7-0-4, 8-0-0, 8-1-0,
> 8-2-0, 8-3-0, 8-4-0, 9-0-0, 9-0-1, 9-0-2, 9-0-3, 9-0-4]>
> 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:147)
> at
> org.apache.flume.channel.file.TestFileChannel.testThreaded(TestFileChannel.java:329)
> {noformat}
--
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
