----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57797/#review169523 -----------------------------------------------------------
geode-core/src/main/java/org/apache/geode/internal/cache/DiskEntry.java Line 458 (original), 458 (patched) <https://reviews.apache.org/r/57797/#comment241889> Instead of directly reading the private inst var call the gettor method here. geode-core/src/main/java/org/apache/geode/internal/cache/DiskEntry.java Lines 1020 (patched) <https://reviews.apache.org/r/57797/#comment241890> once again use the gettor instead of the inst var geode-core/src/main/java/org/apache/geode/internal/cache/DiskEntry.java Lines 1063 (patched) <https://reviews.apache.org/r/57797/#comment241892> I'd like to see these "updateRecoveredEntry" static methods to be made instance methods on the RecoveredEntry class named "update". You can then get rid of the check in them that "newValue == null" since that will never happen on an instance method. All the callers I see of this have a non-null RecoveredEntry so this null check was not needed. geode-core/src/main/java/org/apache/geode/internal/cache/DiskEntry.java Lines 1741 (patched) <https://reviews.apache.org/r/57797/#comment241887> a better name for this would be "valueRecovered". Also, make it "final". geode-core/src/main/java/org/apache/geode/internal/cache/DiskEntry.java Lines 1810 (patched) <https://reviews.apache.org/r/57797/#comment241888> Note the "Vaule" typo in these two methods. Change them to "getValueRecovered" and "setValueRecovered". geode-core/src/main/java/org/apache/geode/internal/cache/DiskEntry.java Lines 1814 (patched) <https://reviews.apache.org/r/57797/#comment241891> Remove this settor since you never use it and the field should only be set in the constructor - Darrel Schneider On March 20, 2017, 3:47 p.m., Eric Shu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57797/ > ----------------------------------------------------------- > > (Updated March 20, 2017, 3:47 p.m.) > > > Review request for geode, anilkumar gingade and Darrel Schneider. > > > Bugs: GEODE-2535 > https://issues.apache.org/jira/browse/GEODE-2535 > > > Repository: geode > > > Description > ------- > > Use the boolean flag to determine if an entry is in memory or on disk so that > stats are correctly updated. > Avoid negating keyId by using the boolean flag. > > > Diffs > ----- > > geode-core/src/main/java/org/apache/geode/internal/cache/DiskEntry.java > d00f7a0 > geode-core/src/main/java/org/apache/geode/internal/cache/DiskId.java > 8d2c675 > geode-core/src/main/java/org/apache/geode/internal/cache/DiskStoreImpl.java > 642eed3 > geode-core/src/main/java/org/apache/geode/internal/cache/Oplog.java 27f41a2 > > geode-core/src/test/java/org/apache/geode/internal/cache/DiskRegRecoveryJUnitTest.java > 3fd091b > > geode-core/src/test/java/org/apache/geode/internal/cache/OplogJUnitTest.java > 7549ea7 > > > Diff: https://reviews.apache.org/r/57797/diff/1/ > > > Testing > ------- > > precheckin. > > > Thanks, > > Eric Shu > >
