Support lease renewal for distributed locks

[stefanvassilev]

Expected Behavior

One should be able to renew lock obtained from lockRegistry, as if one wants to hold onto a lock for longer than TTL.

Current Behavior

Calling jdbcLockRegistry.tryLock() from current thread would update the lock, which seems to renew it, however it would increase the hold's count as well, causing jdbcLock.unlock()' not to release the lock as it would only decrease delegate's hold's by one see: https://github.com/spring-projects/spring-integration/blob/62d472f679c6dba9b0810bc8b5ef20d0cccc329c/spring-integration-jdbc/src/main/java/org/springframework/integration/jdbc/lock/JdbcLockRegistry.java#L236

One can workaround this by using the 'DefaultLockRepository' to require a lock re-acquire a lock, which would update CREATED_DATE of the lock without increasing delegate's hold count.

Context

It would be great if this is supported from jdbcRegistry rather than going over the workaround I mentioned above.

[artembilan]

Hi @stefanvassilev !

I'm not sure that I fully understood how you workaround it and what the change you would like to do, but it looks more like you are looking for something like this: https://github.com/spring-projects/spring-integration/issues/3095. In other words a contract like this: tryLock(long time, TimeUnit unit, long leaseTime, TimeUnit leaseUnit) and lock(long leaseTime, TimeUnit timeUnit);

Please, share more info how we should fix it from your perspective.

Thanks

[stefanvassilev]

hi @artembilan,

thanks for the quick response.

The problem that I'm describing is the following:

Imagine you have two instances which use 'lockRegistry', instance A and instance B.

Instance 'A' acquires lock with name lockA and does some work, which takes more time than the TTL, so when instance 'B' tries to acquire lockA to acquire it it will succeed, as lockA would be expired and will get deleted.

My proposal would be to include a method to renew to the lock from instance A so that instance B won't be able to acquire it, as it won't be expired. The issue you linked would probably solve to a degree my problem, however, from API standpoint one should be probably be able to decide for how long the lock should is maintained.

I hope this clarifies what I'm trying to describe.

[artembilan]

Hm. Probably it sounds like an addition to the mentioned tryLock(long time, TimeUnit unit, long leaseTime, TimeUnit leaseUnit)API - renewLease(long leaseTime)...

[stefanvassilev]

seems reasonable to me :-)

[artembilan]

So, since we agree about this feature, I make it official. Let's see what we can do for the next 5.4 version!

Probably not all distributed locks could have that feature. I don't see a way in Curator Framework and looks like Hazelcast went a raft algorithm way, so there is no lease in their new FencedLock at all.

[artembilan]

Reopen since we need to revise all other LockRegistry implementations to be sure that they can support this renewal feature.

[lubronzhan]

Thanks @artembilan Is there any plan to add this renewLock to this part of logic? https://github.com/spring-projects/spring-integration/blob/master/spring-integration-core/src/main/java/org/springframework/integration/support/leader/LockRegistryLeaderInitiator.java#L400-L408 Or we have to wait for modification on other LockRegistry first?

[artembilan]

Hm. I think with a renewable feature we need to have a slightly different logic in that initiator. Probably the whole main loop should be adjusted respectively.

I didn't think it through yet, but if you have something in mind, feel free to raise a Pull Request and we will look together what and how should be fixed over there to support that RenewableLockRegistry.

Good catch though!