I am using RedisLockRegistry. After uploading the version to 5.5.7.RELEASE, a issue occurred.

[junhoi-pk]

In what version(s) of Spring Integration are you seeing this issue?

5.5.7.RELEASE (with spring boot version 2.6.2)

Describe the bug

        Lock lock = redisLockRegistry.obtain(key);
        boolean acquired = false;
        try {
            acquired = lock.tryLock(ttl, TimeUnit.MILLISECONDS);
            if (!acquired) {
                throw new RuntimeException(String.format("Failed to acquire lock [%s]", key));
            }
            // code
        } catch (InterruptedException e) {
            log.error(Throwables.getStackTraceAsString(e));
            Thread.currentThread().interrupt();
            throw new RuntimeException(String.format("Interrupted in locking [%s]", key));
        } finally {
            if (acquired) {
                try {
                    lock.unlock();
                } catch (Exception e) {
                    log.error(Throwables.getStackTraceAsString(e));
                }
            }
        }

To Reproduce

After updating the version, acquired is intermittently returned as false in the following code.
When I measured the time, it is presumed that it probably took a timeout and returned false.

It seems that unlock doesn't work properly locally, presumably intermittently.

There are several instances, but it seems that local lock release does not work properly as it only occurs on the instance with the upgraded version.

Expected behavior

A clear and concise description of what you expected to happen.

Reports that include a sample will take priority over reports that do not.
At times, we may require a sample, so it is good to try and include a sample up front.

[artembilan]

@Meteorkor ,

would you mind to take a look here?
Feels like a regression after introduction of pub-sub recently.

[Meteorkor]

@artembilan
If a vm uses tryLock() and another vm unlock it, it will wake up with an unlock message and try to acquire the lock, but if it fails(the other vm obtain lock), vm will return false.

There seems to be a problem that returns false without waiting for the time passed to tryLock(ttl).
I think we should retry until the time passed from subscribeLock()

And if you mix the versions, it will be a problem, but is this part okay?
Version 5.5.7 wakes up through unlock message, but lower version doesn't publish unlock message, so you may not be able to wake up.

[junhoi-pk]

@Meteorkor

Thank you for answer.
I tested it, but it returns false after waiting all the expire time.

Also, when I mixed it with the lower version, the problem seemed to occur, so I updated all the servers, but it occurred continuously.

Thx

[Meteorkor]

@junhoi-pk
Thank you for test.

Could you please upload the test code?

I know it still happens in mixed versions.
The fix is for the case of waking up early without trying until time.
but, There seems to be a slightly different problem.

[Meteorkor]

If you can't provide sample code that shows this problem.

It would also be helpful to provide a thread-dump in case of a waiting state.

[junhoi-pk]

@Meteorkor

I tested one more time.

If the versions are mixed, it waits for the expired time and returns false,
If the version has been updated in all instances, it returns false immediately without waiting.

I use this feature very often and it generates quite a bit of traffic.

Below is my code.

public class RedisLockAdvice {

    private final static ExpressionEvaluator<String> EVALUATOR = new ExpressionEvaluator<>();
    private final ApplicationContext applicationContext;

    @Around(value = "@annotation(redisLock)")
    public Object redisLock(ProceedingJoinPoint joinPoint, RedisLock redisLock) throws Throwable {
        Object[] args = joinPoint.getArgs();
        Class clazz = joinPoint.getTarget().getClass();
        Method method = ((MethodSignature) joinPoint.getSignature()).getMethod();
        EvaluationContext evaluationContext = EVALUATOR.createEvaluationContext(joinPoint.getTarget(), clazz, method, args);
        AnnotatedElementKey methodKey = new AnnotatedElementKey(method, clazz);

        String key = EVALUATOR.condition(redisLock.key(), methodKey, evaluationContext, String.class);
        RedisLockTemplate template = applicationContext.getBean(redisLock.lockTemplate(), RedisLockTemplate.class);
        RedisLockRegistry redisLockRegistry = template.getRedisLockRegistry();

        Lock lock = redisLockRegistry.obtain(key);
        boolean acquired = false;
        try {
            acquired = lock.tryLock(template.getTtl() + redisLock.afterDelay() + redisLock.beforeDelay(), TimeUnit.MILLISECONDS);
            if (!acquired) {
                throw new RuntimeException(String.format("Failed to acquire lock [%s]", key));
            }
            return joinPoint.proceed();
        } catch (InterruptedException e) {
            log.error(Throwables.getStackTraceAsString(e));
            Thread.currentThread().interrupt();
            throw new RuntimeException(String.format("Interrupted in locking [%s]", key));
        } finally {
            if (acquired) {
                try {
                    lock.unlock();
                } catch (Exception e) {
                    log.error(Throwables.getStackTraceAsString(e));
                }
            }
        }
    }
}

public class RedisLockTemplate {

    @Getter(AccessLevel.PACKAGE)
    private final RedisLockRegistry redisLockRegistry;

    @Getter(AccessLevel.PACKAGE)
    private final long ttl;

    public RedisLockTemplate(RedisConnectionFactory redisConnectionFactory, String name, long ttl) {
        this.redisLockRegistry = new RedisLockRegistry(redisConnectionFactory, name, ttl);
        this.ttl = ttl;
    }
}

@Configuration
public class RedisLockConfig {
    @Bean
    @Primary
    public RedisLockTemplate lockTemplate(RedisConnectionFactory redisConnectionFactory) {
        return new RedisLockTemplate(redisConnectionFactory, "common", 10000);
    }

    @Bean
    public RedisLockTemplate userLockTemplate(RedisConnectionFactory redisConnectionFactory) {
        return new RedisLockTemplate(redisConnectionFactory, "user", 5000);
    }
}

@Target({ElementType.TYPE, ElementType.METHOD})
@Retention(RetentionPolicy.RUNTIME)
public @interface RedisLock {
    String key();
    String lockTemplate() default "lockTemplate";
}

    @RedisLock(key = "'payment:' + #userId")
    public void test(Long userId) {
        // blabla
    }

[Meteorkor]

@junhoi-pk

If the version has been updated in all instances, it returns false immediately without waiting.

Does mean that even if the PR contents are applied, false is immediately returned?

[junhoi-pk]

@Meteorkor

Oh, I see.
But I can't use only the PR you coded, so I'll apply the update when it's officially released.

Thx

[Meteorkor]

@junhoi-pk

Thank you for answer.

oh i misunderstood
I understood that there was still a problem even after reflecting the PR code.

There doesn't seem to be any problem with that code.
Let's see if there are any other issues.

If you use a mixture of versions, there may be a problem of waiting until the expired time.
(It is possible to add logic to check repeatedly as in the past, but this is a worrying part because it is considered unnecessary.)

[junhoi-pk]

@Meteorkor

If the mixed version is not used, will the problem of returning false immediately be fixed with the PR?

[Meteorkor]

@junhoi-pk
Yes, the problem will be resolved.

After waking up with an unlock message, there seems to be a problem when trying to acquire a lock but failing.

It has been fixed because it is returning false without retrying even though the expiration time has not yet expired.