You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Sai Boorlagadda <sb...@pivotal.io> on 2016/05/24 21:36:09 UTC
Review Request 47793: GEODE-93: Entry count stats are incorrect with
PR with entry eviction and async disk
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47793/
-----------------------------------------------------------
Review request for geode, Darrel Schneider, Eric Shu, Scott Jewell, and Ken Howe.
Bugs: GEODE-93
https://issues.apache.org/jira/browse/GEODE-93
Repository: geode
Description
-------
- Stats are not updated when a tombstone is scheduled/unscheduled.
- All stats are updated when the data is finally flushed to the disk rather than doing it eagerly and compensating when on a new operation if the data is not yet written to disk
Diffs
-----
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/DiskEntry.java 54ecb0434298ab6af2af8724a33048732735fc7d
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalRegion.java d28496cc6e58cf061ce996d45526f3a0bd3ab10f
geode-core/src/test/java/com/gemstone/gemfire/internal/cache/PartitionedRegionStatsJUnitTest.java 1a3277ca7b9597327e9ce42a61976ad731cc7c1c
Diff: https://reviews.apache.org/r/47793/diff/
Testing
-------
precheckin
Thanks,
Sai Boorlagadda
Re: Review Request 47793: GEODE-93: Entry count stats are incorrect
with PR with entry eviction and async disk
Posted by Ken Howe <kh...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47793/#review134650
-----------------------------------------------------------
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/DiskEntry.java (line 1070)
<https://reviews.apache.org/r/47793/#comment199493>
Delete, variable is not referenced in the new version
- Ken Howe
On May 24, 2016, 9:36 p.m., Sai Boorlagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47793/
> -----------------------------------------------------------
>
> (Updated May 24, 2016, 9:36 p.m.)
>
>
> Review request for geode, Darrel Schneider, Eric Shu, Scott Jewell, and Ken Howe.
>
>
> Bugs: GEODE-93
> https://issues.apache.org/jira/browse/GEODE-93
>
>
> Repository: geode
>
>
> Description
> -------
>
> - Stats are not updated when a tombstone is scheduled/unscheduled.
> - All stats are updated when the data is finally flushed to the disk rather than doing it eagerly and compensating when on a new operation if the data is not yet written to disk
>
>
> Diffs
> -----
>
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/DiskEntry.java 54ecb0434298ab6af2af8724a33048732735fc7d
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalRegion.java d28496cc6e58cf061ce996d45526f3a0bd3ab10f
> geode-core/src/test/java/com/gemstone/gemfire/internal/cache/PartitionedRegionStatsJUnitTest.java 1a3277ca7b9597327e9ce42a61976ad731cc7c1c
>
> Diff: https://reviews.apache.org/r/47793/diff/
>
>
> Testing
> -------
>
> precheckin
>
>
> Thanks,
>
> Sai Boorlagadda
>
>
Re: Review Request 47793: GEODE-93: Entry count stats are incorrect
with PR with entry eviction and async disk
Posted by Sai Boorlagadda <sb...@pivotal.io>.
> On May 24, 2016, 10:17 p.m., Darrel Schneider wrote:
> > geode-core/src/main/java/com/gemstone/gemfire/internal/cache/DiskEntry.java, line 1077
> > <https://reviews.apache.org/r/47793/diff/1/?file=1392875#file1392875line1077>
> >
> > It looks to me like every place we call incrementBucketStats we also now do corresponding updates to dr inVM, onDisk, and BytesOnDisk.
> >
> > How about refactoring this code so that this common pattern is in a single method?
> > You could rename incrementBucketStats to "updateOverflowStats" and add a DiskRegionView parameter.
I agree it needs some refactoring. Will create a new ticket for the re-factoring effort.
- Sai
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47793/#review134647
-----------------------------------------------------------
On May 24, 2016, 9:36 p.m., Sai Boorlagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47793/
> -----------------------------------------------------------
>
> (Updated May 24, 2016, 9:36 p.m.)
>
>
> Review request for geode, Darrel Schneider, Eric Shu, Scott Jewell, and Ken Howe.
>
>
> Bugs: GEODE-93
> https://issues.apache.org/jira/browse/GEODE-93
>
>
> Repository: geode
>
>
> Description
> -------
>
> - Stats are not updated when a tombstone is scheduled/unscheduled.
> - All stats are updated when the data is finally flushed to the disk rather than doing it eagerly and compensating when on a new operation if the data is not yet written to disk
>
>
> Diffs
> -----
>
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/DiskEntry.java 54ecb0434298ab6af2af8724a33048732735fc7d
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalRegion.java d28496cc6e58cf061ce996d45526f3a0bd3ab10f
> geode-core/src/test/java/com/gemstone/gemfire/internal/cache/PartitionedRegionStatsJUnitTest.java 1a3277ca7b9597327e9ce42a61976ad731cc7c1c
>
> Diff: https://reviews.apache.org/r/47793/diff/
>
>
> Testing
> -------
>
> precheckin
>
>
> Thanks,
>
> Sai Boorlagadda
>
>
Re: Review Request 47793: GEODE-93: Entry count stats are incorrect
with PR with entry eviction and async disk
Posted by Darrel Schneider <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47793/#review134647
-----------------------------------------------------------
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/DiskEntry.java (line 1077)
<https://reviews.apache.org/r/47793/#comment199494>
It looks to me like every place we call incrementBucketStats we also now do corresponding updates to dr inVM, onDisk, and BytesOnDisk.
How about refactoring this code so that this common pattern is in a single method?
You could rename incrementBucketStats to "updateOverflowStats" and add a DiskRegionView parameter.
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/DiskEntry.java (line 1079)
<https://reviews.apache.org/r/47793/#comment199495>
Why no call of incrementBucketStats here?
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/DiskEntry.java (line 1772)
<https://reviews.apache.org/r/47793/#comment199491>
Is anything in this comment still needed?
I know that onDisk was NOT already inced.
I'm not sure if we need to say anything about the old size.
- Darrel Schneider
On May 24, 2016, 2:36 p.m., Sai Boorlagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47793/
> -----------------------------------------------------------
>
> (Updated May 24, 2016, 2:36 p.m.)
>
>
> Review request for geode, Darrel Schneider, Eric Shu, Scott Jewell, and Ken Howe.
>
>
> Bugs: GEODE-93
> https://issues.apache.org/jira/browse/GEODE-93
>
>
> Repository: geode
>
>
> Description
> -------
>
> - Stats are not updated when a tombstone is scheduled/unscheduled.
> - All stats are updated when the data is finally flushed to the disk rather than doing it eagerly and compensating when on a new operation if the data is not yet written to disk
>
>
> Diffs
> -----
>
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/DiskEntry.java 54ecb0434298ab6af2af8724a33048732735fc7d
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalRegion.java d28496cc6e58cf061ce996d45526f3a0bd3ab10f
> geode-core/src/test/java/com/gemstone/gemfire/internal/cache/PartitionedRegionStatsJUnitTest.java 1a3277ca7b9597327e9ce42a61976ad731cc7c1c
>
> Diff: https://reviews.apache.org/r/47793/diff/
>
>
> Testing
> -------
>
> precheckin
>
>
> Thanks,
>
> Sai Boorlagadda
>
>
Re: Review Request 47793: GEODE-93: Entry count stats are incorrect
with PR with entry eviction and async disk
Posted by Darrel Schneider <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47793/#review135999
-----------------------------------------------------------
Ship it!
Ship It!
- Darrel Schneider
On June 2, 2016, 2:26 p.m., Sai Boorlagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47793/
> -----------------------------------------------------------
>
> (Updated June 2, 2016, 2:26 p.m.)
>
>
> Review request for geode, Darrel Schneider, Eric Shu, Scott Jewell, and Ken Howe.
>
>
> Bugs: GEODE-93
> https://issues.apache.org/jira/browse/GEODE-93
>
>
> Repository: geode
>
>
> Description
> -------
>
> - Stats are not updated when a tombstone is scheduled/unscheduled.
> - All stats are updated when the data is finally flushed to the disk rather than doing it eagerly and compensating when on a new operation if the data is not yet written to disk
>
>
> Diffs
> -----
>
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/DiskEntry.java e015460ee412644b3bc62beb71403b9bd299ca48
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalRegion.java 8295487ef9e8930a929dfe8a10396bc83755ddc1
> geode-core/src/test/java/com/gemstone/gemfire/internal/cache/PartitionedRegionStatsJUnitTest.java 1a3277ca7b9597327e9ce42a61976ad731cc7c1c
> geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/parallel/ParallelGatewaySenderQueueOverflowDUnitTest.java 7169b2e64071511e4732321882490e9b0b769025
>
> Diff: https://reviews.apache.org/r/47793/diff/
>
>
> Testing
> -------
>
> precheckin
>
>
> Thanks,
>
> Sai Boorlagadda
>
>
Re: Review Request 47793: GEODE-93: Entry count stats are incorrect
with PR with entry eviction and async disk
Posted by Darrel Schneider <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47793/#review138045
-----------------------------------------------------------
Ship it!
Ship It!
- Darrel Schneider
On June 15, 2016, 10:14 a.m., Sai Boorlagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47793/
> -----------------------------------------------------------
>
> (Updated June 15, 2016, 10:14 a.m.)
>
>
> Review request for geode, Darrel Schneider, Eric Shu, Scott Jewell, and Ken Howe.
>
>
> Bugs: GEODE-93
> https://issues.apache.org/jira/browse/GEODE-93
>
>
> Repository: geode
>
>
> Description
> -------
>
> - Stats are not updated when a tombstone is scheduled/unscheduled.
> - All stats are updated when the data is finally flushed to the disk rather than doing it eagerly and compensating when on a new operation if the data is not yet written to disk
>
>
> Diffs
> -----
>
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/DiskEntry.java 5da0d9ae0370fe8d3c43b8294928ae3842f6bcbe
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalRegion.java 8b9664fed26eebfc56b0b9362765a0d82981832b
> geode-core/src/test/java/com/gemstone/gemfire/internal/cache/PartitionedRegionStatsJUnitTest.java 1a3277ca7b9597327e9ce42a61976ad731cc7c1c
> geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/parallel/ParallelGatewaySenderQueueOverflowDUnitTest.java e2fe10b089b10e8ffb6d999199899a5d768177e1
>
> Diff: https://reviews.apache.org/r/47793/diff/
>
>
> Testing
> -------
>
> precheckin
>
>
> Thanks,
>
> Sai Boorlagadda
>
>
Re: Review Request 47793: GEODE-93: Entry count stats are incorrect
with PR with entry eviction and async disk
Posted by Sai Boorlagadda <sb...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47793/
-----------------------------------------------------------
(Updated June 15, 2016, 5:14 p.m.)
Review request for geode, Darrel Schneider, Eric Shu, Scott Jewell, and Ken Howe.
Changes
-------
* Fixed a recovery stats test failure
* Refactored stats update logic for recovered entry case
Bugs: GEODE-93
https://issues.apache.org/jira/browse/GEODE-93
Repository: geode
Description
-------
- Stats are not updated when a tombstone is scheduled/unscheduled.
- All stats are updated when the data is finally flushed to the disk rather than doing it eagerly and compensating when on a new operation if the data is not yet written to disk
Diffs (updated)
-----
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/DiskEntry.java 5da0d9ae0370fe8d3c43b8294928ae3842f6bcbe
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalRegion.java 8b9664fed26eebfc56b0b9362765a0d82981832b
geode-core/src/test/java/com/gemstone/gemfire/internal/cache/PartitionedRegionStatsJUnitTest.java 1a3277ca7b9597327e9ce42a61976ad731cc7c1c
geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/parallel/ParallelGatewaySenderQueueOverflowDUnitTest.java e2fe10b089b10e8ffb6d999199899a5d768177e1
Diff: https://reviews.apache.org/r/47793/diff/
Testing
-------
precheckin
Thanks,
Sai Boorlagadda
Re: Review Request 47793: GEODE-93: Entry count stats are incorrect
with PR with entry eviction and async disk
Posted by Sai Boorlagadda <sb...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47793/
-----------------------------------------------------------
(Updated June 2, 2016, 9:26 p.m.)
Review request for geode, Darrel Schneider, Eric Shu, Scott Jewell, and Ken Howe.
Bugs: GEODE-93
https://issues.apache.org/jira/browse/GEODE-93
Repository: geode
Description
-------
- Stats are not updated when a tombstone is scheduled/unscheduled.
- All stats are updated when the data is finally flushed to the disk rather than doing it eagerly and compensating when on a new operation if the data is not yet written to disk
Diffs (updated)
-----
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/DiskEntry.java e015460ee412644b3bc62beb71403b9bd299ca48
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalRegion.java 8295487ef9e8930a929dfe8a10396bc83755ddc1
geode-core/src/test/java/com/gemstone/gemfire/internal/cache/PartitionedRegionStatsJUnitTest.java 1a3277ca7b9597327e9ce42a61976ad731cc7c1c
geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/parallel/ParallelGatewaySenderQueueOverflowDUnitTest.java 7169b2e64071511e4732321882490e9b0b769025
Diff: https://reviews.apache.org/r/47793/diff/
Testing
-------
precheckin
Thanks,
Sai Boorlagadda