----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60446/#review179017 -----------------------------------------------------------
Ship it! All my concerns are about refactoring and code hygiene and at least one of them was brough up by Galen. Can we merge this as it is and address the refactoring concerns as a follow up commit that goes to develop but not to the release branch? - Alexander Murmann On June 26, 2017, 10:24 p.m., Bruce Schuchardt wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60446/ > ----------------------------------------------------------- > > (Updated June 26, 2017, 10:24 p.m.) > > > Review request for geode and Barry Oglesby. > > > Bugs: GEODE-3072 > https://issues.apache.org/jira/browse/GEODE-3072 > > > Repository: geode > > > Description > ------- > > EventID and ThreadIdentifier hold the serialized form of portions of an > InternalDistributedMember identifier. This serialized form changed in v1.0.0 > and was causing .equals and .hashCode for these objects to return false when > they should have returned true owing to additional data being in the > serialized form of the identifier. > > This change set modifies the equals and hashCode methods of the classes to > ensure that they return the correct results in the presence of this > additional ID data. > > I created a dunit test to reproduce the problem but I think the behavior of > HARegionQueues isn't reliable enough to check in that test. I'm afraid that > it would end up being a "flaky" test that fails periodically. Instead, I'm > relying on a new junit test and the test that Barry already checked in. > > Note: I've also included two other things in this change set. > > First, LocalRegion.dumpBackingMap() is a test-hook that should log its > results at "info" level. I used it in debugging this problem. > > Second, I've added a new backward-compatibility version so now we have 100 > and 110. > > > Diffs > ----- > > > geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java > a1b4a9d5684d0148bd9e72c00c674afa299b2b9d > geode-core/src/main/java/org/apache/geode/internal/cache/EventID.java > 4d2ddc11bb1bd36446dc30b2033a6bc1ed455c98 > > geode-core/src/main/java/org/apache/geode/internal/cache/ha/ThreadIdentifier.java > ec165a5af210966241fdc1cee81702231557cc8c > > geode-core/src/test/java/org/apache/geode/internal/cache/ha/ThreadIdentifierJUnitTest.java > 29b22be6cc866217f165b755f11e68e1343843fe > > geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ClientServerMiscBCDUnitTest.java > 5fb8fa201b4c059d5d458a6af0915273f600f69f > geode-old-versions/build.gradle d85eb0b7dea6d3fa6b19a0030e64433cb4cb7cb9 > > > Diff: https://reviews.apache.org/r/60446/diff/2/ > > > Testing > ------- > > > File Attachments > ---------------- > > diffall.txt > > https://reviews.apache.org/media/uploaded/files/2017/06/26/98d794d9-6e21-4a1b-8ee5-2394ac2baa6f__diffall.txt > diffall.txt > > https://reviews.apache.org/media/uploaded/files/2017/06/26/cc4dfcc9-dc0b-48da-b97b-337563b127fe__diffall.txt > diffall.txt > > https://reviews.apache.org/media/uploaded/files/2017/06/26/de1405e6-38f0-4972-adf7-e43e568ff5ad__diffall.txt > diffall.txt > > https://reviews.apache.org/media/uploaded/files/2017/06/26/8f143bb9-0225-4e3b-ace1-09614cf5efe8__diffall.txt > > > Thanks, > > Bruce Schuchardt > >
