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

Lock in use should throw a specific error #288

Open
slavafomin opened this issue Dec 15, 2023 · 3 comments
Open

Lock in use should throw a specific error #288

slavafomin opened this issue Dec 15, 2023 · 3 comments

Comments

@slavafomin
Copy link

Hello!

Consider the following code:

const redlock = new Redlock([redis]);

const promises = [];

promises.push(run());
promises.push(run());

await Promise.all(promises);

async function run() {

  try {

    await redlock.using(['foo'], 10_000, {}, async () => {
      logger.warn(`Processing`);
      await wait(5_000);
    });

  } catch (error: unknown) {

    logError({ logger, error });

  }

}

It tries to acquire the same lock two times: the first time successful and the second time with error (which is expected). However, the error that is thrown is: ExecutionError: The operation was unable to achieve a quorum during its retry window.. I'm not sure about the quorum, but shouldn't the ResourceLockedError be thrown instead?

Also, what retryCount and retryDelay parameters are used for exactly? Are they used for multiple tries when acquiring a quorum or are they also used while waiting for existing lock to release?

Thank you.

@JohnWilliamForcier-Spiria
Copy link

JohnWilliamForcier-Spiria commented Jan 4, 2024

My workaround is to used the error attempts returned in the catch,


await redlockService
   .using([key], 1000, settings, async () => {
      // method here
   })
   .catch(async (err: ExecutionError | ResourceLockedError) => {
      let isAlreadyLocked = err instanceof ResourceLockedError;
      if (err instanceof ExecutionError && err.attempts.length) {
         const attemptError = (await err.attempts.at(0)).votesAgainst.values().next().value;
         isAlreadyLocked = attemptError instanceof ResourceLockedError;
      }

      if (!isAlreadyLocked) {
          throw err;
      }
   });

@cswww
Copy link

cswww commented Aug 20, 2024

我的解决方法是使用 catch 中返回的错误尝试,


await redlockService
   .using([key], 1000, settings, async () => {
      // method here
   })
   .catch(async (err: ExecutionError | ResourceLockedError) => {
      let isAlreadyLocked = err instanceof ResourceLockedError;
      if (err instanceof ExecutionError && err.attempts.length) {
         const attemptError = (await err.attempts.at(0)).votesAgainst.values().next().value;
         isAlreadyLocked = attemptError instanceof ResourceLockedError;
      }

      if (!isAlreadyLocked) {
          throw err;
      }
   });

This cannot truly lock the key

@anteqkois
Copy link

@slavafomin @JohnWilliamForcier-Spiria
I have resolved this issue with these changes. The key is to overwrite release method and check if lock expired.

import Redlock from 'redlock'
import { redis } from '../storage/redis'

export const redlock = new Redlock([redis], {
  // The expected clock drift; for more details see:
  // http://redis.io/topics/distlock
  driftFactor: 0.01, // multiplied by lock ttl to determine drift time
  // The max number of times Redlock will attempt to lock a resource
  // before erroring.
  retryCount: 30,
  // the time in ms between attempts
  retryDelay: 2000, // time in ms
  // the max time in ms randomly added to retries
  // to improve performance under high contention
  // see https://www.awsarchitectureblog.com/2015/03/backoff.html
  retryJitter: 200, // time in ms
  // The minimum remaining time on a lock before an extension is automatically
  // attempted with the `using` API.
  automaticExtensionThreshold: 500, // time in ms
})

// Save the original acquire method
const originalAcquire = Redlock.prototype.acquire

// Override the release method for all lock instances
Redlock.prototype.acquire = async function (...args) {
  const duration = args[1] // this is a duration value

  // use the duration to create additional settings
  args[2] = {
    retryCount: Math.ceil((duration / 2_000) * 1.5),
    retryDelay: 2_000,
    ...args[2],
  }

  return originalAcquire.apply(this, args) // Call the original release method
}

// Save the original release method
const originalRelease = Redlock.prototype.release

// Override the release method for all lock instances
Redlock.prototype.release = async function (...args) {
  const now = new Date().getTime()

  if (args[0] && args[0].expiration > now) {
    // Check if the lock still exists
    return originalRelease.apply(this, args) // Call the original release method
  }

  return {
    attempts: [],
  }
}

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

4 participants