You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by es...@apache.org on 2018/04/12 16:44:49 UTC

[geode] branch develop updated: GEODE-5039: Change AbstractRegion to correctly construct EvictionAttributesMutator (#1766)

This is an automated email from the ASF dual-hosted git repository.

eshu11 pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git


The following commit(s) were added to refs/heads/develop by this push:
     new c62846d  GEODE-5039: Change AbstractRegion to correctly construct EvictionAttributesMutator (#1766)
c62846d is described below

commit c62846d643d35c68945320dc8a0e85197b28b0f6
Author: pivotal-eshu <es...@pivotal.io>
AuthorDate: Thu Apr 12 09:44:44 2018 -0700

    GEODE-5039: Change AbstractRegion to correctly construct EvictionAttributesMutator (#1766)
---
 .../geode/internal/cache/AbstractRegion.java       | 212 ++++++++++-----------
 ...victionAttributesMutatorImplRegressionTest.java |  75 ++++++++
 2 files changed, 177 insertions(+), 110 deletions(-)

diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegion.java b/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegion.java
index bc5f792..1a68d82 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegion.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegion.java
@@ -83,7 +83,6 @@ import org.apache.geode.distributed.internal.DistributionAdvisor;
 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.membership.InternalDistributedMember;
 import org.apache.geode.internal.DataSerializableFixedID;
 import org.apache.geode.internal.cache.extension.Extensible;
 import org.apache.geode.internal.cache.extension.ExtensionPoint;
@@ -223,10 +222,7 @@ public abstract class AbstractRegion implements InternalRegion, AttributesMutato
   /** Attributes that define this Region as a PartitionedRegion */
   protected PartitionAttributes partitionAttributes;
 
-  protected EvictionAttributesImpl evictionAttributes = new EvictionAttributesImpl();
-
-  protected EvictionAttributesMutator evictionAttributesMutator =
-      new EvictionAttributesMutatorImpl(this, this.evictionAttributes);
+  private final EvictionAttributesImpl evictionAttributes;
 
   /** The membership attributes defining required roles functionality */
   protected MembershipAttributes membershipAttributes;
@@ -272,7 +268,105 @@ public abstract class AbstractRegion implements InternalRegion, AttributesMutato
     this.isPdxTypesRegion = PeerTypeRegistration.REGION_NAME.equals(regionName);
     this.lastAccessedTime = new AtomicLong(cacheTimeMillis());
     this.lastModifiedTime = new AtomicLong(this.lastAccessedTime.get());
-    setAttributes(attrs, regionName, internalRegionArgs);
+    this.dataPolicy = attrs.getDataPolicy(); // do this one first
+    this.scope = attrs.getScope();
+
+    this.offHeap = attrs.getOffHeap();
+
+    // fix bug #52033 by invoking setOffHeap now (localMaxMemory may now be the temporary
+    // placeholder for off-heap until DistributedSystem is created
+    // found non-null PartitionAttributes and offHeap is true so let's setOffHeap on PA now
+    PartitionAttributes<?, ?> partitionAttributes1 = attrs.getPartitionAttributes();
+    if (this.offHeap && partitionAttributes1 != null) {
+      PartitionAttributesImpl impl = (PartitionAttributesImpl) partitionAttributes1;
+      impl.setOffHeap(true);
+    }
+
+    evictionAttributes = new EvictionAttributesImpl(attrs.getEvictionAttributes());
+    if (attrs.getPartitionAttributes() != null && this.evictionAttributes != null
+        && this.evictionAttributes.getAlgorithm().isLRUMemory()
+        && attrs.getPartitionAttributes().getLocalMaxMemory() != 0 && this.evictionAttributes
+            .getMaximum() != attrs.getPartitionAttributes().getLocalMaxMemory()) {
+      logger.warn(LocalizedMessage.create(LocalizedStrings.Mem_LRU_Eviction_Attribute_Reset,
+          new Object[] {regionName, this.evictionAttributes.getMaximum(),
+              attrs.getPartitionAttributes().getLocalMaxMemory()}));
+      this.evictionAttributes.setMaximum(attrs.getPartitionAttributes().getLocalMaxMemory());
+    }
+
+    storeCacheListenersField(attrs.getCacheListeners());
+    assignCacheLoader(attrs.getCacheLoader());
+    assignCacheWriter(attrs.getCacheWriter());
+    this.regionTimeToLive = attrs.getRegionTimeToLive().getTimeout();
+    this.regionTimeToLiveExpirationAction = attrs.getRegionTimeToLive().getAction();
+    setRegionTimeToLiveAtts();
+    this.regionIdleTimeout = attrs.getRegionIdleTimeout().getTimeout();
+    this.regionIdleTimeoutExpirationAction = attrs.getRegionIdleTimeout().getAction();
+    setRegionIdleTimeoutAttributes();
+    this.entryTimeToLive = attrs.getEntryTimeToLive().getTimeout();
+    this.entryTimeToLiveExpirationAction = attrs.getEntryTimeToLive().getAction();
+    setEntryTimeToLiveAttributes();
+    this.customEntryTimeToLive = attrs.getCustomEntryTimeToLive();
+    this.entryIdleTimeout = attrs.getEntryIdleTimeout().getTimeout();
+    this.entryIdleTimeoutExpirationAction = attrs.getEntryIdleTimeout().getAction();
+    setEntryIdleTimeoutAttributes();
+    this.customEntryIdleTimeout = attrs.getCustomEntryIdleTimeout();
+    updateEntryExpiryPossible();
+    this.statisticsEnabled = attrs.getStatisticsEnabled();
+    this.ignoreJTA = attrs.getIgnoreJTA();
+    this.isLockGrantor = attrs.isLockGrantor();
+    this.keyConstraint = attrs.getKeyConstraint();
+    this.valueConstraint = attrs.getValueConstraint();
+    this.initialCapacity = attrs.getInitialCapacity();
+    this.loadFactor = attrs.getLoadFactor();
+    this.concurrencyLevel = attrs.getConcurrencyLevel();
+    this.setConcurrencyChecksEnabled(
+        attrs.getConcurrencyChecksEnabled() && supportsConcurrencyChecks());
+    this.earlyAck = attrs.getEarlyAck();
+    this.gatewaySenderIds = attrs.getGatewaySenderIds();
+    this.asyncEventQueueIds = attrs.getAsyncEventQueueIds();
+    initializeVisibleAsyncEventQueueIds(internalRegionArgs);
+    setAllGatewaySenderIds();
+    this.enableSubscriptionConflation = attrs.getEnableSubscriptionConflation();
+    this.publisher = attrs.getPublisher();
+    this.enableAsyncConflation = attrs.getEnableAsyncConflation();
+    this.indexMaintenanceSynchronous = attrs.getIndexMaintenanceSynchronous();
+    this.mcastEnabled = attrs.getMulticastEnabled();
+    this.partitionAttributes = attrs.getPartitionAttributes();
+    this.membershipAttributes = attrs.getMembershipAttributes();
+    this.subscriptionAttributes = attrs.getSubscriptionAttributes();
+    this.cloningEnable = attrs.getCloningEnabled();
+    this.poolName = attrs.getPoolName();
+    if (this.poolName != null) {
+      PoolImpl cp = getPool();
+      if (cp == null) {
+        throw new IllegalStateException(
+            LocalizedStrings.AbstractRegion_THE_CONNECTION_POOL_0_HAS_NOT_BEEN_CREATED
+                .toLocalizedString(this.poolName));
+      }
+      cp.attach();
+      if (cp.getMultiuserAuthentication() && !this.getDataPolicy().isEmpty()) {
+        throw new IllegalStateException(
+            "Region must have empty data-policy " + "when multiuser-authentication is true.");
+      }
+    }
+
+    this.diskStoreName = attrs.getDiskStoreName();
+    this.isDiskSynchronous = attrs.isDiskSynchronous();
+    if (this.diskStoreName == null) {
+      this.diskWriteAttributes = attrs.getDiskWriteAttributes();
+      this.isDiskSynchronous = this.diskWriteAttributes.isSynchronous(); // fixes bug 41313
+      this.diskDirs = attrs.getDiskDirs();
+      this.diskSizes = attrs.getDiskDirSizes();
+    }
+
+    this.compressor = attrs.getCompressor();
+    // enable concurrency checks for persistent regions
+    if (!attrs.getConcurrencyChecksEnabled() && attrs.getDataPolicy().withPersistence()
+        && supportsConcurrencyChecks()) {
+      throw new IllegalStateException(
+          LocalizedStrings.AttributesFactory_CONCURRENCY_CHECKS_MUST_BE_ENABLED
+              .toLocalizedString());
+    }
   }
 
   @TestingOnly
@@ -282,6 +376,7 @@ public abstract class AbstractRegion implements InternalRegion, AttributesMutato
     this.isPdxTypesRegion = false;
     this.lastAccessedTime = new AtomicLong(0);
     this.lastModifiedTime = new AtomicLong(0);
+    evictionAttributes = new EvictionAttributesImpl();
   }
 
   /**
@@ -1527,109 +1622,6 @@ public abstract class AbstractRegion implements InternalRegion, AttributesMutato
     return this.evictionAttributes != null && !this.evictionAttributes.getAlgorithm().isNone();
   }
 
-  private void setAttributes(RegionAttributes attrs, String regionName,
-      InternalRegionArguments internalRegionArgs) {
-    this.dataPolicy = attrs.getDataPolicy(); // do this one first
-    this.scope = attrs.getScope();
-
-    this.offHeap = attrs.getOffHeap();
-
-    // fix bug #52033 by invoking setOffHeap now (localMaxMemory may now be the temporary
-    // placeholder for off-heap until DistributedSystem is created
-    // found non-null PartitionAttributes and offHeap is true so let's setOffHeap on PA now
-    PartitionAttributes<?, ?> partitionAttributes = attrs.getPartitionAttributes();
-    if (this.offHeap && partitionAttributes != null) {
-      PartitionAttributesImpl impl = (PartitionAttributesImpl) partitionAttributes;
-      impl.setOffHeap(true);
-    }
-
-    this.evictionAttributes = new EvictionAttributesImpl(attrs.getEvictionAttributes());
-    if (attrs.getPartitionAttributes() != null && this.evictionAttributes != null
-        && this.evictionAttributes.getAlgorithm().isLRUMemory()
-        && attrs.getPartitionAttributes().getLocalMaxMemory() != 0 && this.evictionAttributes
-            .getMaximum() != attrs.getPartitionAttributes().getLocalMaxMemory()) {
-      logger.warn(LocalizedMessage.create(LocalizedStrings.Mem_LRU_Eviction_Attribute_Reset,
-          new Object[] {regionName, this.evictionAttributes.getMaximum(),
-              attrs.getPartitionAttributes().getLocalMaxMemory()}));
-      this.evictionAttributes.setMaximum(attrs.getPartitionAttributes().getLocalMaxMemory());
-    }
-
-    storeCacheListenersField(attrs.getCacheListeners());
-    assignCacheLoader(attrs.getCacheLoader());
-    assignCacheWriter(attrs.getCacheWriter());
-    this.regionTimeToLive = attrs.getRegionTimeToLive().getTimeout();
-    this.regionTimeToLiveExpirationAction = attrs.getRegionTimeToLive().getAction();
-    setRegionTimeToLiveAtts();
-    this.regionIdleTimeout = attrs.getRegionIdleTimeout().getTimeout();
-    this.regionIdleTimeoutExpirationAction = attrs.getRegionIdleTimeout().getAction();
-    setRegionIdleTimeoutAttributes();
-    this.entryTimeToLive = attrs.getEntryTimeToLive().getTimeout();
-    this.entryTimeToLiveExpirationAction = attrs.getEntryTimeToLive().getAction();
-    setEntryTimeToLiveAttributes();
-    this.customEntryTimeToLive = attrs.getCustomEntryTimeToLive();
-    this.entryIdleTimeout = attrs.getEntryIdleTimeout().getTimeout();
-    this.entryIdleTimeoutExpirationAction = attrs.getEntryIdleTimeout().getAction();
-    setEntryIdleTimeoutAttributes();
-    this.customEntryIdleTimeout = attrs.getCustomEntryIdleTimeout();
-    updateEntryExpiryPossible();
-    this.statisticsEnabled = attrs.getStatisticsEnabled();
-    this.ignoreJTA = attrs.getIgnoreJTA();
-    this.isLockGrantor = attrs.isLockGrantor();
-    this.keyConstraint = attrs.getKeyConstraint();
-    this.valueConstraint = attrs.getValueConstraint();
-    this.initialCapacity = attrs.getInitialCapacity();
-    this.loadFactor = attrs.getLoadFactor();
-    this.concurrencyLevel = attrs.getConcurrencyLevel();
-    this.setConcurrencyChecksEnabled(
-        attrs.getConcurrencyChecksEnabled() && supportsConcurrencyChecks());
-    this.earlyAck = attrs.getEarlyAck();
-    this.gatewaySenderIds = attrs.getGatewaySenderIds();
-    this.asyncEventQueueIds = attrs.getAsyncEventQueueIds();
-    initializeVisibleAsyncEventQueueIds(internalRegionArgs);
-    setAllGatewaySenderIds();
-    this.enableSubscriptionConflation = attrs.getEnableSubscriptionConflation();
-    this.publisher = attrs.getPublisher();
-    this.enableAsyncConflation = attrs.getEnableAsyncConflation();
-    this.indexMaintenanceSynchronous = attrs.getIndexMaintenanceSynchronous();
-    this.mcastEnabled = attrs.getMulticastEnabled();
-    this.partitionAttributes = attrs.getPartitionAttributes();
-    this.membershipAttributes = attrs.getMembershipAttributes();
-    this.subscriptionAttributes = attrs.getSubscriptionAttributes();
-    this.cloningEnable = attrs.getCloningEnabled();
-    this.poolName = attrs.getPoolName();
-    if (this.poolName != null) {
-      PoolImpl cp = getPool();
-      if (cp == null) {
-        throw new IllegalStateException(
-            LocalizedStrings.AbstractRegion_THE_CONNECTION_POOL_0_HAS_NOT_BEEN_CREATED
-                .toLocalizedString(this.poolName));
-      }
-      cp.attach();
-      if (cp.getMultiuserAuthentication() && !this.getDataPolicy().isEmpty()) {
-        throw new IllegalStateException(
-            "Region must have empty data-policy " + "when multiuser-authentication is true.");
-      }
-    }
-
-    this.diskStoreName = attrs.getDiskStoreName();
-    this.isDiskSynchronous = attrs.isDiskSynchronous();
-    if (this.diskStoreName == null) {
-      this.diskWriteAttributes = attrs.getDiskWriteAttributes();
-      this.isDiskSynchronous = this.diskWriteAttributes.isSynchronous(); // fixes bug 41313
-      this.diskDirs = attrs.getDiskDirs();
-      this.diskSizes = attrs.getDiskDirSizes();
-    }
-
-    this.compressor = attrs.getCompressor();
-    // enable concurrency checks for persistent regions
-    if (!attrs.getConcurrencyChecksEnabled() && attrs.getDataPolicy().withPersistence()
-        && supportsConcurrencyChecks()) {
-      throw new IllegalStateException(
-          LocalizedStrings.AttributesFactory_CONCURRENCY_CHECKS_MUST_BE_ENABLED
-              .toLocalizedString());
-    }
-  }
-
   /** is this a region that supports versioning? */
   public abstract boolean supportsConcurrencyChecks();
 
@@ -1674,7 +1666,7 @@ public abstract class AbstractRegion implements InternalRegion, AttributesMutato
 
   @Override
   public EvictionAttributesMutator getEvictionAttributesMutator() {
-    return this.evictionAttributesMutator;
+    return new EvictionAttributesMutatorImpl(this, this.evictionAttributes);
   }
 
   /**
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/EvictionAttributesMutatorImplRegressionTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/EvictionAttributesMutatorImplRegressionTest.java
new file mode 100644
index 0000000..a16e982
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/EvictionAttributesMutatorImplRegressionTest.java
@@ -0,0 +1,75 @@
+/*
+ * 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.cache;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.rules.TestName;
+
+import org.apache.geode.cache.AttributesMutator;
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.CacheFactory;
+import org.apache.geode.cache.EvictionAction;
+import org.apache.geode.cache.EvictionAlgorithm;
+import org.apache.geode.cache.EvictionAttributes;
+import org.apache.geode.cache.EvictionAttributesMutator;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionAttributes;
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.test.junit.categories.IntegrationTest;
+
+@Category(IntegrationTest.class)
+public class EvictionAttributesMutatorImplRegressionTest {
+  private Region<String, String> region;
+
+  @Rule
+  public TestName testName = new TestName();
+
+  @Before
+  public void setUp() throws Exception {
+    region = createRegion();
+  }
+
+  private Region<String, String> createRegion() throws Exception {
+    int initMaximum = 1;
+    Cache cache = new CacheFactory().set("locators", "").set("mcast-port", "0").create();
+    Region<String, String> aRegion =
+        cache.<String, String>createRegionFactory(RegionShortcut.REPLICATE)
+            .setEvictionAttributes(EvictionAttributes.createLRUEntryAttributes(initMaximum))
+            .create(testName.getMethodName());
+
+    RegionAttributes<String, String> attributes = aRegion.getAttributes();
+    EvictionAttributes evictionAttributes = attributes.getEvictionAttributes();
+    assertThat(evictionAttributes.getMaximum()).isEqualTo(initMaximum);
+    return aRegion;
+  }
+
+  @Test
+  public void verifySetMaximum() {
+    AttributesMutator<String, String> attributesMutator = region.getAttributesMutator();
+    EvictionAttributesMutator evictionAttributesMutator =
+        attributesMutator.getEvictionAttributesMutator();
+    int updatedMaximum = 2;
+    evictionAttributesMutator.setMaximum(updatedMaximum);
+    RegionAttributes<String, String> attributes = region.getAttributes();
+    EvictionAttributes evictionAttributes = attributes.getEvictionAttributes();
+    assertThat(evictionAttributes.getMaximum()).isEqualTo(updatedMaximum);
+  }
+
+}

-- 
To stop receiving notification emails like this one, please contact
eshu11@apache.org.