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


I too am concerned about the thread safety. I'll take a look at some of the 
classes tonight or tomorrow, and will offer some suggestions. I'll be 
considering atomicity, visibility, and HB relationships per JCIP.

- Will


On 2012-03-19 03:59:00, Juhani Connolly wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4400/
> -----------------------------------------------------------
> 
> (Updated 2012-03-19 03:59:00)
> 
> 
> Review request for Flume.
> 
> 
> Summary
> -------
> 
> I went over the basic interfaces and added some javadoc for developers of new 
> components.
> 
> I am pretty concerned about the fact that pretty much  everything is not 
> thread safe because off the lack of guarantees provided by some of the 
> interfaces(Configurable in particular).
> 
> A lot of existing components assume thread safety(which is somewhat given by 
> isolation of process() type methods to a single runner thread). Should we be 
> fixing these components, or making some more guarantees about when and how 
> configuration calls should  be called(e.g. only callable on a stopped 
> component?)
> 
> 
> This addresses bug Flume-1026.
>     https://issues.apache.org/jira/browse/Flume-1026
> 
> 
> Diffs
> -----
> 
>   flume-ng-core/src/main/java/org/apache/flume/Channel.java 9d1c14c 
>   flume-ng-core/src/main/java/org/apache/flume/PollableSource.java 8806040 
>   flume-ng-core/src/main/java/org/apache/flume/Sink.java 4a706cf 
>   flume-ng-core/src/main/java/org/apache/flume/SinkProcessor.java ea6000b 
>   flume-ng-core/src/main/java/org/apache/flume/Source.java f4e9481 
>   flume-ng-core/src/main/java/org/apache/flume/Transaction.java 5b1fee5 
>   flume-ng-core/src/main/java/org/apache/flume/conf/Configurable.java 0fa4839 
> 
> Diff: https://reviews.apache.org/r/4400/diff
> 
> 
> Testing
> -------
> 
> Doc only patch
> 
> 
> Thanks,
> 
> Juhani
> 
>

Reply via email to