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

Wrong acquire() lock duration #299

Open
kotosha-real opened this issue Jun 8, 2024 · 0 comments
Open

Wrong acquire() lock duration #299

kotosha-real opened this issue Jun 8, 2024 · 0 comments

Comments

@kotosha-real
Copy link

kotosha-real commented Jun 8, 2024

Hello!

I noticed a significant difference in logic between the current GitHub source code and the latest release.

Latest release

  1. Storing the timestamp in the start variable at the start of the acquire() method.
  2. Attempt to acquire the lock.
  3. Calculate the expiration of the returning Lock instance based on the start variable.

This results in the Lock instance being created expired if there are enough retries, because the start is never recalculated. At the same time, the lock in Redis is perfectly fine. It leads to many problems that are not easy to trace.

Source code

  1. Attempt to acquire the lock.
  2. Calculate the expiration of the returning lock instance based on the start timestamp of the successful attempt.

This logic is fine, no bugs here. It was introduced in #276, but never published.

@mike-marcacci can you please release a new version with this commit?

Until then, this problem could be solved with a custom retry system that fits your scenario. For me it was:

const acquireLock = async (redlock: Redlock, resourses: string[], duration: number, timeout = 1000): Promise<Lock> =>
    new Promise((res) => {
        const interval = setInterval(async () => {
            try {
                const lock = await redlock.acquire(resourses, duration, { retryCount: 0 });

                clearInterval(interval);

                res(lock);
                // eslint-disable-next-line no-empty
            } catch (err) {}
        }, timeout);
    });
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant