----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56506/#review164991 -----------------------------------------------------------
geode-core/src/main/java/org/apache/geode/internal/cache/Oplog.java (line 5192) <https://reviews.apache.org/r/56506/#comment236950> I think the code would be easier to understand if these variables were declared inside the "if" block or "do" block that uses them instead of all at the beginning of the method. Doing this can also make refactoring large blocks into their own method easier. My understanding is that you will not get any performance improvement over declaring all local variables at the start of a method. I'd also encourage using "final" on local variables that that code only initializes and never change. geode-core/src/main/java/org/apache/geode/internal/cache/Oplog.java (line 5213) <https://reviews.apache.org/r/56506/#comment236951> What we have seen so far is that when something is wrong it has always been the position in "bb". This code basically says that if the result of "channel.write" is not consistent with the "bb" position then use the "channel" position as the final authority on what is correct. It is possible that the "bb" position is correct according to the "channel" position. So when you fix the "bb" position to be consistent with the channel, I think you should also fix the result of "channel.write" (stored in "channelBytesWritten"). In the retry block you can just update both of these. Then move the inc of "flushed" to be done after this block (right before "while bb.hasRemaining"). geode-core/src/main/java/org/apache/geode/internal/cache/Oplog.java (line 5223) <https://reviews.apache.org/r/56506/#comment236824> It would be nice to include more info in this IOException related to the inconsistency in channleBytesWritten vs. bytes read from bb. - Darrel Schneider On Feb. 9, 2017, 10:11 a.m., Ken Howe wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56506/ > ----------------------------------------------------------- > > (Updated Feb. 9, 2017, 10:11 a.m.) > > > Review request for geode, anilkumar gingade and Darrel Schneider. > > > Repository: geode > > > Description > ------- > > Implemented limited retries in two forms of Oplog.flush() when > channel.write() is called. > If write() returns bytes witten less than the change in the ByteBuffer > positions, then reset > buffer positions and re-try writing for a limited number of times. Throws > IOException if the write doesn't succeeded after a few retries (max > number of retries is defined by a static). > > Added new unit tests. > > > Diffs > ----- > > geode-core/src/main/java/org/apache/geode/internal/cache/Oplog.java > 0b98364b743c39e69773d586f9c793eb7de71b8d > > geode-core/src/test/java/org/apache/geode/internal/cache/OplogFlushTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/56506/diff/ > > > Testing > ------- > > Started precheckin > > > Thanks, > > Ken Howe > >
