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 2018/12/12 21:25:52 UTC
[geode] branch feature/GEODE-6195 updated: GEODE-6195: Check if
returned value is caused by a retried putIfAbsent operation
This is an automated email from the ASF dual-hosted git repository.
eshu11 pushed a commit to branch feature/GEODE-6195
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/feature/GEODE-6195 by this push:
new 3bd928b GEODE-6195: Check if returned value is caused by a retried putIfAbsent operation
3bd928b is described below
commit 3bd928b66d6e83e622cd33c89afdb8a4244d68f7
Author: eshu <es...@pivotal.io>
AuthorDate: Wed Dec 12 13:18:39 2018 -0800
GEODE-6195: Check if returned value is caused by a retried putIfAbsent operation
* Treats a retried putIfAbsent operation a successful one if returned value is the value to be put
* Move the check value methods to ValueComparisonHelper class.
---
.../apache/geode/internal/cache/LocalRegion.java | 66 +++--
.../internal/cache/ValueComparisonHelper.java | 272 +++++++++++++++++++++
.../cache/entries/AbstractRegionEntry.java | 250 +------------------
.../geode/internal/cache/LocalRegionTest.java | 122 ++++++++-
4 files changed, 437 insertions(+), 273 deletions(-)
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java b/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
index 488cd58..273a9be 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
@@ -3060,7 +3060,7 @@ public class LocalRegion extends AbstractRegion implements LoaderHelperFactory,
void checkPutIfAbsentResult(EntryEventImpl event, Object value, Object result) {
if (result != null) {
// we may see a non null result possibly due to retry
- if (event.hasRetried() && putIfAbsentResultHasSameValue(value, result)) {
+ if (event.hasRetried() && putIfAbsentResultHasSameValue(true, value, result)) {
if (logger.isDebugEnabled()) {
logger.debug("retried putIfAbsent and result is the value to be put,"
+ " treat as a successful putIfAbsent");
@@ -3072,11 +3072,29 @@ public class LocalRegion extends AbstractRegion implements LoaderHelperFactory,
}
}
- boolean putIfAbsentResultHasSameValue(Object value, Object result) {
- if (Token.isInvalid(result)) {
- return value == null;
+ boolean putIfAbsentResultHasSameValue(boolean isClient, Object valueToBePut, Object result) {
+ if (Token.isInvalid(result) || result == null) {
+ return valueToBePut == null;
+ }
+
+ boolean isCompressedOffHeap =
+ isClient ? false : getAttributes().getOffHeap() && getAttributes().getCompressor() != null;
+ return ValueComparisonHelper.checkEquals(valueToBePut, result, isCompressedOffHeap, getCache());
+ }
+
+ boolean bridgePutIfAbsentResultHasSameValue(byte[] valueToBePut, boolean isValueToBePutObject,
+ Object result) {
+ if (Token.isInvalid(result) || result == null) {
+ return valueToBePut == null;
+ }
+
+ boolean isCompressedOffHeap =
+ getAttributes().getOffHeap() && getAttributes().getCompressor() != null;
+ if (isValueToBePutObject) {
+ return ValueComparisonHelper.checkEquals(EntryEventImpl.deserialize(valueToBePut), result,
+ isCompressedOffHeap, getCache());
}
- return result.equals(value);
+ return ValueComparisonHelper.checkEquals(valueToBePut, result, isCompressedOffHeap, getCache());
}
/**
@@ -5682,7 +5700,6 @@ public class LocalRegion extends AbstractRegion implements LoaderHelperFactory,
try {
oldEntry = this.entries.basicPut(event, lastModified, ifNew, ifOld, expectedOldValue,
requireOldValue, overwriteDestroyed);
-
} catch (ConcurrentCacheModificationException ignore) {
// this can happen in a client cache when another thread
// managed to slip in its version info to the region entry before this
@@ -5706,7 +5723,6 @@ public class LocalRegion extends AbstractRegion implements LoaderHelperFactory,
return true;
}
}
-
return oldEntry != null;
}
@@ -11616,7 +11632,15 @@ public class LocalRegion extends AbstractRegion implements LoaderHelperFactory,
final boolean ifOld = false;
final boolean requireOldValue = true;
if (!basicPut(event, ifNew, ifOld, oldValue, requireOldValue)) {
- return event.getOldValue();
+ Object result = event.getOldValue();
+ if (event.isPossibleDuplicate() && putIfAbsentResultHasSameValue(false, value, result)) {
+ if (logger.isDebugEnabled()) {
+ logger.debug("possible duplicate putIfAbsent event and result is the value to be put,"
+ + " treat this as a successful putIfAbsent");
+ }
+ return null;
+ }
+ return result;
} else {
if (!getDataView().isDeferredStats()) {
getCachePerfStats().endPut(startPut, false);
@@ -11852,14 +11876,26 @@ public class LocalRegion extends AbstractRegion implements LoaderHelperFactory,
if (basicPut) {
clientEvent.setVersionTag(event.getVersionTag());
clientEvent.isConcurrencyConflict(event.isConcurrencyConflict());
- } else if (oldValue == null) {
- // fix for 42189, putIfAbsent on server can return null if the
- // operation was not performed (oldValue in cache was null).
- // We return the INVALID token instead of null to distinguish
- // this case from successful operation
- return Token.INVALID;
- }
+ } else {
+ assert (value instanceof byte[]);
+ if (event.isPossibleDuplicate()
+ && bridgePutIfAbsentResultHasSameValue((byte[]) value, isObject, oldValue)) {
+ // result is possibly due to the retry
+ if (logger.isDebugEnabled()) {
+ logger.debug("retried putIfAbsent and got oldValue as the value to be put,"
+ + " treat this as a successful putIfAbsent");
+ }
+ return null;
+ }
+ if (oldValue == null) {
+ // fix for 42189, putIfAbsent on server can return null if the
+ // operation was not performed (oldValue in cache was null).
+ // We return the INVALID token instead of null to distinguish
+ // this case from successful operation
+ return Token.INVALID;
+ }
+ }
return oldValue;
} finally {
event.release();
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/ValueComparisonHelper.java b/geode-core/src/main/java/org/apache/geode/internal/cache/ValueComparisonHelper.java
new file mode 100644
index 0000000..4f9c63e
--- /dev/null
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/ValueComparisonHelper.java
@@ -0,0 +1,272 @@
+/*
+ * 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.internal.cache;
+
+import java.io.IOException;
+import java.util.Arrays;
+
+import org.apache.geode.internal.HeapDataOutputStream;
+import org.apache.geode.internal.InternalDataSerializer;
+import org.apache.geode.internal.Version;
+import org.apache.geode.internal.offheap.StoredObject;
+import org.apache.geode.internal.offheap.annotations.Unretained;
+import org.apache.geode.pdx.PdxInstance;
+import org.apache.geode.pdx.PdxSerializable;
+import org.apache.geode.pdx.PdxSerializationException;
+import org.apache.geode.pdx.PdxSerializer;
+
+public class ValueComparisonHelper {
+
+ public static boolean checkEquals(@Unretained Object v1, @Unretained Object v2,
+ boolean isCompressedOffHeap, InternalCache cache) {
+ // need to give PdxInstance#equals priority
+ if (v1 instanceof PdxInstance) {
+ return checkPdxEquals((PdxInstance) v1, v2, cache);
+ } else if (v2 instanceof PdxInstance) {
+ return checkPdxEquals((PdxInstance) v2, v1, cache);
+ } else if (v1 instanceof StoredObject) {
+ return checkOffHeapEquals((StoredObject) v1, v2, cache);
+ } else if (v2 instanceof StoredObject) {
+ return checkOffHeapEquals((StoredObject) v2, v1, cache);
+ } else if (v1 instanceof CachedDeserializable) {
+ return checkCDEquals((CachedDeserializable) v1, v2, isCompressedOffHeap, cache);
+ } else if (v2 instanceof CachedDeserializable) {
+ return checkCDEquals((CachedDeserializable) v2, v1, isCompressedOffHeap, cache);
+ } else {
+ return basicEquals(v1, v2);
+ }
+ }
+
+ private static boolean basicEquals(Object v1, Object v2) {
+ if (v2 != null) {
+ if (v2.getClass().isArray()) {
+ // fix for 52093
+ if (v2 instanceof byte[]) {
+ if (v1 instanceof byte[]) {
+ return Arrays.equals((byte[]) v2, (byte[]) v1);
+ } else {
+ return false;
+ }
+ } else if (v2 instanceof Object[]) {
+ if (v1 instanceof Object[]) {
+ return Arrays.deepEquals((Object[]) v2, (Object[]) v1);
+ } else {
+ return false;
+ }
+ } else if (v2 instanceof int[]) {
+ if (v1 instanceof int[]) {
+ return Arrays.equals((int[]) v2, (int[]) v1);
+ } else {
+ return false;
+ }
+ } else if (v2 instanceof long[]) {
+ if (v1 instanceof long[]) {
+ return Arrays.equals((long[]) v2, (long[]) v1);
+ } else {
+ return false;
+ }
+ } else if (v2 instanceof boolean[]) {
+ if (v1 instanceof boolean[]) {
+ return Arrays.equals((boolean[]) v2, (boolean[]) v1);
+ } else {
+ return false;
+ }
+ } else if (v2 instanceof short[]) {
+ if (v1 instanceof short[]) {
+ return Arrays.equals((short[]) v2, (short[]) v1);
+ } else {
+ return false;
+ }
+ } else if (v2 instanceof char[]) {
+ if (v1 instanceof char[]) {
+ return Arrays.equals((char[]) v2, (char[]) v1);
+ } else {
+ return false;
+ }
+ } else if (v2 instanceof float[]) {
+ if (v1 instanceof float[]) {
+ return Arrays.equals((float[]) v2, (float[]) v1);
+ } else {
+ return false;
+ }
+ } else if (v2 instanceof double[]) {
+ if (v1 instanceof double[]) {
+ return Arrays.equals((double[]) v2, (double[]) v1);
+ } else {
+ return false;
+ }
+ }
+ // fall through and call equals method
+ }
+ return v2.equals(v1);
+ } else {
+ return v1 == null;
+ }
+ }
+
+ /**
+ * This method fixes bug 43643
+ */
+ private static boolean checkPdxEquals(PdxInstance pdx, Object obj, InternalCache cache) {
+ if (!(obj instanceof PdxInstance)) {
+ // obj may be a CachedDeserializable in which case we want to convert it to a PdxInstance even
+ // if we are not readSerialized.
+ if (obj instanceof CachedDeserializable) {
+ CachedDeserializable cdObj = (CachedDeserializable) obj;
+ if (!cdObj.isSerialized()) {
+ // obj is actually a byte[] which will never be equal to a PdxInstance
+ return false;
+ }
+ Object cdVal = cdObj.getValue();
+ if (cdVal instanceof byte[]) {
+ byte[] cdValBytes = (byte[]) cdVal;
+ PdxInstance pi = InternalDataSerializer.readPdxInstance(cdValBytes, cache);
+ if (pi != null) {
+ return pi.equals(pdx);
+ } else {
+ // since obj is serialized as something other than pdx it must not equal our pdx
+ return false;
+ }
+ } else {
+ // remove the cd wrapper so that obj is the actual value we want to compare.
+ obj = cdVal;
+ }
+ }
+ if (obj != null && obj.getClass().getName().equals(pdx.getClassName())) {
+ PdxSerializer pdxSerializer;
+ if (obj instanceof PdxSerializable) {
+ pdxSerializer = null;
+ } else {
+ pdxSerializer = cache.getPdxSerializer();
+ }
+ if (pdxSerializer != null || obj instanceof PdxSerializable) {
+ // try to convert obj to a PdxInstance
+ HeapDataOutputStream hdos = new HeapDataOutputStream(Version.CURRENT);
+ try {
+ if (InternalDataSerializer.autoSerialized(obj, hdos)
+ || InternalDataSerializer.writePdx(hdos, cache, obj, pdxSerializer)) {
+ PdxInstance pi = InternalDataSerializer.readPdxInstance(hdos.toByteArray(), cache);
+ if (pi != null) {
+ obj = pi;
+ }
+ }
+ } catch (IOException | PdxSerializationException ignore) {
+ // we are not able to convert it so just fall through
+ }
+ }
+ }
+ }
+ return basicEquals(obj, pdx);
+ }
+
+ private static boolean checkOffHeapEquals(@Unretained StoredObject ohVal, @Unretained Object obj,
+ InternalCache cache) {
+ if (ohVal.isSerializedPdxInstance()) {
+ PdxInstance pi = InternalDataSerializer.readPdxInstance(ohVal.getSerializedValue(), cache);
+ return ValueComparisonHelper.checkPdxEquals(pi, obj, cache);
+ }
+ if (obj instanceof StoredObject) {
+ return ohVal.checkDataEquals((StoredObject) obj);
+ } else {
+ byte[] serializedObj;
+ if (obj instanceof CachedDeserializable) {
+ CachedDeserializable cdObj = (CachedDeserializable) obj;
+ if (!ohVal.isSerialized()) {
+ assert cdObj.isSerialized();
+ return false;
+ }
+ serializedObj = cdObj.getSerializedValue();
+ } else if (obj instanceof byte[]) {
+ if (ohVal.isSerialized()) {
+ return false;
+ }
+ serializedObj = (byte[]) obj;
+ } else {
+ if (!ohVal.isSerialized()) {
+ return false;
+ }
+ if (obj == null || obj == Token.NOT_AVAILABLE || Token.isInvalidOrRemoved(obj)) {
+ return false;
+ }
+ serializedObj = EntryEventImpl.serialize(obj);
+ }
+ return ohVal.checkDataEquals(serializedObj);
+ }
+ }
+
+ private static boolean checkCDEquals(CachedDeserializable cd, Object obj,
+ boolean isCompressedOffHeap, InternalCache cache) {
+ if (!cd.isSerialized()) {
+ // cd is an actual byte[].
+ byte[] ba2;
+ if (obj instanceof CachedDeserializable) {
+ CachedDeserializable cdObj = (CachedDeserializable) obj;
+ if (!cdObj.isSerialized()) {
+ return false;
+ }
+ ba2 = (byte[]) cdObj.getDeserializedForReading();
+ } else if (obj instanceof byte[]) {
+ ba2 = (byte[]) obj;
+ } else {
+ return false;
+ }
+ byte[] ba1 = (byte[]) cd.getDeserializedForReading();
+ return Arrays.equals(ba1, ba2);
+ }
+ Object cdVal = cd.getValue();
+ if (cdVal instanceof byte[]) {
+ byte[] cdValBytes = (byte[]) cdVal;
+ PdxInstance pi = InternalDataSerializer.readPdxInstance(cdValBytes, cache);
+ if (pi != null) {
+ return ValueComparisonHelper.checkPdxEquals(pi, obj, cache);
+ }
+ if (isCompressedOffHeap) {
+ // fix for bug 52248
+ byte[] serializedObj;
+ if (obj instanceof CachedDeserializable) {
+ serializedObj = ((CachedDeserializable) obj).getSerializedValue();
+ } else {
+ serializedObj = EntryEventImpl.serialize(obj);
+ }
+ return Arrays.equals(cdValBytes, serializedObj);
+ } else {
+ /*
+ * To be more compatible with previous releases do not compare the serialized forms here.
+ * Instead deserialize and call the equals method.
+ */
+ Object deserializedObj;
+ if (obj instanceof CachedDeserializable) {
+ deserializedObj = ((CachedDeserializable) obj).getDeserializedForReading();
+ } else {
+ if (obj == null || obj == Token.NOT_AVAILABLE || Token.isInvalidOrRemoved(obj)) {
+ return false;
+ }
+ // TODO OPTIMIZE: Before serializing all of obj we could get the top
+ // level class name of cdVal and compare it to the top level class name of obj.
+ deserializedObj = obj;
+ }
+ return basicEquals(deserializedObj, cd.getDeserializedForReading());
+ }
+ } else {
+ // prefer object form
+ if (obj instanceof CachedDeserializable) {
+ // TODO OPTIMIZE: Before deserializing all of obj we could get the top
+ // class name of cdVal and the top level class name of obj and compare.
+ obj = ((CachedDeserializable) obj).getDeserializedForReading();
+ }
+ return basicEquals(cdVal, obj);
+ }
+ }
+}
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/entries/AbstractRegionEntry.java b/geode-core/src/main/java/org/apache/geode/internal/cache/entries/AbstractRegionEntry.java
index 2134216..191d9d9 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/entries/AbstractRegionEntry.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/entries/AbstractRegionEntry.java
@@ -18,7 +18,6 @@ import static org.apache.geode.internal.offheap.annotations.OffHeapIdentifier.AB
import static org.apache.geode.internal.offheap.annotations.OffHeapIdentifier.ABSTRACT_REGION_ENTRY_PREPARE_VALUE_FOR_CACHE;
import java.io.IOException;
-import java.util.Arrays;
import org.apache.logging.log4j.Logger;
@@ -42,7 +41,6 @@ import org.apache.geode.distributed.internal.membership.InternalDistributedMembe
import org.apache.geode.internal.Assert;
import org.apache.geode.internal.ByteArrayDataInput;
import org.apache.geode.internal.HeapDataOutputStream;
-import org.apache.geode.internal.InternalDataSerializer;
import org.apache.geode.internal.InternalStatisticsDisabledException;
import org.apache.geode.internal.Version;
import org.apache.geode.internal.cache.CachedDeserializable;
@@ -52,7 +50,6 @@ import org.apache.geode.internal.cache.EntryEventImpl;
import org.apache.geode.internal.cache.FilterProfile;
import org.apache.geode.internal.cache.ImageState;
import org.apache.geode.internal.cache.InitialImageOperation.Entry;
-import org.apache.geode.internal.cache.InternalCache;
import org.apache.geode.internal.cache.InternalCacheEvent;
import org.apache.geode.internal.cache.InternalRegion;
import org.apache.geode.internal.cache.RegionClearedException;
@@ -62,6 +59,7 @@ import org.apache.geode.internal.cache.TXManagerImpl;
import org.apache.geode.internal.cache.TimestampedEntryEventImpl;
import org.apache.geode.internal.cache.Token;
import org.apache.geode.internal.cache.TombstoneService;
+import org.apache.geode.internal.cache.ValueComparisonHelper;
import org.apache.geode.internal.cache.eviction.EvictionList;
import org.apache.geode.internal.cache.persistence.DiskRecoveryStore;
import org.apache.geode.internal.cache.persistence.DiskStoreID;
@@ -88,9 +86,7 @@ import org.apache.geode.internal.util.Versionable;
import org.apache.geode.internal.util.concurrent.CustomEntryConcurrentHashMap;
import org.apache.geode.internal.util.concurrent.CustomEntryConcurrentHashMap.HashEntry;
import org.apache.geode.pdx.PdxInstance;
-import org.apache.geode.pdx.PdxSerializable;
import org.apache.geode.pdx.PdxSerializationException;
-import org.apache.geode.pdx.PdxSerializer;
import org.apache.geode.pdx.internal.ConvertableToBytes;
import org.apache.geode.pdx.internal.PdxInstanceImpl;
@@ -943,251 +939,11 @@ public abstract class AbstractRegionEntry implements HashRegionEntry<Object, Obj
} else {
boolean isCompressedOffHeap =
region.getAttributes().getOffHeap() && region.getAttributes().getCompressor() != null;
- return checkEquals(expectedOldValue, actualValue, isCompressedOffHeap, region.getCache());
+ return ValueComparisonHelper
+ .checkEquals(expectedOldValue, actualValue, isCompressedOffHeap, region.getCache());
}
}
- private static boolean basicEquals(Object v1, Object v2) {
- if (v2 != null) {
- if (v2.getClass().isArray()) {
- // fix for 52093
- if (v2 instanceof byte[]) {
- if (v1 instanceof byte[]) {
- return Arrays.equals((byte[]) v2, (byte[]) v1);
- } else {
- return false;
- }
- } else if (v2 instanceof Object[]) {
- if (v1 instanceof Object[]) {
- return Arrays.deepEquals((Object[]) v2, (Object[]) v1);
- } else {
- return false;
- }
- } else if (v2 instanceof int[]) {
- if (v1 instanceof int[]) {
- return Arrays.equals((int[]) v2, (int[]) v1);
- } else {
- return false;
- }
- } else if (v2 instanceof long[]) {
- if (v1 instanceof long[]) {
- return Arrays.equals((long[]) v2, (long[]) v1);
- } else {
- return false;
- }
- } else if (v2 instanceof boolean[]) {
- if (v1 instanceof boolean[]) {
- return Arrays.equals((boolean[]) v2, (boolean[]) v1);
- } else {
- return false;
- }
- } else if (v2 instanceof short[]) {
- if (v1 instanceof short[]) {
- return Arrays.equals((short[]) v2, (short[]) v1);
- } else {
- return false;
- }
- } else if (v2 instanceof char[]) {
- if (v1 instanceof char[]) {
- return Arrays.equals((char[]) v2, (char[]) v1);
- } else {
- return false;
- }
- } else if (v2 instanceof float[]) {
- if (v1 instanceof float[]) {
- return Arrays.equals((float[]) v2, (float[]) v1);
- } else {
- return false;
- }
- } else if (v2 instanceof double[]) {
- if (v1 instanceof double[]) {
- return Arrays.equals((double[]) v2, (double[]) v1);
- } else {
- return false;
- }
- }
- // fall through and call equals method
- }
- return v2.equals(v1);
- } else {
- return v1 == null;
- }
- }
-
- private static boolean checkEquals(@Unretained Object v1, @Unretained Object v2,
- boolean isCompressedOffHeap, InternalCache cache) {
- // need to give PdxInstance#equals priority
- if (v1 instanceof PdxInstance) {
- return checkPdxEquals((PdxInstance) v1, v2, cache);
- } else if (v2 instanceof PdxInstance) {
- return checkPdxEquals((PdxInstance) v2, v1, cache);
- } else if (v1 instanceof StoredObject) {
- return checkOffHeapEquals((StoredObject) v1, v2, cache);
- } else if (v2 instanceof StoredObject) {
- return checkOffHeapEquals((StoredObject) v2, v1, cache);
- } else if (v1 instanceof CachedDeserializable) {
- return checkCDEquals((CachedDeserializable) v1, v2, isCompressedOffHeap, cache);
- } else if (v2 instanceof CachedDeserializable) {
- return checkCDEquals((CachedDeserializable) v2, v1, isCompressedOffHeap, cache);
- } else {
- return basicEquals(v1, v2);
- }
- }
-
- private static boolean checkOffHeapEquals(@Unretained StoredObject ohVal, @Unretained Object obj,
- InternalCache cache) {
- if (ohVal.isSerializedPdxInstance()) {
- PdxInstance pi = InternalDataSerializer.readPdxInstance(ohVal.getSerializedValue(), cache);
- return checkPdxEquals(pi, obj, cache);
- }
- if (obj instanceof StoredObject) {
- return ohVal.checkDataEquals((StoredObject) obj);
- } else {
- byte[] serializedObj;
- if (obj instanceof CachedDeserializable) {
- CachedDeserializable cdObj = (CachedDeserializable) obj;
- if (!ohVal.isSerialized()) {
- assert cdObj.isSerialized();
- return false;
- }
- serializedObj = cdObj.getSerializedValue();
- } else if (obj instanceof byte[]) {
- if (ohVal.isSerialized()) {
- return false;
- }
- serializedObj = (byte[]) obj;
- } else {
- if (!ohVal.isSerialized()) {
- return false;
- }
- if (obj == null || obj == Token.NOT_AVAILABLE || Token.isInvalidOrRemoved(obj)) {
- return false;
- }
- serializedObj = EntryEventImpl.serialize(obj);
- }
- return ohVal.checkDataEquals(serializedObj);
- }
- }
-
- private static boolean checkCDEquals(CachedDeserializable cd, Object obj,
- boolean isCompressedOffHeap, InternalCache cache) {
- if (!cd.isSerialized()) {
- // cd is an actual byte[].
- byte[] ba2;
- if (obj instanceof CachedDeserializable) {
- CachedDeserializable cdObj = (CachedDeserializable) obj;
- if (!cdObj.isSerialized()) {
- return false;
- }
- ba2 = (byte[]) cdObj.getDeserializedForReading();
- } else if (obj instanceof byte[]) {
- ba2 = (byte[]) obj;
- } else {
- return false;
- }
- byte[] ba1 = (byte[]) cd.getDeserializedForReading();
- return Arrays.equals(ba1, ba2);
- }
- Object cdVal = cd.getValue();
- if (cdVal instanceof byte[]) {
- byte[] cdValBytes = (byte[]) cdVal;
- PdxInstance pi = InternalDataSerializer.readPdxInstance(cdValBytes, cache);
- if (pi != null) {
- return checkPdxEquals(pi, obj, cache);
- }
- if (isCompressedOffHeap) {
- // fix for bug 52248
- byte[] serializedObj;
- if (obj instanceof CachedDeserializable) {
- serializedObj = ((CachedDeserializable) obj).getSerializedValue();
- } else {
- serializedObj = EntryEventImpl.serialize(obj);
- }
- return Arrays.equals(cdValBytes, serializedObj);
- } else {
- /*
- * To be more compatible with previous releases do not compare the serialized forms here.
- * Instead deserialize and call the equals method.
- */
- Object deserializedObj;
- if (obj instanceof CachedDeserializable) {
- deserializedObj = ((CachedDeserializable) obj).getDeserializedForReading();
- } else {
- if (obj == null || obj == Token.NOT_AVAILABLE || Token.isInvalidOrRemoved(obj)) {
- return false;
- }
- // TODO OPTIMIZE: Before serializing all of obj we could get the top
- // level class name of cdVal and compare it to the top level class name of obj.
- deserializedObj = obj;
- }
- return basicEquals(deserializedObj, cd.getDeserializedForReading());
- }
- } else {
- // prefer object form
- if (obj instanceof CachedDeserializable) {
- // TODO OPTIMIZE: Before deserializing all of obj we could get the top
- // class name of cdVal and the top level class name of obj and compare.
- obj = ((CachedDeserializable) obj).getDeserializedForReading();
- }
- return basicEquals(cdVal, obj);
- }
- }
-
- /**
- * This method fixes bug 43643
- */
- private static boolean checkPdxEquals(PdxInstance pdx, Object obj, InternalCache cache) {
- if (!(obj instanceof PdxInstance)) {
- // obj may be a CachedDeserializable in which case we want to convert it to a PdxInstance even
- // if we are not readSerialized.
- if (obj instanceof CachedDeserializable) {
- CachedDeserializable cdObj = (CachedDeserializable) obj;
- if (!cdObj.isSerialized()) {
- // obj is actually a byte[] which will never be equal to a PdxInstance
- return false;
- }
- Object cdVal = cdObj.getValue();
- if (cdVal instanceof byte[]) {
- byte[] cdValBytes = (byte[]) cdVal;
- PdxInstance pi = InternalDataSerializer.readPdxInstance(cdValBytes, cache);
- if (pi != null) {
- return pi.equals(pdx);
- } else {
- // since obj is serialized as something other than pdx it must not equal our pdx
- return false;
- }
- } else {
- // remove the cd wrapper so that obj is the actual value we want to compare.
- obj = cdVal;
- }
- }
- if (obj != null && obj.getClass().getName().equals(pdx.getClassName())) {
- PdxSerializer pdxSerializer;
- if (obj instanceof PdxSerializable) {
- pdxSerializer = null;
- } else {
- pdxSerializer = cache.getPdxSerializer();
- }
- if (pdxSerializer != null || obj instanceof PdxSerializable) {
- // try to convert obj to a PdxInstance
- HeapDataOutputStream hdos = new HeapDataOutputStream(Version.CURRENT);
- try {
- if (InternalDataSerializer.autoSerialized(obj, hdos)
- || InternalDataSerializer.writePdx(hdos, cache, obj, pdxSerializer)) {
- PdxInstance pi = InternalDataSerializer.readPdxInstance(hdos.toByteArray(), cache);
- if (pi != null) {
- obj = pi;
- }
- }
- } catch (IOException | PdxSerializationException ignore) {
- // we are not able to convert it so just fall through
- }
- }
- }
- }
- return basicEquals(obj, pdx);
- }
-
// Do not add any instance fields to this class.
// Instead add them to LeafRegionEntry.cpp
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/LocalRegionTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/LocalRegionTest.java
index 771d360..ba4096b 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/LocalRegionTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/LocalRegionTest.java
@@ -27,6 +27,7 @@ import org.junit.Test;
import org.apache.geode.CancelCriterion;
import org.apache.geode.cache.EntryNotFoundException;
import org.apache.geode.cache.Operation;
+import org.apache.geode.cache.RegionAttributes;
import org.apache.geode.cache.client.internal.ServerRegionProxy;
public class LocalRegionTest {
@@ -35,6 +36,7 @@ public class LocalRegionTest {
private ServerRegionProxy serverRegionProxy;
private Operation operation;
private CancelCriterion cancelCriterion;
+ private RegionAttributes regionAttributes;
private final Object key = new Object();
private final String value = "value";
@@ -45,12 +47,14 @@ public class LocalRegionTest {
event = mock(EntryEventImpl.class);
serverRegionProxy = mock(ServerRegionProxy.class);
cancelCriterion = mock(CancelCriterion.class);
+ regionAttributes = mock(RegionAttributes.class);
when(region.getServerProxy()).thenReturn(serverRegionProxy);
when(event.isFromServer()).thenReturn(false);
when(event.getKey()).thenReturn(key);
when(event.getRawNewValue()).thenReturn(value);
when(region.getCancelCriterion()).thenReturn(cancelCriterion);
+ when(region.getAttributes()).thenReturn(regionAttributes);
}
@Test
@@ -91,7 +95,7 @@ public class LocalRegionTest {
public void checkPutIfAbsentResultThrowsIfEventHasRetriedButResultNotHaveSameValue() {
Object result = new Object();
when(event.hasRetried()).thenReturn(true);
- when(region.putIfAbsentResultHasSameValue(value, result)).thenReturn(false);
+ when(region.putIfAbsentResultHasSameValue(true, value, result)).thenReturn(false);
doCallRealMethod().when(region).checkPutIfAbsentResult(event, value, result);
region.checkPutIfAbsentResult(event, value, result);
@@ -101,43 +105,139 @@ public class LocalRegionTest {
public void checkPutIfAbsentResultSucceedsIfEventHasRetriedAndResultHasSameValue() {
Object result = new Object();
when(event.hasRetried()).thenReturn(true);
- when(region.putIfAbsentResultHasSameValue(value, result)).thenReturn(true);
+ when(region.putIfAbsentResultHasSameValue(true, value, result)).thenReturn(true);
doCallRealMethod().when(region).checkPutIfAbsentResult(event, value, result);
region.checkPutIfAbsentResult(event, value, result);
verify(event).hasRetried();
- verify(region).putIfAbsentResultHasSameValue(value, result);
+ verify(region).putIfAbsentResultHasSameValue(true, value, result);
}
@Test
public void putIfAbsentResultHasSameValueReturnTrueIfResultIsInvalidTokenAndValueToBePutIsNull() {
- when(region.putIfAbsentResultHasSameValue(null, Token.INVALID)).thenCallRealMethod();
+ when(region.putIfAbsentResultHasSameValue(true, null, Token.INVALID)).thenCallRealMethod();
- assertThat(region.putIfAbsentResultHasSameValue(null, Token.INVALID)).isTrue();
+ assertThat(region.putIfAbsentResultHasSameValue(true, null, Token.INVALID)).isTrue();
}
@Test
public void putIfAbsentResultHasSameValueReturnFalseIfResultIsInvalidTokenAndValueToBePutIsNotNull() {
- when(region.putIfAbsentResultHasSameValue(value, Token.INVALID)).thenCallRealMethod();
+ when(region.putIfAbsentResultHasSameValue(true, value, Token.INVALID)).thenCallRealMethod();
- assertThat(region.putIfAbsentResultHasSameValue(value, Token.INVALID)).isFalse();
+ assertThat(region.putIfAbsentResultHasSameValue(true, value, Token.INVALID)).isFalse();
}
@Test
public void putIfAbsentResultHasSameValueReturnTrueIfResultHasSameValue() {
Object result = "value";
- when(region.putIfAbsentResultHasSameValue(value, result)).thenCallRealMethod();
+ when(region.putIfAbsentResultHasSameValue(true, value, result)).thenCallRealMethod();
- assertThat(region.putIfAbsentResultHasSameValue(value, result)).isTrue();
+ assertThat(region.putIfAbsentResultHasSameValue(true, value, result)).isTrue();
+ verify(region, never()).getAttributes();
}
@Test
public void putIfAbsentResultHasSameValueReturnFalseIfResultDoesNotHaveSameValue() {
Object result = "differentValue";
- when(region.putIfAbsentResultHasSameValue(value, result)).thenCallRealMethod();
+ when(region.putIfAbsentResultHasSameValue(true, value, result)).thenCallRealMethod();
- assertThat(region.putIfAbsentResultHasSameValue(value, result)).isFalse();
+ assertThat(region.putIfAbsentResultHasSameValue(true, value, result)).isFalse();
+ verify(region, never()).getAttributes();
+ }
+
+ @Test
+ public void putIfAbsentResultHasSameValueChecksRegionAttributesIfNotFromClient() {
+ Object result = "value";
+ when(region.putIfAbsentResultHasSameValue(false, value, result)).thenCallRealMethod();
+
+ assertThat(region.putIfAbsentResultHasSameValue(false, value, result)).isTrue();
+ verify(region).getAttributes();
+ }
+
+ @Test
+ public void putIfAbsentResultHasSameValueReturnFalseIfResultDoesNotHaveSameValueAndNotFromClient() {
+ Object oldValue = "differentValue";
+ Object result = new VMCachedDeserializable(EntryEventImpl.serialize(oldValue));
+ when(region.putIfAbsentResultHasSameValue(false, value, result)).thenCallRealMethod();
+
+ assertThat(region.putIfAbsentResultHasSameValue(false, value, result)).isFalse();
+ verify(region).getAttributes();
+ }
+
+ @Test
+ public void putIfAbsentResultHasSameValueReturnTrueIfResultHasSameValueAndNotFromClient() {
+ Object result = new VMCachedDeserializable(EntryEventImpl.serialize(value));
+ when(region.putIfAbsentResultHasSameValue(false, value, result)).thenCallRealMethod();
+
+ assertThat(region.putIfAbsentResultHasSameValue(false, value, result)).isTrue();
+ verify(region).getAttributes();
+ }
+
+ @Test
+ public void bridgePutIfAbsentResultHasSameValueCanCheckValueForObject() {
+ Object result = "value";
+ byte[] valueToBePut = EntryEventImpl.serialize(value);
+ when(region.bridgePutIfAbsentResultHasSameValue(valueToBePut, true, result))
+ .thenCallRealMethod();
+
+ assertThat(region.bridgePutIfAbsentResultHasSameValue(valueToBePut, true, result)).isTrue();
+ verify(region).getAttributes();
+ }
+
+ @Test
+ public void bridgePutIfAbsentResultHasSameValueCanCheckValueForNonObjectByteArray() {
+ byte[] valueToBePut = {0, 1, 2, 3};
+ Object result = new VMCachedDeserializable(EntryEventImpl.serialize(valueToBePut));
+ when(region.bridgePutIfAbsentResultHasSameValue(valueToBePut, false, result))
+ .thenCallRealMethod();
+
+ assertThat(region.bridgePutIfAbsentResultHasSameValue(valueToBePut, false, result)).isTrue();
+ verify(region).getAttributes();
+ }
+
+ @Test
+ public void bridgePutIfAbsentResultHasSameValueCanCheckValueForIntArray() {
+ int[] newValue = {0, 1, 2, 3};
+ byte[] valueToBePut = EntryEventImpl.serialize(newValue);
+ Object result = newValue;
+ when(region.bridgePutIfAbsentResultHasSameValue(valueToBePut, true, result))
+ .thenCallRealMethod();
+
+ assertThat(region.bridgePutIfAbsentResultHasSameValue(valueToBePut, true, result)).isTrue();
+ verify(region).getAttributes();
+ }
+
+ @Test
+ public void bridgePutIfAbsentResultHasSameValueCanCheckValueForArrayOfArray() {
+ String[] array1 = {"0", "1", "2"};
+ String[] array2 = {"3", "4", "5"};
+ String[] array3 = {"7"};
+ String[][] newValue = {array1, array2, array3};
+ byte[] valueToBePut = EntryEventImpl.serialize(newValue);
+ Object result = new VMCachedDeserializable(EntryEventImpl.serialize(newValue));
+ when(region.bridgePutIfAbsentResultHasSameValue(valueToBePut, true, result))
+ .thenCallRealMethod();
+
+ assertThat(region.bridgePutIfAbsentResultHasSameValue(valueToBePut, true, result)).isTrue();
+ verify(region).getAttributes();
+ }
+
+ @Test
+ public void bridgePutIfAbsentResultHasSameValueCanCheckDifferentValuesForArrayOfArray() {
+ String[] array1 = {"0", "1", "2"};
+ String[] array2 = {"3", "4", "5"};
+ String[] array3 = {"7"};
+ String[] array4 = {"8"};
+ String[][] newValue = {array1, array2, array3};
+ String[][] returnedValue = {array1, array2, array4};
+ byte[] valueToBePut = EntryEventImpl.serialize(newValue);
+ Object result = new VMCachedDeserializable(EntryEventImpl.serialize(returnedValue));
+ when(region.bridgePutIfAbsentResultHasSameValue(valueToBePut, true, result))
+ .thenCallRealMethod();
+
+ assertThat(region.bridgePutIfAbsentResultHasSameValue(valueToBePut, true, result)).isFalse();
+ verify(region).getAttributes();
}
}