You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by do...@apache.org on 2020/11/11 00:01:43 UTC
[geode] branch support/1.13 updated (32a8a55 -> aaa3d13)
This is an automated email from the ASF dual-hosted git repository.
donalevans pushed a change to branch support/1.13
in repository https://gitbox.apache.org/repos/asf/geode.git.
from 32a8a55 * GEODE-8652: NioSslEngine.close() Bypasses Locks (#5712)
new 0f9d6b2 GEODE-8536: Allow limited retries when creating Lucene IndexWriter (#5659)
new aaa3d13 GEODE-8686: Prevent potential deadlock during GII and tombstone GC (#5707)
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../cache/versions/TombstoneDUnitTest.java | 98 +++++++++++++++-
.../geode/internal/cache/AbstractRegionMap.java | 16 +--
.../IndexRepositoryFactoryDistributedTest.java | 2 -
.../IndexRepositoryFactoryIntegrationTest.java | 123 +++++++++++++++++++++
.../lucene/internal/IndexRepositoryFactory.java | 41 +++++--
.../internal/IndexRepositoryFactoryTest.java | 45 ++++++--
6 files changed, 291 insertions(+), 34 deletions(-)
create mode 100644 geode-lucene/src/integrationTest/java/org/apache/geode/cache/lucene/internal/IndexRepositoryFactoryIntegrationTest.java
[geode] 01/02: GEODE-8536: Allow limited retries when creating
Lucene IndexWriter (#5659)
Posted by do...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
donalevans pushed a commit to branch support/1.13
in repository https://gitbox.apache.org/repos/asf/geode.git
commit 0f9d6b22ec45b9238bec268161eadeb3647ddb49
Author: Donal Evans <do...@pivotal.io>
AuthorDate: Fri Oct 23 12:05:42 2020 -0700
GEODE-8536: Allow limited retries when creating Lucene IndexWriter (#5659)
Authored-by: Donal Evans <do...@vmware.com>
(cherry picked from commit 872718ec9d119e332c328caf4493bdf8e8a83dcf)
---
.../IndexRepositoryFactoryDistributedTest.java | 2 -
.../IndexRepositoryFactoryIntegrationTest.java | 123 +++++++++++++++++++++
.../lucene/internal/IndexRepositoryFactory.java | 41 +++++--
.../internal/IndexRepositoryFactoryTest.java | 45 ++++++--
4 files changed, 193 insertions(+), 18 deletions(-)
diff --git a/geode-lucene/src/distributedTest/java/org/apache/geode/cache/lucene/internal/IndexRepositoryFactoryDistributedTest.java b/geode-lucene/src/distributedTest/java/org/apache/geode/cache/lucene/internal/IndexRepositoryFactoryDistributedTest.java
index a3f245d..bfe921f 100644
--- a/geode-lucene/src/distributedTest/java/org/apache/geode/cache/lucene/internal/IndexRepositoryFactoryDistributedTest.java
+++ b/geode-lucene/src/distributedTest/java/org/apache/geode/cache/lucene/internal/IndexRepositoryFactoryDistributedTest.java
@@ -25,7 +25,6 @@ import java.io.Serializable;
import java.util.concurrent.TimeUnit;
import java.util.stream.IntStream;
-import junitparams.Parameters;
import org.apache.commons.lang3.RandomStringUtils;
import org.awaitility.core.ConditionTimeoutException;
import org.junit.Before;
@@ -111,7 +110,6 @@ public class IndexRepositoryFactoryDistributedTest implements Serializable {
}
@Test
- @Parameters()
public void lockedBucketShouldPreventPrimaryFromMoving() {
dataStore1.invoke(this::initDataStoreAndLuceneIndex);
dataStore1.invoke(() -> LuceneTestUtilities.pauseSender(getCache()));
diff --git a/geode-lucene/src/integrationTest/java/org/apache/geode/cache/lucene/internal/IndexRepositoryFactoryIntegrationTest.java b/geode-lucene/src/integrationTest/java/org/apache/geode/cache/lucene/internal/IndexRepositoryFactoryIntegrationTest.java
new file mode 100644
index 0000000..5e79bcf
--- /dev/null
+++ b/geode-lucene/src/integrationTest/java/org/apache/geode/cache/lucene/internal/IndexRepositoryFactoryIntegrationTest.java
@@ -0,0 +1,123 @@
+/*
+ * 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.cache.lucene.internal;
+
+import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+
+import java.io.IOException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
+
+import org.apache.geode.InternalGemFireError;
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.CacheFactory;
+import org.apache.geode.cache.PartitionAttributesFactory;
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.cache.execute.FunctionException;
+import org.apache.geode.cache.lucene.LuceneQuery;
+import org.apache.geode.cache.lucene.LuceneQueryException;
+import org.apache.geode.cache.lucene.LuceneServiceProvider;
+import org.apache.geode.internal.cache.InternalCache;
+
+public class IndexRepositoryFactoryIntegrationTest {
+ private Cache cache;
+ public static final String INDEX_NAME = "testIndex";
+ public static final String REGION_NAME = "testRegion";
+ public static final int NUMBER_OF_BUCKETS = 4;
+ private IndexRepositoryFactory spyFactory;
+ private LuceneQuery<Object, Object> luceneQuery;
+
+ @Before
+ public void setUp() {
+ cache = new CacheFactory().create();
+ String fieldName = "field1";
+ LuceneServiceProvider.get(cache)
+ .createIndexFactory()
+ .setFields(fieldName)
+ .create(INDEX_NAME, REGION_NAME);
+
+ cache.createRegionFactory(RegionShortcut.PARTITION)
+ .setPartitionAttributes(new PartitionAttributesFactory<>()
+ .setTotalNumBuckets(NUMBER_OF_BUCKETS)
+ .create())
+ .create(REGION_NAME);
+
+ spyFactory = spy(new IndexRepositoryFactory());
+ PartitionedRepositoryManager.indexRepositoryFactory = spyFactory;
+
+ luceneQuery = LuceneServiceProvider.get(cache)
+ .createLuceneQueryFactory()
+ .create(INDEX_NAME, REGION_NAME, "hello", fieldName);
+ }
+
+ @After
+ public void tearDown() {
+ ExecutorService lonerDistributionThreads =
+ ((InternalCache) cache).getDistributionManager().getExecutors().getThreadPool();
+ PartitionedRepositoryManager.indexRepositoryFactory = new IndexRepositoryFactory();
+ if (cache != null) {
+ cache.close();
+ }
+ // Wait until the thread pool that uses the modified IndexRepositoryFactory behaviour has
+ // terminated before allowing further tests, to prevent mocking exceptions
+ await().until(lonerDistributionThreads::isTerminated);
+ }
+
+ @Test
+ public void shouldRetryWhenIOExceptionEncounteredOnceDuringComputingRepository()
+ throws IOException, LuceneQueryException {
+ // To ensure that the specific bucket used in the query throws the IOException to trigger the
+ // retry, throw once for every bucket in the region
+
+ doAnswer(new Answer<Object>() {
+ private AtomicInteger times = new AtomicInteger(0);
+
+ @Override
+ public Object answer(InvocationOnMock invocation) throws Throwable {
+ if (times.getAndIncrement() < NUMBER_OF_BUCKETS) {
+ throw new IOException();
+ }
+ return invocation.callRealMethod();
+ }
+ }).when(spyFactory).getIndexWriter(any(), any());
+
+ luceneQuery.findKeys();
+
+ // The invocation should throw once for each bucket, then retry once for each bucket
+ verify(spyFactory, times(NUMBER_OF_BUCKETS * 2)).getIndexWriter(any(), any());
+ }
+
+ @Test
+ public void shouldThrowInternalGemfireErrorWhenIOExceptionEncounteredConsistentlyDuringComputingRepository()
+ throws IOException {
+ doThrow(new IOException()).when(spyFactory).getIndexWriter(any(), any());
+
+ assertThatThrownBy(luceneQuery::findKeys).isInstanceOf(FunctionException.class)
+ .hasCauseInstanceOf(InternalGemFireError.class);
+ }
+}
diff --git a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/IndexRepositoryFactory.java b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/IndexRepositoryFactory.java
index 7674a45..7db8b96 100644
--- a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/IndexRepositoryFactory.java
+++ b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/IndexRepositoryFactory.java
@@ -44,6 +44,7 @@ public class IndexRepositoryFactory {
private static final Logger logger = LogService.getLogger();
public static final String FILE_REGION_LOCK_FOR_BUCKET_ID = "FileRegionLockForBucketId:";
public static final String APACHE_GEODE_INDEX_COMPLETE = "APACHE_GEODE_INDEX_COMPLETE";
+ protected static final int GET_INDEX_WRITER_MAX_ATTEMPTS = 200;
public IndexRepositoryFactory() {}
@@ -74,7 +75,8 @@ public class IndexRepositoryFactory {
* This is a util function just to not let computeIndexRepository be a huge chunk of code.
*/
protected IndexRepository finishComputingRepository(Integer bucketId, LuceneSerializer serializer,
- PartitionedRegion userRegion, IndexRepository oldRepository, InternalLuceneIndex index) {
+ PartitionedRegion userRegion, IndexRepository oldRepository, InternalLuceneIndex index)
+ throws IOException {
LuceneIndexForPartitionedRegion indexForPR = (LuceneIndexForPartitionedRegion) index;
final PartitionedRegion fileRegion = indexForPR.getFileAndChunkRegion();
BucketRegion fileAndChunkBucket = getMatchingBucket(fileRegion, bucketId);
@@ -129,7 +131,7 @@ public class IndexRepositoryFactory {
} catch (IOException e) {
logger.warn("Exception thrown while constructing Lucene Index for bucket:" + bucketId
+ " for file region:" + fileAndChunkBucket.getFullPath(), e);
- return null;
+ throw e;
} catch (CacheClosedException e) {
logger.info("CacheClosedException thrown while constructing Lucene Index for bucket:"
+ bucketId + " for file region:" + fileAndChunkBucket.getFullPath());
@@ -144,11 +146,34 @@ public class IndexRepositoryFactory {
protected IndexWriter buildIndexWriter(int bucketId, BucketRegion fileAndChunkBucket,
LuceneIndexForPartitionedRegion indexForPR) throws IOException {
- // bucketTargetingMap handles partition resolver (via bucketId as callbackArg)
- Map bucketTargetingMap = getBucketTargetingMap(fileAndChunkBucket, bucketId);
- RegionDirectory dir = new RegionDirectory(bucketTargetingMap, indexForPR.getFileSystemStats());
- IndexWriterConfig config = new IndexWriterConfig(indexForPR.getAnalyzer());
+ int attempts = 0;
+ // IOExceptions can occur if the fileAndChunk region is being modified while the IndexWriter is
+ // being initialized, so allow limited retries here to account for that timing window
+ while (true) {
+ // bucketTargetingMap handles partition resolver (via bucketId as callbackArg)
+ Map<Object, Object> bucketTargetingMap = getBucketTargetingMap(fileAndChunkBucket, bucketId);
+ RegionDirectory dir =
+ new RegionDirectory(bucketTargetingMap, indexForPR.getFileSystemStats());
+ IndexWriterConfig config = new IndexWriterConfig(indexForPR.getAnalyzer());
+ try {
+ attempts++;
+ return getIndexWriter(dir, config);
+ } catch (IOException e) {
+ if (attempts >= GET_INDEX_WRITER_MAX_ATTEMPTS) {
+ throw e;
+ }
+ logger.info("Encountered {} while attempting to get IndexWriter for index {}. Retrying...",
+ e, indexForPR.getName());
+ try {
+ Thread.sleep(5);
+ } catch (InterruptedException ignore) {
+ }
+ }
+ }
+ }
+ protected IndexWriter getIndexWriter(RegionDirectory dir, IndexWriterConfig config)
+ throws IOException {
return new IndexWriter(dir, config);
}
@@ -186,8 +211,8 @@ public class IndexRepositoryFactory {
return value;
}
- protected Map getBucketTargetingMap(BucketRegion region, int bucketId) {
- return new BucketTargetingMap(region, bucketId);
+ protected Map<Object, Object> getBucketTargetingMap(BucketRegion region, int bucketId) {
+ return new BucketTargetingMap<>(region, bucketId);
}
protected String getLockName(final BucketRegion fileAndChunkBucket) {
diff --git a/geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/IndexRepositoryFactoryTest.java b/geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/IndexRepositoryFactoryTest.java
index e301dcf..38e6355 100644
--- a/geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/IndexRepositoryFactoryTest.java
+++ b/geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/IndexRepositoryFactoryTest.java
@@ -14,6 +14,7 @@
*/
package org.apache.geode.cache.lucene.internal;
+import static org.apache.geode.cache.lucene.internal.IndexRepositoryFactory.GET_INDEX_WRITER_MAX_ATTEMPTS;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.ArgumentMatchers.any;
@@ -22,11 +23,13 @@ import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import java.io.IOException;
+import org.apache.lucene.index.IndexWriter;
import org.junit.Before;
import org.junit.Test;
@@ -77,7 +80,8 @@ public class IndexRepositoryFactoryTest {
}
@Test
- public void finishComputingRepositoryShouldReturnNullAndCleanOldRepositoryWhenFileAndChunkBucketIsNull() {
+ public void finishComputingRepositoryShouldReturnNullAndCleanOldRepositoryWhenFileAndChunkBucketIsNull()
+ throws IOException {
doReturn(null).when(indexRepositoryFactory).getMatchingBucket(fileRegion, bucketId);
IndexRepository indexRepository = indexRepositoryFactory.finishComputingRepository(0,
@@ -87,7 +91,8 @@ public class IndexRepositoryFactoryTest {
}
@Test
- public void finishComputingRepositoryShouldReturnNullAndCleanOldRepositoryWhenFileAndChunkBucketIsNotPrimary() {
+ public void finishComputingRepositoryShouldReturnNullAndCleanOldRepositoryWhenFileAndChunkBucketIsNotPrimary()
+ throws IOException {
when(fileAndChunkBucketAdvisor.isPrimary()).thenReturn(false);
IndexRepository indexRepository = indexRepositoryFactory.finishComputingRepository(0,
@@ -97,7 +102,8 @@ public class IndexRepositoryFactoryTest {
}
@Test
- public void finishComputingRepositoryShouldReturnOldRepositoryWhenNotNullAndNotClosed() {
+ public void finishComputingRepositoryShouldReturnOldRepositoryWhenNotNullAndNotClosed()
+ throws IOException {
when(oldRepository.isClosed()).thenReturn(false);
when(fileAndChunkBucketAdvisor.isPrimary()).thenReturn(true);
@@ -108,7 +114,8 @@ public class IndexRepositoryFactoryTest {
}
@Test
- public void finishComputingRepositoryShouldReturnNullWhenLockCanNotBeAcquiredAndFileAndChunkBucketIsNotPrimary() {
+ public void finishComputingRepositoryShouldReturnNullWhenLockCanNotBeAcquiredAndFileAndChunkBucketIsNotPrimary()
+ throws IOException {
when(oldRepository.isClosed()).thenReturn(true);
when(fileAndChunkBucketAdvisor.isPrimary()).thenReturn(true).thenReturn(false);
when(distributedLockService.lock(any(), anyLong(), anyLong())).thenReturn(false);
@@ -119,7 +126,7 @@ public class IndexRepositoryFactoryTest {
}
@Test
- public void finishComputingRepositoryShouldReturnNullAndReleaseLockWhenIOExceptionIsThrownWhileBuildingTheIndex()
+ public void finishComputingRepositoryShouldThrowExceptionAndReleaseLockWhenIOExceptionIsThrownWhileBuildingTheIndex()
throws IOException {
when(oldRepository.isClosed()).thenReturn(true);
when(fileAndChunkBucketAdvisor.isPrimary()).thenReturn(true);
@@ -127,9 +134,8 @@ public class IndexRepositoryFactoryTest {
doThrow(new IOException("Test Exception")).when(indexRepositoryFactory)
.buildIndexWriter(bucketId, fileAndChunkBucket, luceneIndex);
- IndexRepository indexRepository = indexRepositoryFactory.finishComputingRepository(0,
- serializer, userRegion, oldRepository, luceneIndex);
- assertThat(indexRepository).isNull();
+ assertThatThrownBy(() -> indexRepositoryFactory.finishComputingRepository(0,
+ serializer, userRegion, oldRepository, luceneIndex)).isInstanceOf(IOException.class);
verify(distributedLockService).unlock(any());
}
@@ -146,4 +152,27 @@ public class IndexRepositoryFactoryTest {
userRegion, oldRepository, luceneIndex)).isInstanceOf(CacheClosedException.class);
verify(distributedLockService).unlock(any());
}
+
+ @Test
+ public void buildIndexWriterRetriesCreatingIndexWriterWhenIOExceptionEncountered()
+ throws IOException {
+ IndexWriter writer = mock(IndexWriter.class);
+ doThrow(new IOException()).doReturn(writer).when(indexRepositoryFactory).getIndexWriter(any(),
+ any());
+ assertThat(indexRepositoryFactory.buildIndexWriter(bucketId, fileAndChunkBucket, luceneIndex))
+ .isEqualTo(writer);
+ verify(indexRepositoryFactory, times(2)).getIndexWriter(any(), any());
+ }
+
+ @Test
+ public void buildIndexWriterThrowsExceptionWhenIOExceptionConsistentlyEncountered()
+ throws IOException {
+ IOException testException = new IOException("Test exception");
+ doThrow(testException).when(indexRepositoryFactory).getIndexWriter(any(), any());
+ assertThatThrownBy(
+ () -> indexRepositoryFactory.buildIndexWriter(bucketId, fileAndChunkBucket, luceneIndex))
+ .isEqualTo(testException);
+ verify(indexRepositoryFactory, times(GET_INDEX_WRITER_MAX_ATTEMPTS)).getIndexWriter(any(),
+ any());
+ }
}
[geode] 02/02: GEODE-8686: Prevent potential deadlock during GII
and tombstone GC (#5707)
Posted by do...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
donalevans pushed a commit to branch support/1.13
in repository https://gitbox.apache.org/repos/asf/geode.git
commit aaa3d13e9c604806624e43ba86d959473226a674
Author: Donal Evans <do...@pivotal.io>
AuthorDate: Tue Nov 10 09:16:00 2020 -0800
GEODE-8686: Prevent potential deadlock during GII and tombstone GC (#5707)
- Do not call AbstractRegionMap.removeTombstone() outside of
TombstoneService class
- Add test to confirm that tombstones are correctly scheduled and
collected with this change
Authored-by: Donal Evans <do...@vmware.com>
(cherry picked from commit 70b1ee8b98f1458620539c4a962e82f8ef35707b)
---
.../cache/versions/TombstoneDUnitTest.java | 98 +++++++++++++++++++++-
.../geode/internal/cache/AbstractRegionMap.java | 16 +---
2 files changed, 98 insertions(+), 16 deletions(-)
diff --git a/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/versions/TombstoneDUnitTest.java b/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/versions/TombstoneDUnitTest.java
index cf4dcb6..8c7fc3a 100644
--- a/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/versions/TombstoneDUnitTest.java
+++ b/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/versions/TombstoneDUnitTest.java
@@ -14,7 +14,11 @@
*/
package org.apache.geode.internal.cache.versions;
+import static org.apache.geode.cache.RegionShortcut.REPLICATE_PERSISTENT;
+import static org.apache.geode.internal.cache.InitialImageOperation.GIITestHookType.AfterReceivedRequestImage;
+import static org.apache.geode.internal.cache.InitialImageOperation.GIITestHookType.DuringApplyDelta;
import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
+import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.assertEquals;
import java.io.Serializable;
@@ -30,6 +34,7 @@ import org.apache.geode.distributed.internal.DistributionMessage;
import org.apache.geode.distributed.internal.DistributionMessageObserver;
import org.apache.geode.internal.cache.DestroyOperation;
import org.apache.geode.internal.cache.DistributedTombstoneOperation;
+import org.apache.geode.internal.cache.InitialImageOperation;
import org.apache.geode.internal.cache.LocalRegion;
import org.apache.geode.test.dunit.AsyncInvocation;
import org.apache.geode.test.dunit.Host;
@@ -133,9 +138,98 @@ public class TombstoneDUnitTest extends JUnit4CacheTestCase {
});
}
- private class RegionObserver extends DistributionMessageObserver implements Serializable {
+ @Test
+ public void tombstoneGCDuringGIICorrectlySchedulesTombstonesForCollection() {
+ VM vm0 = VM.getVM(0);
+ VM vm1 = VM.getVM(1);
+
+ createCache(vm0);
+ createCache(vm1);
+
+ vm0.invoke(() -> {
+ createRegion("TestRegion", true);
+ Region<String, String> region = getCache().getRegion("TestRegion");
+ region.put("K1", "V1");
+ region.put("K2", "V2");
+ });
+
+ vm1.invoke(() -> {
+ createRegion("TestRegion", true);
+ Region<String, String> region = getCache().getRegion("TestRegion");
+ // Ensure that there are local tombstones to be recovered in the member that will request GII
+ region.destroy("K1");
+ region.destroy("K2");
+ closeCache();
+ });
+
+ vm0.invoke(() -> {
+ Region<String, String> region = getCache().getRegion("TestRegion");
+ // Ensure that there are newer tombstones that will be sent via GII
+ region.put("K1", "V3");
+ region.destroy("K1");
+ region.put("K2", "V4");
+ region.destroy("K2");
+ // Trigger a tombstone GC after receiving the GII request message
+ InitialImageOperation.setGIITestHook(
+ new InitialImageOperation.GIITestHook(AfterReceivedRequestImage, "TestRegion") {
+ private static final long serialVersionUID = -3790198435185240444L;
+
+ @Override
+ public void reset() {}
+
+ @Override
+ public void run() {
+ try {
+ performGC(((LocalRegion) region).getTombstoneCount());
+ } catch (Exception ignore) {
+ }
+ }
+ });
+ });
+
+ createCache(vm1);
+
+ vm1.invoke(() -> {
+ InitialImageOperation.setGIITestHook(
+ new InitialImageOperation.GIITestHook(DuringApplyDelta, "TestRegion") {
+ private static final long serialVersionUID = 637083883125364247L;
+ private int entryNumber = 0;
+
+ @Override
+ public void reset() {
+ entryNumber = 0;
+ }
+
+ @Override
+ public void run() {
+ if (entryNumber == 0) {
+ await().alias("Waiting for scheduled tombstone count to be zero")
+ .until(
+ () -> getCache().getTombstoneService().getScheduledTombstoneCount() == 0);
+ }
+ // Confirm that tombstones are correctly scheduled for collection after processing
+ // each new entry received during GII
+ assertThat(getCache().getTombstoneService().getScheduledTombstoneCount())
+ .as("Scheduled tombstone count did not match expected value")
+ .isEqualTo(entryNumber++);
+ }
+ });
+
+ Region<?, ?> region =
+ getCache().<String, String>createRegionFactory(REPLICATE_PERSISTENT).create("TestRegion");
+
+ // Confirm that we are able to collect all tombstones once the region is initialized
+ performGC(((LocalRegion) region).getTombstoneCount());
+ assertEquals(0, ((LocalRegion) region).getTombstoneCount());
+ InitialImageOperation.resetAllGIITestHooks();
+ });
+
+ vm0.invoke(InitialImageOperation::resetAllGIITestHooks);
+ }
- VersionTag versionTag = null;
+ private static class RegionObserver extends DistributionMessageObserver implements Serializable {
+ private static final long serialVersionUID = 6272522949825923089L;
+ VersionTag<?> versionTag;
CountDownLatch tombstoneGcLatch = new CountDownLatch(1);
@Override
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java b/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java
index f1f765e..6e1dd75 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java
@@ -848,20 +848,8 @@ public abstract class AbstractRegionMap extends BaseRegionMap
if (!oldIsDestroyedOrRemoved) {
owner.updateSizeOnRemove(key, oldSize);
}
- if (owner.getServerProxy() == null
- && owner.getVersionVector().isTombstoneTooOld(
- entryVersion.getMemberID(), entryVersion.getRegionVersion())) {
- // the received tombstone has already been reaped, so don't retain it
- if (owner.getIndexManager() != null) {
- owner.getIndexManager().updateIndexes(oldRe, IndexManager.REMOVE_ENTRY,
- IndexProtocol.REMOVE_DUE_TO_GII_TOMBSTONE_CLEANUP);
- }
- removeTombstone(oldRe, entryVersion, false, false);
- return false;
- } else {
- owner.scheduleTombstone(oldRe, entryVersion);
- lruEntryDestroy(oldRe);
- }
+ owner.scheduleTombstone(oldRe, entryVersion);
+ lruEntryDestroy(oldRe);
} else {
int newSize = owner.calculateRegionEntryValueSize(oldRe);
if (!oldIsTombstone) {