This is an automated email from the ASF dual-hosted git repository. echobravo pushed a commit to branch release/1.12.0 in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/release/1.12.0 by this push: new 46a9d94 GEODE-7789: Cache DurableClienAttributes (#4687) 46a9d94 is described below commit 46a9d9468437dba4ee227e74be767f0224d050e5 Author: Juan José Ramos <jujora...@users.noreply.github.com> AuthorDate: Thu Feb 13 10:00:36 2020 +0000 GEODE-7789: Cache DurableClienAttributes (#4687) Keep the last known 'DurableClienAttributes' cached within the 'InternalDistributedMember' class to avoid the overhead of creating a new instance every time. (cherry picked from commit 71e156a55228d89efafd048e1533debba606c064) --- .../codeAnalysis/sanctionedDataSerializables.txt | 6 +- .../membership/InternalDistributedMember.java | 32 ++++-- .../membership/InternalDistributedMemberTest.java | 109 +++++++++++++++++++-- 3 files changed, 132 insertions(+), 15 deletions(-) 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 5e2e57c..3076db7 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 @@ -275,9 +275,9 @@ fromData,17 toData,17 org/apache/geode/distributed/internal/membership/InternalDistributedMember,6 -fromData,12 -fromDataPre_GFE_7_1_0_0,12 -fromDataPre_GFE_9_0_0_0,12 +fromData,17 +fromDataPre_GFE_7_1_0_0,17 +fromDataPre_GFE_9_0_0_0,17 toData,12 toDataPre_GFE_7_1_0_0,12 toDataPre_GFE_9_0_0_0,12 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 151b787..ac2bf6b 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 @@ -60,6 +60,7 @@ public class InternalDistributedMember implements DistributedMember, Externalizable, ProfileId, VersionSource<DistributedMember>, MemberIdentifier, DataSerializableFixedID { private static final long serialVersionUID = -2785249969777296507L; + public static final int DEFAULT_DURABLE_CLIENT_TIMEOUT = 300; @Immutable public static final MemberIdentifierFactoryImpl MEMBER_IDENTIFIER_FACTORY = @@ -69,9 +70,12 @@ public class InternalDistributedMember @MutableForTesting protected static HostnameResolver hostnameResolver = (location) -> InetAddress.getByName(location.getHostName()); + private final MemberIdentifier memberIdentifier; - /** lock object used when getting/setting roles/rolesSet fields */ + @VisibleForTesting + // Cached to prevent creating a new instance every single time. + protected volatile DurableClientAttributes durableClientAttributes; // Used only by deserialization public InternalDistributedMember() { @@ -187,7 +191,8 @@ public class InternalDistributedMember * @throws UnknownHostException if the given hostname cannot be resolved */ public InternalDistributedMember(String host, int p, String n, String u, int vmKind, - String[] groups, DurableClientAttributes attr) throws UnknownHostException { + String[] groups, DurableClientAttributes attr) { + durableClientAttributes = attr; memberIdentifier = MEMBER_IDENTIFIER_FACTORY.create(createMemberData(host, p, n, vmKind, groups, attr, u)); @@ -272,11 +277,19 @@ public class InternalDistributedMember @Override public DurableClientAttributes getDurableClientAttributes() { assert !this.isPartial(); - String durableId = memberIdentifier.getDurableId(); - if (durableId == null || durableId.isEmpty()) { - return new DurableClientAttributes("", 300); + + if (durableClientAttributes == null) { + String durableId = memberIdentifier.getDurableId(); + + if (durableId == null || durableId.isEmpty()) { + durableClientAttributes = new DurableClientAttributes("", DEFAULT_DURABLE_CLIENT_TIMEOUT); + } else { + durableClientAttributes = + new DurableClientAttributes(durableId, memberIdentifier.getDurableTimeout()); + } } - return new DurableClientAttributes(durableId, memberIdentifier.getDurableTimeout()); + + return durableClientAttributes; } /** @@ -336,16 +349,19 @@ public class InternalDistributedMember @Override public void setDurableTimeout(int newValue) { memberIdentifier.setDurableTimeout(newValue); + durableClientAttributes = null; } @Override public void setDurableId(String id) { memberIdentifier.setDurableId(id); + durableClientAttributes = null; } @Override public void setMemberData(MemberData m) { memberIdentifier.setMemberData(m); + durableClientAttributes = null; } @Override @@ -457,6 +473,7 @@ public class InternalDistributedMember @Override public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException { memberIdentifier.readExternal(in); + durableClientAttributes = null; } @Override @@ -483,6 +500,7 @@ public class InternalDistributedMember DeserializationContext context) throws IOException, ClassNotFoundException { memberIdentifier.fromData(in, context); + durableClientAttributes = null; } @Override @@ -490,6 +508,7 @@ public class InternalDistributedMember DeserializationContext context) throws IOException, ClassNotFoundException { memberIdentifier.fromDataPre_GFE_9_0_0_0(in, context); + durableClientAttributes = null; } @Override @@ -497,6 +516,7 @@ public class InternalDistributedMember DeserializationContext context) throws IOException, ClassNotFoundException { memberIdentifier.fromDataPre_GFE_7_1_0_0(in, context); + durableClientAttributes = null; } @Override diff --git a/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/InternalDistributedMemberTest.java b/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/InternalDistributedMemberTest.java index 66f0367..4e889a0 100644 --- a/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/InternalDistributedMemberTest.java +++ b/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/InternalDistributedMemberTest.java @@ -24,7 +24,9 @@ import java.net.UnknownHostException; import org.junit.Test; import org.junit.experimental.categories.Category; +import org.apache.geode.distributed.DurableClientAttributes; import org.apache.geode.distributed.internal.membership.api.MemberData; +import org.apache.geode.distributed.internal.membership.api.MemberIdentifier; import org.apache.geode.test.junit.categories.MembershipTest; /** @@ -37,18 +39,14 @@ public class InternalDistributedMemberTest { public void equalsReturnsTrueForSameMember() { InternalDistributedMember member = new InternalDistributedMember("", 34567); - boolean result = member.equals(member); - - assertThat(result).isTrue(); + assertThat(member.equals(member)).isTrue(); } @Test public void equalsReturnsFalseForNull() { InternalDistributedMember member = new InternalDistributedMember("", 34567); - boolean result = member.equals(null); - - assertThat(result).isFalse(); + assertThat(member.equals(null)).isFalse(); } @Test @@ -183,4 +181,103 @@ public class InternalDistributedMemberTest { assertThat(result).isTrue(); } + @Test + public void getDurableClientAttributesShouldReturnDefaultsWhenCachedInstanceIsNullAndDurableIdIsNull() + throws UnknownHostException { + InetAddress host1 = InetAddress.getByAddress(new byte[] {127, 0, 0, 1}); + MemberData memberData = mock(MemberData.class); + when(memberData.getInetAddress()).thenReturn(host1); + InternalDistributedMember member = new InternalDistributedMember(memberData); + + assertThat(member.durableClientAttributes).isNull(); + assertThat(member.getDurableClientAttributes()).isNotNull(); + assertThat(member.getDurableClientAttributes().getId()).isEqualTo(""); + assertThat(member.getDurableClientAttributes().getTimeout()) + .isEqualTo(InternalDistributedMember.DEFAULT_DURABLE_CLIENT_TIMEOUT); + } + + @Test + public void getDurableClientAttributesShouldReturnDefaultsWhenCachedInstanceIsNullAndDurableIdIsEmpty() + throws UnknownHostException { + InetAddress host1 = InetAddress.getByAddress(new byte[] {127, 0, 0, 1}); + MemberData memberData = mock(MemberData.class); + when(memberData.getDurableId()).thenReturn(""); + when(memberData.getInetAddress()).thenReturn(host1); + InternalDistributedMember member = new InternalDistributedMember(memberData); + + assertThat(member.durableClientAttributes).isNull(); + assertThat(member.getDurableClientAttributes()).isNotNull(); + assertThat(member.getDurableClientAttributes().getId()).isEqualTo(""); + assertThat(member.getDurableClientAttributes().getTimeout()) + .isEqualTo(InternalDistributedMember.DEFAULT_DURABLE_CLIENT_TIMEOUT); + } + + @Test + public void getDurableClientAttributesShouldReturnCustomAttributesWhenCachedInstanceIsNullAndDurableIdIsNotNull() + throws UnknownHostException { + InetAddress host1 = InetAddress.getByAddress(new byte[] {127, 0, 0, 1}); + MemberData memberData = mock(MemberData.class); + when(memberData.getInetAddress()).thenReturn(host1); + when(memberData.getDurableId()).thenReturn("durableId"); + when(memberData.getDurableTimeout()).thenReturn(Integer.MAX_VALUE); + InternalDistributedMember internalDistributedMember = new InternalDistributedMember(memberData); + + assertThat(internalDistributedMember.durableClientAttributes).isNull(); + assertThat(internalDistributedMember.getDurableClientAttributes()).isNotNull(); + assertThat(internalDistributedMember.getDurableClientAttributes().getId()) + .isEqualTo("durableId"); + assertThat(internalDistributedMember.getDurableClientAttributes().getTimeout()) + .isEqualTo(Integer.MAX_VALUE); + } + + @Test + public void getDurableClientAttributesShouldReturnCachedInstanceWhenBackingMemberIdentifierAttributesHaveNotChanged() + throws UnknownHostException { + InetAddress host1 = InetAddress.getByAddress(new byte[] {127, 0, 0, 1}); + MemberData memberData = mock(MemberData.class); + when(memberData.getInetAddress()).thenReturn(host1); + when(memberData.getDurableId()).thenReturn("durableId"); + when(memberData.getDurableTimeout()).thenReturn(Integer.MAX_VALUE); + InternalDistributedMember internalDistributedMember = new InternalDistributedMember(memberData); + assertThat(internalDistributedMember.durableClientAttributes).isNull(); + + // Get Attributes first time - should be instantiated and cached. + DurableClientAttributes attributes = internalDistributedMember.getDurableClientAttributes(); + assertThat(attributes).isNotNull(); + assertThat(attributes.getId()).isEqualTo("durableId"); + assertThat(attributes.getTimeout()).isEqualTo(Integer.MAX_VALUE); + + // Get Attributes again without changing backing store - should return cached instance . + assertThat(internalDistributedMember.getDurableClientAttributes()).isSameAs(attributes); + } + + @Test + public void setDurableTimeOutShouldNullifyCachedDurableClientAttributes() { + InternalDistributedMember member = new InternalDistributedMember("", 34567, "name", "uniqueId", + MemberIdentifier.NORMAL_DM_TYPE, new String[] {}, new DurableClientAttributes("", 500)); + assertThat(member.durableClientAttributes).isNotNull(); + + member.setDurableTimeout(600); + assertThat(member.durableClientAttributes).isNull(); + } + + @Test + public void setDurableIdShouldNullifyCachedDurableClientAttributes() { + InternalDistributedMember member = new InternalDistributedMember("", 34567, "name", "uniqueId", + MemberIdentifier.NORMAL_DM_TYPE, new String[] {}, new DurableClientAttributes("", 500)); + assertThat(member.durableClientAttributes).isNotNull(); + + member.setDurableId("testId"); + assertThat(member.durableClientAttributes).isNull(); + } + + @Test + public void setMemberDataShouldNullifyCachedDurableClientAttributes() { + InternalDistributedMember member = new InternalDistributedMember("", 34567, "name", "uniqueId", + MemberIdentifier.NORMAL_DM_TYPE, new String[] {}, new DurableClientAttributes("", 500)); + assertThat(member.durableClientAttributes).isNotNull(); + + member.setMemberData(mock(MemberData.class)); + assertThat(member.durableClientAttributes).isNull(); + } }