----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/6982/#review12075 -----------------------------------------------------------
I'd recommend exploring the semaphore approach. Frankly, I don't really think there is a need for byte level granularity. A granularity of 50 bytes is fairly reasonable. flume-ng-core/src/main/java/org/apache/flume/channel/MemoryChannel.java <https://reviews.apache.org/r/6982/#comment25730> This is still not thread safe. The transaction objects are thread local. SO if you have bufferedByteCapacity = 10 and 2 txns doing puts, one where the size = 3 and the other where size = 8. This series of events happens: txn 1 increments currentByteUsage atomically setting it to 3. Before txn1 finishes the comparison txn 2 adds 8 to currentByteUsage and does the comparison, and gets true, enters the if block. Now txn1 does the compare, and gets true - and enters the if block. Both txn1 and txn2 fail even though one of the two could easily have gone in. You need to make sure the comparison is atomic. flume-ng-core/src/main/java/org/apache/flume/channel/MemoryChannel.java <https://reviews.apache.org/r/6982/#comment25732> These have threading and efficiency implications. Since these are inside transactions, wouldn't this be much more efficient and thread-safe if you just keep track of the size internally per transaction, rather than sitting in a loop and subtracting ? flume-ng-core/src/main/java/org/apache/flume/channel/MemoryChannel.java <https://reviews.apache.org/r/6982/#comment25733> same as above. flume-ng-core/src/main/java/org/apache/flume/channel/MemoryChannel.java <https://reviews.apache.org/r/6982/#comment25734> Log that defaults were restored because the config had invalid values? flume-ng-core/src/main/java/org/apache/flume/channel/MemoryChannel.java <https://reviews.apache.org/r/6982/#comment25735> The headers take space in memory. It is not reasonable to assume that the headers are "free." Otherwise the estimates are invalid and you could easily overrun memory. HashMaps do have a reasonable memory overhead. - Hari Shreedharan On Sept. 16, 2012, 3:54 a.m., Ted Malaska wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/6982/ > ----------------------------------------------------------- > > (Updated Sept. 16, 2012, 3:54 a.m.) > > > Review request for Flume. > > > Description > ------- > > 1. The user will be able to define a byteCapacity and a > byteCapacityBufferPercentage. > 2. Events byte size will be estimated from there body contents > 3. On put bytes are added to current total > 4. On commit any uncommitted takes are removed from the current byte total > 5. On rollover any uncommitted puts are removed from the current byte total > > > This addresses bug FLUME-1535. > https://issues.apache.org/jira/browse/FLUME-1535 > > > Diffs > ----- > > flume-ng-core/src/main/java/org/apache/flume/channel/MemoryChannel.java > c72e97c > flume-ng-core/src/test/java/org/apache/flume/channel/TestMemoryChannel.java > e070864 > > Diff: https://reviews.apache.org/r/6982/diff/ > > > Testing > ------- > > > Thanks, > > Ted Malaska > >
