node-redlock icon indicating copy to clipboard operation
node-redlock copied to clipboard

Wrong acquire() lock duration

Open kotosha-real opened this issue 8 months ago • 0 comments

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);
    });

kotosha-real avatar Jun 08 '24 08:06 kotosha-real