Explicit locking in JPAStorageService

[Cantor, Scott]

On 11/5/15, 11:11 AM, "dev on behalf of Etienne Dysli-Metref" <dev-bounces at shibboleth.net on behalf of etienne.dysli-metref at switch.ch> wrote:



>On 05/11/15 00:28, Cantor, Scott wrote:
>> Yes, it's quite simple. The databases are broken. They don't always
>> even provide pessimistic locking, in fact, which is the reason for
>> the retry logic being necessary.
>
>My DBA colleagues would disagree. ;)

If they disagree that the database isn't taking the locks, that would be a database-specific determination. If you're having problems (again, outside of that cleanup issue), that itself implies it's probably not taking them as instructed. I've also seen the docs on postgres and mysql, and they both have pages and pages of material on locking behavior. That alone says that they're not just doing what they're told, they're trying to optimize it all and provide a lot of non-standard SQL options for influencing it.

>> Without the proper lock, the retry logic will kick in much more often
>> and fail much more often, under load. The SP code has proven that.
>
>So this retry logic is only there in case locking doesn't work as you
>expect?

Yes, same as in the SP. The IdP code is doing the same operations as the SP code, so it has exactly the same problems. And these are really simple tables. It's literally nothing more than refusing to honor ACID when a select-then-insert/update sequence is necessary.

>I don't know what the ISO/IEC SQL standard says, since it is not freely
>available, but locking a whole table seems excessive where a range lock
>would suffice.

It's not excessive, it's *mandatory*. If you don't take a table lock on a select with a transaction left open, then a follow up insert or update is subject to deadlock. When the operations are fast, that works fine.

>One could also set the isolation level on the JDBC DataSource, but then
>it would apply to all transactions and JPA doesn't provide a way to
>override it.

I was led to understand that wasn't something we could set with JPA. It is entirely fine for that to be a global setting. This code REQUIRES serializable transactions (otherwise known as, well, transactions).

-- Scott