Uniqueness validation is prone to race conditions.

[qubyte]

I'm not really sure what can be done about this, since any fix would likely be at the adaptor level. We currently see uniqueness violations when two model instances with an identical (asserted unique) field are created very close together (i.e. close enough that the uniqueness has been tested for both before the first new instance is created). For example, if you create two new users with the same (should be unique) username at the same time, then they will succeed.

[dgsan]

In MySQL adapter you should be able to use kind: unique for your indexes, which should then make it so one of the two instances gets an error from the DB since the constraint will be DB level. I'm not sure how well it's documented/covered by test cases, though, or how analogous/applicable it is for other adapters. The idea of unique indexes is kind of weird for Redis, just as an example.

[qubyte]

It would be trivial to implement in redis with a small lua script. (I've already been doing this to implement things like increments, which are also prone to race conditions.)

[qubyte]

Of course, it could be that this is already available in the adaptor I use (redis-hq), but I find that code ridiculously opaque. Even with these index fields available, the function that this issue refers to should be removed. It's dangerous and misleading, and I'm seeing the issue manifest.

[randunel]

In mongodb, indices have to be used for atomic inserts with uniqueness. As a mongodb sidenote, $addToSet has to be used for uniqueness in nested arrays, but this validation is not supported by jugglingdb.

Every server has its own implementation, so only the adapters can be responsible for atomic operations and certain validations.

[qubyte]

Another option would be to have the existing API and delegate the uniqueness assertion to the adaptor. Just so long as it never exists in a way that allows this race condition.

[pocesar]

per schema base adapter "lock" would suffice to fight any racing conditions, like find with delete, insert with update, insert with insert, etc. the lock would last as most as a process tick (usually less than 1ms), and should go unnoticed with regards of performance. although the new https://github.com/jugglingdb/validations could implement this and leave out the need to implement it at adapter level.

[qubyte]

If I'm reading that right, you're suggesting a pure JS solution, which probably won't work in Node.js clusters.

[pocesar]

yeah, you are reading right, and you are also right... forgot about clusters. maybe create an intermediary memory repository to keep the read/writes/inserts that are being validated/executed and that could be looked up using cluster API or any other IPC. using a intermediary database for this would defeat the purpose and add a lot of overhead

[dgsan]

I'm against putting uniqueness constraint checking in core javascript. It'd basically be completely redundant with letting the backing DB maintain uniqueness checks for all the adapters backed by SQL.

[pocesar]

it's not the uniqueness that is in Javascript, it's the "lock". for example, InnoDB is row locking, while MYISAM is table locking. If you don't define an UNIQUE index, even with locking, you'll have a racing condition, it WRITES might be executed after READS depending on your configuration and query (LOW PRIORITY or INSERT IGNORE). By offloading this lock to the Javascript code, you don't need to worry about specific atomic writes/reads and locking mechanisms, and since Jugglingdb is all about consistency for many database, I wouldn't see a drawback in implementing this, unless of course, add the complexity of a clustered environment.

[dgsan]

Safe difference. Let the DB handle it. Sure, MySQL has different storage engines with different locking behaviours, but so long as you define your unique constraints/indexes correctly it will take care of itself. Offloading the lock to JS in the case of MySQL just makes it so there are two identical locks controlling one resource, which is silly. If JDB needs consistency of syntax, just let the central method of setting kind: unique be handled by the adapters individually and not by core.

[qubyte]

@pocesar An in-memory intermediary won't work either because it's common to also have multiple webservers hosting your app. The only way to implement this in JS land is to have a gatekeeper service for your database, and that sounds like adding a lot of complexity. The only sane way I can see for this to be done is via the adaptor creating the right constraints / stored procedures for the DB to manage itself.

[qubyte]

@dgsan Can you point me to the area of the mysql adaptor that handles kind: 'unique', and also where in JugglingDB this is handled too? This is absolutely killing us for the redis-hq adaptor and I'll put the time in myself if it means getting this fixed.

[dgsan]

https://github.com/jugglingdb/mysql-adapter/blob/master/lib/mysql.js#L596

Starting at line 596 it handles column property declared stuff, then on 621 it does things declared under indexes. As I think I noted previously it is among the features in need of better unit tests. I can vouch for the multicolumn declaration syntax starting at 621 working, but for all I know the 596 section might be borked. They're also related to sections at lines 710 and 728 to the degree that there's a good opportunity for someone to do some code cleanup.

As far as JDB itself, I don't think it really does much directly related to the feature - it leaves it to the adapter.