spring-projects / spring-integration

Spring Integration provides an extension of the Spring programming model to support the well-known Enterprise Integration Patterns (EIP)
http://projects.spring.io/spring-integration/
Apache License 2.0
1.54k stars 1.11k forks source link

Add configurable TTL for every Lock [INT-4399] #8342

Closed spring-operator closed 5 years ago

spring-operator commented 6 years ago

Nazar Vishka opened INT-4399 and commented

It will be a very useful feature to obtain lock from the same RedisLockRegistry with different TTL. It is a normal situation that for some features 1m lock is enough, and for other ones 10m will be OK. For now the only possible solution is to create bunch of lock repositories with different expireAfter parameter's value and it is annoying. If you could, please extend LockRegistry with new method:

Lock obtain(Object lockKey,  long expireAfter);

Affects: 5.0.1

spring-operator commented 6 years ago

Artem Bilan commented

I don't think that it is going to be so simple and straightforward.

First of all an obtain() contract is like this:

public Lock obtain(Object lockKey) {
    Assert.isInstanceOf(String.class, lockKey);
    String path = (String) lockKey;
    return this.locks.computeIfAbsent(path, RedisLock::new);
}

So, we reuse a RedisLock instance from the cache for the same lockKey. Therefore we are going to redo that logic too hard to bring such an expireAfter option to the target RedisLock.

Moreover we need think here about all other LockRegistry implementations...

What I see here as a solution is something like DistributedLock contract similar to what we have in the Hazelcast with their ILock:

boolean tryLock(long time, TimeUnit unit, long leaseTime, TimeUnit leaseUnit) throws InterruptedException;

void lock(long leaseTime, TimeUnit timeUnit);

This way we still reuse the same Lock object and open hooks to let to override expireAfter per lock operation.

That's still not so easy to implement for every LockRegistry and need to think more about the contract. For example I see like there is something like this getLockService().lockInterruptibly(this.key, waitInterval, getLockLeaseForLock()); in the DistributedRegion of the Gemfire support, but our GemfireLockRegistry is too plain to expose such a functionality.

Therefore I think we should move this issue to the next 5.1 because there is going to be enough breaking changes and the issue is much complicated than it is at a glance. Unless I'm missing something obvious and Gary can share with us his vision.

As a workaround I would suggest you to consider to take a look into exposed on the RedisLockRegistry a management operation:

/**
 * Remove locks last acquired more than 'age' ago that are not currently locked.
 * @param age the time since the lock was last obtained.
 * @throws IllegalStateException if the registry configuration does not support this feature.
 */
void expireUnusedOlderThan(long age);

Thanks

spring-operator commented 6 years ago

Gary Russell commented

Let's consider moving it to the backlog then; since this is "annoying" only (and there is a work around)

spring-operator commented 5 years ago

Artem Bilan commented

Duplicates https://github.com/spring-projects/spring-integration/issues/3095