[hibernate-dev] SQL Server lock hints misunderstanding

[Vlad Mihalcea]

Hi,

Besides UPGRADE_SKIPLOCKED, we also have the option of passing the
SKIP_LOCKED timeout to other LockModes, like PESSIMISTIC_READ:

Query query = session.createQuery(
      "select j from BatchJob j", BatchJob.class )
      .setMaxResults( maxResult )
      .unwrap( Query.class )
      .setLockOptions(
new LockOptions( LockMode.PESSIMISTIC_READ )
.setTimeOut( LockOptions.SKIP_LOCKED )
 )
);

After discussing with Chris and studying the docs in more depth,
I think we can come up with this LockMode to SQL Server Lock Hint mapping:
final String writeLockStr = lockOptions.getTimeOut() ==
LockOptions.SKIP_LOCKED ? "updlock" : "updlock, holdlock";
final String readLockStr = lockOptions.getTimeOut() ==
LockOptions.SKIP_LOCKED ? "updlock" : "holdlock";

final String noWaitStr = lockOptions.getTimeOut() == LockOptions.NO_WAIT ?
", nowait" : "";
final String skipLockStr = lockOptions.getTimeOut() ==
LockOptions.SKIP_LOCKED ? ", readpast" : "";

switch ( lockMode ) {
case UPGRADE:
case PESSIMISTIC_WRITE:
case WRITE: {
return tableName + " with (" + writeLockStr + ", rowlock" + noWaitStr +
skipLockStr + ")";
}
case PESSIMISTIC_READ: {
return tableName + " with (" + readLockStr + ", rowlock" + noWaitStr +
skipLockStr + ")";
}
case UPGRADE_SKIPLOCKED:
return tableName + " with (updlock, rowlock, readpast" + noWaitStr + ")";
default: {
return tableName;
}
}
I added more tests to prove this new lock mode mapping, and I'll send a
Pull Request tomorrow.

Vlad

On Thu, Mar 23, 2017 at 5:41 PM, Steve Ebersole <steve at hibernate.org> wrote:

