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

Do not execute RELEASE_SCRIPT lock failed with one client #292

Open
ievgen-kapinos opened this issue Mar 16, 2024 · 0 comments
Open

Do not execute RELEASE_SCRIPT lock failed with one client #292

ievgen-kapinos opened this issue Mar 16, 2024 · 0 comments

Comments

@ievgen-kapinos
Copy link

ievgen-kapinos commented Mar 16, 2024

Hi !

First of all, thank you for creating so nice library and amazing documentation.

Setup

I'm running Node.js Web application in Kubernetes. Depends on traffic I have 8-30 pods. I do not have Redis Cluster and have only one redis client in code. I'm using redlock (5.0.0-beta.2). So my setup looks like this.

const redisClient = new Redis('my-redis-url', {
    enableOfflineQueue: false, // Do not collect commands in pending queue. Throw exception, if redis not available.
    maxRetriesPerRequest: 0, // Do not retry command. Throw exception, if redis not available.
});
     
const redlock = new Redlock([redisClient], {
   retryCount: 0, // One attempt or throw exception
   automaticExtensionThreshold: 5000,
})

I need lock to select only one (primary) pod to execute background job which updates cache. All other pods(secondary) retry to acquire lock, if failed then sleep for one minute. I'm using this approach:

await redlock.using(['my-backgroung-job-lock'], 15000, async signal => {
   // business code when lock is acquired
}

Investigation

All good on primary pod. I have 6 evalsha requests per minute (60000/ (15000-5000)) = 6

But on secondary pods amount of evalsha requests is looks to big. I expect only one request per pod, but have two. I've investigated code and found this part.

node-redlock/src/index.ts

Lines 330 to 333 in 5138813

} catch (error) {
// If there was an error acquiring the lock, release any partial lock
// state that may exist on a minority of clients.
await this._execute(this.scripts.releaseScript, resources, [value], {

This means that each secondary pod executes ACQUIRE_SCRIPT - that is expected. But if it fails, then RELEASE_SCRIPT executed unconditionally.

Proposal

We can improve performance (decrease amount of requests to Redis) and do not execute RELEASE_SCRIPT when:

  • number of clients is one
  • ACQUIRE_SCRIPT returns zero
@ievgen-kapinos ievgen-kapinos changed the title Do not execute RELEASE_SCRIPT if one client Do not execute RELEASE_SCRIPT lock failed with one client Mar 16, 2024
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