RashadSaleh / mutually-exclusive-functions

Mutually exclusive execution of your JavaScript functions.
MIT License
1 stars 1 forks source link

Explain advantage over resource locking #2

Open mk-pmb opened 5 years ago

mk-pmb commented 5 years ago

Hi! I just skimmed over the readme, and couldn't find why someone could want mutex on a function level instead of on a resource level (in your readme example, a database record). If it's explained, please make it more prominent. If not, please explain it.

RashadSaleh commented 5 years ago

It is not always about resource locking. Consider the following example of a signup function:

async function signup({email, password}) {
  let exists = await db.exists({type: 'user', email: email});
  if (exists) throw new Error("Email already exists.");
  else await db.create({type: 'user', email: email, password: password});
}

If two users try to signup at the same time, the following order of operations might happen:

first signup checks database and finds no such email exists
second signup checks database and finds no such email exists
first signup creates a new user
second signup creates a new user with the same email address

In this case there is no data resource to lock, but instead it is the "execution resource" that needs to be locked: both can't have execution at the same time.

In general, however, I can't recommend one approach over the other (locking/"function mutex"). Preventing race conditions is a tricky business and I simply don't have enough real world experience to give an expert opinion.

I will make sure to incorporate this into the readme, and I thank you for raising this important issue.

mk-pmb commented 5 years ago

I simply don't have enough real world experience to give an expert opinion.

In that case, please do not suggest people use this module unless you have a use case where it is appropriate to mutex functions.

Your new example with creating DB users is misleading. The problem is that mutexing the functions will slow down registration a lot, because it denies not just registering the same email address, but all email addresses, until the current one is decided. This may be ok for small websites, but it is a bad pattern and people shouldn't be encouraged to learn how to do it the wrong way. It is indeed a resource locking problem here, albeit about a mental concept, not about a physical resource. You want to prevent simultaneous signup with the same email address, so "email address meant to be registered" is the resource to lock. The industry standard is to let the DB handle this: Just try to add the record, and if a duplicate field constraint for the email address is reported as error, you know it's already registered. I can think of several exotic scenarios that can not use dedupe via DB, but even in those, there are way better solutions than limiting the function call.

RashadSaleh commented 5 years ago

@mk-pmb

In the readme, I am careful to say:

In these cases, it can be the solution to make sure that competing functions execute one at a time, or what can be called mutually exclusive execution of the functions. This is the solution offered by this package. [emphasis added]

I intended that to mean exactly that I am not saying that it should. If that wasn’t clear enough, I am still to do the edits I mention in my last comment, which should resolve the issue.

However, I do agree that the main use cases for this library are missing from the documentation. The examples I list illustrate the concepts, but I did not intend to provide them as use cases. Yes, database concurrency should be delt with at the database level. However, not all race conditions are data races, so there should be cases where it is purely about execution. I am already working on those.

The rest of your comments where you make assertions about what is a good and what is a bad practice/solution I think can benefit from more qualification. However, do note that I edited your reply so that it more closely adheres to the Community Guidelines.