diff --git a/core/src/main/java/org/infinispan/interceptors/DistributionInterceptor.java b/core/src/main/java/org/infinispan/interceptors/DistributionInterceptor.java index 8b4dd418dd47..260e055762bf 100644 --- a/core/src/main/java/org/infinispan/interceptors/DistributionInterceptor.java +++ b/core/src/main/java/org/infinispan/interceptors/DistributionInterceptor.java @@ -94,7 +94,7 @@ public Object visitGetKeyValueCommand(InvocationContext ctx, GetKeyValueCommand // need to check in the context as well since a null retval is not necessarily an indication of the entry not being // available. It could just have been removed in the same tx beforehand. if (needsRemoteGet(ctx, command.getKey(), returnValue == null)) - returnValue = remoteGetAndStoreInL1(ctx, command.getKey(), isRehashInProgress); + returnValue = remoteGetAndStoreInL1(ctx, command.getKey(), isRehashInProgress, false); return returnValue; } @@ -118,17 +118,17 @@ private boolean needsRemoteGet(InvocationContext ctx, Object key, boolean retval * * @throws Throwable if there are problems */ - private Object remoteGetAndStoreInL1(InvocationContext ctx, Object key, boolean dmWasRehashingDuringLocalLookup) throws Throwable { + private Object remoteGetAndStoreInL1(InvocationContext ctx, Object key, boolean dmWasRehashingDuringLocalLookup, boolean isWrite) throws Throwable { boolean isMappedToLocalNode = false; if (ctx.isOriginLocal() && !(isMappedToLocalNode = dm.isLocal(key)) && isNotInL1(key)) { - return realRemoteGet(ctx, key, true); + return realRemoteGet(ctx, key, true, isWrite); } else { // maybe we are still rehashing as a joiner? ISPN-258 if (isMappedToLocalNode && dmWasRehashingDuringLocalLookup) { if (trace) log.trace("Key is mapped to local node, but a rehash is in progress so may need to look elsewhere"); // try a remote lookup all the same - return realRemoteGet(ctx, key, false); + return realRemoteGet(ctx, key, false, isWrite); } else { if (trace) log.trace("Not doing a remote get for key %s since entry is mapped to current node, or is in L1", key); @@ -137,7 +137,7 @@ private Object remoteGetAndStoreInL1(InvocationContext ctx, Object key, boolean return null; } - private Object realRemoteGet(InvocationContext ctx, Object key, boolean storeInL1) throws Throwable { + private Object realRemoteGet(InvocationContext ctx, Object key, boolean storeInL1, boolean isWrite) throws Throwable { if (trace) log.trace("Doing a remote get for key %s", key); // attempt a remote lookup InternalCacheEntry ice = dm.retrieveFromRemoteSource(key); @@ -153,10 +153,14 @@ private Object realRemoteGet(InvocationContext ctx, Object key, boolean storeInL } else { CacheEntry ce = ctx.lookupEntry(key); if (ce == null || ce.isNull() || ce.isLockPlaceholder()) { - if (ce != null && ce.isChanged()) + if (ce != null && ce.isChanged()) { ce.setValue(ice.getValue()); - else - ctx.putLookedUpEntry(key, ice); + } else { + if (isWrite) + entryFactory.wrapEntryForWriting(ctx, ice, true, false, ctx.hasLockedKey(key), false, false); + else + ctx.putLookedUpEntry(key, ice); + } } } } else { @@ -296,7 +300,7 @@ private void remoteGetBeforeWrite(InvocationContext ctx, boolean isConditionalCo // b) unsafeUnreliableReturnValues is true, we are in a TX and the command is conditional boolean isStillRehashingOnJoin = !dm.isJoinComplete(); if (isNeedReliableReturnValues(ctx) || (isConditionalCommand && ctx.isInTxScope())) { - for (Object k : keygen.getKeys()) remoteGetAndStoreInL1(ctx, k, isStillRehashingOnJoin); + for (Object k : keygen.getKeys()) remoteGetAndStoreInL1(ctx, k, isStillRehashingOnJoin, true); } } diff --git a/core/src/test/java/org/infinispan/distribution/DisabledL1Test.java b/core/src/test/java/org/infinispan/distribution/DisabledL1Test.java index a70daf5d58be..ae1267820c4c 100644 --- a/core/src/test/java/org/infinispan/distribution/DisabledL1Test.java +++ b/core/src/test/java/org/infinispan/distribution/DisabledL1Test.java @@ -27,6 +27,11 @@ import org.infinispan.commands.write.RemoveCommand; import org.testng.annotations.Test; +import java.lang.reflect.Method; + +import static org.infinispan.test.TestingUtil.k; +import static org.infinispan.test.TestingUtil.v; + @Test(groups = "functional", testName = "distribution.DisabledL1Test") public class DisabledL1Test extends BaseDistFunctionalTest { @@ -51,4 +56,11 @@ public void testRemoveFromNonOwner() { assertOnAllCachesAndOwnership("k1", null); } + public void testReplaceFromNonOwner(Method m) { + final String k = k(m); + final String v = v(m); + getOwners(k)[0].put(k, v); + getNonOwners(k)[0].replace(k, v(m, 1)); + } + } diff --git a/core/src/test/java/org/infinispan/distribution/DisabledL1WithRetValsTest.java b/core/src/test/java/org/infinispan/distribution/DisabledL1WithRetValsTest.java new file mode 100644 index 000000000000..59e847b21c78 --- /dev/null +++ b/core/src/test/java/org/infinispan/distribution/DisabledL1WithRetValsTest.java @@ -0,0 +1,65 @@ +/* + * JBoss, Home of Professional Open Source. + * Copyright 2000 - 2011, Red Hat Middleware LLC, and individual contributors + * as indicated by the @author tags. See the copyright.txt file in the + * distribution for a full listing of individual contributors. + * + * This is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as + * published by the Free Software Foundation; either version 2.1 of + * the License, or (at your option) any later version. + * + * This software is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this software; if not, write to the Free + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA, or see the FSF site: http://www.fsf.org. + */ + +package org.infinispan.distribution; + +import org.infinispan.Cache; +import org.infinispan.config.Configuration; +import org.infinispan.test.MultipleCacheManagersTest; +import org.infinispan.util.concurrent.IsolationLevel; +import org.testng.annotations.Test; + +import java.io.Serializable; +import java.lang.reflect.Method; +import java.util.List; +import java.util.Random; + +import static org.infinispan.test.TestingUtil.k; +import static org.infinispan.test.TestingUtil.v; + +/** + * Test distribution when L1 is disabled and return values are needed. + * + * @author Galder Zamarreño + * @since 4.2 + * @since 5.0 + */ +@Test(groups = "functional", testName = "distribution.DisabledL1WithRetValsTest") +public class DisabledL1WithRetValsTest extends BaseDistFunctionalTest { + + public DisabledL1WithRetValsTest() { + l1CacheEnabled = false; + testRetVals = true; + numOwners = 1; + INIT_CLUSTER_SIZE = 2; + } + + public void testReplaceFromNonOwner(Method m) { + final String k = k(m); + final String v = v(m); + Cache ownerCache = getOwners(k, 1)[0]; + ownerCache.put(k, v); + Cache nonOwnerCache = getNonOwners(k, 1)[0]; + nonOwnerCache.replace(k, v(m, 1)); + } + +} diff --git a/core/src/test/java/org/infinispan/jmx/ActivationAndPassivationInterceptorMBeanTest.java b/core/src/test/java/org/infinispan/jmx/ActivationAndPassivationInterceptorMBeanTest.java index 3057f6040ef0..e70b0f5ea790 100644 --- a/core/src/test/java/org/infinispan/jmx/ActivationAndPassivationInterceptorMBeanTest.java +++ b/core/src/test/java/org/infinispan/jmx/ActivationAndPassivationInterceptorMBeanTest.java @@ -93,8 +93,8 @@ public void testActivationOnPut(Method m) throws Exception { assertActivationCount(0); cacheStore.store(InternalEntryFactory.create(k(m), v(m))); assert cacheStore.containsKey(k(m)); - cache.put(k(m), v(m, "2")); - assert cache.get(k(m)).equals(v(m, "2")); + cache.put(k(m), v(m, 2)); + assert cache.get(k(m)).equals(v(m, 2)); assertActivationCount(1); assert !cacheStore.containsKey(k(m)) : "this should only be persisted on evict"; } @@ -116,7 +116,7 @@ public void testActivationOnReplace(Method m) throws Exception { assertActivationCount(0); cacheStore.store(InternalEntryFactory.create(k(m), v(m))); assert cacheStore.containsKey(k(m)); - assert cache.replace(k(m), v(m, "2")).equals(v(m)); + assert cache.replace(k(m), v(m, 2)).equals(v(m)); assertActivationCount(1); assert !cacheStore.containsKey(k(m)); } @@ -129,20 +129,20 @@ public void testActivationOnPutMap(Method m) throws Exception { assert cacheStore.containsKey(k(m)); Map toAdd = new HashMap(); - toAdd.put(k(m), v(m, "2")); + toAdd.put(k(m), v(m, 2)); cache.putAll(toAdd); assertActivationCount(1); - assert cache.get(k(m)).equals(v(m, "2")); + assert cache.get(k(m)).equals(v(m, 2)); assert !cacheStore.containsKey(k(m)); } public void testPassivationOnEvict(Method m) throws Exception { assertPassivationCount(0); cache.put(k(m), v(m)); - cache.put(k(m, "2"), v(m, "2")); + cache.put(k(m, 2), v(m, 2)); cache.evict(k(m)); assertPassivationCount(1); - cache.evict(k(m, "2")); + cache.evict(k(m, 2)); assertPassivationCount(2); cache.evict("not_existing_key"); assertPassivationCount(2); diff --git a/core/src/test/java/org/infinispan/loaders/decorators/AsyncTest.java b/core/src/test/java/org/infinispan/loaders/decorators/AsyncTest.java index 8629ff834da9..ef4ba71414c7 100644 --- a/core/src/test/java/org/infinispan/loaders/decorators/AsyncTest.java +++ b/core/src/test/java/org/infinispan/loaders/decorators/AsyncTest.java @@ -145,7 +145,7 @@ public void testThreadSafetyWritingDiffValuesForKey(Method m) throws Exception { public void testTransactionalModificationsHappenInDiffThread(Method m) throws Exception { try { final GlobalTransactionFactory gtf = new GlobalTransactionFactory(); - final String k1 = k(m, "1"), k2 = k(m, "2"), v1 = v(m, "1"), v2 = v(m, "2"); + final String k1 = k(m, 1), k2 = k(m, 2), v1 = v(m, 1), v2 = v(m, 2); final ConcurrentMap localMods = new ConcurrentHashMap(); final CyclicBarrier barrier = new CyclicBarrier(2); DummyInMemoryCacheStore underlying = new DummyInMemoryCacheStore(); @@ -197,7 +197,7 @@ protected void applyModificationsSync(ConcurrentMap mods) public void testTransactionalModificationsAreCoalesced(Method m) throws Exception { try { final GlobalTransactionFactory gtf = new GlobalTransactionFactory(); - final String k1 = k(m, "1"), k2 = k(m, "2"), k3 = k(m, "3"), v1 = v(m, "1"), v2 = v(m, "2"), v3 = v(m, "3"); + final String k1 = k(m, 1), k2 = k(m, 2), k3 = k(m, 3), v1 = v(m, 1), v2 = v(m, 2), v3 = v(m, 3); final AtomicInteger storeCount = new AtomicInteger(); final AtomicInteger removeCount = new AtomicInteger(); final AtomicInteger clearCount = new AtomicInteger(); diff --git a/core/src/test/java/org/infinispan/test/TestingUtil.java b/core/src/test/java/org/infinispan/test/TestingUtil.java index e04f47ccc124..141bf4bdbfec 100644 --- a/core/src/test/java/org/infinispan/test/TestingUtil.java +++ b/core/src/test/java/org/infinispan/test/TestingUtil.java @@ -859,22 +859,22 @@ public static String tmpDirectory(String basedir, AbstractInfinispanTest test) { return prefix + TEST_PATH + separator + test.getClass().getSimpleName(); } - public static String k(Method method, String index) { + public static String k(Method method, int index) { return new StringBuilder().append("k").append(index).append('-') .append(method.getName()).toString(); } - public static String v(Method method, String index) { + public static String v(Method method, int index) { return new StringBuilder().append("v").append(index).append('-') .append(method.getName()).toString(); } public static String k(Method method) { - return k(method, ""); + return k(method, 0); } - public static Object v(Method method) { - return v(method, ""); + public static String v(Method method) { + return v(method, 0); } public static TransactionTable getTransactionTable(Cache cache) {