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:35 UTC
[geode] branch feature/GEODE-9138-2 created (now 6fafd3a)
This is an automated email from the ASF dual-hosted git repository.
boglesby pushed a change to branch feature/GEODE-9138-2
in repository https://gitbox.apache.org/repos/asf/geode.git.
at 6fafd3a GEODE-9138: Refactored and added tests
This branch includes the following new commits:
new 6fafd3a GEODE-9138: Refactored and added tests
The 1 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.
[geode] 01/01: GEODE-9138: Refactored and added tests
Posted by bo...@apache.org.
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) {