incubator-flume-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Juhani Connolly" <juha...@gmail.com>
Subject Re: Review Request: FLUME-936 MemoryChannel is not thread safe
Date Thu, 02 Feb 2012 00:42:37 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3704/
-----------------------------------------------------------

(Updated 2012-02-02 00:42:37.375992)


Review request for Flume.


Changes
-------

This addresses Prasad's concern regarding the queue getting full up by reducing to one lock


Summary
-------

This is an initial go at fixing the threading issues with memory channel. 

It uses the preliminary work on FLUME-935 and I have included the code from that.

The tagging of the events became unnecessary so I dropped that. One thing that concerns me
slightly is how to deal with not having enough space in the queue to rollback failed takes.
One method would be to keep a minimum buffer of transactionCapacity. Another would be to implement
the queue of queues as suggested in FLUME-889

Anyway, just putting up this early version to see what people think


This addresses bug FLUME-936.
    https://issues.apache.org/jira/browse/FLUME-936


Diffs (updated)
-----

  flume-ng-core/src/main/java/org/apache/flume/channel/BasicChannelSemantics.java PRE-CREATION

  flume-ng-core/src/main/java/org/apache/flume/channel/BasicTransactionSemantics.java PRE-CREATION

  flume-ng-core/src/main/java/org/apache/flume/channel/ChannelUtils.java PRE-CREATION 
  flume-ng-core/src/main/java/org/apache/flume/channel/MemoryChannel.java d379b64 
  flume-ng-core/src/test/java/org/apache/flume/channel/TestFanoutChannel.java ada9a72 
  flume-ng-core/src/test/java/org/apache/flume/channel/TestMemoryChannel.java b44030e 
  flume-ng-core/src/test/java/org/apache/flume/channel/TestMemoryChannelConcurrency.java PRE-CREATION

  flume-ng-core/src/test/java/org/apache/flume/channel/TestMemoryChannelTransaction.java d18045b

  flume-ng-core/src/test/java/org/apache/flume/source/TestExecSource.java 6acbbd5 

Diff: https://reviews.apache.org/r/3704/diff


Testing
-------

The original tests pass, though I had to take out the state checks because of the changes
to semantics from the flume-935 code. I also had to add a transaction.close statement where
semantics were not properly being followed
I have to retrofit my new concurrency test since without the tagged events it cannot fail
without checking that the content is correct. I'll put that up asap, just wanted to get some
eyes on this before I head out.


Thanks,

Juhani


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message