> On July 3, 2017, 9:02 a.m., Dan Smith wrote: > > It looks like this code should work. I'm still a bit worried that we are > > relying on code that tries to deserialize the member id with the wrong > > version and then ignores errors that occur. I'd like to see us revisit > > propegating the version with the membership id bytes.
Yes, we need to have the version info in bytes that we retain in serialized form like this. > On July 3, 2017, 9:02 a.m., Dan Smith wrote: > > geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java > > Lines 129 (patched) > > <https://reviews.apache.org/r/60570/diff/3/?file=1767956#file1767956line129> > > > > I don't see this property ever get used? This is used in ProcessManager. The setting is already in place. > On July 3, 2017, 9:02 a.m., Dan Smith wrote: > > geode-core/src/main/java/org/apache/geode/internal/cache/EventID.java > > Lines 307 (patched) > > <https://reviews.apache.org/r/60570/diff/3/?file=1767958#file1767958line307> > > > > Is this not just targetVersion.equals(GFE_90) ? Anything between 1.0.0-incubating and GEODE-110 could have this problem - Bruce ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60570/#review179513 ----------------------------------------------------------- On June 30, 2017, 4:17 p.m., Bruce Schuchardt wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60570/ > ----------------------------------------------------------- > > (Updated June 30, 2017, 4:17 p.m.) > > > Review request for geode, Alexander Murmann, Barry Oglesby, Galen O'Sullivan, > Hitesh Khamesra, and Brian Rowe. > > > Bugs: GEODE-3153 > https://issues.apache.org/jira/browse/GEODE-3153 > > > Repository: geode > > > Description > ------- > > Another problem was found in backward-compatibility testing. If a 1.0.0 > client was receiving subscription events generated by a 1.0.0 peer "feeder" > member and the events were routed through a 1.0.0 server the client might see > duplicate events when the server is stopped and the client fails over to a > 1.2.0 server holding its redundant subscription queue. This is especially > possible if a large "ack" period is established in the client. > > The problem stems from the EventID deserialization/reserialization of the > memberID bytes when sending to a 1.0 client. It was deserializing using > Version.CURRENT, which ignores the UUID bytes in the serialized ID. Then it > serialized the identifier using the client's version, which includes the UUID > bytes but which are zero due to the version used in deserialization. > > > Diffs > ----- > > > geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java > bc3d708da2ae9a8e386accb8d15e2ed49123241e > geode-core/src/main/java/org/apache/geode/internal/Version.java > 557697159da644915e4ffe2405cdddc9ef37c5ac > geode-core/src/main/java/org/apache/geode/internal/cache/EventID.java > 55c89f1f2e0800371dd4a30c4312c44f942a45ea > > geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ClientServerMiscBCDUnitTest.java > bc48d976096fafe2545e707da68dab5120ddca51 > > geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/ClientServerMiscDUnitTest.java > bfe4646b9abdf6075e8d30fab3d79924faade2aa > > geode-core/src/test/resources/org/apache/geode/codeAnalysis/sanctionedDataSerializables.txt > b69e004d63d74eccd5cd562ea269363ba3f2782e > > > Diff: https://reviews.apache.org/r/60570/diff/3/ > > > Testing > ------- > > new unit tests, precheckin > > > Thanks, > > Bruce Schuchardt > >
