----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53867/#review156309 -----------------------------------------------------------
geode-core/src/main/java/org/apache/geode/pdx/internal/PeerTypeRegistration.java (line 284) <https://reviews.apache.org/r/53867/#comment226524> Shouldn't this be using newEnumId? I think you should delete nextEnumId as it caused confusion here and it's not needed anymore. geode-core/src/main/java/org/apache/geode/pdx/internal/PeerTypeRegistration.java (line 562) <https://reviews.apache.org/r/53867/#comment226531> The check for totalPdxTypeIdInDS == maxTypeId should be moved to this method and you should delete the instance variable totalPdxTypeIdInDS. Otherwise you've introduced a weird dependency between the two methods. Rename this method to indicate it is doing this check. geode-core/src/main/java/org/apache/geode/pdx/internal/PeerTypeRegistration.java (line 596) <https://reviews.apache.org/r/53867/#comment226532> Same comment here - get rid of this.totalEnumIdInDS and throw the exception in this method. If it shouldn't perform the check in some code paths you could parameterize the method to turn the check on/off. - Bruce Schuchardt On Nov. 18, 2016, 12:41 a.m., Hitesh Khamesra wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53867/ > ----------------------------------------------------------- > > (Updated Nov. 18, 2016, 12:41 a.m.) > > > Review request for geode, Bruce Schuchardt and Udo Kohlmeyer. > > > Repository: geode > > > Description > ------- > > Right now pdxtype id has 4 bytes. Out of those 4 bytes, one byte reserved for > distributed-system-id, this make sure type id generated from different > cluster has different id. For rest of the three bytes we just increment > counter to create new pdxtype id. In the field, we have observed that > sometimes this pdxType Id collides. One reason could be they end up having > same distributed-system-id for the different cluster. > Thus to avoid a collision, we will be using hashcode of pdxType for three > bytes of pdxType id. That will reduce the possibility of collision. > > > Diffs > ----- > > geode-core/src/main/java/org/apache/geode/pdx/internal/EnumId.java 5d399eb > geode-core/src/main/java/org/apache/geode/pdx/internal/PdxType.java b586f64 > > geode-core/src/main/java/org/apache/geode/pdx/internal/PeerTypeRegistration.java > 0226cca > geode-core/src/test/java/org/apache/geode/pdx/PdxAttributesJUnitTest.java > c45abce > geode-core/src/test/java/org/apache/geode/pdx/PdxSerializableJUnitTest.java > 5cd822c > > geode-wan/src/test/java/org/apache/geode/internal/cache/wan/WANTestBase.java > f9c18ec > > geode-wan/src/test/java/org/apache/geode/internal/cache/wan/misc/PDXNewWanDUnitTest.java > f1e8f42 > > Diff: https://reviews.apache.org/r/53867/diff/ > > > Testing > ------- > > > Thanks, > > Hitesh Khamesra > >
