From 21ade0c798ed0fbb1444339af396325f324f94cb Mon Sep 17 00:00:00 2001 From: Steve Ebersole Date: Fri, 30 Nov 2012 12:09:15 -0600 Subject: [PATCH] HHH-1168 - Problem combining locking and paging on Oracle --- .../main/java/org/hibernate/LockOptions.java | 61 ++++++++++++++----- .../java/org/hibernate/dialect/Dialect.java | 8 +++ .../hibernate/internal/CoreMessageLogger.java | 12 +++- .../java/org/hibernate/loader/Loader.java | 19 +++++- .../loader/criteria/CriteriaLoader.java | 18 +++++- 5 files changed, 98 insertions(+), 20 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/LockOptions.java b/hibernate-core/src/main/java/org/hibernate/LockOptions.java index a86b8b4f91..016a8ced22 100644 --- a/hibernate-core/src/main/java/org/hibernate/LockOptions.java +++ b/hibernate-core/src/main/java/org/hibernate/LockOptions.java @@ -49,6 +49,7 @@ public class LockOptions implements Serializable { * UPGRADE represents LockMode.UPGRADE (will wait forever for lock and * scope of false meaning only entity is locked) */ + @SuppressWarnings("deprecation") public static final LockOptions UPGRADE = new LockOptions(LockMode.UPGRADE); /** @@ -65,7 +66,9 @@ public class LockOptions implements Serializable { private LockMode lockMode = LockMode.NONE; private int timeout = WAIT_FOREVER; - private Map aliasSpecificLockModes = null; //initialize lazily as LockOptions is frequently created without needing this + + //initialize lazily as LockOptions is frequently created without needing this + private Map aliasSpecificLockModes = null; public LockOptions() { } @@ -114,7 +117,7 @@ public class LockOptions implements Serializable { */ public LockOptions setAliasSpecificLockMode(String alias, LockMode lockMode) { if ( aliasSpecificLockModes == null ) { - aliasSpecificLockModes = new HashMap(); + aliasSpecificLockModes = new HashMap(); } aliasSpecificLockModes.put( alias, lockMode ); return this; @@ -135,7 +138,7 @@ public class LockOptions implements Serializable { if ( aliasSpecificLockModes == null ) { return null; } - return (LockMode) aliasSpecificLockModes.get( alias ); + return aliasSpecificLockModes.get( alias ); } /** @@ -159,6 +162,11 @@ public class LockOptions implements Serializable { return lockMode == null ? LockMode.NONE : lockMode; } + public boolean hasAliasSpecificLockModes() { + return aliasSpecificLockModes != null + && ! aliasSpecificLockModes.isEmpty(); + } + /** * Get the number of aliases that have specific lock modes defined. * @@ -183,6 +191,30 @@ public class LockOptions implements Serializable { return aliasSpecificLockModes.entrySet().iterator(); } + /** + * Currently needed for follow-on locking + * + * @return The greatest of all requested lock modes. + */ + public LockMode findGreatestLockMode() { + LockMode lockModeToUse = getLockMode(); + if ( lockModeToUse == null ) { + lockModeToUse = LockMode.NONE; + } + + if ( aliasSpecificLockModes == null ) { + return lockModeToUse; + } + + for ( LockMode lockMode : aliasSpecificLockModes.values() ) { + if ( lockMode.greaterThan( lockModeToUse ) ) { + lockModeToUse = lockMode; + } + } + + return lockModeToUse; + } + /** * Retrieve the current timeout setting. *

@@ -245,19 +277,20 @@ public class LockOptions implements Serializable { } /** - * Shallow copy From to Dest + * Perform a shallow copy * - * @param from is copied from - * @param dest is copied to - * @return dest + * @param source Source for the copy (copied from) + * @param destination Destination for the copy (copied to) + * + * @return destination */ - public static LockOptions copy(LockOptions from, LockOptions dest) { - dest.setLockMode(from.getLockMode()); - dest.setScope(from.getScope()); - dest.setTimeOut(from.getTimeOut()); - if ( from.aliasSpecificLockModes != null ) { - dest.aliasSpecificLockModes = new HashMap( from.aliasSpecificLockModes ); + public static LockOptions copy(LockOptions source, LockOptions destination) { + destination.setLockMode( source.getLockMode() ); + destination.setScope( source.getScope() ); + destination.setTimeOut( source.getTimeOut() ); + if ( source.aliasSpecificLockModes != null ) { + destination.aliasSpecificLockModes = new HashMap( source.aliasSpecificLockModes ); } - return dest; + return destination; } } diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java b/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java index ce2aa4653f..1b8cd205c4 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java @@ -2400,6 +2400,14 @@ public abstract class Dialect implements ConversionContext { return false; } + /** + * Some dialects have trouble applying pessimistic locking depending upon what other query options are + * specified (paging, ordering, etc). This method allows these dialects to request that locking be applied + * by subsequent selects. + * + * @return {@code true} indicates that the dialect requests that locking be applied by subsequent select; + * {@code false} (the default) indicates that locking should be applied to the main SQL statement.. + */ public boolean useFollowOnLocking() { return false; } diff --git a/hibernate-core/src/main/java/org/hibernate/internal/CoreMessageLogger.java b/hibernate-core/src/main/java/org/hibernate/internal/CoreMessageLogger.java index 20fce8b934..f4e10225c4 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/CoreMessageLogger.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/CoreMessageLogger.java @@ -45,6 +45,7 @@ import org.jboss.logging.Message; import org.jboss.logging.MessageLogger; import org.hibernate.HibernateException; +import org.hibernate.LockMode; import org.hibernate.cache.CacheException; import org.hibernate.dialect.Dialect; import org.hibernate.engine.loading.internal.CollectionLoadContext; @@ -1587,8 +1588,17 @@ public interface CoreMessageLogger extends BasicLogger { @LogMessage(level = WARN) @Message( value = "Encountered request for locking however dialect reports that database prefers locking be done in a " + - "separate select; results will be locked after initial query executes", + "separate select (follow-on locking); results will be locked after initial query executes", id = 444 ) void usingFollowOnLocking(); + + @LogMessage(level = WARN) + @Message( + value = "Alias-specific lock modes requested, which is not currently supported with follow-on locking; " + + "all acquired locks will be [%s]", + id = 445 + ) + void aliasSpecificLockingWithFollowOnLocking(LockMode lockMode); + } diff --git a/hibernate-core/src/main/java/org/hibernate/loader/Loader.java b/hibernate-core/src/main/java/org/hibernate/loader/Loader.java index 67d1907e49..66a618930d 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/Loader.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/Loader.java @@ -255,13 +255,16 @@ public abstract class Loader { List afterLoadActions) { if ( dialect.useFollowOnLocking() ) { LOG.usingFollowOnLocking(); - final LockOptions lockOptions = parameters.getLockOptions(); + // currently only one lock mode is allowed in follow-on locking + final LockMode lockMode = determineFollowOnLockMode( parameters.getLockOptions() ); + final LockOptions lockOptions = new LockOptions( lockMode ); + lockOptions.setTimeOut( parameters.getLockOptions().getTimeOut() ); + lockOptions.setScope( parameters.getLockOptions().getScope() ); afterLoadActions.add( new AfterLoadAction() { - private final LockOptions originalLockOptions = lockOptions.makeCopy(); @Override public void afterLoad(SessionImplementor session, Object entity, Loadable persister) { - ( (Session) session ).buildLockRequest( originalLockOptions ).lock( persister.getEntityName(), entity ); + ( (Session) session ).buildLockRequest( lockOptions ).lock( persister.getEntityName(), entity ); } } ); @@ -271,6 +274,16 @@ public abstract class Loader { return false; } + protected LockMode determineFollowOnLockMode(LockOptions lockOptions) { + final LockMode lockModeToUse = lockOptions.findGreatestLockMode(); + + if ( lockOptions.hasAliasSpecificLockModes() ) { + LOG.aliasSpecificLockingWithFollowOnLocking( lockModeToUse ); + } + + return lockModeToUse; + } + private String prependComment(String sql, QueryParameters parameters) { String comment = parameters.getComment(); if ( comment == null ) { diff --git a/hibernate-core/src/main/java/org/hibernate/loader/criteria/CriteriaLoader.java b/hibernate-core/src/main/java/org/hibernate/loader/criteria/CriteriaLoader.java index c5f0add9a6..521ffac762 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/criteria/CriteriaLoader.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/criteria/CriteriaLoader.java @@ -207,8 +207,9 @@ public class CriteriaLoader extends OuterJoinLoader { if ( dialect.useFollowOnLocking() ) { // Dialect prefers to perform locking in a separate step LOG.usingFollowOnLocking(); - final LockOptions lockOptionsToUse = new LockOptions(); - lockOptionsToUse.setLockMode( lockOptions.getEffectiveLockMode( "this_" ) ); + + final LockMode lockMode = determineFollowOnLockMode( lockOptions ); + final LockOptions lockOptionsToUse = new LockOptions( lockMode ); lockOptionsToUse.setTimeOut( lockOptions.getTimeOut() ); lockOptionsToUse.setScope( lockOptions.getScope() ); @@ -245,6 +246,19 @@ public class CriteriaLoader extends OuterJoinLoader { return dialect.applyLocksToSql( sql, locks, keyColumnNames ); } + + + protected LockMode determineFollowOnLockMode(LockOptions lockOptions) { + final LockMode lockModeToUse = lockOptions.findGreatestLockMode(); + + if ( lockOptions.getAliasLockCount() > 1 ) { + // > 1 here because criteria always uses alias map for the root lock mode (under 'this_') + LOG.aliasSpecificLockingWithFollowOnLocking( lockModeToUse ); + } + + return lockModeToUse; + } + protected LockMode[] getLockModes(LockOptions lockOptions) { final String[] entityAliases = getAliases(); if ( entityAliases == null ) {