This is an automated email from the ASF dual-hosted git repository. burcham pushed a commit to branch support/1.13 in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/support/1.13 by this push: new 1c18206 GEODE-8240: Member Identifier Future Versions 1c18206 is described below commit 1c18206887aac2defe5411b62a44951fc14429ef Author: Bill Burcham <bill.burc...@gmail.com> AuthorDate: Thu Jul 2 09:08:34 2020 -0700 GEODE-8240: Member Identifier Future Versions cherry picked from commit bfe1ca113124ea958dc02dd9be90cdb0aabe5c4d --- .../cache/client/ClientCacheFactoryJUnitTest.java | 6 +- .../cache/ha/HARegionQueueIntegrationTest.java | 2 +- .../codeAnalysis/sanctionedDataSerializables.txt | 93 ++------------ .../internal/ClusterDistributionManager.java | 4 +- .../internal/LonerDistributionManager.java | 4 +- .../membership/InternalDistributedMember.java | 16 ++- .../internal/cache/AbstractUpdateOperation.java | 2 +- .../internal/cache/DistributedPutAllOperation.java | 5 +- .../cache/DistributedRemoveAllOperation.java | 10 +- ...xpireDisconnectedClientTransactionsMessage.java | 4 +- .../geode/internal/cache/GemFireCacheImpl.java | 3 +- .../internal/cache/InitialImageOperation.java | 16 +-- .../org/apache/geode/internal/cache/Oplog.java | 3 +- .../geode/internal/cache/PartitionedRegion.java | 4 +- .../cache/PartitionedRegionQueryEvaluator.java | 2 +- .../geode/internal/cache/ServerPingMessage.java | 2 +- .../cache/partitioned/IndexCreationMsg.java | 2 +- .../cache/partitioned/PutAllPRMessage.java | 7 +- .../internal/cache/partitioned/QueryMessage.java | 4 +- .../cache/partitioned/RemoveAllPRMessage.java | 8 +- .../geode/internal/cache/tx/DistTxEntryEvent.java | 4 +- .../internal/cache/tx/RemotePutAllMessage.java | 2 +- .../internal/cache/tx/RemoteRemoveAllMessage.java | 2 +- .../configuration/domain/Configuration.java | 6 +- .../operation/RestoreRedundancyPerformer.java | 2 +- .../management/internal/util/ManagementUtils.java | 3 +- ...eDisconnectedClientTransactionsMessageTest.java | 2 +- .../operation/RestoreRedundancyPerformerTest.java | 8 +- .../RollingUpgrade2DUnitTestBase.java | 3 +- .../rollingupgrade/RollingUpgradeDUnitTest.java | 23 +++- .../cli/commands/CreateGatewaySenderCommand.java | 3 +- .../internal/cli/commands/RedundancyCommand.java | 2 +- .../commands/CreateGatewaySenderCommandTest.java | 4 +- .../org/apache/geode/test/version/TestVersion.java | 11 -- .../apache/geode/test/version/VersionManager.java | 4 +- .../cache/lucene/internal/LuceneServiceImpl.java | 9 +- .../internal/distributed/LuceneQueryFunction.java | 2 +- .../gms/GMSMemberDataVersionJUnitTest.java | 99 +++++++++++++++ .../gms/membership/GMSJoinLeaveJUnitTest.java | 30 +++++ .../internal/membership/api/MemberData.java | 4 +- .../internal/membership/api/MemberIdentifier.java | 3 +- .../internal/membership/gms/GMSMemberData.java | 46 +++---- .../internal/membership/gms/GMSMembership.java | 9 +- .../internal/membership/gms/GMSMembershipView.java | 19 +++ .../membership/gms/MemberIdentifierImpl.java | 12 +- .../membership/gms/membership/GMSJoinLeave.java | 2 + geode-old-versions/README.md | 55 +++++++++ .../geode/internal/serialization/Version.java | 82 +------------ .../internal/serialization/VersionOrdinal.java | 83 +++++++++++++ .../internal/serialization/VersionOrdinalImpl.java | 136 +++++++++++++++++++++ .../serialization/VersionedDataStream.java | 13 +- .../geode/internal/serialization/Versioning.java | 37 ++++++ .../internal/DSFIDSerializerImpl.java | 19 +-- .../internal/serialization/VersionJUnitTest.java | 18 +++ .../serialization/VersionOrdinalImplJUnitTest.java | 127 +++++++++++++++++++ .../TcpServerProductVersionDUnitTest.java | 11 +- 56 files changed, 776 insertions(+), 316 deletions(-) diff --git a/geode-core/src/integrationTest/java/org/apache/geode/cache/client/ClientCacheFactoryJUnitTest.java b/geode-core/src/integrationTest/java/org/apache/geode/cache/client/ClientCacheFactoryJUnitTest.java index e914f0f..77ccabb 100644 --- a/geode-core/src/integrationTest/java/org/apache/geode/cache/client/ClientCacheFactoryJUnitTest.java +++ b/geode-core/src/integrationTest/java/org/apache/geode/cache/client/ClientCacheFactoryJUnitTest.java @@ -355,7 +355,7 @@ public class ClientCacheFactoryJUnitTest { (InternalDistributedMember) clientCache.getDistributedSystem().getDistributedMember(); MemberIdentifier gmsID = memberID; memberID.setVersionObjectForTest(Version.GFE_82); - assertThat(memberID.getVersionObject()).isEqualTo(Version.GFE_82); + assertThat(memberID.getVersionOrdinalObject()).isEqualTo(Version.GFE_82); ClientProxyMembershipID clientID = ClientProxyMembershipID.getClientId(memberID); HeapDataOutputStream out = new HeapDataOutputStream(Version.GFE_82); @@ -367,7 +367,7 @@ public class ClientCacheFactoryJUnitTest { ClientProxyMembershipID newID = DataSerializer.readObject(in); InternalDistributedMember newMemberID = (InternalDistributedMember) newID.getDistributedMember(); - assertThat(newMemberID.getVersionObject()).isEqualTo(Version.GFE_82); + assertThat(newMemberID.getVersionOrdinalObject()).isEqualTo(Version.GFE_82); assertThat(newID.getClientVersion()).isEqualTo(Version.GFE_82); assertThat(newMemberID.getUuidLeastSignificantBits()).isEqualTo(0); @@ -383,7 +383,7 @@ public class ClientCacheFactoryJUnitTest { Version.CURRENT); newID = DataSerializer.readObject(in); newMemberID = (InternalDistributedMember) newID.getDistributedMember(); - assertThat(newMemberID.getVersionObject()).isEqualTo(Version.CURRENT); + assertThat(newMemberID.getVersionOrdinalObject()).isEqualTo(Version.CURRENT); assertThat(newID.getClientVersion()).isEqualTo(Version.CURRENT); assertThat(newMemberID.getUuidLeastSignificantBits()) diff --git a/geode-core/src/integrationTest/java/org/apache/geode/internal/cache/ha/HARegionQueueIntegrationTest.java b/geode-core/src/integrationTest/java/org/apache/geode/internal/cache/ha/HARegionQueueIntegrationTest.java index 4778608..293bfd9 100644 --- a/geode-core/src/integrationTest/java/org/apache/geode/internal/cache/ha/HARegionQueueIntegrationTest.java +++ b/geode-core/src/integrationTest/java/org/apache/geode/internal/cache/ha/HARegionQueueIntegrationTest.java @@ -509,7 +509,7 @@ public class HARegionQueueIntegrationTest { private InternalDistributedMember createMember() { // Create an InternalDistributedMember InternalDistributedMember member = mock(InternalDistributedMember.class); - when(member.getVersionObject()).thenReturn(Version.CURRENT); + when(member.getVersionOrdinalObject()).thenReturn(Version.CURRENT); return member; } diff --git a/geode-core/src/integrationTest/resources/org/apache/geode/codeAnalysis/sanctionedDataSerializables.txt b/geode-core/src/integrationTest/resources/org/apache/geode/codeAnalysis/sanctionedDataSerializables.txt index 094a8ea..1a65b6c 100644 --- a/geode-core/src/integrationTest/resources/org/apache/geode/codeAnalysis/sanctionedDataSerializables.txt +++ b/geode-core/src/integrationTest/resources/org/apache/geode/codeAnalysis/sanctionedDataSerializables.txt @@ -282,74 +282,6 @@ toData,12 toDataPre_GFE_7_1_0_0,12 toDataPre_GFE_9_0_0_0,12 -org/apache/geode/distributed/internal/membership/adapter/LocalViewMessage,2 -fromData,8 -toData,8 - -org/apache/geode/distributed/internal/membership/gms/GMSMembershipView,2 -fromData,121 -toData,72 - -org/apache/geode/distributed/internal/membership/gms/locator/FindCoordinatorRequest,2 -fromData,112 -toData,132 - -org/apache/geode/distributed/internal/membership/gms/locator/FindCoordinatorResponse,2 -fromData,130 -toData,114 - -org/apache/geode/distributed/internal/membership/gms/locator/GetViewRequest,2 -fromData,1 -toData,1 - -org/apache/geode/distributed/internal/membership/gms/locator/GetViewResponse,2 -fromData,20 -toData,17 - -org/apache/geode/distributed/internal/membership/gms/messages/FinalCheckPassedMessage,2 -fromData,20 -toData,17 - -org/apache/geode/distributed/internal/membership/gms/messages/HeartbeatMessage,2 -fromData,11 -toData,11 - -org/apache/geode/distributed/internal/membership/gms/messages/HeartbeatRequestMessage,2 -fromData,30 -toData,27 - -org/apache/geode/distributed/internal/membership/gms/messages/InstallViewMessage,2 -fromData,60 -toData,56 - -org/apache/geode/distributed/internal/membership/gms/messages/JoinRequestMessage,2 -fromData,63 -toData,60 - -org/apache/geode/distributed/internal/membership/gms/messages/JoinResponseMessage,2 -fromData,63 -toData,57 - -org/apache/geode/distributed/internal/membership/gms/messages/LeaveRequestMessage,2 -fromData,28 -toData,25 - -org/apache/geode/distributed/internal/membership/gms/messages/NetworkPartitionMessage,2 -fromData,1 -toData,1 - -org/apache/geode/distributed/internal/membership/gms/messages/RemoveMemberMessage,2 -fromData,28 -toData,25 - -org/apache/geode/distributed/internal/membership/gms/messages/SuspectMembersMessage,2 -fromData,63 -toData,92 - -org/apache/geode/distributed/internal/membership/gms/messages/ViewAckMessage,2 -fromData,40 -toData,37 - org/apache/geode/distributed/internal/streaming/StreamingOperation$RequestStreamingMessage,2 fromData,17 toData,17 @@ -358,14 +290,6 @@ org/apache/geode/distributed/internal/streaming/StreamingOperation$StreamingRepl fromData,425 toData,86 -org/apache/geode/distributed/internal/tcpserver/InfoResponse,2 -fromData,9 -toData,9 - -org/apache/geode/distributed/internal/tcpserver/VersionResponse,2 -fromData,11 -toData,11 - org/apache/geode/internal/DSFIDFactory,2 fromData,8 toData,8 @@ -578,9 +502,6 @@ org/apache/geode/internal/admin/remote/LicenseInfoResponse,2 fromData,18 toData,15 -org/apache/geode/internal/admin/remote/MissingPersistentIDsRequest,1 -fromData,7 - org/apache/geode/internal/admin/remote/MissingPersistentIDsResponse,2 fromData,129 toData,115 @@ -947,7 +868,7 @@ org/apache/geode/internal/cache/DistributedPutAllOperation$PutAllEntryData,1 toData,252 org/apache/geode/internal/cache/DistributedPutAllOperation$PutAllMessage,2 -fromData,217 +fromData,214 toData,188 org/apache/geode/internal/cache/DistributedRegionFunctionStreamingMessage,2 @@ -955,7 +876,7 @@ fromData,177 toData,174 org/apache/geode/internal/cache/DistributedRemoveAllOperation$RemoveAllMessage,2 -fromData,217 +fromData,194 toData,188 org/apache/geode/internal/cache/DistributedTombstoneOperation$TombstoneMessage,2 @@ -1648,7 +1569,7 @@ fromData,17 toData,17 org/apache/geode/internal/cache/partitioned/PutAllPRMessage,2 -fromData,187 +fromData,164 toData,201 org/apache/geode/internal/cache/partitioned/PutAllPRMessage$PutAllReplyMessage,2 @@ -1676,7 +1597,7 @@ fromData,54 toData,35 org/apache/geode/internal/cache/partitioned/RemoveAllPRMessage,2 -fromData,194 +fromData,164 toData,201 org/apache/geode/internal/cache/partitioned/RemoveAllPRMessage$RemoveAllReplyMessage,2 @@ -1922,7 +1843,7 @@ fromData,45 toData,112 org/apache/geode/internal/cache/tx/RemotePutAllMessage,2 -fromData,227 +fromData,224 toData,180 org/apache/geode/internal/cache/tx/RemotePutAllMessage$PutAllReplyMessage,2 @@ -1938,7 +1859,7 @@ fromData,84 toData,95 org/apache/geode/internal/cache/tx/RemoteRemoveAllMessage,2 -fromData,207 +fromData,204 toData,180 org/apache/geode/internal/cache/tx/RemoteRemoveAllMessage$RemoveAllReplyMessage,2 @@ -2042,7 +1963,7 @@ fromData,17 toData,17 org/apache/geode/management/internal/configuration/domain/Configuration,2 -fromData,110 +fromData,111 toData,62 org/apache/geode/management/internal/configuration/domain/ConfigurationChangeResult,2 diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/ClusterDistributionManager.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/ClusterDistributionManager.java index 022fa59..338a8b7 100644 --- a/geode-core/src/main/java/org/apache/geode/distributed/internal/ClusterDistributionManager.java +++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/ClusterDistributionManager.java @@ -1544,13 +1544,13 @@ public class ClusterDistributionManager implements DistributionManager { @Override public void retainMembersWithSameOrNewerVersion(Collection<InternalDistributedMember> members, Version version) { - members.removeIf(id -> id.getVersionObject().compareTo(version) < 0); + members.removeIf(id -> id.getVersionOrdinalObject().compareTo(version) < 0); } @Override public void removeMembersWithSameOrNewerVersion(Collection<InternalDistributedMember> members, Version version) { - members.removeIf(id -> id.getVersionObject().compareTo(version) >= 0); + members.removeIf(id -> id.getVersionOrdinalObject().compareTo(version) >= 0); } @Override diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/LonerDistributionManager.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/LonerDistributionManager.java index f2e9f51..114a0e1 100644 --- a/geode-core/src/main/java/org/apache/geode/distributed/internal/LonerDistributionManager.java +++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/LonerDistributionManager.java @@ -189,7 +189,7 @@ public class LonerDistributionManager implements DistributionManager { Version version) { for (Iterator<InternalDistributedMember> it = members.iterator(); it.hasNext();) { InternalDistributedMember id = it.next(); - if (id.getVersionObject().compareTo(version) < 0) { + if (id.getVersionOrdinalObject().compareTo(version) < 0) { it.remove(); } } @@ -200,7 +200,7 @@ public class LonerDistributionManager implements DistributionManager { Version version) { for (Iterator<InternalDistributedMember> it = members.iterator(); it.hasNext();) { InternalDistributedMember id = it.next(); - if (id.getVersionObject().compareTo(version) >= 0) { + if (id.getVersionOrdinalObject().compareTo(version) >= 0) { it.remove(); } } 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 ef6102e..a8e35e2 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 @@ -49,6 +49,7 @@ import org.apache.geode.internal.serialization.DataSerializableFixedID; import org.apache.geode.internal.serialization.DeserializationContext; import org.apache.geode.internal.serialization.SerializationContext; import org.apache.geode.internal.serialization.Version; +import org.apache.geode.internal.serialization.VersionOrdinal; import org.apache.geode.logging.internal.OSProcess; /** @@ -549,8 +550,21 @@ public class InternalDistributedMember } @Override + public VersionOrdinal getVersionOrdinalObject() { + return memberIdentifier.getVersionOrdinalObject(); + } + + /** + * If this member runs a version known in this JVM then return that Version. + * If this member does not run a known version then return Version.CURRENT. + * + * In various serialization scenarios we want the well-known version for this + * member, or, if it doesn't have a well-known version, we want the current + * (in this JVM) software version. Rather than have that logic spread around in + * the serialization code, it is centralized here. + */ public Version getVersionObject() { - return memberIdentifier.getVersionObject(); + return Version.fromOrdinalNoThrow(getVersionOrdinalObject().ordinal(), false); } @Override diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractUpdateOperation.java b/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractUpdateOperation.java index dd6614c..972c652 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractUpdateOperation.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractUpdateOperation.java @@ -344,7 +344,7 @@ public abstract class AbstractUpdateOperation extends DistributedCacheOperation String msg = String.format("memberID cannot be null for persistent regions: %s", tag); - RuntimeException ex = (sender.getVersionObject().compareTo(Version.GFE_80) < 0) + RuntimeException ex = (sender.getVersionOrdinalObject().isOlderThan(Version.GFE_80)) ? new InternalGemFireException(msg) : new InvalidVersionException(msg); throw ex; } diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/DistributedPutAllOperation.java b/geode-core/src/main/java/org/apache/geode/internal/cache/DistributedPutAllOperation.java index 74d7eef..8895e84 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/DistributedPutAllOperation.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/DistributedPutAllOperation.java @@ -335,8 +335,7 @@ public class DistributedPutAllOperation extends AbstractUpdateOperation { * Constructor to use when receiving a putall from someone else */ public PutAllEntryData(DataInput in, DeserializationContext context, EventID baseEventID, - int idx, Version version, - ByteArrayDataInput bytesIn) throws IOException, ClassNotFoundException { + int idx) throws IOException, ClassNotFoundException { this.key = context.getDeserializer().readObject(in); byte flgs = in.readByte(); if ((flgs & IS_OBJECT) != 0) { @@ -1214,7 +1213,7 @@ public class DistributedPutAllOperation extends AbstractUpdateOperation { final Version version = StaticSerialization.getVersionForDataStreamOrNull(in); final ByteArrayDataInput bytesIn = new ByteArrayDataInput(); for (int i = 0; i < this.putAllDataSize; i++) { - this.putAllData[i] = new PutAllEntryData(in, context, eventId, i, version, bytesIn); + this.putAllData[i] = new PutAllEntryData(in, context, eventId, i); } boolean hasTags = in.readBoolean(); diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/DistributedRemoveAllOperation.java b/geode-core/src/main/java/org/apache/geode/internal/cache/DistributedRemoveAllOperation.java index b3135d5..f76c701 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/DistributedRemoveAllOperation.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/DistributedRemoveAllOperation.java @@ -50,12 +50,9 @@ import org.apache.geode.internal.cache.versions.VersionTag; import org.apache.geode.internal.offheap.annotations.Released; import org.apache.geode.internal.offheap.annotations.Retained; import org.apache.geode.internal.offheap.annotations.Unretained; -import org.apache.geode.internal.serialization.ByteArrayDataInput; import org.apache.geode.internal.serialization.DataSerializableFixedID; import org.apache.geode.internal.serialization.DeserializationContext; import org.apache.geode.internal.serialization.SerializationContext; -import org.apache.geode.internal.serialization.StaticSerialization; -import org.apache.geode.internal.serialization.Version; import org.apache.geode.logging.internal.log4j.api.LogService; /** @@ -318,8 +315,7 @@ public class DistributedRemoveAllOperation extends AbstractUpdateOperation { /** * Constructor to use when receiving a putall from someone else */ - public RemoveAllEntryData(DataInput in, EventID baseEventID, int idx, Version version, - ByteArrayDataInput bytesIn, + public RemoveAllEntryData(DataInput in, EventID baseEventID, int idx, DeserializationContext context) throws IOException, ClassNotFoundException { this.key = context.getDeserializer().readObject(in); this.oldValue = null; @@ -991,10 +987,8 @@ public class DistributedRemoveAllOperation extends AbstractUpdateOperation { this.removeAllDataSize = (int) InternalDataSerializer.readUnsignedVL(in); this.removeAllData = new RemoveAllEntryData[this.removeAllDataSize]; if (this.removeAllDataSize > 0) { - final Version version = StaticSerialization.getVersionForDataStreamOrNull(in); - final ByteArrayDataInput bytesIn = new ByteArrayDataInput(); for (int i = 0; i < this.removeAllDataSize; i++) { - this.removeAllData[i] = new RemoveAllEntryData(in, eventId, i, version, bytesIn, context); + this.removeAllData[i] = new RemoveAllEntryData(in, eventId, i, context); } boolean hasTags = in.readBoolean(); diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/ExpireDisconnectedClientTransactionsMessage.java b/geode-core/src/main/java/org/apache/geode/internal/cache/ExpireDisconnectedClientTransactionsMessage.java index 32c6dc2..3ffb3fa 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/ExpireDisconnectedClientTransactionsMessage.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/ExpireDisconnectedClientTransactionsMessage.java @@ -47,7 +47,7 @@ public class ExpireDisconnectedClientTransactionsMessage Set newVersionRecipients = new HashSet(); for (InternalDistributedMember recipient : recipients) { // to geode 1.7.0 and later version servers - if (recipient.getVersionObject().compareTo(Version.GEODE_1_7_0) >= 0) { + if (recipient.getVersionOrdinalObject().isNotOlderThan(Version.GEODE_1_7_0)) { newVersionRecipients.add(recipient); } } @@ -78,7 +78,7 @@ public class ExpireDisconnectedClientTransactionsMessage InternalDistributedMember sender = getSender(); if (cache != null) { TXManagerImpl mgr = cache.getTXMgr(); - if (sender.getVersionObject().compareTo(Version.GEODE_1_7_0) >= 0) { + if (sender.getVersionOrdinalObject().isNotOlderThan(Version.GEODE_1_7_0)) { // schedule to expire disconnected client transaction. mgr.expireDisconnectedClientTransactions(this.txIds, false); } else { diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java b/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java index 1b6d46f..c4c8db5 100755 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java @@ -4314,7 +4314,8 @@ public class GemFireCacheImpl implements InternalCache, InternalClientCache, Has // This block prevents sending a message to old members that do not know about // the RemoveCacheServerProfileMessage - otherMembers.removeIf(member -> Version.GEODE_1_5_0.compareTo(member.getVersionObject()) > 0); + otherMembers + .removeIf(member -> Version.GEODE_1_5_0.compareTo(member.getVersionOrdinalObject()) > 0); if (!otherMembers.isEmpty()) { if (logger.isDebugEnabled()) { diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/InitialImageOperation.java b/geode-core/src/main/java/org/apache/geode/internal/cache/InitialImageOperation.java index 89d543c..a7d0687 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/InitialImageOperation.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/InitialImageOperation.java @@ -338,14 +338,14 @@ public class InitialImageOperation { final ClusterDistributionManager dm = (ClusterDistributionManager) this.region.getDistributionManager(); boolean allowDeltaGII = true; - if (FORCE_FULL_GII || recipient.getVersionObject().compareTo(Version.GFE_80) < 0) { + if (FORCE_FULL_GII || recipient.getVersionOrdinalObject().isOlderThan(Version.GFE_80)) { allowDeltaGII = false; } Set keysOfUnfinishedOps = null; RegionVersionVector received_rvv = null; RegionVersionVector remote_rvv = null; if (this.region.getConcurrencyChecksEnabled() - && recipient.getVersionObject().compareTo(Version.GFE_80) >= 0) { + && recipient.getVersionOrdinalObject().isNotOlderThan(Version.GFE_80)) { if (internalBeforeRequestRVV != null && internalBeforeRequestRVV.getRegionName().equals(this.region.getName())) { internalBeforeRequestRVV.run(); @@ -746,7 +746,7 @@ public class InitialImageOperation { Set recipients = this.region.getCacheDistributionAdvisor().adviseReplicates(); for (Iterator it = recipients.iterator(); it.hasNext();) { InternalDistributedMember mbr = (InternalDistributedMember) it.next(); - if (mbr.getVersionObject().compareTo(Version.GFE_80) < 0) { + if (mbr.getVersionOrdinalObject().isOlderThan(Version.GFE_80)) { it.remove(); } } @@ -815,7 +815,7 @@ public class InitialImageOperation { * @param entries entries to add to the region * @return false if should abort (region was destroyed or cache was closed) */ - boolean processChunk(List entries, InternalDistributedMember sender, Version remoteVersion) + boolean processChunk(List entries, InternalDistributedMember sender) throws IOException, ClassNotFoundException { final boolean isDebugEnabled = logger.isDebugEnabled(); final boolean isTraceEnabled = logger.isTraceEnabled(); @@ -1190,7 +1190,7 @@ public class InitialImageOperation { region.recordEventState(msg.getSender(), msg.eventState); } if (msg.versionVector != null - && msg.getSender().getVersionObject().compareTo(Version.GFE_80) < 0 + && msg.getSender().getVersionOrdinalObject().isOlderThan(Version.GFE_80) && region.getConcurrencyChecksEnabled()) { // for older version, save received rvv from RegionStateMessage logger.debug("Applying version vector to {}: {}", region.getName(), msg.versionVector); @@ -1322,7 +1322,7 @@ public class InitialImageOperation { // bug 37461: don't allow abort flag to be reset boolean isAborted = this.abort; // volatile fetch if (!isAborted) { - isAborted = !processChunk(m.entries, m.getSender(), m.remoteVersion); + isAborted = !processChunk(m.entries, m.getSender()); if (isAborted) { this.abort = true; // volatile store } else { @@ -1599,7 +1599,7 @@ public class InitialImageOperation { } public boolean goWithFullGII(DistributedRegion rgn, RegionVersionVector requesterRVV) { - if (getSender().getVersionObject().compareTo(Version.GFE_80) < 0) { + if (getSender().getVersionOrdinalObject().isOlderThan(Version.GFE_80)) { // pre-8.0 could not handle a delta-GII return true; } @@ -1744,7 +1744,7 @@ public class InitialImageOperation { if (eventState != null && eventState.size() > 0) { RegionStateMessage.send(dm, getSender(), this.processorId, eventState, true); } - } else if (getSender().getVersionObject().compareTo(Version.GFE_80) < 0) { + } else if (getSender().getVersionOrdinalObject().isOlderThan(Version.GFE_80)) { // older versions of the product expect a RegionStateMessage at this point if (rgn.getConcurrencyChecksEnabled() && this.versionVector == null && !recoveringForLostMember) { diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/Oplog.java b/geode-core/src/main/java/org/apache/geode/internal/cache/Oplog.java index d195476..bb80a06 100755 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/Oplog.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/Oplog.java @@ -7227,7 +7227,8 @@ public class Oplog implements CompactableOplog, Flushable { @Override public boolean fillInValue(InternalRegion region, InitialImageOperation.Entry entry, - ByteArrayDataInput in, DistributionManager distributionManager, final Version version) { + ByteArrayDataInput in, DistributionManager distributionManager, + final Version version) { return false; } diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java b/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java index 53754e3..da3be92 100755 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java @@ -4803,7 +4803,7 @@ public class PartitionedRegion extends LocalRegion oneBucketKeys.clear(); oneBucketKeys.put(e.getKey(), e.getValue()); try { - if (entry.getKey().getVersionObject().compareTo(Version.GFE_80) < 0) { + if (entry.getKey().getVersionOrdinalObject().isOlderThan(Version.GFE_80)) { failures.putAll(nodeToBuckets.get(entry.getKey())); continue; } @@ -4855,7 +4855,7 @@ public class PartitionedRegion extends LocalRegion bucketId.clear(); bucketId.add(bucket); try { - if (entry.getKey().getVersionObject().compareTo(Version.GFE_80) < 0) { + if (entry.getKey().getVersionOrdinalObject().isOlderThan(Version.GFE_80)) { failures.addAll(nodeToBuckets.get(entry.getKey())); continue; } diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionQueryEvaluator.java b/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionQueryEvaluator.java index 0e4780c..1c7d4bb 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionQueryEvaluator.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionQueryEvaluator.java @@ -186,7 +186,7 @@ public class PartitionedRegionQueryEvaluator extends StreamingPartitionOperation // we will have to sort it boolean sortNeeded = false; List<CompiledSortCriterion> orderByAttribs = null; - if (sender.getVersionObject().compareTo(Version.GFE_90) < 0) { + if (sender.getVersionOrdinalObject().isOlderThan(Version.GFE_90)) { CompiledSelect cs = this.query.getSimpleSelect(); if (cs != null && cs.isOrderBy()) { sortNeeded = true; diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/ServerPingMessage.java b/geode-core/src/main/java/org/apache/geode/internal/cache/ServerPingMessage.java index e91f356..8f8d337 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/ServerPingMessage.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/ServerPingMessage.java @@ -65,7 +65,7 @@ public class ServerPingMessage extends PooledDistributionMessage { // filtered recipients for (InternalDistributedMember recipient : recipients) { - if (Version.GFE_81.compareTo(recipient.getVersionObject()) <= 0) { + if (Version.GFE_81.compareTo(recipient.getVersionOrdinalObject()) <= 0) { filteredRecipients.add(recipient); } } diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/IndexCreationMsg.java b/geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/IndexCreationMsg.java index 88d94b3..6b9534f 100755 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/IndexCreationMsg.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/IndexCreationMsg.java @@ -357,7 +357,7 @@ public class IndexCreationMsg extends PartitionMessage { } for (InternalDistributedMember rec : recipients) { - if (rec.getVersionObject().compareTo(Version.GFE_81) < 0) { + if (rec.getVersionOrdinalObject().isOlderThan(Version.GFE_81)) { throw new UnsupportedOperationException( "Indexes should not be created during rolling upgrade"); } diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/PutAllPRMessage.java b/geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/PutAllPRMessage.java index 5c2cf3d..0c690c5 100755 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/PutAllPRMessage.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/PutAllPRMessage.java @@ -68,11 +68,8 @@ import org.apache.geode.internal.cache.versions.VersionTag; import org.apache.geode.internal.logging.log4j.LogMarker; import org.apache.geode.internal.offheap.annotations.Released; import org.apache.geode.internal.offheap.annotations.Retained; -import org.apache.geode.internal.serialization.ByteArrayDataInput; import org.apache.geode.internal.serialization.DeserializationContext; import org.apache.geode.internal.serialization.SerializationContext; -import org.apache.geode.internal.serialization.StaticSerialization; -import org.apache.geode.internal.serialization.Version; import org.apache.geode.logging.internal.log4j.api.LogService; /** @@ -230,10 +227,8 @@ public class PutAllPRMessage extends PartitionMessageWithDirectReply { this.putAllPRDataSize = (int) InternalDataSerializer.readUnsignedVL(in); this.putAllPRData = new PutAllEntryData[putAllPRDataSize]; if (this.putAllPRDataSize > 0) { - final Version version = StaticSerialization.getVersionForDataStreamOrNull(in); - final ByteArrayDataInput bytesIn = new ByteArrayDataInput(); for (int i = 0; i < this.putAllPRDataSize; i++) { - this.putAllPRData[i] = new PutAllEntryData(in, context, null, i, version, bytesIn); + this.putAllPRData[i] = new PutAllEntryData(in, context, null, i); } boolean hasTags = in.readBoolean(); diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/QueryMessage.java b/geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/QueryMessage.java index 1566ba4..d96e573 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/QueryMessage.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/QueryMessage.java @@ -123,7 +123,7 @@ public class QueryMessage extends StreamingPartitionOperation.StreamingPartition } } Object data = this.currentResultIterator.next(); - boolean isPostGFE_8_1 = this.getSender().getVersionObject().compareTo(Version.GFE_81) > 0; + boolean isPostGFE_8_1 = this.getSender().getVersionOrdinalObject().isNewerThan(Version.GFE_81); // There is a bug in older versions of GFE such that the query node expects the structs to have // type as ObjectTypes only & not specific types. So the new version needs to send the @@ -190,7 +190,7 @@ public class QueryMessage extends StreamingPartitionOperation.StreamingPartition logger.debug("Started executing query from remote node: {}", query.getQueryString()); } isQueryTraced = - query.isTraced() && this.sender.getVersionObject().compareTo(Version.GFE_81) >= 0; + query.isTraced() && this.sender.getVersionOrdinalObject().isNotOlderThan(Version.GFE_81); // Adds a query trace info object to the results list for remote queries PRQueryTraceInfo queryTraceInfo = null; diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/RemoveAllPRMessage.java b/geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/RemoveAllPRMessage.java index 6e05a41..6f355d6 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/RemoveAllPRMessage.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/RemoveAllPRMessage.java @@ -68,11 +68,8 @@ import org.apache.geode.internal.cache.versions.VersionTag; import org.apache.geode.internal.logging.log4j.LogMarker; import org.apache.geode.internal.offheap.annotations.Released; import org.apache.geode.internal.offheap.annotations.Retained; -import org.apache.geode.internal.serialization.ByteArrayDataInput; import org.apache.geode.internal.serialization.DeserializationContext; import org.apache.geode.internal.serialization.SerializationContext; -import org.apache.geode.internal.serialization.StaticSerialization; -import org.apache.geode.internal.serialization.Version; import org.apache.geode.logging.internal.log4j.api.LogService; /** @@ -220,15 +217,12 @@ public class RemoveAllPRMessage extends PartitionMessageWithDirectReply { if ((flags & HAS_BRIDGE_CONTEXT) != 0) { this.bridgeContext = DataSerializer.readObject(in); } - Version sourceVersion = StaticSerialization.getVersionForDataStream(in); this.callbackArg = DataSerializer.readObject(in); this.removeAllPRDataSize = (int) InternalDataSerializer.readUnsignedVL(in); this.removeAllPRData = new RemoveAllEntryData[removeAllPRDataSize]; if (this.removeAllPRDataSize > 0) { - final Version version = StaticSerialization.getVersionForDataStreamOrNull(in); - final ByteArrayDataInput bytesIn = new ByteArrayDataInput(); for (int i = 0; i < this.removeAllPRDataSize; i++) { - this.removeAllPRData[i] = new RemoveAllEntryData(in, null, i, version, bytesIn, context); + this.removeAllPRData[i] = new RemoveAllEntryData(in, null, i, context); } boolean hasTags = in.readBoolean(); diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tx/DistTxEntryEvent.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tx/DistTxEntryEvent.java index 1c7c5d1..40ddf56 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/tx/DistTxEntryEvent.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tx/DistTxEntryEvent.java @@ -156,7 +156,7 @@ public class DistTxEntryEvent extends EntryEventImpl { final Version version = StaticSerialization.getVersionForDataStreamOrNull(in); final ByteArrayDataInput bytesIn = new ByteArrayDataInput(); for (int i = 0; i < putAllSize; i++) { - putAllEntries[i] = new PutAllEntryData(in, context, this.eventID, i, version, bytesIn); + putAllEntries[i] = new PutAllEntryData(in, context, this.eventID, i); } boolean hasTags = in.readBoolean(); @@ -206,7 +206,7 @@ public class DistTxEntryEvent extends EntryEventImpl { final Version version = StaticSerialization.getVersionForDataStreamOrNull(in); final ByteArrayDataInput bytesIn = new ByteArrayDataInput(); for (int i = 0; i < removeAllSize; i++) { - removeAllData[i] = new RemoveAllEntryData(in, this.eventID, i, version, bytesIn, context); + removeAllData[i] = new RemoveAllEntryData(in, this.eventID, i, context); } boolean hasTags = in.readBoolean(); diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tx/RemotePutAllMessage.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tx/RemotePutAllMessage.java index 7399969..1cbb8df 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/tx/RemotePutAllMessage.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tx/RemotePutAllMessage.java @@ -240,7 +240,7 @@ public class RemotePutAllMessage extends RemoteOperationMessageWithDirectReply { final Version version = StaticSerialization.getVersionForDataStreamOrNull(in); final ByteArrayDataInput bytesIn = new ByteArrayDataInput(); for (int i = 0; i < this.putAllDataCount; i++) { - this.putAllData[i] = new PutAllEntryData(in, context, this.eventId, i, version, bytesIn); + this.putAllData[i] = new PutAllEntryData(in, context, this.eventId, i); } boolean hasTags = in.readBoolean(); diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tx/RemoteRemoveAllMessage.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tx/RemoteRemoveAllMessage.java index a2e95a3..8c6371c 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/tx/RemoteRemoveAllMessage.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tx/RemoteRemoveAllMessage.java @@ -235,7 +235,7 @@ public class RemoteRemoveAllMessage extends RemoteOperationMessageWithDirectRepl final Version version = StaticSerialization.getVersionForDataStreamOrNull(in); final ByteArrayDataInput bytesIn = new ByteArrayDataInput(); for (int i = 0; i < this.removeAllDataCount; i++) { - this.removeAllData[i] = new RemoveAllEntryData(in, this.eventId, i, version, bytesIn, + this.removeAllData[i] = new RemoveAllEntryData(in, this.eventId, i, context); } diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/domain/Configuration.java b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/domain/Configuration.java index c9af54d..501ade2 100644 --- a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/domain/Configuration.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/domain/Configuration.java @@ -39,6 +39,8 @@ import org.xml.sax.SAXException; import org.apache.geode.DataSerializable; import org.apache.geode.DataSerializer; import org.apache.geode.internal.serialization.Version; +import org.apache.geode.internal.serialization.VersionOrdinal; +import org.apache.geode.internal.serialization.Versioning; import org.apache.geode.management.configuration.Deployment; import org.apache.geode.management.internal.configuration.utils.XmlUtils; @@ -185,8 +187,8 @@ public class Configuration implements DataSerializable { .forEach(deployment -> deployments.put(deployment.getFileName(), deployment)); } else { // version of the data we are reading (1.12 or later) - Version version = Version.fromOrdinalNoThrow(Version.readOrdinal(in), true); - if (version.compareTo(Version.GEODE_1_12_0) >= 0) { + final VersionOrdinal version = Versioning.getVersionOrdinal(Version.readOrdinal(in)); + if (version.isNotOlderThan(Version.GEODE_1_12_0)) { deployments.putAll(DataSerializer.readHashMap(in)); } } diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/operation/RestoreRedundancyPerformer.java b/geode-core/src/main/java/org/apache/geode/management/internal/operation/RestoreRedundancyPerformer.java index e5a4d6a..1376817 100644 --- a/geode-core/src/main/java/org/apache/geode/management/internal/operation/RestoreRedundancyPerformer.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/operation/RestoreRedundancyPerformer.java @@ -128,7 +128,7 @@ public class RestoreRedundancyPerformer RebalanceOperationPerformer.MemberPRInfo prInfo) { return prInfo.dsMemberList.stream() .map(InternalDistributedMember.class::cast) - .filter(member -> member.getVersionObject().compareTo(ADDED_VERSION) >= 0) + .filter(member -> member.getVersionOrdinalObject().compareTo(ADDED_VERSION) >= 0) .collect(Collectors.toList()); } diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/util/ManagementUtils.java b/geode-core/src/main/java/org/apache/geode/management/internal/util/ManagementUtils.java index a6e8d75..95bc5b5 100644 --- a/geode-core/src/main/java/org/apache/geode/management/internal/util/ManagementUtils.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/util/ManagementUtils.java @@ -75,7 +75,8 @@ public class ManagementUtils { public static Set<DistributedMember> getNormalMembersWithSameOrNewerVersion(InternalCache cache, Version version) { return getAllNormalMembers(cache).stream().filter( - member -> ((InternalDistributedMember) member).getVersionObject().compareTo(version) >= 0) + member -> ((InternalDistributedMember) member).getVersionOrdinalObject() + .compareTo(version) >= 0) .collect(Collectors.toSet()); } diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/ExpireDisconnectedClientTransactionsMessageTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/ExpireDisconnectedClientTransactionsMessageTest.java index 73606f6..4f5f9d3 100644 --- a/geode-core/src/test/java/org/apache/geode/internal/cache/ExpireDisconnectedClientTransactionsMessageTest.java +++ b/geode-core/src/test/java/org/apache/geode/internal/cache/ExpireDisconnectedClientTransactionsMessageTest.java @@ -44,7 +44,7 @@ public class ExpireDisconnectedClientTransactionsMessageTest { when(dm.getCache()).thenReturn(cache); when(cache.getTXMgr()).thenReturn(txManager); doReturn(sender).when(message).getSender(); - when(sender.getVersionObject()).thenReturn(version); + when(sender.getVersionOrdinalObject()).thenReturn(version); } @Test diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/operation/RestoreRedundancyPerformerTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/operation/RestoreRedundancyPerformerTest.java index 5f675af..c46f1a2 100644 --- a/geode-core/src/test/java/org/apache/geode/management/internal/operation/RestoreRedundancyPerformerTest.java +++ b/geode-core/src/test/java/org/apache/geode/management/internal/operation/RestoreRedundancyPerformerTest.java @@ -93,9 +93,9 @@ public class RestoreRedundancyPerformerTest { when(internalCacheForClientAccess.getInternalDistributedSystem()) .thenReturn(internalDistributedSystem); - when(server1.getVersionObject()) + when(server1.getVersionOrdinalObject()) .thenReturn(RestoreRedundancyPerformer.ADDED_VERSION); - when(server2.getVersionObject()) + when(server2.getVersionOrdinalObject()) .thenReturn(RestoreRedundancyPerformer.ADDED_VERSION); restoreRedundancyPerformer = new RestoreRedundancyPerformer(); @@ -239,9 +239,9 @@ public class RestoreRedundancyPerformerTest { underRedundancyRegionResults.put(REGION_1, regionRedundancyStatusImpl); - when(server1.getVersionObject()) + when(server1.getVersionOrdinalObject()) .thenReturn(Version.GEODE_1_2_0); - when(server2.getVersionObject()) + when(server2.getVersionOrdinalObject()) .thenReturn(Version.GEODE_1_9_0); diff --git a/geode-core/src/upgradeTest/java/org/apache/geode/internal/cache/rollingupgrade/RollingUpgrade2DUnitTestBase.java b/geode-core/src/upgradeTest/java/org/apache/geode/internal/cache/rollingupgrade/RollingUpgrade2DUnitTestBase.java index 3559110..4e4a503 100755 --- a/geode-core/src/upgradeTest/java/org/apache/geode/internal/cache/rollingupgrade/RollingUpgrade2DUnitTestBase.java +++ b/geode-core/src/upgradeTest/java/org/apache/geode/internal/cache/rollingupgrade/RollingUpgrade2DUnitTestBase.java @@ -1071,7 +1071,8 @@ public abstract class RollingUpgrade2DUnitTestBase extends JUnit4DistributedTest private static void assertVersion(GemFireCache cache, short ordinal) { DistributedSystem system = cache.getDistributedSystem(); int thisOrdinal = - ((InternalDistributedMember) system.getDistributedMember()).getVersionObject().ordinal(); + ((InternalDistributedMember) system.getDistributedMember()).getVersionOrdinalObject() + .ordinal(); if (ordinal != thisOrdinal) { throw new Error( "Version ordinal:" + thisOrdinal + " was not the expected ordinal of:" + ordinal); diff --git a/geode-core/src/upgradeTest/java/org/apache/geode/internal/cache/rollingupgrade/RollingUpgradeDUnitTest.java b/geode-core/src/upgradeTest/java/org/apache/geode/internal/cache/rollingupgrade/RollingUpgradeDUnitTest.java index 089b4ff..5cce73c 100644 --- a/geode-core/src/upgradeTest/java/org/apache/geode/internal/cache/rollingupgrade/RollingUpgradeDUnitTest.java +++ b/geode-core/src/upgradeTest/java/org/apache/geode/internal/cache/rollingupgrade/RollingUpgradeDUnitTest.java @@ -15,6 +15,7 @@ package org.apache.geode.internal.cache.rollingupgrade; import static org.apache.geode.test.awaitility.GeodeAwaitility.await; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertTrue; import java.io.File; @@ -47,11 +48,15 @@ import org.apache.geode.cache30.CacheSerializableRunnable; import org.apache.geode.distributed.DistributedSystem; import org.apache.geode.distributed.Locator; import org.apache.geode.distributed.internal.DistributionConfig; +import org.apache.geode.distributed.internal.DistributionManager; +import org.apache.geode.distributed.internal.InternalDistributedSystem; import org.apache.geode.distributed.internal.InternalLocator; import org.apache.geode.distributed.internal.membership.InternalDistributedMember; +import org.apache.geode.distributed.internal.membership.api.MembershipView; import org.apache.geode.distributed.internal.membership.gms.membership.GMSJoinLeave; import org.apache.geode.internal.AvailablePortHelper; import org.apache.geode.internal.serialization.Version; +import org.apache.geode.internal.serialization.VersionOrdinal; import org.apache.geode.test.dunit.DistributedTestUtils; import org.apache.geode.test.dunit.Host; import org.apache.geode.test.dunit.IgnoredException; @@ -205,6 +210,22 @@ public abstract class RollingUpgradeDUnitTest extends JUnit4DistributedTestCase verifyValues(objectType, regionName, 0, 10, server2); putAndVerify(objectType, server2, regionName, 15, 25, server1); + final short versionOrdinalAfterUpgrade = Version.getCurrentVersion().ordinal(); + locator.invoke(() -> { + + final Locator theLocator = Locator.getLocator(); + final DistributedSystem distributedSystem = theLocator.getDistributedSystem(); + final InternalDistributedSystem ids = + (InternalDistributedSystem) distributedSystem; + final DistributionManager distributionManager = ids.getDistributionManager(); + final MembershipView<InternalDistributedMember> view = + distributionManager.getDistribution().getView(); + + for (final InternalDistributedMember member : view.getMembers()) { + assertThat(member.getVersionOrdinal()).isEqualTo(versionOrdinalAfterUpgrade); + } + }); + } finally { invokeRunnableInVMs(true, invokeStopLocator(), locator); invokeRunnableInVMs(true, invokeCloseCache(), server1, server2); @@ -659,7 +680,7 @@ public abstract class RollingUpgradeDUnitTest extends JUnit4DistributedTestCase private static void assertVersion(Cache cache, short ordinal) { DistributedSystem ds = cache.getDistributedSystem(); InternalDistributedMember member = (InternalDistributedMember) ds.getDistributedMember(); - Version thisVersion = member.getVersionObject(); + final VersionOrdinal thisVersion = member.getVersionOrdinalObject(); short thisOrdinal = thisVersion.ordinal(); if (ordinal != thisOrdinal) { throw new Error( diff --git a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/CreateGatewaySenderCommand.java b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/CreateGatewaySenderCommand.java index c00ffce..9acd0f2 100644 --- a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/CreateGatewaySenderCommand.java +++ b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/CreateGatewaySenderCommand.java @@ -202,7 +202,8 @@ public class CreateGatewaySenderCommand extends SingleGfshCommand { private boolean verifyAllCurrentVersion(Set<DistributedMember> members) { return members.stream().allMatch( - member -> ((InternalDistributedMember) member).getVersionObject().equals(Version.CURRENT)); + member -> ((InternalDistributedMember) member).getVersionOrdinalObject() + .equals(Version.CURRENT)); } private CacheConfig.GatewaySender buildConfiguration(String id, Integer remoteDSId, diff --git a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/RedundancyCommand.java b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/RedundancyCommand.java index fcc1110..43cd2fd 100644 --- a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/RedundancyCommand.java +++ b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/RedundancyCommand.java @@ -91,7 +91,7 @@ public class RedundancyCommand extends GfshCommand { RebalanceOperationPerformer.MemberPRInfo prInfo) { return prInfo.dsMemberList.stream() .map(InternalDistributedMember.class::cast) - .filter(member -> member.getVersionObject().compareTo(ADDED_VERSION) >= 0) + .filter(member -> member.getVersionOrdinalObject().compareTo(ADDED_VERSION) >= 0) .collect(Collectors.toList()); } diff --git a/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/commands/CreateGatewaySenderCommandTest.java b/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/commands/CreateGatewaySenderCommandTest.java index 99aeaaa..ddc4b5f 100644 --- a/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/commands/CreateGatewaySenderCommandTest.java +++ b/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/commands/CreateGatewaySenderCommandTest.java @@ -204,9 +204,9 @@ public class CreateGatewaySenderCommandTest { // Create a set of mixed version members Set<DistributedMember> members = new HashSet<>(); InternalDistributedMember currentVersionMember = mock(InternalDistributedMember.class); - when(currentVersionMember.getVersionObject()).thenReturn(Version.CURRENT); + when(currentVersionMember.getVersionOrdinalObject()).thenReturn(Version.CURRENT); InternalDistributedMember oldVersionMember = mock(InternalDistributedMember.class); - when(oldVersionMember.getVersionObject()).thenReturn(Version.GEODE_1_4_0); + when(oldVersionMember.getVersionOrdinalObject()).thenReturn(Version.GEODE_1_4_0); members.add(currentVersionMember); members.add(oldVersionMember); doReturn(members).when(command).getMembers(any(), any()); diff --git a/geode-junit/src/main/java/org/apache/geode/test/version/TestVersion.java b/geode-junit/src/main/java/org/apache/geode/test/version/TestVersion.java index f961f03..4af0b9e 100644 --- a/geode-junit/src/main/java/org/apache/geode/test/version/TestVersion.java +++ b/geode-junit/src/main/java/org/apache/geode/test/version/TestVersion.java @@ -17,8 +17,6 @@ package org.apache.geode.test.version; import java.io.Serializable; import java.util.Objects; -import org.apache.geode.internal.serialization.Version; - public class TestVersion implements Comparable, Serializable { public static final TestVersion CURRENT_VERSION = new TestVersion(VersionManager.CURRENT_VERSION); @@ -51,15 +49,6 @@ public class TestVersion implements Comparable, Serializable { return new TestVersion(version1).compareTo(new TestVersion(version2)); } - public boolean isSameAs(Version version) { - if (equals(CURRENT_VERSION) && version.equals(Version.getCurrentVersion())) { - return true; - } - return release == version.getRelease() - && minor == version.getMinorVersion() - && major == version.getMajorVersion(); - } - @Override public String toString() { return "" + major + "." + minor + "." + release; diff --git a/geode-junit/src/main/java/org/apache/geode/test/version/VersionManager.java b/geode-junit/src/main/java/org/apache/geode/test/version/VersionManager.java index 3efc602..ef0ecb4 100755 --- a/geode-junit/src/main/java/org/apache/geode/test/version/VersionManager.java +++ b/geode-junit/src/main/java/org/apache/geode/test/version/VersionManager.java @@ -152,7 +152,7 @@ public class VersionManager { } private void findVersions(String fileName) { - // this file is created by the gradle task createClasspathsPropertiesFile + // this file is created by the gradle task :geode-old-versions:createGeodeClasspathsFile readVersionsFile(fileName, (version, path) -> { Optional<String> parsedVersion = parseVersion(version); if (parsedVersion.isPresent()) { @@ -198,7 +198,7 @@ public class VersionManager { } public Properties readPropertiesFile(String fileName) { - // this file is created by the gradle task createClasspathsPropertiesFile + // this file is created by the gradle task :geode-old-versions:createGeodeClasspathsFile Properties props = new Properties(); URL url = VersionManager.class.getResource("/" + fileName); if (url == null) { diff --git a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneServiceImpl.java b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneServiceImpl.java index a3b5ae4..92eff09 100644 --- a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneServiceImpl.java +++ b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneServiceImpl.java @@ -80,6 +80,7 @@ import org.apache.geode.internal.cache.extension.Extensible; import org.apache.geode.internal.cache.xmlcache.XmlGenerator; import org.apache.geode.internal.serialization.DataSerializableFixedID; import org.apache.geode.internal.serialization.Version; +import org.apache.geode.internal.serialization.VersionOrdinal; import org.apache.geode.logging.internal.log4j.api.LogService; import org.apache.geode.management.internal.beans.CacheServiceMBeanBase; import org.apache.geode.util.internal.GeodeGlossary; @@ -237,11 +238,11 @@ public class LuceneServiceImpl implements InternalLuceneService { protected void validateAllMembersAreTheSameVersion(PartitionedRegion region) { Set<InternalDistributedMember> remoteMembers = region.getRegionAdvisor().adviseAllPRNodes(); - Version localVersion = - cache.getDistributionManager().getDistributionManagerId().getVersionObject(); + final VersionOrdinal localVersion = + cache.getDistributionManager().getDistributionManagerId().getVersionOrdinalObject(); if (!remoteMembers.isEmpty()) { for (InternalDistributedMember remoteMember : remoteMembers) { - if (!remoteMember.getVersionObject().equals(localVersion)) { + if (!remoteMember.getVersionOrdinalObject().equals(localVersion)) { throw new IllegalStateException( "The lucene index cannot be created on a existing region if all members hosting the region : " + region.getFullPath() + ", are not the same Apache Geode version "); @@ -723,7 +724,7 @@ public class LuceneServiceImpl implements InternalLuceneService { private boolean isAnyRemoteMemberVersionLessThanGeode1_7_0( Set<InternalDistributedMember> remoteMembers) { for (InternalDistributedMember remoteMember : remoteMembers) { - if (remoteMember.getVersionObject().ordinal() < Version.GEODE_1_7_0.ordinal()) { + if (remoteMember.getVersionOrdinalObject().ordinal() < Version.GEODE_1_7_0.ordinal()) { return true; } } diff --git a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/distributed/LuceneQueryFunction.java b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/distributed/LuceneQueryFunction.java index af1075e..d20b664 100644 --- a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/distributed/LuceneQueryFunction.java +++ b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/distributed/LuceneQueryFunction.java @@ -96,7 +96,7 @@ public class LuceneQueryFunction implements InternalFunction<LuceneFunctionConte // Hence the query waits for the repositories to be ready instead of throwing the exception if (!remoteMembers.isEmpty()) { for (InternalDistributedMember remoteMember : remoteMembers) { - if (remoteMember.getVersionObject().ordinal() < Version.GEODE_1_6_0.ordinal()) { + if (remoteMember.getVersionOrdinalObject().ordinal() < Version.GEODE_1_6_0.ordinal()) { // re-execute but wait till indexing is complete execute(ctx, true); return; diff --git a/geode-membership/src/integrationTest/java/org/apache/geode/distributed/internal/membership/gms/GMSMemberDataVersionJUnitTest.java b/geode-membership/src/integrationTest/java/org/apache/geode/distributed/internal/membership/gms/GMSMemberDataVersionJUnitTest.java new file mode 100644 index 0000000..2e34bc4 --- /dev/null +++ b/geode-membership/src/integrationTest/java/org/apache/geode/distributed/internal/membership/gms/GMSMemberDataVersionJUnitTest.java @@ -0,0 +1,99 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more contributor license + * agreements. See the NOTICE file distributed with this work for additional information regarding + * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. You may obtain a + * copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ + +package org.apache.geode.distributed.internal.membership.gms; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; + +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.io.IOException; +import java.net.InetAddress; + +import org.assertj.core.api.AbstractShortAssert; +import org.junit.Test; + +import org.apache.geode.distributed.internal.membership.api.MemberData; +import org.apache.geode.internal.serialization.DSFIDSerializer; +import org.apache.geode.internal.serialization.DSFIDSerializerFactory; +import org.apache.geode.internal.serialization.DeserializationContext; +import org.apache.geode.internal.serialization.SerializationContext; +import org.apache.geode.internal.serialization.Version; + +/** + * MemberData has to be able to hold an unknown version ordinal since, during a rolling upgrade, + * we may receive a MemberData from a member running a future version of the product. + */ +public class GMSMemberDataVersionJUnitTest { + + private final short unknownVersionOrdinal = + (short) (Version.CURRENT_ORDINAL + 1); + + @Test + public void testConstructor1() { + final MemberDataBuilderImpl builder = MemberDataBuilderImpl.newBuilder(null, null); + builder.setVersionOrdinal(unknownVersionOrdinal); + validate(builder.build()); + } + + @Test + public void testConstructor2() { + final GMSMemberData memberData = + new GMSMemberData(mock(InetAddress.class), 0, unknownVersionOrdinal, 0, 0, 0); + validate(memberData); + } + + @Test + public void testReadEssentialData() throws IOException, ClassNotFoundException { + + final MemberDataBuilderImpl builder = MemberDataBuilderImpl.newBuilder(null, null); + builder.setVersionOrdinal(unknownVersionOrdinal); + final MemberData member = builder.build(); + + final ByteArrayOutputStream baos = new ByteArrayOutputStream(); + final DataOutput dataOutput = new DataOutputStream(baos); + final DSFIDSerializer dsfidSerializer = new DSFIDSerializerFactory().create(); + final SerializationContext serializationContext = + dsfidSerializer.createSerializationContext(dataOutput); + member.writeEssentialData(dataOutput, serializationContext); + + final ByteArrayInputStream bais = new ByteArrayInputStream(baos.toByteArray()); + final DataInputStream stream = new DataInputStream(bais); + final DeserializationContext deserializationContext = + dsfidSerializer.createDeserializationContext(stream); + final DataInput dataInput = new DataInputStream(bais); + final GMSMemberData newMember = new GMSMemberData(); + newMember.readEssentialData(dataInput, deserializationContext); + + validate(newMember); + } + + @Test + public void testSetVersionOrdinal() { + final GMSMemberData memberData = new GMSMemberData(); + memberData.setVersionOrdinal(unknownVersionOrdinal); + validate(memberData); + } + + private AbstractShortAssert<?> validate(final MemberData memberData) { + return assertThat(memberData.getVersionOrdinal()).isEqualTo(unknownVersionOrdinal); + } + +} diff --git a/geode-membership/src/integrationTest/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java b/geode-membership/src/integrationTest/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java index c65f8ef..3a86a1e 100644 --- a/geode-membership/src/integrationTest/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java +++ b/geode-membership/src/integrationTest/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java @@ -1570,6 +1570,36 @@ public class GMSJoinLeaveJUnitTest { } } + // GEODE-8240 could cause this member's identifier to have the wrong version so patch it up + @Test + public void repairWrongVersionInView() throws Exception { + + initMocks(); + + List<MemberIdentifier> viewmembers = + Arrays.asList(new MemberIdentifier[] {mockMembers[0], gmsJoinLeaveMemberId}); + + final GMSMembershipView<MemberIdentifier> viewWithWrongVersion = + new GMSMembershipView<>(mockMembers[0], 2, viewmembers); + + // clone member ID + final MemberIdentifierImpl myMemberIDWithWrongVersion = + new MemberIdentifierImpl(gmsJoinLeaveMemberId.getMemberData()); + + // this test must live in the 1.12 and later lines so pick a pre-1.12 version + final Version oldVersion = Version.GEODE_1_11_0; + myMemberIDWithWrongVersion.setVersionObjectForTest(oldVersion); + + viewWithWrongVersion.remove(gmsJoinLeaveMemberId); + viewWithWrongVersion.add(myMemberIDWithWrongVersion); + + gmsJoinLeave.installView(viewWithWrongVersion); + + assertThat( + gmsJoinLeave.getView().getCanonicalID(gmsJoinLeaveMemberId).getVersionOrdinalObject()) + .isEqualTo(Version.getCurrentVersion()); + } + private void becomeCoordinatorForTest(GMSJoinLeave gmsJoinLeave) { synchronized (gmsJoinLeave.getViewInstallationLock()) { gmsJoinLeave.becomeCoordinator(); diff --git a/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/api/MemberData.java b/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/api/MemberData.java index 6483fb5..d1b5ad7 100644 --- a/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/api/MemberData.java +++ b/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/api/MemberData.java @@ -22,7 +22,7 @@ import org.jgroups.util.UUID; import org.apache.geode.internal.serialization.DeserializationContext; import org.apache.geode.internal.serialization.SerializationContext; -import org.apache.geode.internal.serialization.Version; +import org.apache.geode.internal.serialization.VersionOrdinal; /** * MemberIdentifiers are created with a MemberData component. Use MemberDataBuilder to create @@ -45,7 +45,7 @@ public interface MemberData { short getVersionOrdinal(); - Version getVersion(); + VersionOrdinal getVersionOrdinalObject(); String getUniqueTag(); diff --git a/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/api/MemberIdentifier.java b/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/api/MemberIdentifier.java index 7f16c65..90c4327 100644 --- a/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/api/MemberIdentifier.java +++ b/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/api/MemberIdentifier.java @@ -29,6 +29,7 @@ import org.apache.geode.internal.serialization.DataSerializableFixedID; import org.apache.geode.internal.serialization.DeserializationContext; import org.apache.geode.internal.serialization.SerializationContext; import org.apache.geode.internal.serialization.Version; +import org.apache.geode.internal.serialization.VersionOrdinal; /** * MemberIdentifier should be implemented by a user of GMS if the default member identifier @@ -131,7 +132,7 @@ public interface MemberIdentifier extends DataSerializableFixedID { /** * Get the Geode version of this member */ - Version getVersionObject(); + VersionOrdinal getVersionOrdinalObject(); /** * Replace the current member data with the given member data. This can be used to fill out a diff --git a/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMemberData.java b/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMemberData.java index ad00642..85087c8 100644 --- a/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMemberData.java +++ b/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMemberData.java @@ -27,8 +27,9 @@ import org.apache.geode.distributed.internal.membership.api.MemberIdentifier; import org.apache.geode.internal.serialization.DeserializationContext; import org.apache.geode.internal.serialization.SerializationContext; import org.apache.geode.internal.serialization.StaticSerialization; -import org.apache.geode.internal.serialization.UnsupportedSerializationVersionException; import org.apache.geode.internal.serialization.Version; +import org.apache.geode.internal.serialization.VersionOrdinal; +import org.apache.geode.internal.serialization.Versioning; /** * GMSMember contains data that is required to identify a member of the cluster. @@ -79,7 +80,15 @@ public class GMSMemberData implements MemberData, Comparable<GMSMemberData> { * displayable. */ private String uniqueTag = null; - private transient Version versionObj = Version.CURRENT; + + /** + * versionOrdinal is stored here as a VersionOrdinal and not a Version, because + * GMSMemberData needs to sometimes store the version of a new product version, + * e.g. during rolling upgrade members with old versions receive member identifiers + * from members with new (unknown) versions. + */ + private transient VersionOrdinal versionOrdinal = + Versioning.getVersionOrdinal(Version.CURRENT.ordinal()); /** * whether this is a partial member ID (without roles, durable attributes). We use partial IDs in @@ -102,7 +111,7 @@ public class GMSMemberData implements MemberData, Comparable<GMSMemberData> { * @param networkPartitionDetectionEnabled whether the member has network partition detection * enabled * @param preferredForCoordinator whether the member can be group coordinator - * @param version the member's version ordinal + * @param versionOrdinal the member's version ordinal * @param msbs - most significant bytes of UUID * @param lsbs - least significant bytes of UUID */ @@ -112,7 +121,7 @@ public class GMSMemberData implements MemberData, Comparable<GMSMemberData> { String name, String[] groups, String durableId, int durableTimeout, boolean networkPartitionDetectionEnabled, boolean preferredForCoordinator, - short version, + short versionOrdinal, long msbs, long lsbs, byte memberWeight, boolean isPartial, String uniqueTag) { this.inetAddr = i; this.hostName = hostName; @@ -127,7 +136,7 @@ public class GMSMemberData implements MemberData, Comparable<GMSMemberData> { this.durableTimeout = durableTimeout; this.networkPartitionDetectionEnabled = networkPartitionDetectionEnabled; this.preferredForCoordinator = preferredForCoordinator; - setVersionObject(version); + this.versionOrdinal = Versioning.getVersionOrdinal(versionOrdinal); this.uuidMSBs = msbs; this.uuidLSBs = lsbs; this.memberWeight = memberWeight; @@ -135,19 +144,12 @@ public class GMSMemberData implements MemberData, Comparable<GMSMemberData> { this.uniqueTag = uniqueTag; } - private void setVersionObject(short versionOrdinal) { - try { - this.versionObj = Version.fromOrdinal(versionOrdinal); - } catch (UnsupportedSerializationVersionException e) { - this.versionObj = Version.CURRENT; - } - } - - public GMSMemberData(InetAddress i, int p, short version, long msbs, long lsbs, int viewId) { + public GMSMemberData(InetAddress i, int p, short versionOrdinal, long msbs, long lsbs, + int viewId) { this.inetAddr = i; this.hostName = i.getHostName(); this.udpPort = p; - setVersionObject(version); + this.versionOrdinal = Versioning.getVersionOrdinal(versionOrdinal); this.uuidMSBs = msbs; this.uuidLSBs = lsbs; this.vmViewId = viewId; @@ -176,7 +178,7 @@ public class GMSMemberData implements MemberData, Comparable<GMSMemberData> { this.durableId = other.durableId; this.durableTimeout = other.durableTimeout; this.groups = other.groups; - this.versionObj = other.versionObj; + this.versionOrdinal = other.versionOrdinal; this.uuidLSBs = other.uuidLSBs; this.uuidMSBs = other.uuidMSBs; this.isPartial = other.isPartial; @@ -220,12 +222,12 @@ public class GMSMemberData implements MemberData, Comparable<GMSMemberData> { @Override public short getVersionOrdinal() { - return this.versionObj.ordinal(); + return versionOrdinal.ordinal(); } @Override - public Version getVersion() { - return versionObj; + public VersionOrdinal getVersionOrdinalObject() { + return versionOrdinal; } @Override @@ -235,7 +237,7 @@ public class GMSMemberData implements MemberData, Comparable<GMSMemberData> { @Override public void setVersionOrdinal(short versionOrdinal) { - setVersionObject(versionOrdinal); + this.versionOrdinal = Versioning.getVersionOrdinal(versionOrdinal); } @Override @@ -506,7 +508,7 @@ public class GMSMemberData implements MemberData, Comparable<GMSMemberData> { @Override public void setVersion(Version v) { - this.versionObj = v; + setVersionOrdinal(v.ordinal()); } @Override @@ -581,7 +583,7 @@ public class GMSMemberData implements MemberData, Comparable<GMSMemberData> { @Override public void readEssentialData(DataInput in, DeserializationContext context) throws IOException, ClassNotFoundException { - setVersionObject(Version.readOrdinal(in)); + setVersionOrdinal(Version.readOrdinal(in)); int flags = in.readShort(); this.networkPartitionDetectionEnabled = (flags & NPD_ENABLED_BIT) != 0; diff --git a/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMembership.java b/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMembership.java index 8707467..a791e46 100644 --- a/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMembership.java +++ b/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMembership.java @@ -61,6 +61,7 @@ import org.apache.geode.distributed.internal.membership.api.QuorumChecker; import org.apache.geode.distributed.internal.membership.api.StopShunningMarker; import org.apache.geode.distributed.internal.membership.gms.interfaces.Manager; import org.apache.geode.internal.serialization.Version; +import org.apache.geode.internal.serialization.VersionOrdinal; import org.apache.geode.logging.internal.executors.LoggingExecutors; import org.apache.geode.logging.internal.executors.LoggingThread; import org.apache.geode.util.internal.GeodeGlossary; @@ -377,19 +378,19 @@ public class GMSMembership<ID extends MemberIdentifier> implements Membership<ID latestViewWriteLock.lock(); try { // first determine the version for multicast message serialization - Version version = Version.CURRENT; + VersionOrdinal version = Version.CURRENT; for (final Entry<ID, Long> internalIDLongEntry : surpriseMembers .entrySet()) { ID mbr = internalIDLongEntry.getKey(); - Version itsVersion = mbr.getVersionObject(); + final VersionOrdinal itsVersion = mbr.getVersionOrdinalObject(); if (itsVersion != null && version.compareTo(itsVersion) < 0) { version = itsVersion; } } for (ID mbr : newView.getMembers()) { - Version itsVersion = mbr.getVersionObject(); + final VersionOrdinal itsVersion = mbr.getVersionOrdinalObject(); if (itsVersion != null && itsVersion.compareTo(version) < 0) { - version = mbr.getVersionObject(); + version = mbr.getVersionOrdinalObject(); } } disableMulticastForRollingUpgrade = !version.equals(Version.CURRENT); diff --git a/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMembershipView.java b/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMembershipView.java index d0b0e48..362212f 100644 --- a/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMembershipView.java +++ b/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMembershipView.java @@ -654,4 +654,23 @@ public class GMSMembershipView<ID extends MemberIdentifier> implements DataSeria return NETVIEW; } + /** + * GEODE-8240 could cause a coordinator to produce a view with a wrong version during + * rolling upgrade. This method lets a view recipient repair the damaged version in + * its own member identifier. + * + * Mutates the version of the member identifier corresponding to memberID in this view. + * + * Remove this method when version Geode version 1.12.0 is no longer running in the wild. + * + * @param memberID is the identifier of the member of interest + */ + public void correctWrongVersionIn(final ID memberID) { + final ID oldID = getCanonicalID(memberID); + if (!oldID.getVersionOrdinalObject().equals(Version.getCurrentVersion())) { + // don't remove/add the ID lest we change it's relative position in the list + oldID.setVersionObjectForTest(Version.getCurrentVersion()); + } + } + } diff --git a/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/MemberIdentifierImpl.java b/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/MemberIdentifierImpl.java index 2b6e1dc..697d30a 100644 --- a/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/MemberIdentifierImpl.java +++ b/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/MemberIdentifierImpl.java @@ -46,6 +46,8 @@ import org.apache.geode.internal.serialization.DeserializationContext; import org.apache.geode.internal.serialization.SerializationContext; import org.apache.geode.internal.serialization.StaticSerialization; import org.apache.geode.internal.serialization.Version; +import org.apache.geode.internal.serialization.VersionOrdinal; +import org.apache.geode.internal.serialization.Versioning; /** * An implementation of {@link MemberIdentifier} @@ -419,7 +421,7 @@ public class MemberIdentifierImpl implements MemberIdentifier, DataSerializableF // add version if not current short version = memberData.getVersionOrdinal(); if (version != Version.CURRENT.ordinal()) { - sb.append("(version:").append(Version.toString(version)).append(')'); + sb.append("(version:").append(Versioning.getVersionOrdinal(version)).append(')'); } // leave out Roles on purpose @@ -973,7 +975,7 @@ public class MemberIdentifierImpl implements MemberIdentifier, DataSerializableF // add version if not current short version = memberData.getVersionOrdinal(); if (version != Version.CURRENT.ordinal()) { - sb.append("(version:").append(Version.toString(version)).append(')'); + sb.append("(version:").append(Versioning.getVersionOrdinal(version)).append(')'); } return sb.toString(); @@ -981,10 +983,12 @@ public class MemberIdentifierImpl implements MemberIdentifier, DataSerializableF public void setVersionObjectForTest(Version v) { memberData.setVersion(v); + cachedToString = null; } - public Version getVersionObject() { - return memberData.getVersion(); + @Override + public VersionOrdinal getVersionOrdinalObject() { + return memberData.getVersionOrdinalObject(); } @Override diff --git a/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java b/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java index 537ec02..59783e9 100644 --- a/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java +++ b/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java @@ -1467,6 +1467,8 @@ public class GMSJoinLeave<ID extends MemberIdentifier> implements JoinLeave<ID> } } + newView.correctWrongVersionIn(localAddress); + if (isJoined && isNetworkPartition(newView, true)) { if (quorumRequired) { Set<ID> crashes = newView.getActualCrashedMembers(currentView); diff --git a/geode-old-versions/README.md b/geode-old-versions/README.md new file mode 100644 index 0000000..164c416 --- /dev/null +++ b/geode-old-versions/README.md @@ -0,0 +1,55 @@ +# `geode-old-versions` Module +This module provides old Geode product versions for use by DUnit upgrade tests. + +Subprojects are added to this project from the outside. For example Geode's `settings.gradle` has code like this: + +```groovy +['1.0.0-incubating', + /* etc */ + '1.12.0'].each { + include 'geode-old-versions:'.concat(it) +} +``` + +For each subproject this project will download the artifacts (from whatever Maven repositories) are in effect. It'll unpack artifacts into subproject-specific build dirs. + +After everything is downloaded, two manifest-like files will be created for use by e.g. `VersionManager`: + +*. `geodeOldVersionInstalls.txt`: a map of version to install directory +*. `geodeOldVersionClasspaths.txt`: a map of version to Java classpath + +## Testing Your Upgrade Bug Fixes + +If you find a Geode rolling upgrade bug that necessitates a patch to an old line of development, you'd like to be able to branch that old line of development and build-and-publish artifacts locally. You'd then like this module to consume your locally-built artifacts instead of the ones acquired from remote Maven repositories. + +Here's an example of how you can convince Gradle to use your locally-published artifacts. + +Let's say you've found a rolling upgrade bug that necessitates a change to Geode 1.12.0 (an old version.) + +1\. clear, from the Gradle cache, 1.12.0 artifacts downloaded from non-local Maven repos +```shell script +cd geode # from the root of your Git clone +find ./.gradle -name "*1.12.0*" | xargs rm -fr {} \; +find ~/.gradle -name "*1.12.0*" | xargs rm -fr {} \; +``` +2\. delete the (synthesized) 1.12.0 subproject +```shell script +rm -fr ./geode-old-versions/1.12.0 +``` +3\. manually rebuild (via Gradle, not IntelliJ) the `geode-old-versions` module +```shell script +./gradlew :geode-old-versions:build +``` +4\. verify we are seeing the locally-built-and-published code (see sha and branch) +```shell script +./geode-old-versions/1.12.0/build/apache-geode-1.12.0/bin/gfsh version --full +``` + +e.g. I named my 1.12.0-based feature branch `feature/GEODE-8240-1-12-0-version-ordinal` and I see: + +``` +Source-Repository: feature/GEODE-8240-1-12-0-version-ordinal +Source-Revision: c38a0aa0df2f89fc657aa4f1e15fc152df32c99c +``` + +Now any `upgradeTest` e.g. `RollingUpgradeRollServersOnReplicatedRegion_dataserializable` that calls for Geode 1.12.0 will get the locally-published one. \ No newline at end of file diff --git a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/Version.java b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/Version.java index 6f93c06..d75755b 100644 --- a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/Version.java +++ b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/Version.java @@ -35,7 +35,7 @@ import org.apache.geode.annotations.Immutable; * @since GemFire 5.7 */ @Immutable -public class Version implements Comparable<Version> { +public class Version extends VersionOrdinalImpl { /** The name of this version */ private final transient String name; @@ -52,9 +52,6 @@ public class Version implements Comparable<Version> { private final byte release; private final byte patch; - /** byte used as ordinal to represent this <code>Version</code> */ - private final short ordinal; - public static final int HIGHEST_VERSION = 120; @Immutable @@ -313,13 +310,13 @@ public class Version implements Comparable<Version> { /** Creates a new instance of <code>Version</code> */ private Version(String product, String name, byte major, byte minor, byte release, byte patch, short ordinal) { + super(ordinal); this.productName = product; this.name = name; this.majorVersion = major; this.minorVersion = minor; this.release = release; this.patch = patch; - this.ordinal = ordinal; this.methodSuffix = this.productName + "_" + this.majorVersion + "_" + this.minorVersion + "_" + this.release + "_" + this.patch; if (ordinal != TOKEN_ORDINAL) { @@ -521,11 +518,6 @@ public class Version implements Comparable<Version> { return this.patch; } - public short ordinal() { - return this.ordinal; - } - - /** * Returns whether this <code>Version</code> is compatible with the input <code>Version</code> * @@ -537,39 +529,6 @@ public class Version implements Comparable<Version> { } /** - * Finds the Version instance corresponding to the given ordinal and returns the result of - * compareTo(Version) - * - * @param other the ordinal of the other Version object - * @return negative if this version is older, positive if this version is newer, 0 if this is the - * same version - */ - public int compareTo(short other) { - // first try to find the actual Version object - Version v = fromOrdinalNoThrow(other, false); - if (v == null) { - // failing that we use the old method of comparing Versions: - return this.ordinal() - other; - } - return compareTo(v); - } - - /** - * {@inheritDoc} - */ - @Override - public int compareTo(Version other) { - if (other != null) { - // 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; - } - } - - /** * Returns a string representation for this <code>Version</code>. * * @return the name of this operation. @@ -579,40 +538,6 @@ public class Version implements Comparable<Version> { return this.productName + " " + this.name; } - public static String toString(short ordinal) { - if (ordinal <= CURRENT.ordinal) { - try { - return fromOrdinal(ordinal).toString(); - } catch (UnsupportedSerializationVersionException uve) { - // ignored in toString() - } - } - return "UNKNOWN[ordinal=" + ordinal + ']'; - } - - @Override - public boolean equals(Object other) { - if (other == this) - return true; - if (other != null && other.getClass() == Version.class) { - return this.ordinal == ((Version) other).ordinal; - } else { - return false; - } - } - - public boolean equals(Version other) { - return other != null && this.ordinal == other.ordinal; - } - - @Override - public int hashCode() { - int result = 17; - final int mult = 37; - result = mult * result + this.ordinal; - return result; - } - public byte[] toBytes() { byte[] bytes = new byte[2]; bytes[0] = (byte) (ordinal >> 8); @@ -629,7 +554,4 @@ public class Version implements Comparable<Version> { .collect(Collectors.toList()); } - public boolean isCurrentVersion() { - return this.ordinal == CURRENT.ordinal; - } } diff --git a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/VersionOrdinal.java b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/VersionOrdinal.java new file mode 100644 index 0000000..d09a8ca --- /dev/null +++ b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/VersionOrdinal.java @@ -0,0 +1,83 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more contributor license + * agreements. See the NOTICE file distributed with this work for additional information regarding + * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. You may obtain a + * copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ + +package org.apache.geode.internal.serialization; + +/** + * VersionOrdinal is able to represent not only currently-known + * Geode versions but future versions as well. This is necessary + * because during rolling upgrades Geode manipulates member + * identifiers for members running newer versions of the software. + * In that case we receive the ordinal over the network + * (serialization) but we don't know other version details such as + * major/minor/patch version, which are known to the Version class. + * + * Implementations must define equals() and hashCode() based on + * ordinal() result. And since this interface extends Comparable, + * implementations must define compareTo() as well. + * + * Unlike Version (a subtype of VersionOrdinal which acts like an + * enumerated type), VersionOrdinal does not, in general, guarantee + * that if vo1.equals(vo2) then vo1 == vo2. + * + * Use the Versioning factory class to construct objects implementing + * this interface. All instances of known versions are defined as + * constants in the Version class, e.g. Version.GEODE_1_11_0 + */ +public interface VersionOrdinal extends Comparable<VersionOrdinal> { + + /** + * @return the short ordinal value for comparison implementations + */ + public short ordinal(); + + /* + * What follows is a bunch of comparison methods phrased in terms of version age: + * older/newer. + */ + + /** + * Test if this version is older than given version. + * + * @param version to compare to this version + * @return true if this is older than version, otherwise false. + */ + boolean isOlderThan(VersionOrdinal version); + + /** + * Test if this version is not older than given version. + * + * @param version to compare to this version + * @return true if this is the same version or newer, otherwise false. + */ + boolean isNotOlderThan(VersionOrdinal version); + + /** + * Test if this version is newer than given version. + * + * @param version to compare to this version + * @return true if this is newer than version, otherwise false. + */ + boolean isNewerThan(VersionOrdinal version); + + /** + * Test if this version is not newer than given version. + * + * @param version to compare to this version + * @return true if this is the same version or older, otherwise false. + */ + boolean isNotNewerThan(VersionOrdinal version); + +} diff --git a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/VersionOrdinalImpl.java b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/VersionOrdinalImpl.java new file mode 100644 index 0000000..aa6774c --- /dev/null +++ b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/VersionOrdinalImpl.java @@ -0,0 +1,136 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more contributor license + * agreements. See the NOTICE file distributed with this work for additional information regarding + * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. You may obtain a + * copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ + +package org.apache.geode.internal.serialization; + +public class VersionOrdinalImpl implements VersionOrdinal { + + protected final short ordinal; + + /** + * Package-private so only the Versioning factory can access this constructor. + * + */ + VersionOrdinalImpl(final short ordinal) { + this.ordinal = ordinal; + } + + @Override + public short ordinal() { + return ordinal; + } + + @Override + public int compareTo(final VersionOrdinal other) { + if (other == null) { + return 1; + } else { + return compareTo(other.ordinal()); + } + } + + /** + * TODO: eliminate this legacy method in favor of requiring callers to construct a + * VersionOrdinalImpl. Inline this logic up in compareTo(VersionOrdinal). + */ + public int compareTo(final short other) { + // short min/max can't overflow int, so use (a-b) + final int thisOrdinal = this.ordinal; + final int otherOrdinal = other; + return thisOrdinal - otherOrdinal; + } + + @Override + public boolean equals(final Object other) { + if (other == this) + return true; + if (other instanceof VersionOrdinalImpl) { + return this.ordinal == ((VersionOrdinalImpl) other).ordinal; + } else { + return false; + } + } + + public boolean equals(final VersionOrdinal other) { + return other != null && this.ordinal == other.ordinal(); + } + + @Override + public int hashCode() { + int result = 17; + final int mult = 37; + result = mult * result + this.ordinal; + return result; + } + + @Override + public String toString() { + return toString(ordinal); + } + + /** + * TODO: eliminate this legacy method in favor of requiring callers to construct a + * VersionOrdinalImpl. Inline this logic up in toString(). + */ + public static String toString(short ordinal) { + return "VersionOrdinal[ordinal=" + ordinal + ']'; + } + + + /** + * Test if this version is older than given version. + * + * @param version to compare to this version + * @return true if this is older than version, otherwise false. + */ + @Override + public final boolean isOlderThan(final VersionOrdinal version) { + return compareTo(version) < 0; + } + + /** + * Test if this version is not older than given version. + * + * @param version to compare to this version + * @return true if this is the same version or newer, otherwise false. + */ + @Override + public final boolean isNotOlderThan(final VersionOrdinal version) { + return compareTo(version) >= 0; + } + + /** + * Test if this version is newer than given version. + * + * @param version to compare to this version + * @return true if this is newer than version, otherwise false. + */ + @Override + public final boolean isNewerThan(final VersionOrdinal version) { + return compareTo(version) > 0; + } + + /** + * Test if this version is not newer than given version. + * + * @param version to compare to this version + * @return true if this is the same version or older, otherwise false. + */ + @Override + public final boolean isNotNewerThan(final VersionOrdinal version) { + return compareTo(version) <= 0; + } + +} diff --git a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/VersionedDataStream.java b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/VersionedDataStream.java index f9762c7..c1a4eb3 100644 --- a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/VersionedDataStream.java +++ b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/VersionedDataStream.java @@ -31,11 +31,14 @@ import java.io.DataOutput; public interface VersionedDataStream { /** - * If the remote peer to which this input/output is connected has a lower version that this - * member, then this returns the {@link Version} of the peer else null. If the peer has a - * higher - * {@link Version}, then this member cannot do any adjustment to serialization and its the remote - * peer's responsibility to adjust the serialization/deserialization according to this peer. + * If the remote peer to which this input/output is connected has a version ordinal + * for which a {@link Version} is known (locally) then that {@link Version} is returned, + * otherwise null is returned. + * + * If the peer has a version ordinal for which no {@link Version} is locally known, + * then this member cannot do any adjustment to serialization and it's the remote + * peer's responsibility to adjust the serialization/deserialization according to + * this peer. */ Version getVersion(); } diff --git a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/Versioning.java b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/Versioning.java new file mode 100644 index 0000000..b1ef6f9 --- /dev/null +++ b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/Versioning.java @@ -0,0 +1,37 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more contributor license + * agreements. See the NOTICE file distributed with this work for additional information regarding + * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. You may obtain a + * copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ + +package org.apache.geode.internal.serialization; + +/** + * This is a factory for getting VersionOrdinal instances. It's aware of the whole + * VersionOrdinal/Version hierarchy, so when asked for a VersionOrdinal that represents + * a known version (a Version) it returns a reference to one of those. + * + * This ensures that toString() on any VersionOrdinal, if that object represents a + * known version, will render itself as a Version. + */ +public class Versioning { + private Versioning() {} + + public static VersionOrdinal getVersionOrdinal(final short ordinal) { + try { + return Version.fromOrdinal(ordinal); + } catch (final UnsupportedSerializationVersionException e) { + return new VersionOrdinalImpl(ordinal); + } + } + +} diff --git a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/internal/DSFIDSerializerImpl.java b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/internal/DSFIDSerializerImpl.java index e44011b..134f48d 100644 --- a/geode-serialization/src/main/java/org/apache/geode/internal/serialization/internal/DSFIDSerializerImpl.java +++ b/geode-serialization/src/main/java/org/apache/geode/internal/serialization/internal/DSFIDSerializerImpl.java @@ -41,7 +41,6 @@ import org.apache.geode.internal.serialization.SerializationContext; import org.apache.geode.internal.serialization.SerializationVersions; import org.apache.geode.internal.serialization.StaticSerialization; import org.apache.geode.internal.serialization.Version; -import org.apache.geode.internal.serialization.VersionedDataStream; public class DSFIDSerializerImpl implements DSFIDSerializer { @@ -190,7 +189,7 @@ public class DSFIDSerializerImpl implements DSFIDSerializer { boolean invoked = false; Version v = context.getSerializationVersion(); - if (!v.isCurrentVersion()) { + if (!Version.CURRENT.equals(v)) { // get versions where DataOutput was upgraded SerializationVersions sv = (SerializationVersions) ds; Version[] versions = sv.getSerializationVersions(); @@ -220,20 +219,6 @@ public class DSFIDSerializerImpl implements DSFIDSerializer { } } - /** - * Get the Version of the peer or disk store that created this {@link DataOutput}. - * Returns - * zero if the version is same as this member's. - */ - public Version getVersionForDataStreamOrNull(DataOutput out) { - // check if this is a versioned data output - if (out instanceof VersionedDataStream) { - return ((VersionedDataStream) out).getVersion(); - } else { - return null; - } - } - private Object readDSFID(final DataInput in) throws IOException, ClassNotFoundException { checkIn(in); DSCODE dsHeaderType = DscodeHelper.toDSCODE(in.readByte()); @@ -308,7 +293,7 @@ public class DSFIDSerializerImpl implements DSFIDSerializer { try { boolean invoked = false; Version v = context.getSerializationVersion(); - if (!v.isCurrentVersion() && ds instanceof SerializationVersions) { + if (!Version.CURRENT.equals(v) && ds instanceof SerializationVersions) { // get versions where DataOutput was upgraded SerializationVersions vds = (SerializationVersions) ds; Version[] versions = vds.getSerializationVersions(); diff --git a/geode-serialization/src/test/java/org/apache/geode/internal/serialization/VersionJUnitTest.java b/geode-serialization/src/test/java/org/apache/geode/internal/serialization/VersionJUnitTest.java index 5b610c1..20a7066 100644 --- a/geode-serialization/src/test/java/org/apache/geode/internal/serialization/VersionJUnitTest.java +++ b/geode-serialization/src/test/java/org/apache/geode/internal/serialization/VersionJUnitTest.java @@ -14,6 +14,7 @@ */ package org.apache.geode.internal.serialization; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -42,6 +43,8 @@ public class VersionJUnitTest { compare(Version.GEODE_1_9_0, Version.GEODE_1_8_0); compare(Version.GEODE_1_10_0, Version.GEODE_1_9_0); compare(Version.GEODE_1_11_0, Version.GEODE_1_10_0); + compare(Version.GEODE_1_12_0, Version.GEODE_1_11_0); + compare(Version.GEODE_1_13_0, Version.GEODE_1_12_0); } private void compare(Version later, Version earlier) { @@ -68,4 +71,19 @@ public class VersionJUnitTest { throws UnsupportedSerializationVersionException { Version.fromOrdinal(Version.CURRENT_ORDINAL); } + + @Test + public void ordinalImplMatchesVersion() { + /* + * We are not using the Version.getVersionOrdinal(short) factory method here + * because we intend to test that Version and VersionOrdinal are cross-comparable. + * The factory would return Version.GFE_82 which would foil our testing. + */ + final VersionOrdinalImpl versionOrdinal = new VersionOrdinalImpl(Version.GFE_82.ordinal); + assertThat(Version.GFE_82.equals(versionOrdinal)) + .as("GFE_82 Version equals VersionOrdinal").isTrue(); + assertThat(versionOrdinal.equals(Version.GFE_82)) + .as("GFE_82 VersionOrdinal equals Version").isTrue(); + } + } diff --git a/geode-serialization/src/test/java/org/apache/geode/internal/serialization/VersionOrdinalImplJUnitTest.java b/geode-serialization/src/test/java/org/apache/geode/internal/serialization/VersionOrdinalImplJUnitTest.java new file mode 100644 index 0000000..c339402 --- /dev/null +++ b/geode-serialization/src/test/java/org/apache/geode/internal/serialization/VersionOrdinalImplJUnitTest.java @@ -0,0 +1,127 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more contributor license + * agreements. See the NOTICE file distributed with this work for additional information regarding + * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. You may obtain a + * copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ + +package org.apache.geode.internal.serialization; + +import static org.assertj.core.api.Assertions.assertThat; + +import junit.framework.TestCase; +import org.junit.Test; + +public class VersionOrdinalImplJUnitTest extends TestCase { + + @Test + public void testEqualMinSameIdentity() { + final VersionOrdinal versionOrdinal = new VersionOrdinalImpl(Short.MIN_VALUE); + validateEqual(versionOrdinal, versionOrdinal); + } + + @Test + public void testEqualMinDifferentIdentity() { + validateEqual(new VersionOrdinalImpl(Short.MIN_VALUE), new VersionOrdinalImpl(Short.MIN_VALUE)); + } + + @Test + public void testEqualMaxSameIdentity() { + final VersionOrdinalImpl versionOrdinal = new VersionOrdinalImpl(Short.MAX_VALUE); + validateEqual(versionOrdinal, versionOrdinal); + } + + @Test + public void testEqualMaxDifferentIdentity() { + validateEqual(new VersionOrdinalImpl(Short.MAX_VALUE), new VersionOrdinalImpl(Short.MAX_VALUE)); + } + + @Test + public void testUnequalVersionOrdinals() { + validateUnequal((short) 7, (short) 8); + } + + @Test + public void testUnequalVersionOrdinalsLimits() { + validateUnequal(Short.MIN_VALUE, Short.MAX_VALUE); + } + + @Test + public void testHashMin() { + validateHash(Short.MIN_VALUE); + } + + @Test + public void testHashMax() { + validateHash(Short.MAX_VALUE); + } + + @Test + public void testStringMin() { + assertThat(new VersionOrdinalImpl(Short.MIN_VALUE).toString()) + .isEqualTo("VersionOrdinal[ordinal=-32768]"); + } + + @Test + public void testStringMax() { + assertThat(new VersionOrdinalImpl(Short.MAX_VALUE).toString()) + .isEqualTo("VersionOrdinal[ordinal=32767]"); + } + + @Test + public void testCompareToNull() { + assertThat(new VersionOrdinalImpl((short) 3).compareTo(null)).isEqualTo(1); + } + + @Test + public void testEqualsIncompatible() { + assertThat(new VersionOrdinalImpl((short) 6).equals("howdy!")).isFalse(); + } + + private void validateEqual(final VersionOrdinal a, final VersionOrdinal b) { + assertThat(a.compareTo(b)).isEqualTo(0); + assertThat(a.equals(b)).isTrue(); + assertThat(a.isNewerThan(b)).isFalse(); + assertThat(a.isNotNewerThan(b)).isTrue(); + assertThat(a.isOlderThan(b)).isFalse(); + assertThat(a.isNotOlderThan(b)).isTrue(); + } + + private void validateUnequal(final short smallerShort, final short largerShort) { + final VersionOrdinal smaller = new VersionOrdinalImpl(smallerShort); + final VersionOrdinal larger = new VersionOrdinalImpl(largerShort); + + assertThat(smaller.compareTo(larger)).isLessThan(0); + assertThat(smaller.equals(larger)).isFalse(); + assertThat(smaller.isNewerThan(larger)).isFalse(); + assertThat(smaller.isOlderThan(larger)).isTrue(); + + assertThat(larger.compareTo(smaller)).isGreaterThan(0); + assertThat(larger.equals(smaller)).isFalse(); + assertThat(larger.isNewerThan(smaller)).isTrue(); + assertThat(larger.isOlderThan(smaller)).isFalse(); + + // now test the boolean inverses + + assertThat(smaller.isNewerThan(larger)).isNotEqualTo(smaller.isNotNewerThan(larger)); + assertThat(larger.isNewerThan(smaller)).isNotEqualTo(larger.isNotNewerThan(smaller)); + assertThat(smaller.isOlderThan(larger)).isNotEqualTo(smaller.isNotOlderThan(larger)); + assertThat(larger.isOlderThan(smaller)).isNotEqualTo(larger.isNotOlderThan(smaller)); + } + + private void validateHash(final short ordinal) { + final VersionOrdinal a = new VersionOrdinalImpl(ordinal); + final VersionOrdinal b = new VersionOrdinalImpl(ordinal); + assertThat(a.equals(b)).isTrue(); + assertThat(a.hashCode()).isEqualTo(b.hashCode()); + } + +} diff --git a/geode-tcp-server/src/distributedTest/java/org/apache/geode/distributed/internal/tcpserver/TcpServerProductVersionDUnitTest.java b/geode-tcp-server/src/distributedTest/java/org/apache/geode/distributed/internal/tcpserver/TcpServerProductVersionDUnitTest.java index 0fd5d06..3530ba6 100644 --- a/geode-tcp-server/src/distributedTest/java/org/apache/geode/distributed/internal/tcpserver/TcpServerProductVersionDUnitTest.java +++ b/geode-tcp-server/src/distributedTest/java/org/apache/geode/distributed/internal/tcpserver/TcpServerProductVersionDUnitTest.java @@ -49,7 +49,6 @@ import org.apache.geode.internal.InternalDataSerializer; import org.apache.geode.internal.net.SocketCreator; import org.apache.geode.internal.net.SocketCreatorFactory; import org.apache.geode.internal.security.SecurableCommunicationChannel; -import org.apache.geode.internal.serialization.Version; import org.apache.geode.test.awaitility.GeodeAwaitility; import org.apache.geode.test.dunit.DistributedTestUtils; import org.apache.geode.test.dunit.Host; @@ -143,10 +142,12 @@ public class TcpServerProductVersionDUnitTest implements Serializable { @Test public void testAllMessageTypes() { - int clientVMNumber = versions.clientProductVersion.isSameAs(Version.CURRENT) - ? DUnitLauncher.DEBUGGING_VM_NUM : 0; - int locatorVMNumber = versions.locatorProductVersion.isSameAs(Version.CURRENT) - ? DUnitLauncher.DEBUGGING_VM_NUM : 0; + int clientVMNumber = + versions.clientProductVersion.equals(TestVersion.CURRENT_VERSION) + ? DUnitLauncher.DEBUGGING_VM_NUM : 0; + int locatorVMNumber = + versions.locatorProductVersion.equals(TestVersion.CURRENT_VERSION) + ? DUnitLauncher.DEBUGGING_VM_NUM : 0; VM clientVM = Host.getHost(0).getVM(versions.clientProductVersion.toString(), clientVMNumber); VM locatorVM = Host.getHost(0).getVM(versions.locatorProductVersion.toString(), locatorVMNumber);