> On Aug. 9, 2012, 10:56 p.m., Hari Shreedharan wrote: > > flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestFileChannel.java, > > line 305 > > <https://reviews.apache.org/r/6411/diff/4/?file=135714#file135714line305> > > > > I didn't really understand what is being tested here. > > > > It seems like the take will clear up one slot for the put and the put > > can get written to the channel before it stops. > > > > Now when the channel starts the puts which filled up the channel are > > replayed and then the blocked-put is replayed before the take? Wouldn't > > that cause an exception? Or I am missing something here? > > > > > > Hari Shreedharan wrote: > Also where do you actually test what events you picked up from the > channel? I think I need some help understanding this test. > > Brock Noland wrote: > I agree, that test needs a good comment. > > We are testing that we can replay a log file despite the PUT having an > earlier transaction id than the TAKE. The transaction ID is assigned on > transaction create. If we replayed by transaction id, the put would come > before the take while when replaying based on the write order id, the put > comes after the take. This is import when the the queue is full.
Ah now that makes sense. I was trying to understand the txn id comment and its context. - Hari ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/6411/#review10094 ----------------------------------------------------------- On Aug. 8, 2012, 5:51 p.m., Brock Noland wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/6411/ > ----------------------------------------------------------- > > (Updated Aug. 8, 2012, 5:51 p.m.) > > > Review request for Flume. > > > Description > ------- > > This resolves FLUME-1432 which describes a problem with how we replayed logs. > Instead of replaying one log at a time, we replay all the logs in the order > they were written. We do this by changing the "timestamp" field to be a > globally ordered number which allows us to do a merge of all the log files. > This should be faster and also fix some problems where we could not replay > some checkpoints. > > Its worth noting that the condition in testRaceFoundInFLUME1432 was observed > prior to this patch and will not pass if we simply used the transaction id to > order edits due to that being generated on transaction create or the previous > value of the field, timestamp, because they were observed to be non-unique on > a 2 core host. > > If committed FLUME-1433 can be closed as well since this change fixes that > test as well. > > > This addresses bug FLUME-1432. > https://issues.apache.org/jira/browse/FLUME-1432 > > > Diffs > ----- > > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Commit.java > 03b1060 > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FileChannel.java > cc8f89a > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FlumeEventQueue.java > e692934 > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Log.java > 2b733b1 > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFile.java > 5615c6d > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogRecord.java > PRE-CREATION > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Put.java > bcd37ab > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/ReplayHandler.java > da2d068 > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Rollback.java > b42501f > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Take.java > 42b197f > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/TransactionEventRecord.java > c222bd1 > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/WriteOrderOracle.java > PRE-CREATION > > flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestCheckpoint.java > 17a7cf9 > > flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestFileChannel.java > 681ebcc > > flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestLogFile.java > 8995089 > > flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestLogRecord.java > PRE-CREATION > > flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestTransactionEventRecord.java > a46526d > > flume-ng-channels/flume-file-channel/src/test/resources/fileformat-v2-checkpoint.gz > PRE-CREATION > > flume-ng-channels/flume-file-channel/src/test/resources/fileformat-v2-log-1.gz > PRE-CREATION > > flume-ng-channels/flume-file-channel/src/test/resources/fileformat-v2-log-2.gz > PRE-CREATION > > flume-ng-channels/flume-file-channel/src/test/resources/fileformat-v2-log-3.gz > PRE-CREATION > > flume-ng-channels/flume-file-channel/src/test/resources/fileformat-v2-pre-FLUME-1432-checkpoint.gz > PRE-CREATION > > flume-ng-channels/flume-file-channel/src/test/resources/fileformat-v2-pre-FLUME-1432-log-1.gz > PRE-CREATION > > flume-ng-channels/flume-file-channel/src/test/resources/fileformat-v2-pre-FLUME-1432-log-2.gz > PRE-CREATION > > flume-ng-channels/flume-file-channel/src/test/resources/fileformat-v2-pre-FLUME-1432-log-3.gz > PRE-CREATION > > flume-ng-sinks/flume-ng-hbase-sink/src/test/java/org/apache/flume/sink/hbase/TestAsyncHBaseSink.java > 641952a > > Diff: https://reviews.apache.org/r/6411/diff/ > > > Testing > ------- > > All unit tests pass. A unit test is added, testRaceFoundInFLUME1432 which > found a problem with the way we previously replayed logs. It passes with > these changes. I also tested on a two node flume installation stopping and > restarting a node several times. > > > Thanks, > > Brock Noland > >
