Browse files

ISPN-1483 - Don't clear the locks when the prepare fails with optimis…

…tic locking

This change is only to make optimistic locking and pessimistic the same, the behaviour didn't change.
With pessimitic locking we can't clear the locks in the prepare command because that will prevent the rollback command from running on the remote nodes.
With optimistic locking it doesn't matter because the transaction will have at least one modification in it, forcing the rollback command to run on the remote nodes.
  • Loading branch information...
1 parent c49538a commit 44fbb9eb78305195856bfb2ca65505f23704fe1a Dan Berindei committed with galderz Nov 2, 2011
View
2 core/src/main/java/org/infinispan/interceptors/locking/OptimisticLockingInterceptor.java
@@ -65,7 +65,7 @@ public Object visitPrepareCommand(TxInvocationContext ctx, PrepareCommand comman
}
return invokeNextAndCommitIf1Pc(ctx, command);
} catch (Throwable te) {
- lockManager.unlockAll(ctx);
+ // don't remove the locks here, the rollback command will clear them
throw te;
}
}
View
1 core/src/main/java/org/infinispan/interceptors/locking/PessimisticLockingInterceptor.java
@@ -84,6 +84,7 @@ public Object visitPrepareCommand(TxInvocationContext ctx, PrepareCommand comman
abortIfRemoteTransactionInvalid(ctx, command);
return invokeNextAndCommitIf1Pc(ctx, command);
} catch (Throwable t) {
+ // don't remove the locks here, the rollback command will clear them
throw t;
}
}
View
87 core/src/test/java/org/infinispan/lock/StaleLocksOnPrepareFailureTest.java
@@ -0,0 +1,87 @@
+/*
+ * JBoss, Home of Professional Open Source
+ * Copyright 2010 Red Hat Inc. and/or its affiliates and other
+ * contributors as indicated by the @author tags. All rights reserved.
+ * See the copyright.txt 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.lock;
+
+import org.infinispan.Cache;
+import org.infinispan.commands.tx.PrepareCommand;
+import org.infinispan.config.Configuration;
+import org.infinispan.distribution.MagicKey;
+import org.infinispan.interceptors.DistributionInterceptor;
+import org.infinispan.interceptors.InterceptorChain;
+import org.infinispan.manager.EmbeddedCacheManager;
+import org.infinispan.test.MultipleCacheManagersTest;
+import org.infinispan.test.TestingUtil;
+import org.infinispan.test.fwk.CleanupAfterMethod;
+import org.infinispan.test.fwk.TestCacheManagerFactory;
+import org.testng.annotations.Test;
+
+@Test(testName = "lock.StaleLocksOnPrepareFailureTest", groups = "functional")
+@CleanupAfterMethod
+public class StaleLocksOnPrepareFailureTest extends MultipleCacheManagersTest {
+
+ public static final int NUM_CACHES = 10;
+
+ @Override
+ protected void createCacheManagers() throws Throwable {
+ Configuration cfg = TestCacheManagerFactory.getDefaultConfiguration(true, Configuration.CacheMode.DIST_SYNC);
+ cfg.setNumOwners(NUM_CACHES);
+ cfg.setLockAcquisitionTimeout(100);
+ for (int i = 0; i < NUM_CACHES; i++) {
+ EmbeddedCacheManager cm = TestCacheManagerFactory.createClusteredCacheManager(cfg);
+ registerCacheManager(cm);
+ }
+ waitForClusterToForm();
+ }
+
+ public void testModsCommit() throws Exception {
+ doTest(true);
+ }
+
+ private void doTest(boolean mods) throws Exception {
+ Cache<Object, Object> c1 = cache(0);
+ Cache<Object, Object> c2 = cache(NUM_CACHES /2);
+
+ // force the prepare command to fail on c2
+ FailInterceptor interceptor = new FailInterceptor();
+ interceptor.failFor(PrepareCommand.class);
+ InterceptorChain ic = TestingUtil.extractComponent(c2, InterceptorChain.class);
+ ic.addInterceptorBefore(interceptor, DistributionInterceptor.class);
+
+ MagicKey k1 = new MagicKey(c1, "k1");
+
+ tm(c1).begin();
+ c1.put(k1, "v1");
+
+ try {
+ tm(c1).commit();
+ assert false : "Commit should have failed";
+ } catch (Exception e) {
+ // expected
+ }
+
+ for (int i = 0; i < NUM_CACHES; i++) {
+ assertNotLocked(cache(i), k1);
+ }
+ }
+}
+

0 comments on commit 44fbb9e

Please sign in to comment.