Unnecessary Value parameter for release/2,3

[hamidreza-s]

As type of locker_db is set so for every unique key there is just one unique value, so I wonder what is the purpose of passing Value as second parameter to release/2,3 functions?

In the release callback of locker's gen_server, the passed Value parameter just is used to matched with the Value returned from ets table, which seems to be unnecessary.

If it is the case, I can fix it and send a pull request.

[knutin]

Hey,

The purpose of including the value in all the calls (including extending the lease) is to ensure that no other client has changed the value in the time passed between acquiring the lock and the next operation.

Let's walk through an example where you lock lock user ids. The value is the pid of the process currently handling that user.

1. At startup of the process the lock is acquired, mapping user id to process id (which includes the node name)
2. Now let's say the process accidentally gets stuck or hibernates or something and is unable to extend it's lease within the required time.
3. The user reconnects and hits another node in the cluster, which is able to acquire the lock because the lease has expired.
4. That first process wakes up and tries to extend it's lock. As far as it can tell, the lock is still there so all is good, right? No, because the value has changed and the lock is now "owned" by a different process. Any operation attempted by this process should fail.

The approach taken here is pretty rudimentary and it doesn't solve the ABA problem. (That's something for locker 2.0 ...) For example there's no true ownership system (which could allow locker to only accept change requests from the owner). There's also no versioning of the value (which would solve the ABA problem). Also maybe the underlying two-phase lock with quorum mechanism could be replaced with the Raft algorithm. These are just thoughts at the moment, I don't have any plans to write another distributed locking system in the near future..

Best, Knut

On Mon, 13 Jun 2016 at 14:08 Hamidreza Soleimani notifications@github.com wrote:

As type of locker_db is set so for every unique key there is just one unique value, so I wonder what is the purpose of passing Value as second parameter to release/2,3 functions?

In the release callback https://github.com/wooga/locker/blob/master/src/locker.erl#L410-L428 of locker's gen_server, the passed Value parameter just is used to matched with the Value returned from ets table, which seems to be unnecessary.

If it is the case, I can fix it and send a pull request.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/wooga/locker/issues/11, or mute the thread https://github.com/notifications/unsubscribe/AAWPxFfupWPtc15avj8B_Y3uHJPyKsETks5qLVZkgaJpZM4I0Tyf .

[hamidreza-s]

Hi,

Thanks for the detailed explanation. Yes it seems that for solving the ABA problem when it comes to changing the state of previously locked items it can be a smart solution and works.

Assuming that each item in locker is a process name as key and process ID (PID) as value and also each process locks, extends, updates or releases itself. Now if calling the extend, update and release API for changing the state of the process is done by the process itself, so locker could compare the PID of the caller and the PID which is stored as value.

I know that it needs changing the API and might limit the use cases of locker or such, but I was just thinking out load.

Cheers.

[knutin]

Yes, the original plan was to do have some kind of ownership system, but if I remember correctly we ran into use cases where you don't want that. Without process ownership locker is more generic and could be useful in more situations, however maybe that's not a necessity.

There's users relying on the current semantics so we can't change locker itself at this point, except to fix bugs and add features. That's why I'm thinking that locker 2.0 would be a separate project in a separate repo.

Knut

On Tue, 14 Jun 2016 at 08:14 Hamidreza Soleimani notifications@github.com wrote:

Hi,

Thanks for the detailed explanation. Yes it seems that for solving the ABA problem when it comes to changing the state of previously locked items it can be a smart solution and works.

Assuming that each item in locker is a process name as key and process ID (PID) as value and also each process locks, extends, updates or releases itself. Now if calling the extend, update and release API for changing the state of the process is done by the process itself, so locker could compare the PID of the caller and the PID which is stored as value.

I know that it needs changing the API and might limit the use cases of locker or such, but I was just thinking out load.

Cheers.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/wooga/locker/issues/11#issuecomment-225798290, or mute the thread https://github.com/notifications/unsubscribe/AAWPxE0Q2Tf-2OFQG_GgXnvPiFbn1G5-ks5qLlTKgaJpZM4I0Tyf .

[hamidreza-s]

Yes, it is reasonable to go that way. I am interested to write such thing because I need it every so often. However if you start it first, and need some contributors, just ping me :)

[knutin]

I don't have any near terms plan on this, so if you start it and need contributors please ping me :)

On Tue, 14 Jun 2016 at 10:56 Hamidreza Soleimani notifications@github.com wrote:

Yes, it is reasonable to go that way. I am interested to write such thing because I need it every so often. However if you start it first, and need some contributors, just ping me :)

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/wooga/locker/issues/11#issuecomment-225834681, or mute the thread https://github.com/notifications/unsubscribe/AAWPxDA_oy1_2MIGCtx3LLXAwkMENRB-ks5qLnqygaJpZM4I0Tyf .