> On Jan. 16, 2013, 9:28 p.m., Brock Noland wrote: > > flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/AsyncHBaseSink.java, > > line 208 > > <https://reviews.apache.org/r/8968/diff/4/?file=249254#file249254line208> > > > > Curious why we converted to nano seconds from milleseconds? IMHO ms are > > easier to comprehend
Yes, they are easier to comprehend, but I wanted to keep the delay to a minimum. Earlier, we weren't adjusting the timeout, so it was "ok" to wait in milliseconds, now we are - so minimize the time lost due to the couple of calculations that are taking place. If a part of a ms (say 2.9 ms) has passed we end up waiting extra (0.9 ms in the example) in the wait - not that this will affect it much, but I'd rather have a smaller least count. Performance-wise I don't see a difference, because it looks like internally it uses nanos itself. > On Jan. 16, 2013, 9:28 p.m., Brock Noland wrote: > > flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/AsyncHBaseSink.java, > > line 215 > > <https://reviews.apache.org/r/8968/diff/4/?file=249254#file249254line215> > > > > It looks like it's possible for timeRemaining to be negative. That is > > if the condition returned with 1 nanosecond left and then it took longer > > than to to check the callbacks. Condition.await() java doc doesn't say what > > occurs with a negative number. Good catch. > On Jan. 16, 2013, 9:28 p.m., Brock Noland wrote: > > flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/AsyncHBaseSink.java, > > line 445 > > <https://reviews.apache.org/r/8968/diff/4/?file=249254#file249254line445> > > > > How come we are converting back and forth between nanos and millis? I want to sleep for slightly less than the timeout, so I either need to do this for timeout or the hardcoded "slight difference" - or I could hardcode.. > On Jan. 16, 2013, 9:28 p.m., Brock Noland wrote: > > flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/AsyncHBaseSink.java, > > line 197 > > <https://reviews.apache.org/r/8968/diff/4/?file=249254#file249254line197> > > > > Just curious as to why this was added or perhaps why it was missing? I > > am not too familiar with the async hbase api. This does not change anything specifically, since the flush interval is set to 0, the buffer is flushed immediately anyway. I added this to safeguard against any future change in that behavior of the API. - Hari ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8968/#review15412 ----------------------------------------------------------- On Jan. 16, 2013, 8:47 p.m., Hari Shreedharan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8968/ > ----------------------------------------------------------- > > (Updated Jan. 16, 2013, 8:47 p.m.) > > > Review request for Flume. > > > Description > ------- > > Made changes to calculate remaining time and wait only for that long > > > This addresses bug FLUME-1842. > https://issues.apache.org/jira/browse/FLUME-1842 > > > Diffs > ----- > > flume-ng-doc/sphinx/FlumeUserGuide.rst 58a115e > > flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/AsyncHBaseSink.java > 6b34873 > > flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/HBaseSinkConfigurationConstants.java > fad026c > > flume-ng-sinks/flume-ng-hbase-sink/src/test/java/org/apache/flume/sink/hbase/TestAsyncHBaseSink.java > 1f61406 > > Diff: https://reviews.apache.org/r/8968/diff/ > > > Testing > ------- > > Added unit test. Current tests pass > > > Thanks, > > Hari Shreedharan > >
