Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add idleBetweenTries property to RedisLockRegistry #9540

Closed
Ichanskiy opened this issue Oct 8, 2024 · 0 comments · Fixed by #9541
Closed

Add idleBetweenTries property to RedisLockRegistry #9540

Ichanskiy opened this issue Oct 8, 2024 · 0 comments · Fixed by #9541

Comments

@Ichanskiy
Copy link
Contributor

Ichanskiy commented Oct 8, 2024

Hello,
We are using RedisLockRegistry in our project and noticed that, there is a Redis query every 100 milliseconds to check for the lock:

@Override
protected boolean tryRedisLockInner(long time) throws InterruptedException {
    long now = System.currentTimeMillis();
    if (time == -1L) {
        while (!obtainLock()) {
            Thread.sleep(100); //NOSONAR
        }
        return true;
    } else {
        long expire = now + TimeUnit.MILLISECONDS.convert(time, TimeUnit.MILLISECONDS);
        boolean acquired;
        while (!(acquired = obtainLock()) && System.currentTimeMillis() < expire) { //NOSONAR
            Thread.sleep(100); //NOSONAR
        }
        return acquired;
    }
}		

The issue is that we can't configure the sleep duration between these requests (Thread.sleep(100); //NOSONAR), resulting in a large number of Redis requests.
In JdbcLockRegistry, there is a similar property, idleBetweenTries, that allows this configuration.

My suggestion is to add a similar configuration option for RedisLockRegistry. Specifically:

  1. Add a field:
    private Duration idleBetweenTries = Duration.ofMillis(DEFAULT_IDLE);
  2. Provide a setter method:
public void setIdleBetweenTries(Duration idleBetweenTries) {
    Assert.notNull(idleBetweenTries, "'idleBetweenTries' must not be null");
    this.idleBetweenTries = idleBetweenTries;
}
  1. Use the idleBetweenTries in the tryRedisLockInner() method:
@Override
protected boolean tryRedisLockInner(long time) throws InterruptedException {
    long now = System.currentTimeMillis();
    if (time == -1L) {
        while (!obtainLock()) {
            Thread.sleep(idleBetweenTries.toMillis()); //NOSONAR
        }
        return true;
    } else {
        long expire = now + TimeUnit.MILLISECONDS.convert(time, TimeUnit.MILLISECONDS);
        boolean acquired;
        while (!(acquired = obtainLock()) && System.currentTimeMillis() < expire) { //NOSONAR
            Thread.sleep(idleBetweenTries.toMillis()); //NOSONAR
        }
        return acquired;
    }
}
@Ichanskiy Ichanskiy added status: waiting-for-triage The issue need to be evaluated and its future decided type: enhancement labels Oct 8, 2024
Ichanskiy pushed a commit to Ichanskiy/spring-integration that referenced this issue Oct 8, 2024
@artembilan artembilan added in: redis and removed status: waiting-for-triage The issue need to be evaluated and its future decided labels Oct 8, 2024
@artembilan artembilan added this to the 6.4.0-RC1 milestone Oct 8, 2024
Ichanskiy pushed a commit to Ichanskiy/spring-integration that referenced this issue Oct 9, 2024
Ichanskiy pushed a commit to Ichanskiy/spring-integration that referenced this issue Oct 9, 2024
artembilan pushed a commit that referenced this issue Oct 9, 2024
Fixes: #9540
Issue link: #9540

**Auto-cherry-pick to `6.3.x`**
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment