> On July 13, 2015, 4:31 p.m., Darrel Schneider wrote: > > gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/DiskEntry.java, > > line 1099 > > <https://reviews.apache.org/r/36419/diff/1/?file=1008832#file1008832line1099> > > > > why create a new var (oldValueLength)? > > It looks like it is only used in one place. > > Why not just change line 1134 to pass -did.getValueLength()
We need to read the oldValueLength before modifying the entry. If we called did.getValueLenth further down, the entry would already have been modified. > On July 13, 2015, 4:31 p.m., Darrel Schneider wrote: > > gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/DiskEntry.java, > > line 1134 > > <https://reviews.apache.org/r/36419/diff/1/?file=1008832#file1008832line1134> > > > > Since every other place we incNumOverflowBytesOnDisk we also call > > incrementBucketStats could you add a comment explaining why > > incrementBucketStats is not called here? Will do. It looks like it's because this is only called during recovery when there is no BucketRegion. - Dan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36419/#review91483 ----------------------------------------------------------- On July 11, 2015, 12:24 a.m., Dan Smith wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36419/ > ----------------------------------------------------------- > > (Updated July 11, 2015, 12:24 a.m.) > > > Review request for geode, Darrel Schneider and Hitesh Khamesra. > > > Repository: geode > > > Description > ------- > > These data are used in rebalancing, so we should have a stat that tracks > them. > > > Diffs > ----- > > > gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/DiskEntry.java > b67d0f10c63a19307288d5c6f2f9538eb280bfe4 > > gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/DiskRegion.java > e8548edadf89411af2538a30d183c697fe71a917 > > gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/DiskRegionStats.java > c2f6eee778b97d02c72b4d9a9720a45929da6990 > > gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/PartitionedRegion.java > bc164a82c25f6d22cce7abf399d5d57cb38f1630 > > gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/PartitionedRegionStats.java > 563c38b5380c7d704a7f4cf0cebdb405547cc992 > > Diff: https://reviews.apache.org/r/36419/diff/ > > > Testing > ------- > > > Thanks, > > Dan Smith > >
