You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by es...@apache.org on 2021/01/25 22:21:34 UTC
[geode] branch support/1.13 updated: GEODE-8833: Set possible
dupliate flag for persistent region (#5904)
This is an automated email from the ASF dual-hosted git repository.
eshu11 pushed a commit to branch support/1.13
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/support/1.13 by this push:
new eaf7f26 GEODE-8833: Set possible dupliate flag for persistent region (#5904)
eaf7f26 is described below
commit eaf7f2632e0d7e9e98141ccab5508f6304db1851
Author: Eric Shu <es...@pivotal.io>
AuthorDate: Fri Jan 15 14:41:05 2021 -0800
GEODE-8833: Set possible dupliate flag for persistent region (#5904)
For persistent region, even though a version tag is not recovered, the possibleDuplicate
flag is still needed to be set for the client retry message.
(cherry picked from commit 69ba7dd3f3053bd773c2737f32c3b90f6aba51b3)
---
.../internal/cache/tier/sockets/command/Put65.java | 28 +++++++-----
.../cache/tier/sockets/command/Put65Test.java | 51 ++++++++++++++++++++++
2 files changed, 69 insertions(+), 10 deletions(-)
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/Put65.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/Put65.java
index 89a6d37..dfeca87 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/Put65.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/Put65.java
@@ -229,16 +229,8 @@ public class Put65 extends BaseCommand {
// msg.isRetry might be set by v7.0 and later clients
if (clientMessage.isRetry()) {
- // if (logger.isDebugEnabled()) {
- // logger.debug("DEBUG: encountered isRetry in Put65");
- // }
- clientEvent.setPossibleDuplicate(true);
- if (region.getAttributes().getConcurrencyChecksEnabled()) {
- // recover the version tag from other servers
- clientEvent.setRegion(region);
- if (!recoverVersionTagForRetriedOperation(clientEvent)) {
- clientEvent.setPossibleDuplicate(false); // no-one has seen this event
- }
+ if (shouldSetPossibleDuplicate(region, clientEvent)) {
+ clientEvent.setPossibleDuplicate(true);
}
}
@@ -494,6 +486,22 @@ public class Put65 extends BaseCommand {
}
+ boolean shouldSetPossibleDuplicate(LocalRegion region, EventIDHolder clientEvent) {
+ boolean shouldSetPossibleDuplicate = true;
+ if (region.getAttributes().getConcurrencyChecksEnabled()) {
+ // recover the version tag from other servers
+ clientEvent.setRegion(region);
+ boolean withPersistence = region.getAttributes().getDataPolicy().withPersistence();
+ if (!recoverVersionTagForRetriedOperation(clientEvent) && !withPersistence) {
+ // For persistent region, it is possible that all persistent copies went offline.
+ // Do not reset possible duplicate in this case, as persistent data
+ // can be recovered during the retry after recover of version tag failed.
+ shouldSetPossibleDuplicate = false; // no-one has seen this event
+ }
+ }
+ return shouldSetPossibleDuplicate;
+ }
+
protected void writeReply(Message origMsg, ServerConnection servConn, boolean sendOldValue,
boolean oldValueIsObject, Object oldValue, VersionTag tag) throws IOException {
Message replyMsg = servConn.getReplyMessage();
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/Put65Test.java b/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/Put65Test.java
index f643ccf..5f25f27 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/Put65Test.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/Put65Test.java
@@ -18,6 +18,7 @@ import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Matchers.eq;
import static org.mockito.Matchers.isA;
+import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
@@ -29,10 +30,14 @@ import org.junit.experimental.categories.Category;
import org.mockito.ArgumentCaptor;
import org.mockito.InjectMocks;
import org.mockito.Mock;
+import org.mockito.Mockito;
import org.mockito.MockitoAnnotations;
import org.apache.geode.CancelCriterion;
+import org.apache.geode.cache.DataPolicy;
+import org.apache.geode.cache.RegionAttributes;
import org.apache.geode.cache.operations.PutOperationContext;
+import org.apache.geode.internal.cache.EventIDHolder;
import org.apache.geode.internal.cache.InternalCache;
import org.apache.geode.internal.cache.LocalRegion;
import org.apache.geode.internal.cache.TXManagerImpl;
@@ -97,6 +102,12 @@ public class Put65Test {
private Message errorResponseMessage;
@Mock
private Message replyMessage;
+ @Mock
+ private RegionAttributes attributes;
+ @Mock
+ private EventIDHolder clientEvent;
+ @Mock
+ private DataPolicy dataPolicy;
@InjectMocks
private Put65 put65;
@@ -157,6 +168,9 @@ public class Put65Test {
when(this.valuePart.getSerializedForm()).thenReturn(VALUE);
when(this.valuePart.isObject()).thenReturn(true);
+
+ when(localRegion.getAttributes()).thenReturn(attributes);
+ when(attributes.getDataPolicy()).thenReturn(dataPolicy);
}
@Test
@@ -255,4 +269,41 @@ public class Put65Test {
verify(this.errorResponseMessage).send(this.serverConnection);
}
+ @Test
+ public void shouldSetPossibleDuplicateReturnsTrueIfConcurrencyChecksNotEnabled() {
+
+ when(attributes.getConcurrencyChecksEnabled()).thenReturn(false);
+
+ assertThat(put65.shouldSetPossibleDuplicate(localRegion, clientEvent)).isTrue();
+ }
+
+ @Test
+ public void shouldSetPossibleDuplicateReturnsTrueIfRecoveredVersionTagForRetriedOperation() {
+ Put65 spy = Mockito.spy(put65);
+ when(attributes.getConcurrencyChecksEnabled()).thenReturn(true);
+ doReturn(true).when(spy).recoverVersionTagForRetriedOperation(clientEvent);
+
+ assertThat(spy.shouldSetPossibleDuplicate(localRegion, clientEvent)).isTrue();
+ }
+
+ @Test
+ public void shouldSetPossibleDuplicateReturnsFalseIfNotRecoveredVersionTagAndNoPersistence() {
+ Put65 spy = Mockito.spy(put65);
+ when(attributes.getConcurrencyChecksEnabled()).thenReturn(true);
+ when(dataPolicy.withPersistence()).thenReturn(false);
+ doReturn(false).when(spy).recoverVersionTagForRetriedOperation(clientEvent);
+
+ assertThat(spy.shouldSetPossibleDuplicate(localRegion, clientEvent)).isFalse();
+ }
+
+ @Test
+ public void shouldSetPossibleDuplicateReturnsTrueIfNotRecoveredVersionTagAndWithPersistence() {
+ Put65 spy = Mockito.spy(put65);
+ when(attributes.getConcurrencyChecksEnabled()).thenReturn(true);
+ when(dataPolicy.withPersistence()).thenReturn(true);
+ doReturn(false).when(spy).recoverVersionTagForRetriedOperation(clientEvent);
+
+ assertThat(spy.shouldSetPossibleDuplicate(localRegion, clientEvent)).isTrue();
+ }
+
}