See more info here: #3095.

We probably really talk about the same matter - lease, TTL - should lend at the same functionality in the target lock implementation.

Not sure if we can do it for the current 5.5, but definitely worth to revise in 6.0 even if it is going to be a big breaking change.

WDYT?

@artembilan Thanks. Yes, I think we are on the same page! I call it TTL here because it is named TTL in the code of current implementation of org.springframework.integration.jdbc.lock.DefaultLockRepository.

	/**
	 * Specify the time (in milliseconds) to expire dead locks.
	 * @param timeToLive the time to expire dead locks.
	 */
	public void setTimeToLive(int timeToLive) {
		this.ttl = timeToLive;
	}

What I noticed is that in the mentioned issue you decide to introduce a new method tryLock with support of TTL (lease) into the Lock instance. I'm not sure how would you do it, as JDK's Lock doesn't support it. This will require client-code to cast Lock to something else and that's imply that client-code know what implementation of Lock it uses.

My suggestion here is to modify (or, more specifically, overload) LockRegistry.obtain() method. When you obtain the lock with specified TTL, this TTL will be just remembered inside the Lock implementation. And it will be applied only after you call some variant of lock() method on the given Lock instance.

public Lock obtain(Object lockKey, Duration ttl) {
	String path = pathFor((String) lockKey);
	return this.locks.computeIfAbsent(path, (key) -> new JdbcLock(this.client, this.idleBetweenTries, key, ttl)); // note new TTL argument here
}

And because TTL is not absolute instant of time, but relative value, it should work even if you trying to lock() long time after you called obtain(key, ttl).

I also discovered #3272 just now and see that my second "workaround" is already implemented in RenewableLockRegistry (in v5.4, while I use 5.3 so wasn't aware). Cool! :)

By the way, if this feature will be implemented, it would be better to change "renew" implementation to update new EXPIRATION_DATE field instead of CREATED_DATE. CREATED_DATE must never be changed after Lock creation then.

Actually, this not only desirable, but required, because new "expiration query" will have to delete expired locks based on EXPIRATION_DATE field instead of CREATED_DATE. Because 2 locks could be created at the same time but with different expiration dates.

Looks, like CREATED_DATE field will be redundant then. Could only be used for manual overview (how long this Lock is alive).

I think we applied the renewal logic on the CREATED_DATE to avoid breaking changes in the minor release.
For version 6.0 we definitely can consider to to have a new EXPIRATION_DATE column with all the mentioned lease or TTL logic.

My point against obtain(Object lockKey, Duration ttl) is because different instances may try to use different values for ttl when it is still the same lock instance. The ttl is really not an object property, but rather state. Therefore my point about new tryLock() API when any instance may decide how long it would like to keep the lock. In that linked issue I suggest to introduce a DistributedLock contract which is going to be returned from existing obtain() API. However not all LockRegistry are about distributed locks or just cannot implement DistributedLock contract (ZookeeperLockRegistry ?). So, we probably just need to introduce a generic argument on the LockRegistry to let the target impl to dictate us what the Lock extension we are going to deal with. This way a new tryLock(ttl) API will make it into end-user code for any peculiar decisions.

Does it make sense?

Yes, I think it does make sense.

You are talking about the situation when obtained Lock instance is shared between some bean instances/threads (of the same application instance) etc. And each bean/thread may want to set different TTL when trying to acquire that lock.

I just didn't thought about it this way. In my mind the relationship between obtain() and lock() methods was 1-to-1, which is not true. In reality it's 1-to-many. You can call obtain() one time and then call lock() multiple times on the same Lock (or DistributedLock) instance (potentially, with different TTL). There is also caching of obtained Lock instances in the registry implementations. So, you are right.

But then, it looks like renew() method should be part of DistributedLock contract too instead of be part of LockRegistry contract. What do you think?

Yes, that makes sense. Thanks.

We have renewal contract on the registry at the moment because of to big breaking change with a new DistributedLock abstraction. We will revise it the next version.

Feel free to fork the project and try something yourself! we also can review small PRs even if the feature is not complete yet or can't make it into the current version.

Any news on this enhancement?

See my previous comment:

because of to big breaking change with a new DistributedLock abstraction. We will revise it the next version.

So, we are going to tackle this in the next 6.0 version.

@artembilan Any news on this issue ? version version 6.0 is about to be release right ?

No, we didn't have a chance to look into this.
So, it is unlikely this will make it into the current 6.0 RC1 in two weeks.

Feel free to contribute some if you'd like.

Thank you for understanding!