JDBC provider - unlock logic ignores the lockedBy property

[rshisman]

hi @lukas-krecan

the unlock for the JDBC provider actually updated the lockUntil property in the database:
public String getUnlockStatement() { return "UPDATE " + tableName() + " SET " + lockUntil() + " = :unlockTime WHERE " + name() + " = :name"; }
Isn't it more safe to add "+ lockedBy() + " = :lockedBy" to the where clause?

[lukas-krecan]

Hi, originally, the purpose of lockedBy was only to ease debugging. Later I used it for lock extension and I agree that the fact that it's not used in unlocking does not feel consistent.
Let's think about the trade-offs:

• In normal situation, when the lock is held by only one node it does not matter if we take lockedBy into account or not - the condition will be always true. The only interesting case is a wierd situation when the lock is held by multiple nodes at once. I do not think it can happen but lets assume it can.
• If we take lockedBy into account, unlock may fail. What to do in such case? Try again? Throw an exception? Only log it and otherwise ignore it? The best way would IMO be to log it - we would see that the unexpected situation happened. This is the difference from the lock extension - the API forces you to expect that the extension may fail, there is nothing like that in unlock.

But let me think about it, not unlocking due to differing lockedBy and logging an error may be a bit better then unlocking a lock held by somebody else. The thing is that I do not like to do changes that do not have a clear benenfit because there are always some unintended consequeces.