> Part of the problem here is that the actual effects of these lock-modes
> really depend (in part) on the transaction-isolation-level configured for
> the Connection Hibernate uses, but unfortunately we do not necessarily know
> the isolation level - we do have a setting for letting the user configure
> an isolation level, but that is only intended for use by
> ConnectionProviders as they configure themselves.  Basically it allows the
> ConnectionProvider to set the isolation of the Connections it provides to
> the configured isolation, but Hibernate does not require this setting.  We
> could look to leverage Connection#getTransactionIsolation to know the
> isolation level if not configured via `hibernate.connection.isolation` -
> however that does not work in practice for various reasons.
>
> JPA says we can assume "at least" read-committed.
>
> {quote}
> This specification assumes the use of optimistic concurrency control.  It
> assumes that the databases to which persistence units are mapped will be
> accessed by the implementation using read-committed isolation (or a vendor
> equivalent in which long-term read locks are not held)...
> {quote}
>
> Here, for example you are asking about PESSIMISTIC_WRITE, PESSIMISTIC_READ
> and UPGRADE_SKIPLOCKED.  UPGRADE_SKIPLOCKED is a special case of
> PESSIMISTIC_WRITE, so let's ignore that for now.  So the intended outcome
> of both PESSIMISTIC_WRITE and PESSIMISTIC_READ are defined by JPA very well
> in section "3.4.4.2 PESSIMISTIC_READ, PESSIMISTIC_WRITE,
> PESSIMISTIC_FORCE_INCREMENT".  More-or-less:
>
> 1) PESSIMISTIC_READ  == repeatable read
> 2) PESSIMISTIC_WRITE == exclusive (write) lock
>
> Further, just to throw in here that JPA actually allows for a provider to
> essentially treat PESSIMISTIC_READ as if PESSIMISTIC_WRITE were requested.
> In general, we can be more restrictive but not less.
>
> AFAIU SQL Server uses the isolation level to figure out the types of locks
> to acquire unless specific lock hints (updlock, etc) are specified.  The
> thing I am unclear on though is exactly how SQL Server meshes specified
> lock hints with isolation level.  Some of the lock hint descriptions do
> explicitly discuss the effect of isolation.  Based on what I read, it would
> seem that the following would be reasonable:
>
> 1) PESSIMISTIC_READ -> `with (updlock, rowlock)`
> 2) PESSIMISTIC_WRITE -> `with (holdlock, rowlock)`
>
> As far as UPGRADE_SKIPLOCKED, I'd personally think that the intent better
> fits mapping that to PESSIMISTIC_WRITE + skipping.  But for SQL Server that
> would mean, essentially, PESSIMISTIC_READ + skipping because we'd have to
> use updlock rather than holdlock.  Since UPGRADE_SKIPLOCKED is proprietary
> (JPA does not support such a concept) this is fine I guess.
>
> On Thu, Mar 23, 2017 at 8:18 AM Christian Beikov <
> christian.beikov at gmail.com>
> wrote:
>
> > I'm not using SQL Server myself but this sounds reasonable.
> >
> >
> > Mit freundlichen Grüßen,
> > ------------------------------------------------------------------------
> > *Christian Beikov*
> > Am 23.03.2017 um 11:53 schrieb Vlad Mihalcea:
> > > --works
> > > select TOP(?) abstractsk0_.id as id1_0_, abstractsk0_.processed as
> > > processe2_0_ from BatchJob abstractsk0_ with (updlock, rowlock,
> readpast)
> > >
> > > --fails
> > > select TOP(?) abstractsk0_.id as id1_0_, abstractsk0_.processed as
> > > processe2_0_ from BatchJob abstractsk0_ with (holdlock, rowlock,
> > readpast)
> > >
> > > Hi,
> > >
> > > While working on this issue which adds support for SKIP_LOCKED for SQL
> > > server:
> > >
> > > https://hibernate.atlassian.net/browse/HHH-10654
> > >
> > > I came to question the way we use the lock hints based on the JPA or
> > > Hibernate LockMode(Type).
> > >
> > > Currently, we do like this:
> > >
> > > - PESSIMISTIC_WRITE -> UPDLOCK
> > > - PESSIMISTIC_READ  -> HOLDLOCK
> > >
> > > That's surprising since the HOLDLOCK is actually more restrictive than
> > > UPDLOCK.
> > >
> > > According to the officiala documentation  (
> > > https://msdn.microsoft.com/en-us/library/ms187373.aspx ) :
> > >
> > > UPDLOCK:
> > >
> > > "
> > > Specifies that update locks are to be taken and held until the
> > transaction
> > > completes.
> > > UPDLOCK takes update locks for read operations only at the row-level or
> > > page-level.
> > > If UPDLOCK is combined with TABLOCK,
> > > or a table-level lock is taken for some other reason, an exclusive (X)
> > lock
> > > will be taken instead.
> > > "
> > >
> > > HOLDLOCK:
> > >
> > > "
> > > Is equivalent to SERIALIZABLE. For more information, see SERIALIZABLE
> > later
> > > in this topic.
> > > HOLDLOCK applies only to the table or view for which it is specified
> > > and only for the duration of the transaction defined by the statement
> > that
> > > it is used in.
> > > "
> > >
> > > Now, the difference between these two is that UPDLOCK takes shared
> > > row-level locks while
> > > HOLDLOCK goes byond that and takes range locks as well.
> > >
> > > This assumption is backed by these StackOverflow answers:
> > >
> > >
> > http://stackoverflow.com/questions/7843733/confused-
> about-updlock-holdlock
> > >
> > >
> > http://stackoverflow.com/questions/42580238/why-does-
> sql-server-explicit-predicate-locking-disallow-insert-statements-outsid
> > >
> > > For SKIP_LOCKED, which is READPAST in SQL Server, we can't use HOLDLOCK
> > at
> > > all so we need to use UPDLOCK instead.
> > >
> > > Now, I think that both PESSIMISTIC_READ and PESSIMISTIC_WRITE should
> use
> > > HOLDLOCK,
> > > and only if we specify SKIP_LOCKED, we then switch to UPDLOCK instead.
> > >
> > > Let me know what you think?
> > >
> > > Vlad
> > > _______________________________________________
> > > hibernate-dev mailing list
> > > hibernate-dev at lists.jboss.org
> > > https://lists.jboss.org/mailman/listinfo/hibernate-dev
> >
> > _______________________________________________
> > hibernate-dev mailing list
> > hibernate-dev at lists.jboss.org
> > https://lists.jboss.org/mailman/listinfo/hibernate-dev
> >
> _______________________________________________
> hibernate-dev mailing list
> hibernate-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/hibernate-dev
>