GEODE-2915 Messages rejected due to unknown "vmkind" The fix for GEODE_2875 has exacerbated this problem, which we used to only see in cases where disable-tcp=true or when multicast was enabled.
The problem is that JGroupsMessenger is not sending the "vmkind" of the sender in message headers. This part of the header comes from GMSMember.writeEssentialData(). I've changed it here to include the vmKind if the recipient isn't using geode 1.0, which doesn't expect the version byte. Project: http://git-wip-us.apache.org/repos/asf/geode/repo Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/973eb33e Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/973eb33e Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/973eb33e Branch: refs/heads/feature/GEODE-2580 Commit: 973eb33e20993331d192c2ba85d7c9d35f209bdf Parents: a445853 Author: Bruce Schuchardt <bschucha...@pivotal.io> Authored: Tue May 16 13:45:49 2017 -0700 Committer: Bruce Schuchardt <bschucha...@pivotal.io> Committed: Tue May 16 13:47:03 2017 -0700 ---------------------------------------------------------------------- .../membership/InternalDistributedMember.java | 11 + .../internal/membership/gms/GMSMember.java | 8 +- .../gms/messenger/JGroupsMessenger.java | 3 + .../gms/mgr/GMSMembershipManager.java | 28 +- .../java/org/apache/geode/internal/Version.java | 49 +- .../cache/tier/sockets/CommandInitializer.java | 11 +- .../geode/internal/i18n/LocalizedStrings.java | 2 +- .../configuration/domain/XmlEntity.java | 10 +- .../membership/gms/GMSMemberJUnitTest.java | 47 + .../test/dunit/standalone/ProcessManager.java | 2 +- .../sanctionedDataSerializables.txt | 1074 +++++++++--------- geode-old-versions/build.gradle | 2 +- 12 files changed, 652 insertions(+), 595 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/geode/blob/973eb33e/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java index b993b53..7170f20 100755 --- a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java +++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java @@ -165,6 +165,17 @@ public class InternalDistributedMember implements DistributedMember, Externaliza cachedToString = null; } + /** + * Replace the current NetMember with the given member. This can be used to fill out an + * InternalDistributedMember that was created from a partial NetMember created by + * readEssentialData. + * + * @param m the replacement NetMember + */ + public void setNetMember(NetMember m) { + this.netMbr = m; + } + // private void checkHostName() { // // bug #44858: debug method to find who is putting a host name instead of addr into an ID // if (!SocketCreator.resolve_dns http://git-wip-us.apache.org/repos/asf/geode/blob/973eb33e/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMember.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMember.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMember.java index b7079f8..c82d97e 100755 --- a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMember.java +++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMember.java @@ -20,6 +20,7 @@ import org.apache.geode.distributed.internal.DistributionConfig; import org.apache.geode.distributed.internal.membership.MemberAttributes; import org.apache.geode.distributed.internal.membership.NetMember; import org.apache.geode.internal.DataSerializableFixedID; +import org.apache.geode.internal.InternalDataSerializer; import org.apache.geode.internal.Version; import org.apache.geode.internal.i18n.LocalizedStrings; import org.jgroups.util.UUID; @@ -460,7 +461,9 @@ public class GMSMember implements NetMember, DataSerializableFixedID { out.writeInt(vmViewId); out.writeLong(uuidMSBs); out.writeLong(uuidLSBs); - + if (InternalDataSerializer.getVersionForDataStream(out).compareTo(Version.GEODE_120) >= 0) { + out.writeByte(vmKind); + } } @Override @@ -487,6 +490,9 @@ public class GMSMember implements NetMember, DataSerializableFixedID { this.vmViewId = in.readInt(); this.uuidMSBs = in.readLong(); this.uuidLSBs = in.readLong(); + if (InternalDataSerializer.getVersionForDataStream(in).compareTo(Version.GEODE_120) >= 0) { + this.vmKind = in.readByte(); + } } @Override http://git-wip-us.apache.org/repos/asf/geode/blob/973eb33e/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/messenger/JGroupsMessenger.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/messenger/JGroupsMessenger.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/messenger/JGroupsMessenger.java index bfc8b61..b07aa59 100644 --- a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/messenger/JGroupsMessenger.java +++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/messenger/JGroupsMessenger.java @@ -990,6 +990,9 @@ public class JGroupsMessenger implements Messenger { short ordinal = Version.readOrdinal(dis); + // logger.info("JGroupsMessenger read ordinal {} version is {}. My version is {}", + // ordinal, Version.fromOrdinalOrCurrent(ordinal), Version.CURRENT); + if (ordinal < Version.CURRENT_ORDINAL) { dis = new VersionedDataInputStream(dis, Version.fromOrdinalNoThrow(ordinal, true)); } http://git-wip-us.apache.org/repos/asf/geode/blob/973eb33e/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/mgr/GMSMembershipManager.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/mgr/GMSMembershipManager.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/mgr/GMSMembershipManager.java index 8cdd6a5..a41e08a 100644 --- a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/mgr/GMSMembershipManager.java +++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/mgr/GMSMembershipManager.java @@ -472,17 +472,21 @@ public class GMSMembershipManager implements MembershipManager, Manager { InternalDistributedMember m = newView.getMembers().get(i); // Once a member has been seen via a view, remove them from the - // newborn set - boolean wasSurprise = surpriseMembers.remove(m) != null; - - // bug #45155 - membership view processing was slow, causing a member to connect as - // "surprise" - // and the surprise timeout removed the member and shunned it, keeping it from being - // recognized as a valid member when it was finally seen in a view - // if (isShunned(m)) { - // warnShuns.add(m); - // continue; - // } + // newborn set. Replace the netmember of the surpriseMember ID + // in case it was a partial ID and is being retained by DistributionManager + // or some other object + boolean wasSurprise = surpriseMembers.containsKey(m); + if (wasSurprise) { + for (Iterator<Map.Entry<InternalDistributedMember, Long>> iterator = + surpriseMembers.entrySet().iterator(); iterator.hasNext();) { + Entry<InternalDistributedMember, Long> entry = iterator.next(); + if (entry.getKey().equals(m)) { + entry.getKey().setNetMember(m.getNetMember()); + iterator.remove(); + break; + } + } + } // if it's in a view, it's no longer suspect suspectedMembers.remove(m); @@ -491,7 +495,7 @@ public class GMSMembershipManager implements MembershipManager, Manager { continue; // already seen } - // ARB: unblock any waiters for this particular member. + // unblock any waiters for this particular member. // i.e. signal any waiting threads in tcpconduit. String authInit = this.services.getConfig().getDistributionConfig().getSecurityPeerAuthInit(); http://git-wip-us.apache.org/repos/asf/geode/blob/973eb33e/geode-core/src/main/java/org/apache/geode/internal/Version.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/internal/Version.java b/geode-core/src/main/java/org/apache/geode/internal/Version.java index e058edb..1c131e8 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/Version.java +++ b/geode-core/src/main/java/org/apache/geode/internal/Version.java @@ -59,7 +59,7 @@ public class Version implements Comparable<Version> { /** byte used as ordinal to represent this <code>Version</code> */ private final short ordinal; - public static final int HIGHEST_VERSION = 55; + public static final int HIGHEST_VERSION = 60; private static final Version[] VALUES = new Version[HIGHEST_VERSION + 1]; @@ -183,18 +183,23 @@ public class Version implements Comparable<Version> { private static final byte GEODE_110_ORDINAL = 50; public static final Version GEODE_110 = - new Version("GEODE", "1.1.0", (byte) 9, (byte) 0, (byte) 1, (byte) 0, GEODE_110_ORDINAL); + new Version("GEODE", "1.1.0", (byte) 1, (byte) 1, (byte) 0, (byte) 0, GEODE_110_ORDINAL); - private static final byte GFE_91_ORDINAL = 55; + private static final byte GEODE_111_ORDINAL = 55; - public static final Version GFE_91 = - new Version("GFE", "9.1", (byte) 9, (byte) 1, (byte) 0, (byte) 0, GFE_91_ORDINAL); + public static final Version GEODE_111 = + new Version("GEODE", "1.1.1", (byte) 1, (byte) 1, (byte) 1, (byte) 0, GEODE_111_ORDINAL); + + private static final byte GEODE_120_ORDINAL = 60; + + public static final Version GEODE_120 = + new Version("GEODE", "1.2.0", (byte) 1, (byte) 2, (byte) 0, (byte) 0, GEODE_120_ORDINAL); /** * This constant must be set to the most current version of the product. !!! NOTE: update * HIGHEST_VERSION when changing CURRENT !!! */ - public static final Version CURRENT = GFE_91; + public static final Version CURRENT = GEODE_120; /** * A lot of versioning code needs access to the current version's ordinal @@ -504,34 +509,10 @@ public class Version implements Comparable<Version> { @Override public int compareTo(Version other) { if (other != null) { - // [bruce] old implementation used ordinals for comparison, but this requires - // ordinals to be in increasing order, which may not always be possible - // // byte min/max can't overflow int, so use (a-b) - // final int thisOrdinal = this.ordinal; - // final int otherOrdinal = o.ordinal; - // return (thisOrdinal - otherOrdinal); - // [bruce] new implementation uses major/minor/patch/build - if (this.majorVersion > other.majorVersion) { - return 1; - } else if (other.majorVersion > this.majorVersion) { - return -1; - } - if (this.minorVersion > other.minorVersion) { - return 1; - } else if (other.minorVersion > this.minorVersion) { - return -1; - } - if (this.release > other.release) { - return 1; - } else if (other.release > this.release) { - return -1; - } - if (this.patch > other.patch) { - return 1; - } else if (other.patch > this.patch) { - return -1; - } - return 0; + // byte min/max can't overflow int, so use (a-b) + final int thisOrdinal = this.ordinal; + final int otherOrdinal = other.ordinal; + return (thisOrdinal - otherOrdinal); } else { return 1; } http://git-wip-us.apache.org/repos/asf/geode/blob/973eb33e/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/CommandInitializer.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/CommandInitializer.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/CommandInitializer.java index 71586a0..9995e46 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/CommandInitializer.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/CommandInitializer.java @@ -336,9 +336,14 @@ public class CommandInitializer { ALL_COMMANDS.put(Version.GEODE_110, geode110Commands); } { - Map<Integer, Command> gfe91Commands = new HashMap<Integer, Command>(); - gfe91Commands.putAll(ALL_COMMANDS.get(Version.GEODE_110)); - ALL_COMMANDS.put(Version.GFE_91, gfe91Commands); + Map<Integer, Command> geode111Commands = new HashMap<Integer, Command>(); + geode111Commands.putAll(ALL_COMMANDS.get(Version.GEODE_110)); + ALL_COMMANDS.put(Version.GEODE_111, geode111Commands); + } + { + Map<Integer, Command> commands = new HashMap<Integer, Command>(); + commands.putAll(ALL_COMMANDS.get(Version.GEODE_111)); + ALL_COMMANDS.put(Version.GEODE_120, commands); } } http://git-wip-us.apache.org/repos/asf/geode/blob/973eb33e/geode-core/src/main/java/org/apache/geode/internal/i18n/LocalizedStrings.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/internal/i18n/LocalizedStrings.java b/geode-core/src/main/java/org/apache/geode/internal/i18n/LocalizedStrings.java index 1b33094..2fb8c8d 100755 --- a/geode-core/src/main/java/org/apache/geode/internal/i18n/LocalizedStrings.java +++ b/geode-core/src/main/java/org/apache/geode/internal/i18n/LocalizedStrings.java @@ -4222,7 +4222,7 @@ public class LocalizedStrings { public static final StringId AbstractDistributionConfig_UNEXPECTED_PROBLEM_GETTING_INETADDRESS_0 = new StringId(3548, "Unexpected problem getting inetAddress: {0}"); public static final StringId DistributionManager_UNKNOWN_MEMBER_TYPE_0 = - new StringId(3549, "Unknown member type: {0}"); + new StringId(3549, "Unknown member type: {0}"); public static final StringId DistributionManager_UNKNOWN_PROCESSOR_TYPE = new StringId(3550, "unknown processor type {0}"); public static final StringId DLockRequestProcessor_UNKNOWN_RESPONSE_CODE_0 = http://git-wip-us.apache.org/repos/asf/geode/blob/973eb33e/geode-core/src/main/java/org/apache/geode/management/internal/configuration/domain/XmlEntity.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/domain/XmlEntity.java b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/domain/XmlEntity.java index be74e84..0dbe7e5 100644 --- a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/domain/XmlEntity.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/domain/XmlEntity.java @@ -410,12 +410,12 @@ public class XmlEntity implements VersionedDataSerializable { @Override public void toData(DataOutput out) throws IOException { - toDataPre_GFE_9_1_0_0(out); + toDataPre_GEODE_1_1_1_0(out); DataSerializer.writeString(this.childPrefix, out); DataSerializer.writeString(this.childNamespace, out); } - public void toDataPre_GFE_9_1_0_0(DataOutput out) throws IOException { + public void toDataPre_GEODE_1_1_1_0(DataOutput out) throws IOException { DataSerializer.writeString(this.type, out); DataSerializer.writeObject(this.attributes, out); DataSerializer.writeString(this.xmlDefinition, out); @@ -426,12 +426,12 @@ public class XmlEntity implements VersionedDataSerializable { @Override public void fromData(DataInput in) throws IOException, ClassNotFoundException { - fromDataPre_GFE_9_1_0_0(in); + fromDataPre_GEODE_1_1_1_0(in); this.childPrefix = DataSerializer.readString(in); this.childNamespace = DataSerializer.readString(in); } - public void fromDataPre_GFE_9_1_0_0(DataInput in) throws IOException, ClassNotFoundException { + public void fromDataPre_GEODE_1_1_1_0(DataInput in) throws IOException, ClassNotFoundException { this.type = DataSerializer.readString(in); this.attributes = DataSerializer.readObject(in); this.xmlDefinition = DataSerializer.readString(in); @@ -452,7 +452,7 @@ public class XmlEntity implements VersionedDataSerializable { @Override public Version[] getSerializationVersions() { - return new Version[] {Version.GFE_91}; + return new Version[] {Version.GEODE_111}; } /** http://git-wip-us.apache.org/repos/asf/geode/blob/973eb33e/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/GMSMemberJUnitTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/GMSMemberJUnitTest.java b/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/GMSMemberJUnitTest.java index f471ad9..b5e4cfc 100644 --- a/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/GMSMemberJUnitTest.java +++ b/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/GMSMemberJUnitTest.java @@ -17,8 +17,17 @@ package org.apache.geode.distributed.internal.membership.gms; import static org.junit.Assert.*; import static org.mockito.Mockito.*; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.DataInput; +import java.io.DataInputStream; +import java.io.DataOutput; +import java.io.DataOutputStream; import java.net.InetAddress; +import org.apache.geode.internal.HeapDataOutputStream; +import org.apache.geode.internal.Version; +import org.apache.geode.internal.VersionedDataInputStream; import org.jgroups.util.UUID; import org.junit.Test; import org.junit.experimental.categories.Category; @@ -177,4 +186,42 @@ public class GMSMemberJUnitTest { member.setUUID(uuid); assertNotNull(member.getUUID()); } + + /** + * <p> + * GEODE-2875 - adds vmKind to on-wire form of GMSMember.writeEssentialData + * </p> + * <p> + * This must be backward-compatible with Geode 1.0 (Version.GFE_90) + * </p> + * + * @throws Exception + */ + @Test + public void testGMSMemberBackwardCompatibility() throws Exception { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + MemberAttributes attributes = new MemberAttributes(10, 20, 1, 2, "member", null, null); + GMSMember member = new GMSMember(); + member.setAttributes(attributes); + DataOutput dataOutput = new DataOutputStream(baos); + member.writeEssentialData(dataOutput); + + // vmKind should be transmitted to a member with the current version + ByteArrayInputStream bais = new ByteArrayInputStream(baos.toByteArray()); + DataInput dataInput = new DataInputStream(bais); + GMSMember newMember = new GMSMember(); + newMember.readEssentialData(dataInput); + assertEquals(1, newMember.getVmKind()); + + // vmKind should not be transmitted to a member with version GFE_90 or earlier + dataOutput = new HeapDataOutputStream(Version.GFE_90); + member.writeEssentialData(dataOutput); + bais = new ByteArrayInputStream(baos.toByteArray()); + dataInput = new VersionedDataInputStream(new DataInputStream(bais), Version.GFE_90); + newMember = new GMSMember(); + newMember.readEssentialData(dataInput); + assertEquals(0, newMember.getVmKind()); + } + + } http://git-wip-us.apache.org/repos/asf/geode/blob/973eb33e/geode-core/src/test/java/org/apache/geode/test/dunit/standalone/ProcessManager.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/test/dunit/standalone/ProcessManager.java b/geode-core/src/test/java/org/apache/geode/test/dunit/standalone/ProcessManager.java old mode 100644 new mode 100755 index 4e0e82f..21b79e8 --- a/geode-core/src/test/java/org/apache/geode/test/dunit/standalone/ProcessManager.java +++ b/geode-core/src/test/java/org/apache/geode/test/dunit/standalone/ProcessManager.java @@ -76,7 +76,7 @@ public class ProcessManager { workingDir.mkdirs(); } else if (!bouncedVM || DUnitLauncher.MAKE_NEW_WORKING_DIRS) { try { - Files.delete(workingDir.toPath()); + FileUtils.deleteDirectory(workingDir); } catch (IOException e) { // This delete is occasionally failing on some platforms, maybe due to a lingering // process. Allow the process to be launched anyway.
