GEODE-8748: Prevents NPE on alter expiry on proxy regions (#5790)
* When a region is created with zero local memory, there is no bucket regions created.
* When alter region is called on this proxy region to alter expiry, it tries to manipulate the bucket region.
* But there are no buckets and hence NPE is thrown.
* In this commit a null check is done before alter the bucket regions.
* Configs still need to changed to prevent restart issues.
* This similar to the check done in setEntryTimeToLive in PartitionedRegion classdiff --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 ebc778a..afafb8b 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
@@ -9362,8 +9362,10 @@
* updatePRConfig(...) to make changes that cause bucket creation to live lock
*/
PartitionRegionHelper.assignBucketsToPartitions(this);
- dataStore.lockBucketCreationAndVisit(
- (bucketId, r) -> r.getAttributesMutator().setEntryTimeToLive(timeToLive));
+ if (dataStore != null) {
+ dataStore.lockBucketCreationAndVisit(
+ (bucketId, r) -> r.getAttributesMutator().setEntryTimeToLive(timeToLive));
+ }
updatePartitionRegionConfig(config -> config.setEntryTimeToLive(timeToLive));
return attr;
}
@@ -9378,8 +9380,10 @@
public CustomExpiry setCustomEntryTimeToLive(CustomExpiry custom) {
CustomExpiry expiry = super.setCustomEntryTimeToLive(custom);
// Set to Bucket regions as well
- dataStore.lockBucketCreationAndVisit(
- (bucketId, r) -> r.getAttributesMutator().setCustomEntryTimeToLive(custom));
+ if (dataStore != null) {
+ dataStore.lockBucketCreationAndVisit(
+ (bucketId, r) -> r.getAttributesMutator().setCustomEntryTimeToLive(custom));
+ }
return expiry;
}
@@ -9404,9 +9408,10 @@
*/
PartitionRegionHelper.assignBucketsToPartitions(this);
// Set to Bucket regions as well
- dataStore.lockBucketCreationAndVisit(
- (bucketId, r) -> r.getAttributesMutator().setEntryIdleTimeout(idleTimeout));
-
+ if (dataStore != null) {
+ dataStore.lockBucketCreationAndVisit(
+ (bucketId, r) -> r.getAttributesMutator().setEntryIdleTimeout(idleTimeout));
+ }
updatePartitionRegionConfig(config -> config.setEntryIdleTimeout(idleTimeout));
return attr;
}
@@ -9421,8 +9426,10 @@
public CustomExpiry setCustomEntryIdleTimeout(CustomExpiry custom) {
CustomExpiry expiry = super.setCustomEntryIdleTimeout(custom);
// Set to Bucket regions as well
- dataStore.lockBucketCreationAndVisit(
- (bucketId, r) -> r.getAttributesMutator().setCustomEntryIdleTimeout(custom));
+ if (dataStore != null) {
+ dataStore.lockBucketCreationAndVisit(
+ (bucketId, r) -> r.getAttributesMutator().setCustomEntryIdleTimeout(custom));
+ }
return expiry;
}
diff --git a/geode-gfsh/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/AlterTimeToLiveExpirationOnProxyRegionDUnitTest.java b/geode-gfsh/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/AlterTimeToLiveExpirationOnProxyRegionDUnitTest.java
new file mode 100644
index 0000000..a451c4f
--- /dev/null
+++ b/geode-gfsh/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/AlterTimeToLiveExpirationOnProxyRegionDUnitTest.java
@@ -0,0 +1,74 @@
+/*
+ * 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.management.internal.cli.commands;
+
+import junitparams.JUnitParamsRunner;
+import junitparams.Parameters;
+import junitparams.naming.TestCaseName;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+
+import org.apache.geode.test.dunit.rules.ClusterStartupRule;
+import org.apache.geode.test.dunit.rules.MemberVM;
+import org.apache.geode.test.junit.categories.EvictionTest;
+import org.apache.geode.test.junit.rules.GfshCommandRule;
+
+@Category({EvictionTest.class})
+@RunWith(JUnitParamsRunner.class)
+public class AlterTimeToLiveExpirationOnProxyRegionDUnitTest {
+ @Rule
+ public ClusterStartupRule clusterStartupRule = new ClusterStartupRule();
+ @Rule
+ public GfshCommandRule gfsh = new GfshCommandRule();
+
+ public Object[] getRegionTypePairs() {
+ return new Object[] {
+ new Object[] {"REPLICATE", "REPLICATE_PROXY"},
+ new Object[] {"PARTITION", "PARTITION_PROXY"},
+ new Object[] {"PARTITION_REDUNDANT", "PARTITION_PROXY_REDUNDANT"}
+ };
+ }
+
+ @Test
+ @Parameters(method = "getRegionTypePairs")
+ @TestCaseName("[{index}] {method} Non Proxy Region Type:{0}; Proxy Region Type:{1}")
+ public void whenExpirationIsSetUsingAlterOnProxyRegionThenItShouldNotThrowException(
+ String nonProxyRegionType, String proxyRegionType) throws Exception {
+ MemberVM locator = clusterStartupRule.startLocatorVM(0);
+ MemberVM server1 = clusterStartupRule.startServerVM(1, "non-proxy", locator.getPort());
+ MemberVM server2 = clusterStartupRule.startServerVM(2, "proxy", locator.getPort());
+ gfsh.connectAndVerify(locator);
+
+ gfsh.executeAndAssertThat(
+ "create region --name=region --type=" + nonProxyRegionType
+ + " --enable-statistics=true --group=non-proxy")
+ .statusIsSuccess();
+ gfsh.executeAndAssertThat(
+ "create region --name=region --type=" + proxyRegionType
+ + " --enable-statistics=true --group=proxy")
+ .statusIsSuccess();
+
+ gfsh.executeAndAssertThat(
+ "alter region --name=region --entry-time-to-live-expiration=1000 --entry-time-to-live-expiration-action=destroy --group=non-proxy")
+ .statusIsSuccess();
+ gfsh.executeAndAssertThat(
+ "alter region --name=region --entry-time-to-live-expiration=1000 --entry-time-to-live-expiration-action=destroy --group=proxy")
+ .statusIsSuccess();
+
+ }
+
+}
