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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

#82: Implement custom delay strategy support #122

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

meredian
Copy link

@meredian meredian commented Dec 6, 2021

Issue: #82

@mike-marcacci Sketch implementation of custom retry strategy. I've though about few options & decided to go with Strategy option since I decided to minimise changes to lock code, and best approach looked like this to me:

export interface RetryStrategy {
  getRetryCount: () => number;
  getRetryDelay: (attemptNumber: number) => number;
}

After reviewing implementation I had a second thought that it can be simplified with just passing some retryDelays: Array<number> parameter to config directly, and instead of strategy as object use function to generate arbitrary delay values.

e.g:

const redlock = new Redlock( ... {
  retryDelays: exponentialDelays({ startDelay: 200, expFactor: 2, maxDelay: 1000, retryCount: 5 })
});

being just passing an array:

const redlock = new Redlock( ... {
  retryDelays: [200, 400, 800, 1000, 1000]
});

It feels to me that it's even better considering overall node-redlock strive to simplicity 馃憤 Waiting for feedback.

@onichandame
Copy link

Just a thought of mine. A pre-defined array may not satisfy if infinite retries are demanded. An iterator/generator may be more suitable in this case.

@meredian
Copy link
Author

@onichandame I'm not sure that infinite retry is safe - since many clients can compete for acquiring a locks (e.g. you get a large burst of concurrent requests for single resource), you may end up with indeterministic time. Basically service may stop waiting for lock for unknown period of time, but you won't notice that. With fixed retry count there is a guaranty that lock will be either acquired or failed in some limited time.

@onichandame
Copy link

@meredian I agree that infinite retries are not desirable in most cases. Forgive my bad example, my main point is that iterators offer many more possibilities than a static array.
Another example would be exponential delay, where the delay period is dependent on the previous one. With iterators it can be done simply:

function *getDelay(){
  let count=0;
  while (count < 5)
    yield 2 ** (count ++)
}
const redlock = new Redlock( ... {
  retryDelays: getDelay()
});

The logic here is more clear and maintainable than giving an expanded array.

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

Successfully merging this pull request may close these issues.

None yet

2 participants