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/04/30 21:44:43 UTC
[geode] branch feature/GEODE-9138 updated: GEODE-9138: Moved debug
log message and added tests
This is an automated email from the ASF dual-hosted git repository.
boglesby pushed a commit to branch feature/GEODE-9138
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/feature/GEODE-9138 by this push:
new c962ce1 GEODE-9138: Moved debug log message and added tests
c962ce1 is described below
commit c962ce110fee508a0634d927f5556e7ff6561f7e
Author: Barry Oglesby <bo...@pivotal.io>
AuthorDate: Fri Apr 30 14:39:34 2021 -0700
GEODE-9138: Moved debug log message and added tests
---
.../DistributedEventTrackerIntegrationTest.java | 119 ++++++++++++++++-----
.../apache/geode/internal/cache/BucketRegion.java | 2 +-
.../cache/event/DistributedEventTracker.java | 43 +++++---
3 files changed, 121 insertions(+), 43 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 d20eb66..7138db3 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,6 +15,7 @@
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.internal.cache.event.DistributedEventTracker.EVENT_HAS_PREVIOUSLY_BEEN_SEEN_PREFIX;
import static org.assertj.core.api.Assertions.assertThat;
@@ -28,12 +29,13 @@ import java.util.Arrays;
import java.util.Collection;
import java.util.Optional;
+import junitparams.JUnitParamsRunner;
+import junitparams.Parameters;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.experimental.categories.Category;
import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
import org.apache.geode.cache.Operation;
import org.apache.geode.cache.partition.PartitionRegionHelper;
@@ -48,21 +50,11 @@ 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})
+@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();
@@ -75,20 +67,22 @@ public class DistributedEventTrackerIntegrationTest {
}
@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) server.createRegion(REPLICATE, 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) server.createRegion(PARTITION, testName.getMethodName());
// Assign buckets so that the BucketRegions and their EventTrackers are created
PartitionRegionHelper.assignBucketsToPartitions(region);
@@ -97,19 +91,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) server.createRegion(REPLICATE, 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) server.createRegion(PARTITION_REDUNDANT, 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) server.createRegion(PARTITION, 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 =
@@ -118,6 +162,22 @@ 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();
@@ -127,7 +187,7 @@ public class DistributedEventTrackerIntegrationTest {
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);
@@ -143,4 +203,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..e901eff 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,27 @@ 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());
+ }
+ logger.warn(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 +405,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 +425,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) {