bsm / redislock

Simplified distributed locking implementation using Redis
Other
1.45k stars 157 forks source link

Add multi-lock support for atomic multi-lock. #70

Closed bhallionOhbibi closed 11 months ago

bhallionOhbibi commented 1 year ago

Main changes

The purpose of thoses changes is to be able to obtain multiple lock in a single atomic operation. This is useful in many situation were you need to lock resources access by multiple process.

For exemple, if we have:

We want Process A to be able to lock resource 1 and therefore preventing Process B to access it until Process A is done.

I'v been siting on this update since August 2022 and have been using it on production without issues.

Other changes

bhallionOhbibi commented 1 year ago

Is there any chance someone takes a look a this ? :smile:

dim commented 1 year ago

@bhallionOhbibi apologies for the delay, but we have been exceptionally busy lately. We will take a look in the next few days

mxmCherry commented 1 year ago

Note: I'm all in for the Lua scripts instead of using Redis Pipelines here. We don't need any retvals from loop-ed batch operations (done within loops here as there's no / I have found no multi-key-EXPIRE command etc), so let's not waste "traffic" if we can do all the checks we want right in Lua.

bhallionOhbibi commented 1 year ago

Thanks for the review and for considering merging this. I suppose no changes are needed on my side since you plan on polish this to your taste (which I agree with btw). I wont touch the code anymore, unless you need me to.

mxmCherry commented 1 year ago

This one is now up to @dim (ping-ping). I can take over then, unless dim pushes for Pipelines (IMO, I personally prefer PR-ed Lua approach).

bhallionOhbibi commented 1 year ago

Ping

tomaswarynyca commented 1 year ago

@bhallionOhbibi do you have enough time to fix the problems?

bhallionOhbibi commented 1 year ago

I'll try to adresses thoses. I may have some questions

tomaswarynyca commented 1 year ago

@bhallionOhbibi I am very pleased to hear that you will be able to continue with the work! :+1: @dim @mxmCherry I'm sure they will help you with the questions you have!

bhallionOhbibi commented 1 year ago

I made a major update to this whole PR. I think there is no breaking changes and the changes are up to date with the latest version of main.

This is my second dive into lua scripting, bear with me if this is not very idiomatic. Feel free to comment on anything of course. I hope this only needs few changes to be of your liking before merging.

dim commented 1 year ago

Alternatively, I would not be opposed to having an ObtainMany which (atomically) obtains a slice of []*Lock. For example:

type MultiLock []*Lock

func (l MultiLock) MinTTL(ctx context.Context) (time.Duration, error)
func (l MultiLock) Release(ctx context.Context) error

func (c *Client) ObtainMany(ctx context.Context, keys []string, ttl time.Duration, opt *Options) (MultiLock, error)
bhallionOhbibi commented 1 year ago

About multi-lock and hypothetical partially held locks:

Theorically, you cannot hold one lock and not the other. msetnx make sure that you either set all the locks or none. Also, all locks held together share the same value and ttl. Therefore once you know that you and only you own all the locks, any of the commands (Refresh, TTL, Release) will come together nicely.

And for each of theses, if somehow you dont hold one lock, an error is returned and no refresh or release is done on any of the key among the multilock.

tomaswarynyca commented 1 year ago

@dim ping, what do you think of the changes made?

dim commented 1 year ago

I will take a look next week. I think most of my concerns have been addressed through the new release.lua, but need to double-check.

dim commented 1 year ago

It's a relatively big change, I think v0.10.0 is justified

tomaswarynyca commented 1 year ago

ping @bhallionOhbibi

bhallionOhbibi commented 11 months ago

Sorry ! I was on a very long vacation during october. I'll check your feedback soon. Thanks

bhallionOhbibi commented 11 months ago

I updated the code according to your feedback. I also updated the changelog. Please let me know if you're ok with the change on release.lua.

bhallionOhbibi commented 11 months ago

Ping :)

dim commented 11 months ago

Thanks for the epic effort :)

bhallionOhbibi commented 11 months ago

Thank you for all your feedbacks and time dedicated to this