Author: mircea.markus Date: 2007-12-10 18:47:41 -0500 (Mon, 10 Dec 2007) New Revision: 4825 Modified: core/trunk/src/main/java/org/jboss/cache/InvocationContext.java core/trunk/src/main/java/org/jboss/cache/interceptors/MethodDispacherInterceptor.java core/trunk/src/main/java/org/jboss/cache/interceptors/PessimisticLockInterceptor.java core/trunk/src/main/java/org/jboss/cache/transaction/TransactionTable.java Log: code cleanups in PesimisticInterceptor Modified: core/trunk/src/main/java/org/jboss/cache/InvocationContext.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/InvocationContext.java 2007-12-10 21:57:40 UTC (rev 4824) +++ core/trunk/src/main/java/org/jboss/cache/InvocationContext.java 2007-12-10 23:47:41 UTC (rev 4825) @@ -250,4 +250,18 @@ ctx.methodCall = methodCall; return ctx; } + + /** + * If the acq timeout if overwritten for current call, then return that one. + * If not overwritten return default value. + */ + public long getContextLockAcquisitionTimeout(long defaultFalue) { + long timeout = defaultFalue; + if (getOptionOverrides() != null + && getOptionOverrides().getLockAcquisitionTimeout() >= 0) + { + timeout = getOptionOverrides().getLockAcquisitionTimeout(); + } + return timeout; + } } Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/MethodDispacherInterceptor.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/interceptors/MethodDispacherInterceptor.java 2007-12-10 21:57:40 UTC (rev 4824) +++ core/trunk/src/main/java/org/jboss/cache/interceptors/MethodDispacherInterceptor.java 2007-12-10 23:47:41 UTC (rev 4825) @@ -32,9 +32,6 @@ * * @author Mircea.Markus(a)jboss.com * @version 2.2 - * todo - gtx is contained in InvocationContext. Check wheter passing method calls is or isn't redundant - * todo - check wheter is possible to group methods, e.g. MethodDeclarations.putMethods, treansationableMethods? - * todo - Refactor stuff in pessimistic lock interceptor * todo - Refactor stuff in txint * todo - revisit backward compatibility */ @@ -50,67 +47,7 @@ processOverwritternMethods(); } - /** - * Builds the list of methods that are overwiritten. - */ - private void processOverwritternMethods() - { - checkIfOverwritten(MethodDeclarations.putDataEraseMethodLocal_id, "handlePutDataEraseMethod",InvocationContext.class, GlobalTransaction.class, Fqn.class, Map.class, boolean.class, boolean.class); - checkIfOverwritten(MethodDeclarations.putDataMethodLocal_id, "handlePutDataMethod", InvocationContext.class, GlobalTransaction.class, Fqn.class, Map.class, boolean.class); - checkIfOverwritten(MethodDeclarations.putForExternalReadMethodLocal_id, "handlePutForExternalReadMethod",InvocationContext.class, GlobalTransaction.class, Fqn.class, Object.class, Object.class); - checkIfOverwritten(MethodDeclarations.putKeyValMethodLocal_id, "handlePutKeyValueMethod", InvocationContext.class, GlobalTransaction.class, Fqn.class, Object.class, Object.class, boolean.class); - checkIfOverwritten(MethodDeclarations.moveMethodLocal_id, "handleMoveMethod",InvocationContext.class, Fqn.class, Fqn.class); - checkIfOverwritten(MethodDeclarations.addChildMethodLocal_id, "handleAddChildMethod",InvocationContext.class, GlobalTransaction.class, Fqn.class, Object.class, Node.class, boolean.class); - checkIfOverwritten(MethodDeclarations.getKeyValueMethodLocal_id, "handleGetKeyValueMethod", InvocationContext.class, Fqn.class, Object.class, boolean.class); - checkIfOverwritten(MethodDeclarations.getNodeMethodLocal_id, "handleGetNodeMethod", InvocationContext.class, Fqn.class); - checkIfOverwritten(MethodDeclarations.getChildrenNamesMethodLocal_id, "handleGetChildrenNamesMethod",InvocationContext.class, Fqn.class); - checkIfOverwritten(MethodDeclarations.releaseAllLocksMethodLocal_id, "handleReleaseAllLocksMethod",InvocationContext.class, Fqn.class); - checkIfOverwritten(MethodDeclarations.printMethodLocal_id, "handlePrintMethod",InvocationContext.class, Fqn.class); - checkIfOverwritten(MethodDeclarations.getKeysMethodLocal_id, "handleGetKeysMethod", InvocationContext.class, Fqn.class); - checkIfOverwritten(MethodDeclarations.getDataMapMethodLocal_id, "handleGetDataMapMethod", InvocationContext.class, Fqn.class); - checkIfOverwritten(MethodDeclarations.rollbackMethod_id, "handleRollbackMethod", InvocationContext.class, GlobalTransaction.class); - checkIfOverwritten(MethodDeclarations.removeNodeMethodLocal_id, "handleRemoveNodeMethod", InvocationContext.class, GlobalTransaction.class, Fqn.class, boolean.class); - checkIfOverwritten(MethodDeclarations.removeKeyMethodLocal_id, "handleRemoveKeyMethod",InvocationContext.class, GlobalTransaction.class, Fqn.class, Object.class, boolean.class); - checkIfOverwritten(MethodDeclarations.removeDataMethodLocal_id, "handleRemoveDataMethod", InvocationContext.class, GlobalTransaction.class, Fqn.class, boolean.class); - checkIfOverwritten(MethodDeclarations.commitMethod_id, "handleCommitMethod",InvocationContext.class, GlobalTransaction.class); - checkIfOverwritten(MethodDeclarations.optimisticPrepareMethod_id, "handleOptimisticPrepareMethod", InvocationContext.class, GlobalTransaction.class, List.class, Map.class, Address.class, boolean.class); - checkIfOverwritten(MethodDeclarations.prepareMethod_id, "handlePrepareMethod", InvocationContext.class, GlobalTransaction.class, List.class, Address.class, boolean.class); - checkIfOverwritten(MethodDeclarations.evictNodeMethodLocal_id, "handleEvictMethod", InvocationContext.class, Fqn.class); - checkIfOverwritten(MethodDeclarations.evictVersionedNodeMethodLocal_id, "handleEvictVersionedNodeMethod", InvocationContext.class, Fqn.class, DataVersion.class); - checkIfOverwritten(MethodDeclarations.existsMethod_id, "handleExistsMethod", InvocationContext.class, Fqn.class); - checkIfOverwritten(MethodDeclarations.putDataEraseVersionedMethodLocal_id, "handlePutDataEraseVersionedMethod",InvocationContext.class, GlobalTransaction.class, Fqn.class, Map.class, boolean.class, boolean.class, DataVersion.class); - checkIfOverwritten(MethodDeclarations.putDataVersionedMethodLocal_id, "handlePutDataVersionedMethod", InvocationContext.class, GlobalTransaction.class, Fqn.class, Map.class, Boolean.class, DataVersion.class); - checkIfOverwritten(MethodDeclarations.putKeyValVersionedMethodLocal_id, "handlePutKeyValueVersionedMethod", InvocationContext.class, GlobalTransaction.class, Fqn.class, Object.class, Object.class, boolean.class, DataVersion.class); - checkIfOverwritten(MethodDeclarations.putForExternalReadVersionedMethodLocal_id, "handlePutForExternalReadVersionedMethod", InvocationContext.class, GlobalTransaction.class, Fqn.class, Object.class, Object.class, DataVersion.class); - checkIfOverwritten(MethodDeclarations.dataGravitationCleanupMethod_id, "handleDataGravitationCleanupMethod", InvocationContext.class, GlobalTransaction.class, Fqn.class, Fqn.class); - checkIfOverwritten(MethodDeclarations.removeNodeVersionedMethodLocal_id, "handleRemoveNodeVersionedMethod", InvocationContext.class, GlobalTransaction.class, Fqn.class, boolean.class, DataVersion.class); - checkIfOverwritten(MethodDeclarations.removeKeyVersionedMethodLocal_id, "handleRemoveKeyVersionedMethod",InvocationContext.class, GlobalTransaction.class, Fqn.class, Object.class, boolean.class, DataVersion.class); - checkIfOverwritten(MethodDeclarations.removeDataVersionedMethodLocal_id, "handleRemoveDataVersionedMethod", InvocationContext.class, GlobalTransaction.class, Fqn.class, boolean.class, DataVersion.class); - checkIfOverwritten(MethodDeclarations.blockChannelMethodLocal_id, "handleBlockChannelMethod",InvocationContext.class); - checkIfOverwritten(MethodDeclarations.unblockChannelMethodLocal_id, "handleUnblockChannelMethod", InvocationContext.class); - checkIfOverwritten(MethodDeclarations.lockMethodLocal_id, "handleLockMethod", InvocationContext.class, Fqn.class, NodeLock.LockType.class, boolean.class); - - } - - private void checkIfOverwritten(int putDataEraseMethodLocal_id, String methodName, Class... args) - { - Class currentClass = getClass(); - //if this is a > 1 inheritace deepth and the method was overwritten in the parent. We also have to look into parents - while (currentClass != MethodDispacherInterceptor.class) - { - try - { - currentClass.getDeclaredMethod(methodName, args); - this.overwrittenMethods.add(putDataEraseMethodLocal_id); - } catch (NoSuchMethodException e) - { - //ignore - } - currentClass = (Class) currentClass.getGenericSuperclass(); - } - } - - /** + /** * Acts like a 'switch case' that delegates the call to the appropriate method. */ public Object invoke(InvocationContext ctx) throws Throwable @@ -240,7 +177,7 @@ return result; } - /** + /** * Handles {@link org.jboss.cache.CacheImpl#_lock(org.jboss.cache.Fqn, org.jboss.cache.lock.NodeLock.LockType, boolean)} */ protected Object handleLockMethod(InvocationContext ctx, Fqn fqn, NodeLock.LockType lockType, boolean recursive) throws Throwable @@ -248,7 +185,7 @@ return defaultHandlersBehavior(); } - /** + /** * Handles {@link org.jboss.cache.CacheImpl#_unblock()} */ protected Object handleUnblockChannelMethod(InvocationContext ctx) throws Throwable @@ -256,7 +193,7 @@ return defaultHandlersBehavior(); } - /** + /** * Handles {@link org.jboss.cache.CacheImpl#_block()} */ protected Object handleBlockChannelMethod(InvocationContext ctx) throws Throwable @@ -264,7 +201,7 @@ return defaultHandlersBehavior(); } - /** + /** * Handles {@link org.jboss.cache.CacheImpl#_removeData(org.jboss.cache.transaction.GlobalTransaction, org.jboss.cache.Fqn, boolean, org.jboss.cache.optimistic.DataVersion)} */ protected Object handleRemoveDataVersionedMethod(InvocationContext ctx, GlobalTransaction gtx, Fqn fqn, boolean createUndoOps, DataVersion dv) throws Throwable @@ -272,7 +209,7 @@ return defaultHandlersBehavior(); } - /** + /** * Handles {@link org.jboss.cache.CacheImpl#_remove(org.jboss.cache.transaction.GlobalTransaction, org.jboss.cache.Fqn, Object, boolean, org.jboss.cache.optimistic.DataVersion)} */ protected Object handleRemoveKeyVersionedMethod(InvocationContext ctx, GlobalTransaction gtx, Fqn fqn, Object key, boolean createUndoOps, DataVersion dv) throws Throwable @@ -280,7 +217,7 @@ return defaultHandlersBehavior(); } - /** + /** * Handles {@link org.jboss.cache.CacheImpl#_remove(org.jboss.cache.transaction.GlobalTransaction, org.jboss.cache.Fqn, boolean, org.jboss.cache.optimistic.DataVersion)} */ protected Object handleRemoveNodeVersionedMethod(InvocationContext ctx, GlobalTransaction gtx, Fqn fqn, boolean createUndoOps, DataVersion dv) throws Throwable @@ -288,7 +225,7 @@ return defaultHandlersBehavior(); } - /** + /** * Handles {@link org.jboss.cache.CacheImpl#_dataGravitationCleanup(org.jboss.cache.transaction.GlobalTransaction, org.jboss.cache.Fqn, org.jboss.cache.Fqn)} */ protected Object handleDataGravitationCleanupMethod(InvocationContext ctx, GlobalTransaction globalTransaction, Fqn primary, Fqn backup) throws Throwable @@ -296,7 +233,7 @@ return defaultHandlersBehavior(); } - /** + /** * Handles {@link org.jboss.cache.CacheImpl#_putForExternalRead(org.jboss.cache.transaction.GlobalTransaction, org.jboss.cache.Fqn, Object, Object, org.jboss.cache.optimistic.DataVersion)} */ protected Object handlePutForExternalReadVersionedMethod(InvocationContext ctx, GlobalTransaction gtx, Fqn fqn, Object key, Object value, DataVersion dv) throws Throwable @@ -304,7 +241,7 @@ return defaultHandlersBehavior(); } - /** + /** * Handles {@link org.jboss.cache.CacheImpl#_put(org.jboss.cache.transaction.GlobalTransaction, org.jboss.cache.Fqn, Object, Object, boolean, org.jboss.cache.optimistic.DataVersion)} */ protected Object handlePutKeyValueVersionedMethod(InvocationContext ctx, GlobalTransaction gtx, Fqn fqn, Object key, Object value, boolean createUndoOps, DataVersion dv) throws Throwable @@ -312,7 +249,7 @@ return defaultHandlersBehavior(); } - /** + /** * Handles {@link org.jboss.cache.CacheImpl#_put(org.jboss.cache.transaction.GlobalTransaction, org.jboss.cache.Fqn, java.util.Map, boolean, org.jboss.cache.optimistic.DataVersion)} */ protected Object handlePutDataVersionedMethod(InvocationContext ctx, GlobalTransaction globalTransaction, Fqn fqn, Map map, Boolean createUndoOps, DataVersion dataVersion) throws Throwable @@ -320,7 +257,7 @@ return defaultHandlersBehavior(); } - /** + /** * Handles {@link org.jboss.cache.CacheImpl#_put(org.jboss.cache.transaction.GlobalTransaction, org.jboss.cache.Fqn, java.util.Map, boolean, boolean, org.jboss.cache.optimistic.DataVersion)} */ protected Object handlePutDataEraseVersionedMethod(InvocationContext ctx, GlobalTransaction gtx, Fqn fqn, Map data, boolean createUndoOps, boolean eraseContent, DataVersion dv) throws Throwable @@ -328,7 +265,7 @@ return defaultHandlersBehavior(); } - /** + /** * Handles {@link org.jboss.cache.CacheImpl#exists(String)} */ protected Object handleExistsMethod(InvocationContext ctx, Fqn fqn) throws Throwable @@ -336,22 +273,22 @@ return defaultHandlersBehavior(); } - /** + /** * used for logging various steps. if null is returned than nothing is logged. */ protected abstract Log getLog(); - /** + /** * Each interceptor should extend this if it does not need any processing for current call. * An sample usage would be: this interceptor is only interested if thre is one transaction going on. If so all - * handleXYZ would know that we have a transaction going and would not check its state. + * handleXYZ would know that we have a transaction going and would not check its state. */ protected boolean skipMethodCall(InvocationContext ctx) { return false; } - /** + /** * Handles {@link org.jboss.cache.CacheImpl#_evict(org.jboss.cache.Fqn, org.jboss.cache.optimistic.DataVersion)} */ protected Object handleEvictVersionedNodeMethod(InvocationContext ctx, Fqn fqn, DataVersion dataVersion) throws Throwable @@ -359,7 +296,7 @@ return defaultHandlersBehavior(); } - /** + /** * Handles {@link org.jboss.cache.CacheImpl#evict(org.jboss.cache.Fqn)} */ protected Object handleEvictMethod(InvocationContext ctx, Fqn fqn) throws Throwable @@ -367,7 +304,7 @@ return defaultHandlersBehavior(); } - /** + /** * Handles {@link org.jboss.cache.CacheImpl#prepare(org.jboss.cache.transaction.GlobalTransaction, java.util.List, org.jgroups.Address, boolean)} */ protected Object handlePrepareMethod(InvocationContext ctx, GlobalTransaction gtx, List modification, Address coordinator, boolean onePhaseCommit) throws Throwable @@ -375,7 +312,7 @@ return defaultHandlersBehavior(); } - /** + /** * Handles {@link org.jboss.cache.CacheImpl#optimisticPrepare(org.jboss.cache.transaction.GlobalTransaction, java.util.List, java.util.Map, org.jgroups.Address, boolean)} */ protected Object handleOptimisticPrepareMethod(InvocationContext ctx, GlobalTransaction gtx, List modifications, Map data, Address address, boolean onePhaseCommit) throws Throwable @@ -383,7 +320,7 @@ return defaultHandlersBehavior(); } - /** + /** * Handles {@link org.jboss.cache.CacheImpl#commit(org.jboss.cache.transaction.GlobalTransaction)} */ protected Object handleCommitMethod(InvocationContext ctx, GlobalTransaction globalTransaction) throws Throwable @@ -391,7 +328,7 @@ return defaultHandlersBehavior(); } - /** + /** * Handles {@link org.jboss.cache.CacheImpl#_removeData(org.jboss.cache.transaction.GlobalTransaction, Fqn, boolean)} */ protected Object handleRemoveDataMethod(InvocationContext ctx, GlobalTransaction tx, Fqn fqn, boolean createUndoOps) throws Throwable @@ -399,7 +336,7 @@ return defaultHandlersBehavior(); } - /** + /** * Handles {@link org.jboss.cache.CacheImpl#_remove(org.jboss.cache.transaction.GlobalTransaction, String, Object, boolean)} */ protected Object handleRemoveKeyMethod(InvocationContext ctx, GlobalTransaction tx, Fqn fqn, Object key, boolean createUndoOps) throws Throwable @@ -407,8 +344,7 @@ return defaultHandlersBehavior(); } - - /** + /** * Handles {@link org.jboss.cache.CacheImpl#_remove(org.jboss.cache.transaction.GlobalTransaction, String, boolean)} */ protected Object handleRemoveNodeMethod(InvocationContext ctx, GlobalTransaction tx, Fqn fqn, boolean createUndoOps) throws Throwable @@ -416,7 +352,8 @@ return defaultHandlersBehavior(); } - /** + + /** * Handles {@link org.jboss.cache.CacheImpl#rollback(org.jboss.cache.transaction.GlobalTransaction)} */ protected Object handleRollbackMethod(InvocationContext ctx, GlobalTransaction globalTransaction) throws Throwable @@ -424,7 +361,7 @@ return defaultHandlersBehavior(); } - /** + /** * Handles {@link org.jboss.cache.CacheImpl#_getData(org.jboss.cache.Fqn)} */ protected Object handleGetDataMapMethod(InvocationContext ctx, Fqn fqn) throws Throwable @@ -432,7 +369,7 @@ return defaultHandlersBehavior(); } - /** + /** * Handles {@link org.jboss.cache.CacheImpl#getKeys(Fqn)} */ protected Object handleGetKeysMethod(InvocationContext ctx, Fqn fqn) throws Throwable @@ -440,7 +377,7 @@ return defaultHandlersBehavior(); } - /** + /** * Handles {@link org.jboss.cache.CacheImpl#_print(org.jboss.cache.Fqn)} */ protected Object handlePrintMethod(InvocationContext ctx, Fqn fqn) throws Throwable @@ -448,7 +385,7 @@ return defaultHandlersBehavior(); } - /** + /** * Handles {@link org.jboss.cache.CacheImpl#_releaseAllLocks(org.jboss.cache.Fqn)} */ protected Object handleReleaseAllLocksMethod(InvocationContext ctx, Fqn fqn) throws Throwable @@ -456,7 +393,7 @@ return defaultHandlersBehavior(); } - /** + /** * Handles {@link org.jboss.cache.CacheImpl#_getChildrenNames(org.jboss.cache.Fqn)} */ protected Object handleGetChildrenNamesMethod(InvocationContext ctx, Fqn fqn) throws Throwable @@ -464,7 +401,7 @@ return defaultHandlersBehavior(); } - /** + /** * Handles {@link org.jboss.cache.CacheImpl#_get(org.jboss.cache.Fqn)} */ protected Object handleGetNodeMethod(InvocationContext ctx, Fqn fqn) throws Throwable @@ -472,7 +409,7 @@ return defaultHandlersBehavior(); } - /** + /** * Handles {@link org.jboss.cache.CacheImpl#_get(org.jboss.cache.Fqn, Object, boolean)} */ protected Object handleGetKeyValueMethod(InvocationContext ctx, Fqn fqn, Object key, boolean sendNodeEvent) throws Throwable @@ -480,8 +417,7 @@ return defaultHandlersBehavior(); } - - /** + /** * Handles {@link org.jboss.cache.CacheImpl#_addChild(org.jboss.cache.transaction.GlobalTransaction, org.jboss.cache.Fqn, Object, org.jboss.cache.Node, boolean)} */ protected Object handleAddChildMethod(InvocationContext ctx, GlobalTransaction tx, Fqn parentFqn, Object childName, Node cn, boolean createUndoOps) throws Throwable @@ -489,7 +425,8 @@ return defaultHandlersBehavior(); } - /** + + /** * Handles {@link org.jboss.cache.CacheImpl#_move(org.jboss.cache.Fqn, org.jboss.cache.Fqn)} */ protected Object handleMoveMethod(InvocationContext ctx, Fqn from, Fqn to) throws Throwable @@ -497,7 +434,7 @@ return defaultHandlersBehavior(); } - /** + /** * Handles {@link org.jboss.cache.CacheImpl#_put(org.jboss.cache.transaction.GlobalTransaction, String, Object, Object, boolean)} */ protected Object handlePutKeyValueMethod(InvocationContext ctx, GlobalTransaction gtx, Fqn fqn, Object key, Object value, boolean createUndoOps) throws Throwable @@ -505,7 +442,7 @@ return defaultHandlersBehavior(); } - /** + /** * Handles {@link org.jboss.cache.CacheImpl#_putForExternalRead(org.jboss.cache.transaction.GlobalTransaction, org.jboss.cache.Fqn, Object, Object)} */ protected Object handlePutForExternalReadMethod(InvocationContext ctx, GlobalTransaction tx, Fqn fqn, Object key, Object value) throws Throwable @@ -513,7 +450,7 @@ return defaultHandlersBehavior(); } - /** + /** * Handles {@link org.jboss.cache.CacheImpl#_put(org.jboss.cache.transaction.GlobalTransaction, String, java.util.Map, boolean)} */ protected Object handlePutDataMethod(InvocationContext ctx, GlobalTransaction tx, Fqn fqn, Map data, boolean createUndoOps) throws Throwable @@ -521,7 +458,7 @@ return defaultHandlersBehavior(); } - /** + /** * Handles {@link org.jboss.cache.CacheImpl#_put(org.jboss.cache.transaction.GlobalTransaction, org.jboss.cache.Fqn, java.util.Map, boolean, boolean)} */ protected Object handlePutDataEraseMethod(InvocationContext ctx, GlobalTransaction gt, Fqn fqn, Map newData, boolean createUndoOps, boolean eraseContents) throws Throwable @@ -529,7 +466,7 @@ return defaultHandlersBehavior(); } - /** + /** * Handlers defined here should not be called directlly. There are two scenarios in which a handler might be called: * 1 - DerivedInterceptor.super - pointless call * 2 - if the logic that determines that an handler is overwritten fails. Throwing an exception by default is for @@ -538,5 +475,65 @@ private Object defaultHandlersBehavior() { throw new IllegalStateException("this is either called from a derived class or nt overwritten and accidentally called. Either way, is not correct."); - } + } + + /** + * Builds the list of methods that are overwiritten. + */ + private void processOverwritternMethods() + { + checkIfOverwritten(MethodDeclarations.putDataEraseMethodLocal_id, "handlePutDataEraseMethod",InvocationContext.class, GlobalTransaction.class, Fqn.class, Map.class, boolean.class, boolean.class); + checkIfOverwritten(MethodDeclarations.putDataMethodLocal_id, "handlePutDataMethod", InvocationContext.class, GlobalTransaction.class, Fqn.class, Map.class, boolean.class); + checkIfOverwritten(MethodDeclarations.putForExternalReadMethodLocal_id, "handlePutForExternalReadMethod",InvocationContext.class, GlobalTransaction.class, Fqn.class, Object.class, Object.class); + checkIfOverwritten(MethodDeclarations.putKeyValMethodLocal_id, "handlePutKeyValueMethod", InvocationContext.class, GlobalTransaction.class, Fqn.class, Object.class, Object.class, boolean.class); + checkIfOverwritten(MethodDeclarations.moveMethodLocal_id, "handleMoveMethod",InvocationContext.class, Fqn.class, Fqn.class); + checkIfOverwritten(MethodDeclarations.addChildMethodLocal_id, "handleAddChildMethod",InvocationContext.class, GlobalTransaction.class, Fqn.class, Object.class, Node.class, boolean.class); + checkIfOverwritten(MethodDeclarations.getKeyValueMethodLocal_id, "handleGetKeyValueMethod", InvocationContext.class, Fqn.class, Object.class, boolean.class); + checkIfOverwritten(MethodDeclarations.getNodeMethodLocal_id, "handleGetNodeMethod", InvocationContext.class, Fqn.class); + checkIfOverwritten(MethodDeclarations.getChildrenNamesMethodLocal_id, "handleGetChildrenNamesMethod",InvocationContext.class, Fqn.class); + checkIfOverwritten(MethodDeclarations.releaseAllLocksMethodLocal_id, "handleReleaseAllLocksMethod",InvocationContext.class, Fqn.class); + checkIfOverwritten(MethodDeclarations.printMethodLocal_id, "handlePrintMethod",InvocationContext.class, Fqn.class); + checkIfOverwritten(MethodDeclarations.getKeysMethodLocal_id, "handleGetKeysMethod", InvocationContext.class, Fqn.class); + checkIfOverwritten(MethodDeclarations.getDataMapMethodLocal_id, "handleGetDataMapMethod", InvocationContext.class, Fqn.class); + checkIfOverwritten(MethodDeclarations.rollbackMethod_id, "handleRollbackMethod", InvocationContext.class, GlobalTransaction.class); + checkIfOverwritten(MethodDeclarations.removeNodeMethodLocal_id, "handleRemoveNodeMethod", InvocationContext.class, GlobalTransaction.class, Fqn.class, boolean.class); + checkIfOverwritten(MethodDeclarations.removeKeyMethodLocal_id, "handleRemoveKeyMethod",InvocationContext.class, GlobalTransaction.class, Fqn.class, Object.class, boolean.class); + checkIfOverwritten(MethodDeclarations.removeDataMethodLocal_id, "handleRemoveDataMethod", InvocationContext.class, GlobalTransaction.class, Fqn.class, boolean.class); + checkIfOverwritten(MethodDeclarations.commitMethod_id, "handleCommitMethod",InvocationContext.class, GlobalTransaction.class); + checkIfOverwritten(MethodDeclarations.optimisticPrepareMethod_id, "handleOptimisticPrepareMethod", InvocationContext.class, GlobalTransaction.class, List.class, Map.class, Address.class, boolean.class); + checkIfOverwritten(MethodDeclarations.prepareMethod_id, "handlePrepareMethod", InvocationContext.class, GlobalTransaction.class, List.class, Address.class, boolean.class); + checkIfOverwritten(MethodDeclarations.evictNodeMethodLocal_id, "handleEvictMethod", InvocationContext.class, Fqn.class); + checkIfOverwritten(MethodDeclarations.evictVersionedNodeMethodLocal_id, "handleEvictVersionedNodeMethod", InvocationContext.class, Fqn.class, DataVersion.class); + checkIfOverwritten(MethodDeclarations.existsMethod_id, "handleExistsMethod", InvocationContext.class, Fqn.class); + checkIfOverwritten(MethodDeclarations.putDataEraseVersionedMethodLocal_id, "handlePutDataEraseVersionedMethod",InvocationContext.class, GlobalTransaction.class, Fqn.class, Map.class, boolean.class, boolean.class, DataVersion.class); + checkIfOverwritten(MethodDeclarations.putDataVersionedMethodLocal_id, "handlePutDataVersionedMethod", InvocationContext.class, GlobalTransaction.class, Fqn.class, Map.class, Boolean.class, DataVersion.class); + checkIfOverwritten(MethodDeclarations.putKeyValVersionedMethodLocal_id, "handlePutKeyValueVersionedMethod", InvocationContext.class, GlobalTransaction.class, Fqn.class, Object.class, Object.class, boolean.class, DataVersion.class); + checkIfOverwritten(MethodDeclarations.putForExternalReadVersionedMethodLocal_id, "handlePutForExternalReadVersionedMethod", InvocationContext.class, GlobalTransaction.class, Fqn.class, Object.class, Object.class, DataVersion.class); + checkIfOverwritten(MethodDeclarations.dataGravitationCleanupMethod_id, "handleDataGravitationCleanupMethod", InvocationContext.class, GlobalTransaction.class, Fqn.class, Fqn.class); + checkIfOverwritten(MethodDeclarations.removeNodeVersionedMethodLocal_id, "handleRemoveNodeVersionedMethod", InvocationContext.class, GlobalTransaction.class, Fqn.class, boolean.class, DataVersion.class); + checkIfOverwritten(MethodDeclarations.removeKeyVersionedMethodLocal_id, "handleRemoveKeyVersionedMethod",InvocationContext.class, GlobalTransaction.class, Fqn.class, Object.class, boolean.class, DataVersion.class); + checkIfOverwritten(MethodDeclarations.removeDataVersionedMethodLocal_id, "handleRemoveDataVersionedMethod", InvocationContext.class, GlobalTransaction.class, Fqn.class, boolean.class, DataVersion.class); + checkIfOverwritten(MethodDeclarations.blockChannelMethodLocal_id, "handleBlockChannelMethod",InvocationContext.class); + checkIfOverwritten(MethodDeclarations.unblockChannelMethodLocal_id, "handleUnblockChannelMethod", InvocationContext.class); + checkIfOverwritten(MethodDeclarations.lockMethodLocal_id, "handleLockMethod", InvocationContext.class, Fqn.class, NodeLock.LockType.class, boolean.class); + + } + + private void checkIfOverwritten(int putDataEraseMethodLocal_id, String methodName, Class... args) + { + Class currentClass = getClass(); + //if this is a > 1 inheritace deepth and the method was overwritten in the parent. We also have to look into parents + while (currentClass != MethodDispacherInterceptor.class) + { + try + { + currentClass.getDeclaredMethod(methodName, args); + this.overwrittenMethods.add(putDataEraseMethodLocal_id); + } catch (NoSuchMethodException e) + { + //ignore + } + currentClass = (Class) currentClass.getGenericSuperclass(); + } + } } Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/PessimisticLockInterceptor.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/interceptors/PessimisticLockInterceptor.java 2007-12-10 21:57:40 UTC (rev 4824) +++ core/trunk/src/main/java/org/jboss/cache/interceptors/PessimisticLockInterceptor.java 2007-12-10 23:47:41 UTC (rev 4825) @@ -19,6 +19,14 @@ import java.util.*; +/* +* todo refactorings ideas +* - thre are many places in code that handles that coputes the lock owners: either GTX or Thread.local. The +* lockOwner can be abstractised as a LockOwner that can be extended by CurrentThreadLock owner and + GlobalTransaction owner. This would make the code nicer. +*/ + + /** * An interceptor that handles locking. When a TX is associated, we register * for TX completion and unlock the locks acquired within the scope of the TX. @@ -27,7 +35,6 @@ * * @author Bela Ban * @version $Id$ - * //todo = try to see how acquireLuckWithTimeout works inline */ public class PessimisticLockInterceptor extends MethodDispacherInterceptor { @@ -38,9 +45,7 @@ */ private Map<Thread, List<NodeLock>> lock_table; private long lock_acquisition_timeout; - private LockManager lockManager = new LockManager(); - public void setCache(CacheSPI cache) { super.setCache(cache); @@ -76,17 +81,31 @@ if (ctx.getOptionOverrides() != null && ctx.getOptionOverrides().isSuppressLocking()) { log.trace("Creating nodes if necessary"); - createNodes(fqn, ctx.getGlobalTransaction()); + int treeNodeSize = fqn.size(); + NodeSPI n = cache.getRoot(); + for (int i = 0; i < treeNodeSize; i++) + { + Object childName = fqn.get(i); + Fqn childFqn = new Fqn(childName); + NodeSPI child_node = n.getChildDirect(childFqn); + if (child_node == null) child_node = n.addChildDirect(childFqn); + manageReverseRemove(ctx.getGlobalTransaction(), child_node, true); + n = child_node; + } } else { - acquireLocksWithTimeout(ctx, fqn, NodeLock.LockType.WRITE, false, true, false, false, false, false); + acquireLocksWithTimeout(ctx, fqn, NodeLock.LockType.READ, true, false, false, true); } return nextInterceptor(ctx); } protected Object handleLockMethod(InvocationContext ctx, Fqn fqn, NodeLock.LockType lockType, boolean recursive) throws Throwable { - acquireLocksWithTimeout(ctx, fqn, lockType, recursive, false, false, false, false, false); + acquireLocksWithTimeout(ctx, fqn, lockType, false, false, false, false); + if (recursive) + { + acquireLocksOnChildren(cache.peek(fqn, false), lockType, ctx); + } return null; } @@ -95,7 +114,7 @@ // commit propagated up from the tx interceptor commit(ctx.getGlobalTransaction()); Object retVal = nextInterceptor(ctx); - cleanup(ctx.getGlobalTransaction()); + tx_table.cleanup(ctx.getGlobalTransaction()); return retVal; } @@ -112,44 +131,83 @@ log.trace("bypassed locking as method commit() doesn't require locking"); } Object retVal = nextInterceptor(ctx); - cleanup(globalTransaction); + tx_table.cleanup(globalTransaction); return retVal; } protected Object handleRollbackMethod(InvocationContext ctx, GlobalTransaction globalTransaction) throws Throwable { - rollback(globalTransaction); + TransactionEntry entry = tx_table.get(globalTransaction); if (log.isTraceEnabled()) { + log.trace("called to rollback cache with GlobalTransaction=" + globalTransaction); + } + + if (entry == null) + { + log.error("entry for transaction " + globalTransaction + " not found (transaction has possibly already been rolled back)"); + } + else + { + Iterator removedNodes = entry.getRemovedNodes().iterator(); + CacheImpl cacheImpl = (CacheImpl) cache; + while (removedNodes.hasNext()) + { + Fqn f = (Fqn) removedNodes.next(); + cacheImpl.realRemove(f, false); + + } + // 1. Revert the modifications by running the undo-op list in reverse. This *cannot* throw any exceptions ! + entry.undoOperations(cache); + } + if (log.isTraceEnabled()) + { log.trace("bypassed locking as method rollback() doesn't require locking"); } Object retVal = nextInterceptor(ctx); - cleanup(globalTransaction); + tx_table.cleanup(globalTransaction); return retVal; } protected Object handleMoveMethod(InvocationContext ctx, Fqn from, Fqn to) throws Throwable { - long timeout = getLockAcquisitionTimeout(ctx); + long timeout = ctx.getContextLockAcquisitionTimeout(lock_acquisition_timeout); // this call will ensure the node gets a WL and it's current parent gets RL. if (log.isTraceEnabled()) log.trace("Attempting to get WL on node to be moved [" + from + "]"); - lock(ctx, from, NodeLock.LockType.WRITE, true, false, timeout, true, false, false); - //now for an RL for the new parent. - if (log.isTraceEnabled()) log.trace("Attempting to get RL on new parent [" + to + "]"); - lock(ctx, to, NodeLock.LockType.READ, true, false, timeout, false, false, false); + if (from != null && ! (configuration.getIsolationLevel() == IsolationLevel.NONE)) + { + lock(ctx, from, NodeLock.LockType.WRITE, false, timeout, true, false); + if (ctx.getGlobalTransaction() != null) + { + cache.getTransactionTable().get(ctx.getGlobalTransaction()).addRemovedNode(from); + } + acquireLocksOnChildren(cache.peek(from, true), NodeLock.LockType.WRITE, ctx); + } + if (to != null && !(configuration.getIsolationLevel() == IsolationLevel.NONE)) + { + //now for an RL for the new parent. + if (log.isTraceEnabled()) log.trace("Attempting to get RL on new parent [" + to + "]"); + lock(ctx, to, NodeLock.LockType.READ, false, timeout, false, false); + acquireLocksOnChildren(cache.peek(to, true), NodeLock.LockType.READ, ctx); + } Object retValue = nextInterceptor(ctx); // do a REAL remove here. NodeSPI n = cache.peek(from, true); if (n != null) { - lockManager.getLock(n).releaseAll(Thread.currentThread()); + n.getLock().releaseAll(Thread.currentThread()); } return retValue; } protected Object handleRemoveNodeMethod(InvocationContext ctx, GlobalTransaction tx, Fqn fqn, boolean createUndoOps) throws Throwable { - boolean created = acquireLocksWithTimeout(ctx, fqn, NodeLock.LockType.WRITE, true, true, false, true, false, false); + boolean created = acquireLocksWithTimeout(ctx, fqn, NodeLock.LockType.WRITE, true, false, true, false); + if (ctx.getGlobalTransaction() != null) + { + cache.getTransactionTable().get(ctx.getGlobalTransaction()).addRemovedNode(fqn); + } + acquireLocksOnChildren(cache.getRoot().getChildDirect(fqn), NodeLock.LockType.WRITE, ctx); Object retVal = nextInterceptor(ctx); if (ctx.getGlobalTransaction() == null) { @@ -157,7 +215,7 @@ NodeSPI n = cache.peek(fqn, true); if (n != null) { - lockManager.getLock(n).releaseAll(Thread.currentThread()); + n.getLock().releaseAll(Thread.currentThread()); } } // if this is a delete op and we had to create the node, return a FALSE as nothing *really* was deleted! @@ -166,7 +224,7 @@ protected Object handlePutForExternalReadMethod(InvocationContext ctx, GlobalTransaction tx, Fqn fqn, Object key, Object value) throws Throwable { - acquireLocksWithTimeout(ctx, fqn, NodeLock.LockType.WRITE, false, true, true, false, false, false); + acquireLocksWithTimeout(ctx, fqn, NodeLock.LockType.READ, true, true, false, true); return nextInterceptor(ctx); } @@ -177,104 +235,99 @@ protected Object handleRemoveDataMethod(InvocationContext ctx, GlobalTransaction tx, Fqn fqn, boolean createUndoOps) throws Throwable { - acquireLocksWithTimeout(ctx, fqn, NodeLock.LockType.WRITE, false, false, false, false, false, true); + acquireLocksWithTimeout(ctx, fqn, NodeLock.LockType.WRITE, false, false, false, false); return nextInterceptor(ctx); } protected Object handleAddChildMethod(InvocationContext ctx, GlobalTransaction tx, Fqn parentFqn, Object childName, Node cn, boolean createUndoOps) throws Throwable { - acquireLocksWithTimeout(ctx, parentFqn, NodeLock.LockType.WRITE, false, false, false, false, false, false); + acquireLocksWithTimeout(ctx, parentFqn, NodeLock.LockType.READ, false, false, false, false); return nextInterceptor(ctx); } protected Object handleEvictMethod(InvocationContext ctx, Fqn fqn) throws Throwable { - acquireLocksWithTimeout(ctx, fqn, NodeLock.LockType.WRITE, false, false, true, false, true, false); + acquireLocksWithTimeout(ctx, fqn, NodeLock.LockType.WRITE, false, true, false, false); return nextInterceptor(ctx); } protected Object handleGetKeyValueMethod(InvocationContext ctx, Fqn fqn, Object key, boolean sendNodeEvent) throws Throwable { - return acquireNonRecursiveReadLock(ctx, fqn); + acquireLocksWithTimeout(ctx, fqn, NodeLock.LockType.READ, false, false, false, false); + return nextInterceptor(ctx); } - private Object acquireNonRecursiveReadLock(InvocationContext ctx, Fqn fqn) - throws Throwable + protected Object handleGetNodeMethod(InvocationContext ctx, Fqn fqn) throws Throwable { - acquireLocksWithTimeout(ctx, fqn, NodeLock.LockType.READ, false, false, false, false, false, false); - return nextInterceptor(ctx); + acquireLocksWithTimeout(ctx, fqn, NodeLock.LockType.READ, false, false, false, false); + return nextInterceptor(ctx); } - protected Object handleGetNodeMethod(InvocationContext ctx, Fqn fqn) throws Throwable - { - return acquireNonRecursiveReadLock(ctx, fqn); - } - protected Object handleGetKeysMethod(InvocationContext ctx, Fqn fqn) throws Throwable { - return acquireNonRecursiveReadLock(ctx, fqn); + acquireLocksWithTimeout(ctx, fqn, NodeLock.LockType.READ, false, false, false, false); + return nextInterceptor(ctx); } protected Object handleGetChildrenNamesMethod(InvocationContext ctx, Fqn fqn) throws Throwable { - return acquireNonRecursiveReadLock(ctx, fqn); + acquireLocksWithTimeout(ctx, fqn, NodeLock.LockType.READ, false, false, false, false); + return nextInterceptor(ctx); } protected Object handlePrintMethod(InvocationContext ctx, Fqn fqn) throws Throwable { - return acquireNonRecursiveReadLock(ctx, fqn); + acquireLocksWithTimeout(ctx, fqn, NodeLock.LockType.READ, false, false, false, false); + return nextInterceptor(ctx); } protected Object handleReleaseAllLocksMethod(InvocationContext ctx, Fqn fqn) throws Throwable { - return acquireNonRecursiveReadLock(ctx, fqn); + acquireLocksWithTimeout(ctx, fqn, NodeLock.LockType.READ, false, false, false, false); + return nextInterceptor(ctx); } - private boolean acquireLocksWithTimeout(InvocationContext ctx, Fqn fqn, NodeLock.LockType lock_type, boolean recursive, boolean createIfNotExists, boolean zeroLockTimeout, boolean deleteOperation, boolean evictOperation, boolean removeDataOperation) + private boolean acquireLocksWithTimeout(InvocationContext ctx, Fqn fqn, NodeLock.LockType lockType, + boolean createIfNotExists, boolean zeroLockTimeout, + boolean acquireLockOnParent, boolean reverseRemoveCheck) throws InterruptedException { + if (fqn == null || configuration.getIsolationLevel() == IsolationLevel.NONE) + { + return false; + } boolean created; - long timeout = zeroLockTimeout ? 0 : getLockAcquisitionTimeout(ctx); + long timeout = zeroLockTimeout ? 0 : ctx.getContextLockAcquisitionTimeout(lock_acquisition_timeout); // make sure we can bail out of this loop long cutoffTime = System.currentTimeMillis() + timeout; boolean firstTry = true; do - { - // this is an additional check to make sure we don't try for too long. - if (!firstTry && System.currentTimeMillis() > cutoffTime) throw new TimeoutException("Unable to acquire lock on Fqn " + fqn + " after " + timeout + " millis"); - created = lock(ctx, fqn, lock_type, recursive, createIfNotExists, timeout, deleteOperation, evictOperation, removeDataOperation); + { + // this is an additional check to make sure we don't try for too long. + if (!firstTry && System.currentTimeMillis() > cutoffTime) + { + throw new TimeoutException("Unable to acquire lock on Fqn " + fqn + " after " + timeout + " millis"); + } + created = lock(ctx, fqn, lockType, createIfNotExists, timeout, acquireLockOnParent, reverseRemoveCheck); firstTry = false; } while (createIfNotExists && cache.peek(fqn, true) == null);// keep trying until we have the lock (fixes concurrent remove()) return created; } - //todo move to InvocationContext as it is information expert - private long getLockAcquisitionTimeout(InvocationContext ctx) - { - long timeout = lock_acquisition_timeout; - if (ctx.getOptionOverrides() != null - && ctx.getOptionOverrides().getLockAcquisitionTimeout() >= 0) - { - timeout = ctx.getOptionOverrides().getLockAcquisitionTimeout(); - } - return timeout; - } - - /** - * Locks a given node. - * - * @param lock_type DataNode.LOCK_TYPE_READ, DataNode.LOCK_TYPE_WRITE or DataNode.LOCK_TYPE_NONE - * @param recursive Lock children recursively - * @return true if the node had to be created + * Acquires locks on the node and on its parrents. Read locks are acquired for exsiting ancestors, with two exceptions: + * 1) createIfNotExists is true. If an ancestor is created on the fly, then an WL is acquired by default + * 2) acquireWriteLockOnParent is true. If so AND {@link org.jboss.cache.Node#isLockForChildInsertRemove()} then a read + * lock will be aquired for the parent of the node. + * @param createIfNotExists if true, then missing nodes will be cretaed on the fly. If false, method returns if we + * reach a node that does not exists + * @param reverseRemoveCheck see {@link #manageReverseRemove(org.jboss.cache.transaction.GlobalTransaction, org.jboss.cache.NodeSPI, boolean)} */ - private boolean lock(InvocationContext ctx, Fqn fqn, NodeLock.LockType lock_type, boolean recursive, boolean createIfNotExists, long timeout, boolean isDeleteOperation, boolean isEvictionOperation, boolean isRemoveDataOperation) + private boolean lock(InvocationContext ctx, Fqn fqn, NodeLock.LockType lockType, boolean createIfNotExists, long timeout, + boolean acquireWriteLockOnParent, boolean reverseRemoveCheck) throws TimeoutException, LockingException, InterruptedException { - NodeSPI n; - NodeSPI child_node; - Object child_name; Thread currentThread = Thread.currentThread(); GlobalTransaction gtx = ctx.getGlobalTransaction(); boolean created = false; @@ -283,240 +336,164 @@ { assertTransactionValid(ctx); } - Object owner = (gtx != null) ? gtx : currentThread; - int treeNodeSize; - + NodeSPI currentNode; if (log.isTraceEnabled()) log.trace("Attempting to lock node " + fqn + " for owner " + owner); - - if (fqn == null) - { - log.error("fqn is null - this should not be the case"); - return false; - } - - if (configuration.getIsolationLevel() == IsolationLevel.NONE) - { - lock_type = NodeLock.LockType.NONE; - } - - // we need to make sure this loop doesn't take forever (under a lot of concurrency) either as this can seem like a deadlock. - // apply a similar timeout check as is done in the loop that calls this lock() method. long expiryTime = System.currentTimeMillis() + timeout; - boolean reAcquisitionOnSameNode = false, rerunLoop = true; - - while (rerunLoop) + currentNode = cache.getRoot(); + NodeSPI parent = null; + String childName = null; + int currentIndex = -1; + do { - n = cache.getRoot(); - treeNodeSize = fqn.size(); - - for (int i = -1; i < treeNodeSize; i++) + if (currentNode == null) { - if (rerunLoop) rerunLoop = false; - created = false; - if (i == -1) + if (createIfNotExists) { - // this is the root node - child_name = Fqn.ROOT.getLastElement(); - child_node = n; + currentNode = parent.addChildDirect(new Fqn(childName)); + created = true; + if (log.isTraceEnabled()) log.trace("Child node was null, so created child node " + childName); } else { - child_name = fqn.get(i); - child_node = n.getChildDirect(child_name); - } - - // timeout check - if (reAcquisitionOnSameNode && System.currentTimeMillis() > expiryTime) throw new TimeoutException("Unable to acquire lock on child node " + new Fqn(n.getFqn(), child_name) + " after " + timeout + " millis."); - - if (log.isTraceEnabled()) log.trace("Directly got child node " + child_name); - if (child_node == null && createIfNotExists) - { - child_node = n.addChildDirect(new Fqn(child_name)); - created = true; - if (log.isTraceEnabled()) log.trace("Child node was null, so created child node " + child_name); - } - - if (child_node == null) - { - if (log.isTraceEnabled()) - { - log.trace("failed to find or create child " + child_name + " of node " + n); - } + if (log.isTraceEnabled()) log.trace("failed to find or create child " + childName + " of node " + currentNode); return false; } + } + NodeLock.LockType lockTypeRequired = NodeLock.LockType.READ; + if (created || writeLockNeeded(ctx, lockType, currentIndex, acquireWriteLockOnParent, createIfNotExists, fqn, currentNode)) + { + lockTypeRequired = NodeLock.LockType.WRITE; + } - NodeLock.LockType lockTypeRequired; - if (lock_type == NodeLock.LockType.NONE) - { - n = child_node; - continue; - } - else - { - if (created || writeLockNeeded(ctx, lock_type, i, treeNodeSize, isEvictionOperation, isDeleteOperation, createIfNotExists, isRemoveDataOperation, fqn, child_node)) - { - lockTypeRequired = NodeLock.LockType.WRITE; + manageReverseRemove(gtx, currentNode, reverseRemoveCheck); - } - else - { - lockTypeRequired = NodeLock.LockType.READ; - } - } + // actually acquire the lock we need. This method blocks. + acquireNodeLock(currentNode, owner, gtx, lockTypeRequired, timeout); - // reverse the "remove" if the node has been previously removed in the same tx, if this operation is a put() - if (gtx != null && needToReverseRemove(child_node, tx_table.get(gtx), lock_type, isDeleteOperation, createIfNotExists)) - { - reverseRemove(child_node); - } - - // actually acquire the lock we need. This method blocks. - acquireNodeLock(child_node, owner, gtx, lockTypeRequired, timeout); - - // make sure the lock we acquired isn't on a deleted node/is an orphan!! + // make sure the lock we acquired isn't on a deleted node/is an orphan!! + // look into invalidated nodes as well + NodeSPI repeek = cache.peek(currentNode.getFqn(), true, true); + if (currentNode != repeek) + { + if (log.isTraceEnabled()) log.trace("Was waiting for and obtained a lock on a node that doesn't exist anymore! Attempting lock acquisition again."); + // we have an orphan!! Lose the unnecessary lock and re-acquire the lock (and potentially recreate the node). + // check if the parent exists!! // look into invalidated nodes as well - NodeSPI repeek = cache.peek(child_node.getFqn(), true, true); - if (child_node != repeek)//repeek != null && child_node != repeek)// || repeek == null && created) + currentNode.getLock().releaseAll(owner); + if (cache.peek(parent.getFqn(), true, true) == null) { - log.trace("Was waiting for and obtained a lock on a node that doesn't exist anymore! Attempting lock acquisition again."); - // we have an orphan!! Lose the unnecessary lock and re-acquire the lock (and potentially recreate the node). - // check if the parent exists!! - // look into invalidated nodes as well - if (cache.peek(n.getFqn(), true, true) == null) - { - // crap! - log.trace("Parent has been deleted again. Go through the lock method all over again."); - child_node.getLock().releaseAll(owner); - rerunLoop = true; - i = treeNodeSize; - } - else - { - child_node.getLock().releaseAll(owner); - - // do the loop again, but don't assign child_node to n so that child_node is processed again. - i--; - reAcquisitionOnSameNode = true; - } - - continue; + // crap! + if (log.isTraceEnabled()) log.trace("Parent has been deleted again. Go through the lock method all over again."); + currentNode = cache.getRoot(); + parent = null; } else { - reAcquisitionOnSameNode = false; - } - - if (recursive && isTargetNode(i, treeNodeSize)) - { - Set<NodeLock> acquired_locks = lockManager.acquireAll(child_node, owner, lock_type, timeout); - if (acquired_locks.size() > 0) + // do the loop again, but don't assign child_node to currentNode so that child_node is processed again. + if (System.currentTimeMillis() > expiryTime) { - if (gtx != null) - { - cache.getTransactionTable().addLocks(gtx, acquired_locks); - } - else - { - List<NodeLock> locks = getLocks(currentThread); - locks.addAll(acquired_locks); - } + throw new TimeoutException("Unable to acquire lock on child node " + new Fqn(currentNode.getFqn(), childName) + " after " + timeout + " millis."); } } - n = child_node; } - } - - // Add the Fqn to be removed to the transaction entry so we can clean up after ourselves during commit/rollback - if (isDeleteOperation && gtx != null) cache.getTransactionTable().get(gtx).addRemovedNode(fqn); - + else + { + if (currentNode.getFqn().equals(fqn))//we've just processed the last child + { + break; + } + parent = currentNode; + currentIndex = currentNode.getFqn().size(); + currentNode = currentNode.getChildDirect(fqn.get(currentIndex)); + childName = (String) fqn.get(currentIndex); + } + } while (true); return created; } - private boolean needToReverseRemove(NodeSPI n, TransactionEntry te, NodeLock.LockType lockTypeRequested, boolean isRemoveOperation, boolean createIfNotExists) - { - return !isRemoveOperation && createIfNotExists && lockTypeRequested == NodeLock.LockType.WRITE && n.isDeleted() && te.getRemovedNodes().contains(n.getFqn()); - } - private void reverseRemove(NodeSPI n) + /** + * Acquires nodes on the children of this node. nodes on the node itself are not aquired. + * If the supplied parent node is null the method returns(no op). + */ + private void acquireLocksOnChildren(NodeSPI parentNode, NodeLock.LockType lockType, InvocationContext ctx) + throws InterruptedException { - n.markAsDeleted(false); + if (parentNode == null) + { + return; + } + long timeout = ctx.getContextLockAcquisitionTimeout(lock_acquisition_timeout); + GlobalTransaction gtx = ctx.getGlobalTransaction(); + Object owner = (gtx != null) ? gtx : Thread.currentThread(); + + Set<NodeLock> acquiredLocks = parentNode.getLock().acquireAll(owner, timeout, lockType); + if (acquiredLocks.size() > 0) + { + if (gtx != null) + { + cache.getTransactionTable().addLocks(gtx, acquiredLocks); + } + else + { + List<NodeLock> locks = getLocks(Thread.currentThread()); + locks.addAll(acquiredLocks); + } + } } - private boolean writeLockNeeded(InvocationContext ctx, NodeLock.LockType lock_type, int currentNodeIndex, int treeNodeSize, boolean isEvictOperation, boolean isRemoveOperation, boolean isPutOperation, boolean isRemoveDataOperation, Fqn targetFqn, NodeSPI currentNode) + /** + * Used by {@link #lock(org.jboss.cache.InvocationContext, org.jboss.cache.Fqn, org.jboss.cache.lock.NodeLock.LockType, boolean, long, boolean, boolean)}. + * Determins whter an arbitrary node from the supplied fqn needs an write lock. + */ + private boolean writeLockNeeded(InvocationContext ctx, NodeLock.LockType lockType, int currentNodeIndex, boolean acquireWriteLockOnParent, boolean createIfNotExists, Fqn targetFqn, NodeSPI currentNode) { + int treeNodeSize = targetFqn.size(); // write lock forced!! - boolean isTargetNode = isTargetNode(currentNodeIndex, treeNodeSize); - - if (ctx.getOptionOverrides().isForceWriteLock() && isTargetNode) return true; - + boolean isTargetNode = currentNodeIndex == (treeNodeSize - 1); + if (ctx.getOptionOverrides().isForceWriteLock() && isTargetNode) return true; + //this can be injected, from the caller as a param named wlParent if (currentNode.isLockForChildInsertRemove()) { - if (isRemoveOperation && currentNodeIndex == treeNodeSize - 2) + if (acquireWriteLockOnParent && currentNodeIndex == treeNodeSize - 2) { return true;// we're doing a remove and we've reached the PARENT node of the target to be removed. } - if (!isTargetNode && cache.peek(new Fqn(currentNode.getFqn(), targetFqn.get(currentNodeIndex + 1)), false) == null) { - return isPutOperation;// we're at a node in the tree, not yet at the target node, and we need to create the nextInterceptor node. So we need a WL here. + return createIfNotExists;// we're at a node in the tree, not yet at the target node, and we need to create the next node. So we need a WL here. } } - return lock_type == NodeLock.LockType.WRITE && isTargetNode && (isPutOperation || isRemoveOperation || isEvictOperation || isRemoveDataOperation);//normal operation, write lock explicitly requested and this is the target to be written to. + return lockType == NodeLock.LockType.WRITE && isTargetNode;//write lock explicitly requested and this is the target to be written to. } - private boolean isTargetNode(int nodePosition, int treeNodeSize) + private void acquireNodeLock(NodeSPI node, Object owner, GlobalTransaction gtx, NodeLock.LockType lockType, long lockTimeout) throws LockingException, TimeoutException, InterruptedException { - return nodePosition == (treeNodeSize - 1); - } - - private void acquireNodeLock(NodeSPI node, Object owner, GlobalTransaction gtx, NodeLock.LockType lock_type, long lock_timeout) throws LockingException, TimeoutException, InterruptedException - { - boolean acquired = lockManager.acquire(node, owner, lock_type, lock_timeout); + boolean acquired = node.getLock().acquire(owner, lockTimeout, lockType); if (acquired) { // Record the lock for release on method return or tx commit/rollback - recordNodeLock(gtx, lockManager.getLock(node)); + NodeLock lock = node.getLock(); + if (gtx != null) + { + cache.getTransactionTable().recordNodeLock(gtx, lock); + } + else + { + Thread currentThread = Thread.currentThread(); + List<NodeLock> locks = getLocks(currentThread); + if (!locks.contains(lock)) + { + locks.add(lock); + lock_table.put(currentThread, locks); + } + } } - else - { - //if (log.isDebugEnabled()) log.debug("Unable to acquire lock on node " + node.getFqn()); - //throw new LockingException("Could not acquite lock for " + node.getFqn()); - } } - private void recordNodeLock(GlobalTransaction gtx, NodeLock lock) + private List<NodeLock> getLocks(Thread currentThread) { - if (gtx != null) - { - // add the lock to the list of locks maintained for this transaction - // (needed for release of locks on commit or rollback) - try - { - cache.getTransactionTable().addLock(gtx, lock); - } - catch (CacheException e) - { - // may happen, if the transaction entry does not exist - lock.release(gtx); - throw e; - } - } - else - { - Thread currentThread = Thread.currentThread(); - List<NodeLock> locks = getLocks(currentThread); - if (!locks.contains(lock)) - { - locks.add(lock); - lock_table.put(currentThread, locks); - } - } - } - - //todo move to lock tabe as it is information xprt - private List<NodeLock> getLocks(Thread currentThread) - { // This sort of looks like a get/put race condition, but // since we key off the Thread, it's not List<NodeLock> locks = lock_table.get(currentThread); @@ -529,50 +506,25 @@ } - private void createNodes(Fqn fqn, GlobalTransaction gtx) + /** + * Test if this node needs to be 'undeleted' + * reverse the "remove" if the node has been previously removed in the same tx, if this operation is a put() + */ + private void manageReverseRemove(GlobalTransaction gtx, NodeSPI childNode, boolean reverseRemoveCheck) { - int treeNodeSize; - if ((treeNodeSize = fqn.size()) == 0) return; - NodeSPI n = cache.getRoot(); - for (int i = 0; i < treeNodeSize; i++) + boolean needToReverseRemove = reverseRemoveCheck && childNode.isDeleted() && tx_table.get(gtx).getRemovedNodes().contains(childNode.getFqn()); + if (gtx != null && needToReverseRemove) { - Object child_name = fqn.get(i); - Fqn childFqn = new Fqn(child_name); - - NodeSPI child_node = n.getChildDirect(childFqn); - if (child_node == null) child_node = n.addChildDirect(childFqn); - // test if this node needs to be 'undeleted' - // reverse the "remove" if the node has been previously removed in the same tx, if this operation is a put() - if (gtx != null && needToReverseRemove(child_node, tx_table.get(gtx), NodeLock.LockType.WRITE, false, true)) - { - reverseRemove(child_node); - } - - if (child_node == null) - { - if (log.isTraceEnabled()) - { - log.trace("failed to find or create child " + child_name + " of node " + n.getFqn()); - } - return; - } - n = child_node; + childNode.markAsDeleted(false); } } /** * Remove all locks held by <tt>tx</tt>, remove the transaction from the transaction table - * - * @param gtx - * todo move this logic in txTable as it is information expert for this class (this looks procedural) */ private void commit(GlobalTransaction gtx) { - if (log.isTraceEnabled()) - { - log.trace("committing cache with gtx " + gtx); - } - + if (log.isTraceEnabled()) log.trace("committing cache with gtx " + gtx); TransactionEntry entry = tx_table.get(gtx); if (entry == null) { @@ -590,104 +542,4 @@ } } - /** - * todo move this logic in txTable as it is information expert for this class (this looks procedural) - */ - private void cleanup(GlobalTransaction gtx) - { - if (log.isTraceEnabled()) log.trace("Cleaning up locks for gtx " + gtx); - TransactionEntry entry = tx_table.get(gtx); - // Let's do it in stack style, LIFO - if (entry != null) - entry.releaseAllLocksLIFO(gtx); - else - log.error("No transaction entry present!!", new Throwable()); - -/* - Transaction ltx = entry.getTransaction(); - if (log.isTraceEnabled()) - { - log.trace("removing local transaction " + ltx + " and global transaction " + gtx); - } - tx_table.remove(ltx); - tx_table.remove(gtx); -*/ - } - - /** - * Revert all changes made inside this TX: invoke all method calls of the undo-ops - * list. Then release all locks and remove the TX from the transaction table. - * <ol> - * <li>Revert all modifications done in the current TX<li/> - * <li>Release all locks held by the current TX</li> - * <li>Remove all temporary nodes created by the current TX</li> - * </ol> - * - * todo move this logic in txTable as it is information expert for this class (this looks procedural) - */ - private void rollback(GlobalTransaction tx) - { - TransactionEntry entry = tx_table.get(tx); - - if (log.isTraceEnabled()) - { - log.trace("called to rollback cache with GlobalTransaction=" + tx); - } - - if (entry == null) - { - log.error("entry for transaction " + tx + " not found (transaction has possibly already been rolled back)"); - return; - } - - Iterator removedNodes = entry.getRemovedNodes().iterator(); - CacheImpl cacheImpl = (CacheImpl) cache; - while (removedNodes.hasNext()) - { - Fqn f = (Fqn) removedNodes.next(); - cacheImpl.realRemove(f, false); - - } - - // 1. Revert the modifications by running the undo-op list in reverse. This *cannot* throw any exceptions ! - entry.undoOperations(cache); - - // This was removed as we don't use temporary nodes anymore; we now create undo-operations on put(), e.g. - // put(/a/b/c) on /a, create b and c, plus undo operations _remove(a/b/c) and _remove(/a/b) - - // 2. Remove all temporary nodes. Need to do it backwards since node is LIFO. - // for(ListIterator it=new LinkedList(entry.getNodes()).listIterator(entry.getNodes().size()); - // it.hasPrevious();) { - // node_name=(Fqn)it.previous(); - // try { - // cache._remove(tx, node_name, false); - // } - // catch(Throwable t) { - // log.error("failed removing node \"" + node_name + "\"", t); - // } - // } - - // 3. Finally, release all locks held by this TX - // Let's do it in stack style, LIFO - // Note that the lock could have been released already so don't panic. - } - - private static class LockManager - { - boolean acquire(NodeSPI node, Object owner, NodeLock.LockType lockType, long timeout) throws InterruptedException - { - return getLock(node).acquire(owner, timeout, lockType); - } - - NodeLock getLock(NodeSPI node) - { - return node.getLock(); - } - - public Set<NodeLock> acquireAll(NodeSPI node, Object owner, NodeLock.LockType lockType, long timeout) throws InterruptedException - { - return getLock(node).acquireAll(owner, timeout, lockType); - } - } - } Modified: core/trunk/src/main/java/org/jboss/cache/transaction/TransactionTable.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/transaction/TransactionTable.java 2007-12-10 21:57:40 UTC (rev 4824) +++ core/trunk/src/main/java/org/jboss/cache/transaction/TransactionTable.java 2007-12-10 23:47:41 UTC (rev 4825) @@ -230,6 +230,18 @@ entry.addLocks(locks); } + public void cleanup(GlobalTransaction gtx) + { + if (log.isTraceEnabled()) log.trace("Cleaning up locks for gtx " + gtx); + TransactionEntry entry = this.get(gtx); + // Let's do it in stack style, LIFO + if (entry != null) + entry.releaseAllLocksLIFO(gtx); + else + log.error("No transaction entry present!!", new Throwable()); + } + + /** * Returns summary debug information. */ @@ -264,4 +276,21 @@ return sb.toString(); } + /** + * Add the lock to the list of locks maintained for this transaction + * (needed for release of locks on commit or rollback) + */ + public void recordNodeLock(GlobalTransaction gtx, NodeLock lock) + { + try + { + addLock(gtx, lock); + } + catch (CacheException e) + { + // may happen, if the transaction entry does not exist + lock.release(gtx); + throw e; + } + } }