----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4400/#review6153 -----------------------------------------------------------
Juhani, looks great. I added a few suggestions around wording. I agree with the general concerns. I'm not yet sure what I think we should do about it though. flume-ng-core/src/main/java/org/apache/flume/Channel.java <https://reviews.apache.org/r/4400/#comment13238> "...they may be simultaneously accessed by" (missing "by") flume-ng-core/src/main/java/org/apache/flume/PollableSource.java <https://reviews.apache.org/r/4400/#comment13239> How about: "guaranteed to be called only by a single thread at a time, with no concurrency. Any other mechanism driving a pollable source must follow the same semantics." This sounds a lot like a synchronized call and I wonder if it makes sense to call this out as required by PollableSource implementations. flume-ng-core/src/main/java/org/apache/flume/Sink.java <https://reviews.apache.org/r/4400/#comment13242> How about switching a couple words around: "While the Sink process call is guaranteed to only be accessed by a single thread, other calls may be concurrently accessed and should thus be protected." - Mike 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 > >
