You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Kirk Lund <ki...@gmail.com> on 2016/08/16 01:56:06 UTC
Review Request 51125: GEODE-1782: stat resources with different time
stamps should not be equal
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51125/
-----------------------------------------------------------
Review request for geode, Anthony Baker, Darrel Schneider, Kevin Duling, Lynn Hughes-Godfrey, Lynn Gallinat, and Dan Smith.
Bugs: GEODE-1782
https://issues.apache.org/jira/browse/GEODE-1782
Repository: geode
Description
-------
GEODE-1782: stat resources with different time stamps should not be equal
* StatArchiveWithConsecutiveResourceInstGenerator generates gfs with multiple stat resources of same name but different times
* StatArchiveWithConsecutiveResourceInstIntegrationTest confirms existence of bug GEODE-1782: StatArchiveReader ignores later stats reso
urce with same name as closed stats resource
* ResourceInstTest verifies the underlying issue in StatArchiveReader.ResourceInst.equals and the fix
Note: this changeset builds upon the changeset for GEODE-1782: https://reviews.apache.org/r/51098
Explanation of new tests: I started out with a very large .gfs file and reproduced the bug with StatArchiveWithConsecutiveResourceInstIntegrationTest. Darrel helped identify the underlying cause in StatArchiveReader.ResourceInst.equals. I then wrote the unit test ResourceInstTest to reproduce the bug at the lowest level possible and fix it. Next I created StatArchiveWithConsecutiveResourceInstGenerator to generate a tiny .gfs file for StatArchiveWithConsecutiveResourceInstIntegrationTest so that I could check it in as well since it verifies the bug and the fix at a higher level.
Diffs
-----
geode-core/src/main/java/com/gemstone/gemfire/internal/statistics/StatArchiveReader.java 12637bc
geode-core/src/test/java/com/gemstone/gemfire/internal/statistics/ResourceInstTest.java PRE-CREATION
geode-core/src/test/java/com/gemstone/gemfire/internal/statistics/StatArchiveWithConsecutiveResourceInstGenerator.java PRE-CREATION
geode-core/src/test/java/com/gemstone/gemfire/internal/statistics/StatArchiveWithConsecutiveResourceInstGeneratorTest.java PRE-CREATION
geode-core/src/test/java/com/gemstone/gemfire/internal/statistics/StatArchiveWithConsecutiveResourceInstIntegrationTest.java PRE-CREATION
geode-core/src/test/java/com/gemstone/gemfire/internal/statistics/StatUtils.java PRE-CREATION
geode-core/src/test/resources/com/gemstone/gemfire/internal/statistics/StatArchiveWithConsecutiveResourceInstIntegrationTest.gfs PRE-CREATION
Diff: https://reviews.apache.org/r/51125/diff/
Testing
-------
precheckin
Thanks,
Kirk Lund
Re: Review Request 51125: GEODE-1782: stat resources with different
time stamps should not be equal
Posted by Darrel Schneider <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51125/#review145879
-----------------------------------------------------------
Ship it!
Ship It!
- Darrel Schneider
On Aug. 15, 2016, 6:56 p.m., Kirk Lund wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51125/
> -----------------------------------------------------------
>
> (Updated Aug. 15, 2016, 6:56 p.m.)
>
>
> Review request for geode, Anthony Baker, Darrel Schneider, Kevin Duling, Lynn Hughes-Godfrey, Lynn Gallinat, and Dan Smith.
>
>
> Bugs: GEODE-1782
> https://issues.apache.org/jira/browse/GEODE-1782
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-1782: stat resources with different time stamps should not be equal
>
> * StatArchiveWithConsecutiveResourceInstGenerator generates gfs with multiple stat resources of same name but different times
> * StatArchiveWithConsecutiveResourceInstIntegrationTest confirms existence of bug GEODE-1782: StatArchiveReader ignores later stats reso
> urce with same name as closed stats resource
> * ResourceInstTest verifies the underlying issue in StatArchiveReader.ResourceInst.equals and the fix
>
> Note: this changeset builds upon the changeset for GEODE-1781 (same branch): https://reviews.apache.org/r/51098
>
> Explanation of new tests: I started out with a very large .gfs file and reproduced the bug with StatArchiveWithConsecutiveResourceInstIntegrationTest. Darrel helped identify the underlying cause in StatArchiveReader.ResourceInst.equals. I then wrote the unit test ResourceInstTest to reproduce the bug at the lowest level possible and fix it. Next I created StatArchiveWithConsecutiveResourceInstGenerator to generate a tiny .gfs file for StatArchiveWithConsecutiveResourceInstIntegrationTest so that I could check it in as well since it verifies the bug and the fix at a higher level.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/com/gemstone/gemfire/internal/statistics/StatArchiveReader.java 12637bc
> geode-core/src/test/java/com/gemstone/gemfire/internal/statistics/ResourceInstTest.java PRE-CREATION
> geode-core/src/test/java/com/gemstone/gemfire/internal/statistics/StatArchiveWithConsecutiveResourceInstGenerator.java PRE-CREATION
> geode-core/src/test/java/com/gemstone/gemfire/internal/statistics/StatArchiveWithConsecutiveResourceInstGeneratorTest.java PRE-CREATION
> geode-core/src/test/java/com/gemstone/gemfire/internal/statistics/StatArchiveWithConsecutiveResourceInstIntegrationTest.java PRE-CREATION
> geode-core/src/test/java/com/gemstone/gemfire/internal/statistics/StatUtils.java PRE-CREATION
> geode-core/src/test/resources/com/gemstone/gemfire/internal/statistics/StatArchiveWithConsecutiveResourceInstIntegrationTest.gfs PRE-CREATION
>
> Diff: https://reviews.apache.org/r/51125/diff/
>
>
> Testing
> -------
>
> precheckin
>
>
> Thanks,
>
> Kirk Lund
>
>
Re: Review Request 51125: GEODE-1782: stat resources with different
time stamps should not be equal
Posted by Kirk Lund <ki...@gmail.com>.
> On Aug. 16, 2016, 4:45 p.m., Kevin Duling wrote:
> > In the equals() method, there are several more fields that aren't compared. Should they be?
I discussed this with Darrel and consensus is no the other fields do not help determine identity. All fields actually part of identity are now included in the new equals method.
- Kirk
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51125/#review145877
-----------------------------------------------------------
On Aug. 16, 2016, 1:56 a.m., Kirk Lund wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51125/
> -----------------------------------------------------------
>
> (Updated Aug. 16, 2016, 1:56 a.m.)
>
>
> Review request for geode, Anthony Baker, Darrel Schneider, Kevin Duling, Lynn Hughes-Godfrey, Lynn Gallinat, and Dan Smith.
>
>
> Bugs: GEODE-1782
> https://issues.apache.org/jira/browse/GEODE-1782
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-1782: stat resources with different time stamps should not be equal
>
> * StatArchiveWithConsecutiveResourceInstGenerator generates gfs with multiple stat resources of same name but different times
> * StatArchiveWithConsecutiveResourceInstIntegrationTest confirms existence of bug GEODE-1782: StatArchiveReader ignores later stats reso
> urce with same name as closed stats resource
> * ResourceInstTest verifies the underlying issue in StatArchiveReader.ResourceInst.equals and the fix
>
> Note: this changeset builds upon the changeset for GEODE-1781 (same branch): https://reviews.apache.org/r/51098
>
> Explanation of new tests: I started out with a very large .gfs file and reproduced the bug with StatArchiveWithConsecutiveResourceInstIntegrationTest. Darrel helped identify the underlying cause in StatArchiveReader.ResourceInst.equals. I then wrote the unit test ResourceInstTest to reproduce the bug at the lowest level possible and fix it. Next I created StatArchiveWithConsecutiveResourceInstGenerator to generate a tiny .gfs file for StatArchiveWithConsecutiveResourceInstIntegrationTest so that I could check it in as well since it verifies the bug and the fix at a higher level.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/com/gemstone/gemfire/internal/statistics/StatArchiveReader.java 12637bc
> geode-core/src/test/java/com/gemstone/gemfire/internal/statistics/ResourceInstTest.java PRE-CREATION
> geode-core/src/test/java/com/gemstone/gemfire/internal/statistics/StatArchiveWithConsecutiveResourceInstGenerator.java PRE-CREATION
> geode-core/src/test/java/com/gemstone/gemfire/internal/statistics/StatArchiveWithConsecutiveResourceInstGeneratorTest.java PRE-CREATION
> geode-core/src/test/java/com/gemstone/gemfire/internal/statistics/StatArchiveWithConsecutiveResourceInstIntegrationTest.java PRE-CREATION
> geode-core/src/test/java/com/gemstone/gemfire/internal/statistics/StatUtils.java PRE-CREATION
> geode-core/src/test/resources/com/gemstone/gemfire/internal/statistics/StatArchiveWithConsecutiveResourceInstIntegrationTest.gfs PRE-CREATION
>
> Diff: https://reviews.apache.org/r/51125/diff/
>
>
> Testing
> -------
>
> precheckin
>
>
> Thanks,
>
> Kirk Lund
>
>
Re: Review Request 51125: GEODE-1782: stat resources with different
time stamps should not be equal
Posted by Kevin Duling <kd...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51125/#review145877
-----------------------------------------------------------
In the equals() method, there are several more fields that aren't compared. Should they be?
- Kevin Duling
On Aug. 15, 2016, 6:56 p.m., Kirk Lund wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51125/
> -----------------------------------------------------------
>
> (Updated Aug. 15, 2016, 6:56 p.m.)
>
>
> Review request for geode, Anthony Baker, Darrel Schneider, Kevin Duling, Lynn Hughes-Godfrey, Lynn Gallinat, and Dan Smith.
>
>
> Bugs: GEODE-1782
> https://issues.apache.org/jira/browse/GEODE-1782
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-1782: stat resources with different time stamps should not be equal
>
> * StatArchiveWithConsecutiveResourceInstGenerator generates gfs with multiple stat resources of same name but different times
> * StatArchiveWithConsecutiveResourceInstIntegrationTest confirms existence of bug GEODE-1782: StatArchiveReader ignores later stats reso
> urce with same name as closed stats resource
> * ResourceInstTest verifies the underlying issue in StatArchiveReader.ResourceInst.equals and the fix
>
> Note: this changeset builds upon the changeset for GEODE-1781 (same branch): https://reviews.apache.org/r/51098
>
> Explanation of new tests: I started out with a very large .gfs file and reproduced the bug with StatArchiveWithConsecutiveResourceInstIntegrationTest. Darrel helped identify the underlying cause in StatArchiveReader.ResourceInst.equals. I then wrote the unit test ResourceInstTest to reproduce the bug at the lowest level possible and fix it. Next I created StatArchiveWithConsecutiveResourceInstGenerator to generate a tiny .gfs file for StatArchiveWithConsecutiveResourceInstIntegrationTest so that I could check it in as well since it verifies the bug and the fix at a higher level.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/com/gemstone/gemfire/internal/statistics/StatArchiveReader.java 12637bc
> geode-core/src/test/java/com/gemstone/gemfire/internal/statistics/ResourceInstTest.java PRE-CREATION
> geode-core/src/test/java/com/gemstone/gemfire/internal/statistics/StatArchiveWithConsecutiveResourceInstGenerator.java PRE-CREATION
> geode-core/src/test/java/com/gemstone/gemfire/internal/statistics/StatArchiveWithConsecutiveResourceInstGeneratorTest.java PRE-CREATION
> geode-core/src/test/java/com/gemstone/gemfire/internal/statistics/StatArchiveWithConsecutiveResourceInstIntegrationTest.java PRE-CREATION
> geode-core/src/test/java/com/gemstone/gemfire/internal/statistics/StatUtils.java PRE-CREATION
> geode-core/src/test/resources/com/gemstone/gemfire/internal/statistics/StatArchiveWithConsecutiveResourceInstIntegrationTest.gfs PRE-CREATION
>
> Diff: https://reviews.apache.org/r/51125/diff/
>
>
> Testing
> -------
>
> precheckin
>
>
> Thanks,
>
> Kirk Lund
>
>
Re: Review Request 51125: GEODE-1782: stat resources with different
time stamps should not be equal
Posted by Kirk Lund <ki...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51125/#review145831
-----------------------------------------------------------
One precheckin failures needs resolution. I believe the class is never serialized and should be added to excludedClasses.txt
:geode-core:integrationTest
com.gemstone.gemfire.codeAnalysis.AnalyzeSerializablesJUnitTest > testSerializables FAILED
java.lang.AssertionError: New or moved classes----------------------------------------
com/gemstone/gemfire/internal/statistics/platform/LinuxProcFsStatistics$CPU,false
If the class is not persisted or sent over the wire add it to the excludedClasses.txt file in the
/com/gemstone/gemfire/codeAnalysis/ directory. Otherwise if this doesn't
break backward compatibility move the file actualSerializables.dat to the
codeAnalysis test directory and rename to sanctionedSerializables.txt
at org.junit.Assert.fail(Assert.java:88)
at com.gemstone.gemfire.codeAnalysis.AnalyzeSerializablesJUnitTest.testSerializables(AnalyzeSerializablesJUnitTest.java:230)
3230 tests completed, 1 failed, 175 skipped
- Kirk Lund
On Aug. 16, 2016, 1:56 a.m., Kirk Lund wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51125/
> -----------------------------------------------------------
>
> (Updated Aug. 16, 2016, 1:56 a.m.)
>
>
> Review request for geode, Anthony Baker, Darrel Schneider, Kevin Duling, Lynn Hughes-Godfrey, Lynn Gallinat, and Dan Smith.
>
>
> Bugs: GEODE-1782
> https://issues.apache.org/jira/browse/GEODE-1782
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-1782: stat resources with different time stamps should not be equal
>
> * StatArchiveWithConsecutiveResourceInstGenerator generates gfs with multiple stat resources of same name but different times
> * StatArchiveWithConsecutiveResourceInstIntegrationTest confirms existence of bug GEODE-1782: StatArchiveReader ignores later stats reso
> urce with same name as closed stats resource
> * ResourceInstTest verifies the underlying issue in StatArchiveReader.ResourceInst.equals and the fix
>
> Note: this changeset builds upon the changeset for GEODE-1781 (same branch): https://reviews.apache.org/r/51098
>
> Explanation of new tests: I started out with a very large .gfs file and reproduced the bug with StatArchiveWithConsecutiveResourceInstIntegrationTest. Darrel helped identify the underlying cause in StatArchiveReader.ResourceInst.equals. I then wrote the unit test ResourceInstTest to reproduce the bug at the lowest level possible and fix it. Next I created StatArchiveWithConsecutiveResourceInstGenerator to generate a tiny .gfs file for StatArchiveWithConsecutiveResourceInstIntegrationTest so that I could check it in as well since it verifies the bug and the fix at a higher level.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/com/gemstone/gemfire/internal/statistics/StatArchiveReader.java 12637bc
> geode-core/src/test/java/com/gemstone/gemfire/internal/statistics/ResourceInstTest.java PRE-CREATION
> geode-core/src/test/java/com/gemstone/gemfire/internal/statistics/StatArchiveWithConsecutiveResourceInstGenerator.java PRE-CREATION
> geode-core/src/test/java/com/gemstone/gemfire/internal/statistics/StatArchiveWithConsecutiveResourceInstGeneratorTest.java PRE-CREATION
> geode-core/src/test/java/com/gemstone/gemfire/internal/statistics/StatArchiveWithConsecutiveResourceInstIntegrationTest.java PRE-CREATION
> geode-core/src/test/java/com/gemstone/gemfire/internal/statistics/StatUtils.java PRE-CREATION
> geode-core/src/test/resources/com/gemstone/gemfire/internal/statistics/StatArchiveWithConsecutiveResourceInstIntegrationTest.gfs PRE-CREATION
>
> Diff: https://reviews.apache.org/r/51125/diff/
>
>
> Testing
> -------
>
> precheckin
>
>
> Thanks,
>
> Kirk Lund
>
>
Re: Review Request 51125: GEODE-1782: stat resources with different
time stamps should not be equal
Posted by Kirk Lund <ki...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51125/
-----------------------------------------------------------
(Updated Aug. 16, 2016, 1:56 a.m.)
Review request for geode, Anthony Baker, Darrel Schneider, Kevin Duling, Lynn Hughes-Godfrey, Lynn Gallinat, and Dan Smith.
Changes
-------
Fixed typo in description
Bugs: GEODE-1782
https://issues.apache.org/jira/browse/GEODE-1782
Repository: geode
Description (updated)
-------
GEODE-1782: stat resources with different time stamps should not be equal
* StatArchiveWithConsecutiveResourceInstGenerator generates gfs with multiple stat resources of same name but different times
* StatArchiveWithConsecutiveResourceInstIntegrationTest confirms existence of bug GEODE-1782: StatArchiveReader ignores later stats reso
urce with same name as closed stats resource
* ResourceInstTest verifies the underlying issue in StatArchiveReader.ResourceInst.equals and the fix
Note: this changeset builds upon the changeset for GEODE-1781 (same branch): https://reviews.apache.org/r/51098
Explanation of new tests: I started out with a very large .gfs file and reproduced the bug with StatArchiveWithConsecutiveResourceInstIntegrationTest. Darrel helped identify the underlying cause in StatArchiveReader.ResourceInst.equals. I then wrote the unit test ResourceInstTest to reproduce the bug at the lowest level possible and fix it. Next I created StatArchiveWithConsecutiveResourceInstGenerator to generate a tiny .gfs file for StatArchiveWithConsecutiveResourceInstIntegrationTest so that I could check it in as well since it verifies the bug and the fix at a higher level.
Diffs
-----
geode-core/src/main/java/com/gemstone/gemfire/internal/statistics/StatArchiveReader.java 12637bc
geode-core/src/test/java/com/gemstone/gemfire/internal/statistics/ResourceInstTest.java PRE-CREATION
geode-core/src/test/java/com/gemstone/gemfire/internal/statistics/StatArchiveWithConsecutiveResourceInstGenerator.java PRE-CREATION
geode-core/src/test/java/com/gemstone/gemfire/internal/statistics/StatArchiveWithConsecutiveResourceInstGeneratorTest.java PRE-CREATION
geode-core/src/test/java/com/gemstone/gemfire/internal/statistics/StatArchiveWithConsecutiveResourceInstIntegrationTest.java PRE-CREATION
geode-core/src/test/java/com/gemstone/gemfire/internal/statistics/StatUtils.java PRE-CREATION
geode-core/src/test/resources/com/gemstone/gemfire/internal/statistics/StatArchiveWithConsecutiveResourceInstIntegrationTest.gfs PRE-CREATION
Diff: https://reviews.apache.org/r/51125/diff/
Testing
-------
precheckin
Thanks,
Kirk Lund