----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50680/#review144640 -----------------------------------------------------------
This fix has a problem. It always reads the value from disk if it had overflowed. But just a couple of lines before this basicPut has called setOldValueInEvent. I think the correct fix is to use that old value from the event instead of rereading from disk/memory. - Darrel Schneider On Aug. 1, 2016, 4:17 p.m., Darrel Schneider wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50680/ > ----------------------------------------------------------- > > (Updated Aug. 1, 2016, 4:17 p.m.) > > > Review request for geode, Barry Oglesby, Eric Shu, Scott Jewell, and Ken Howe. > > > Bugs: GEODE-1718 > https://issues.apache.org/jira/browse/GEODE-1718 > > > Repository: geode > > > Description > ------- > > New unit test demonstrated that replace would return false when done on an > overflowed entry. > With the fix the test passes. > Also added a null check in checkPdxEquals to prevent NPE if obj is ever null. > If obj is null checkPdxEquals will now call basicEquals which handles testing > null. > > > Diffs > ----- > > > geode-core/src/main/java/com/gemstone/gemfire/internal/cache/AbstractRegionEntry.java > 15a5bed750b676c668ddde64291a5b315fca90fd > > geode-core/src/main/java/com/gemstone/gemfire/internal/cache/AbstractRegionMap.java > f3cb3d654087c797de430f448348c3ba66846ac4 > > geode-core/src/test/java/com/gemstone/gemfire/internal/cache/ReplaceWithOverflowJUnitTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/50680/diff/ > > > Testing > ------- > > precheckin > > > Thanks, > > Darrel Schneider > >
