You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by bo...@apache.org on 2021/05/03 21:08:36 UTC
[geode] 01/01: GEODE-9138: Refactored and added tests
This is an automated email from the ASF dual-hosted git repository.
boglesby pushed a commit to branch feature/GEODE-9138-2
in repository https://gitbox.apache.org/repos/asf/geode.git
commit 6fafd3a80005b64e54ddc3cc63bf608160d25012
Author: Barry Oglesby <bo...@pivotal.io>
AuthorDate: Mon May 3 14:04:35 2021 -0700
GEODE-9138: Refactored and added tests
---
.../DistributedEventTrackerIntegrationTest.java | 150 ++++++++++++++++-----
.../apache/geode/internal/cache/BucketRegion.java | 2 +-
.../cache/event/DistributedEventTracker.java | 40 +++---
3 files changed, 141 insertions(+), 51 deletions(-)
diff --git a/geode-core/src/integrationTest/java/org/apache/geode/internal/cache/event/DistributedEventTrackerIntegrationTest.java b/geode-core/src/integrationTest/java/org/apache/geode/internal/cache/event/DistributedEventTrackerIntegrationTest.java
index 4384e2a..7f9ca45 100644
--- a/geode-core/src/integrationTest/java/org/apache/geode/internal/cache/event/DistributedEventTrackerIntegrationTest.java
+++ b/geode-core/src/integrationTest/java/org/apache/geode/internal/cache/event/DistributedEventTrackerIntegrationTest.java
@@ -15,7 +15,11 @@
package org.apache.geode.internal.cache.event;
import static org.apache.geode.cache.RegionShortcut.PARTITION;
+import static org.apache.geode.cache.RegionShortcut.PARTITION_REDUNDANT;
import static org.apache.geode.cache.RegionShortcut.REPLICATE;
+import static org.apache.geode.distributed.ConfigurationProperties.LOCATORS;
+import static org.apache.geode.distributed.ConfigurationProperties.LOG_FILE;
+import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
import static org.apache.geode.internal.cache.event.DistributedEventTracker.EVENT_HAS_PREVIOUSLY_BEEN_SEEN_PREFIX;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;
@@ -27,18 +31,22 @@ import java.nio.file.Paths;
import java.util.Arrays;
import java.util.Collection;
import java.util.Optional;
+import java.util.Properties;
+import junitparams.JUnitParamsRunner;
+import junitparams.Parameters;
+import org.junit.After;
import org.junit.Before;
-import org.junit.Ignore;
import org.junit.Rule;
import org.junit.Test;
import org.junit.experimental.categories.Category;
+import org.junit.rules.TemporaryFolder;
import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.CacheFactory;
import org.apache.geode.cache.Operation;
import org.apache.geode.cache.partition.PartitionRegionHelper;
-import org.apache.geode.distributed.internal.InternalDistributedSystem;
import org.apache.geode.internal.cache.BucketRegion;
import org.apache.geode.internal.cache.DistributedRegion;
import org.apache.geode.internal.cache.EntryEventImpl;
@@ -47,50 +55,54 @@ import org.apache.geode.internal.cache.LocalRegion;
import org.apache.geode.internal.cache.PartitionedRegion;
import org.apache.geode.internal.cache.tier.sockets.ClientProxyMembershipID;
import org.apache.geode.test.junit.categories.RegionsTest;
-import org.apache.geode.test.junit.rules.ServerStarterRule;
import org.apache.geode.test.junit.rules.serializable.SerializableTestName;
-import org.apache.geode.test.junit.runners.CategoryWithParameterizedRunnerFactory;
-@RunWith(Parameterized.class)
-@Parameterized.UseParametersRunnerFactory(CategoryWithParameterizedRunnerFactory.class)
@Category({RegionsTest.class})
-@Ignore
+@RunWith(JUnitParamsRunner.class)
public class DistributedEventTrackerIntegrationTest {
- @Parameterized.Parameters(name = "possibleDuplicate={0}")
- public static Collection<Boolean> booleans() {
- return Arrays.asList(true, false);
- }
-
- @Parameterized.Parameter
- public boolean possibleDuplicate;
-
@Rule
public SerializableTestName testName = new SerializableTestName();
@Rule
- public ServerStarterRule server = new ServerStarterRule().withNoCacheServer().withLogFile();
+ public TemporaryFolder temporaryFolder = new TemporaryFolder();
+
+ private Cache cache;
+
+ private File logFile;
@Before
public void setUp() throws Exception {
- server.startServer();
+ logFile = temporaryFolder.newFile(testName.getMethodName() + ".log");
+ Properties properties = new Properties();
+ properties.setProperty(LOCATORS, "");
+ properties.setProperty(MCAST_PORT, "0");
+ properties.setProperty(LOG_FILE, logFile.getAbsolutePath());
+ cache = new CacheFactory(properties).create();
+ }
+
+ @After
+ public void tearDown() throws Exception {
+ cache.close();
}
@Test
- public void testHasSeenEventReplicatedRegion() throws IOException {
+ @Parameters(method = "getPossibleDuplicates")
+ public void testHasSeenEventReplicatedRegion(boolean possibleDuplicate) throws IOException {
// Create the replicated region
- DistributedRegion region = (DistributedRegion) server.createRegion(REPLICATE,
- testName.getMethodName().substring(0, testName.getMethodName().indexOf('[')));
+ DistributedRegion region =
+ (DistributedRegion) cache.createRegionFactory(REPLICATE).create(testName.getMethodName());
// Invoke hasSeenEvent and verify results
- invokeHasSeenEventAndVerifyResults(region);
+ invokeHasSeenEventAndVerifyResults(region, possibleDuplicate, !possibleDuplicate);
}
@Test
- public void testHasSeenEventPartitionedRegion() throws IOException {
+ @Parameters(method = "getPossibleDuplicates")
+ public void testHasSeenEventPartitionedRegion(boolean possibleDuplicate) throws IOException {
// Create the partitioned region
- PartitionedRegion region = (PartitionedRegion) server.createRegion(PARTITION,
- testName.getMethodName().substring(0, testName.getMethodName().indexOf('[')));
+ PartitionedRegion region =
+ (PartitionedRegion) cache.createRegionFactory(PARTITION).create(testName.getMethodName());
// Assign buckets so that the BucketRegions and their EventTrackers are created
PartitionRegionHelper.assignBucketsToPartitions(region);
@@ -99,19 +111,69 @@ public class DistributedEventTrackerIntegrationTest {
BucketRegion br = region.getBucketRegion(0);
// Invoke hasSeenEvent and verify results
- invokeHasSeenEventAndVerifyResults(br);
+ invokeHasSeenEventAndVerifyResults(br, possibleDuplicate, !possibleDuplicate);
}
- private void invokeHasSeenEventAndVerifyResults(DistributedRegion region)
- throws IOException {
+ @Test
+ public void testHasSeenEventNullEvent() throws IOException {
+ // Create the region
+ DistributedRegion region =
+ (DistributedRegion) cache.createRegionFactory(REPLICATE).create(testName.getMethodName());
+
+ // Record an event with a high sequence number
byte[] memberId = new byte[0];
long threadId = 1L;
+ recordHighSequenceNumberEvent(region, memberId, threadId);
- // Create an event with a high sequence number
- EntryEventImpl event1 = createEvent(region, new EventID(memberId, threadId, 1000L), false);
+ // Invoke hasSeenEvent with event id (null event)
+ boolean hasSeenEvent =
+ region.getEventTracker().hasSeenEvent(new EventID(memberId, threadId, 0L));
- // Record the event
- region.getEventTracker().recordEvent(event1);
+ // Verify results
+ verifyResults(region, hasSeenEvent, true);
+ }
+
+ @Test
+ public void testHasSeenEventPartitionedRegionLowRedundancy() throws IOException {
+ // Create the partitioned region
+ PartitionedRegion region = (PartitionedRegion) cache.createRegionFactory(PARTITION_REDUNDANT)
+ .create(testName.getMethodName());
+
+ // Assign buckets so that the BucketRegions and their EventTrackers are created
+ PartitionRegionHelper.assignBucketsToPartitions(region);
+
+ // Get a BucketRegion
+ BucketRegion br = region.getBucketRegion(0);
+
+ // Invoke hasSeenEvent and verify results
+ invokeHasSeenEventAndVerifyResults(br, false, false);
+ }
+
+ @Test
+ public void testHasSeenEventPartitionedRegionInRecovery() throws IOException {
+ // Create the partitioned region
+ PartitionedRegion region =
+ (PartitionedRegion) cache.createRegionFactory(PARTITION).create(testName.getMethodName());
+
+ // Assign buckets so that the BucketRegions and their EventTrackers are created
+ PartitionRegionHelper.assignBucketsToPartitions(region);
+
+ // Get a BucketRegion
+ BucketRegion br = region.getBucketRegion(0);
+
+ // Start recovery in progress
+ region.getPrStats().startRecovery();
+
+ // Invoke hasSeenEvent and verify results
+ invokeHasSeenEventAndVerifyResults(br, false, false);
+ }
+
+ private void invokeHasSeenEventAndVerifyResults(DistributedRegion region,
+ boolean possibleDuplicate, boolean logMessageIsPresent) throws IOException {
+ // Record an event with a high sequence number
+ byte[] memberId = new byte[0];
+ long threadId = 1L;
+ recordHighSequenceNumberEvent(region, memberId, threadId);
// Create an event with a lower sequence number and possibleDuplicate set appropriately
EntryEventImpl event2 =
@@ -120,16 +182,29 @@ public class DistributedEventTrackerIntegrationTest {
// Invoke hasSeenEvent
boolean hasSeenEvent = region.getEventTracker().hasSeenEvent(event2);
+ // Verify results
+ verifyResults(region, hasSeenEvent, logMessageIsPresent);
+ }
+
+ private void recordHighSequenceNumberEvent(DistributedRegion region, byte[] memberId,
+ long threadId) {
+ // Create event with high sequence number
+ EntryEventImpl event1 = createEvent(region, new EventID(memberId, threadId, 1000L), false);
+
+ // Record the event
+ region.getEventTracker().recordEvent(event1);
+ }
+
+ private void verifyResults(DistributedRegion region, boolean hasSeenEvent,
+ boolean logMessageIsPresent) throws IOException {
// Assert hasSeenEvent is true
assertThat(hasSeenEvent).isTrue();
// Verify the log does or does not contain the message depending on possibleDuplicate
- File logFile = ((InternalDistributedSystem) server.getCache().getDistributedSystem())
- .getConfig().getLogFile();
Optional<String> logLine = Files.lines(Paths.get(logFile.getAbsolutePath()))
.filter(line -> line.contains(EVENT_HAS_PREVIOUSLY_BEEN_SEEN_PREFIX))
.findFirst();
- assertThat(logLine.isPresent()).isEqualTo(!possibleDuplicate);
+ assertThat(logLine.isPresent()).isEqualTo(logMessageIsPresent);
// Verify the statistic is incremented
assertThat(region.getCachePerfStats().getPreviouslySeenEvents()).isEqualTo(1);
@@ -145,4 +220,9 @@ public class DistributedEventTrackerIntegrationTest {
event.setContext(mock(ClientProxyMembershipID.class));
return event;
}
+
+ @SuppressWarnings("unused")
+ private Collection<Boolean> getPossibleDuplicates() {
+ return Arrays.asList(true, false);
+ }
}
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/BucketRegion.java b/geode-core/src/main/java/org/apache/geode/internal/cache/BucketRegion.java
index 64bb2a8..d5b63e2 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/BucketRegion.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/BucketRegion.java
@@ -1401,7 +1401,7 @@ public class BucketRegion extends DistributedRegion implements Bucket {
}
public boolean hasLowRedundancy() {
- return redundancy < getBucketAdvisor().getBucketRedundancy();
+ return redundancy > getBucketAdvisor().getBucketRedundancy();
}
@Override
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/event/DistributedEventTracker.java b/geode-core/src/main/java/org/apache/geode/internal/cache/event/DistributedEventTracker.java
index ce24656..56c9f6a 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/event/DistributedEventTracker.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/event/DistributedEventTracker.java
@@ -19,6 +19,7 @@ import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
+import org.apache.commons.lang3.tuple.Pair;
import org.apache.logging.log4j.Logger;
import org.apache.geode.annotations.VisibleForTesting;
@@ -44,13 +45,19 @@ public class DistributedEventTracker implements EventTracker {
@VisibleForTesting
protected static final String EVENT_HAS_PREVIOUSLY_BEEN_SEEN_PREFIX =
- "Event has previously been seen ";
+ "Event has previously been seen";
+
+ protected static final String IGNORING_PREVIOUSLY_SEEN_EVENT_PREFIX =
+ "Ignoring previously seen event due to {}";
private static final String EVENT_HAS_PREVIOUSLY_BEEN_SEEN_PARAMETERS =
- "for region={}; operation={}; key={}; eventId={}; highestSequenceNumberSeen={}";
+ " for region={}; operation={}; key={}; eventId={}; highestSequenceNumberSeen={}";
private static final String EVENT_HAS_PREVIOUSLY_BEEN_SEEN =
EVENT_HAS_PREVIOUSLY_BEEN_SEEN_PREFIX + EVENT_HAS_PREVIOUSLY_BEEN_SEEN_PARAMETERS;
+
+ private static final String IGNORING_PREVIOUSLY_SEEN_EVENT =
+ IGNORING_PREVIOUSLY_SEEN_EVENT_PREFIX + EVENT_HAS_PREVIOUSLY_BEEN_SEEN_PARAMETERS;
/**
* a mapping of originator to the last event applied to this cache
*
@@ -363,11 +370,24 @@ public class DistributedEventTracker implements EventTracker {
if (evh.isRemoved() || evh.getLastSequenceNumber() < eventID.getSequenceID()) {
return false;
}
- if (shouldLogPreviouslySeenEvent(tagHolder, evh)) {
+ Pair<Boolean, String> shouldLogPreviouslySeenEvent =
+ shouldLogPreviouslySeenEvent(tagHolder, evh);
+ if (shouldLogPreviouslySeenEvent.getLeft()) {
logger.info(EVENT_HAS_PREVIOUSLY_BEEN_SEEN, region.getName(),
tagHolder == null ? "unknown" : ((EntryEventImpl) tagHolder).getKey(),
tagHolder == null ? "unknown" : tagHolder.getOperation(), eventID.expensiveToString(),
evh.getLastSequenceNumber());
+ } else {
+ if (logger.isDebugEnabled()) {
+ if (tagHolder == null) {
+ logger.debug(IGNORING_PREVIOUSLY_SEEN_EVENT_PREFIX,
+ shouldLogPreviouslySeenEvent.getRight());
+ } else {
+ logger.debug(IGNORING_PREVIOUSLY_SEEN_EVENT, shouldLogPreviouslySeenEvent.getRight(),
+ region.getName(), ((EntryEventImpl) tagHolder).getKey(), tagHolder.getOperation(),
+ tagHolder.getEventId().expensiveToString(), evh.getLastSequenceNumber());
+ }
+ }
}
// bug #44956 - recover version tag for duplicate event
if (evh.getLastSequenceNumber() == eventID.getSequenceID() && tagHolder != null
@@ -382,7 +402,7 @@ public class DistributedEventTracker implements EventTracker {
}
}
- private boolean shouldLogPreviouslySeenEvent(InternalCacheEvent event,
+ private Pair<Boolean, String> shouldLogPreviouslySeenEvent(InternalCacheEvent event,
EventSequenceNumberHolder evh) {
boolean shouldLogSeenEvent = true;
String message = null;
@@ -402,17 +422,7 @@ public class DistributedEventTracker implements EventTracker {
shouldLogSeenEvent = false;
}
}
- if (!shouldLogSeenEvent && logger.isDebugEnabled()) {
- if (event == null) {
- logger.debug("Ignoring previously seen event due to {}", message);
- } else {
- logger.debug(
- "Ignoring previously seen event due to {} for region={}; operation={}; key={}; eventId={}; highestSequenceNumberSeen={}",
- message, region.getName(), ((EntryEventImpl) event).getKey(), event.getOperation(),
- event.getEventId().expensiveToString(), evh.getLastSequenceNumber());
- }
- }
- return shouldLogSeenEvent;
+ return Pair.of(shouldLogSeenEvent, message);
}
private EventSequenceNumberHolder getSequenceHolderForEvent(EventID eventID) {