search

rethinkdb / rethinkdb_rebirth

The open-source database for the realtime web.
https://rethinkdb.com
Other
999 stars 42 forks source link

Deterministic js #4

Closed asakatida closed 6 years ago

asakatida commented 6 years ago

Description

This builds on #2 and puts the duktape evaluation in process. The JS term is then marked deterministic.

thelinuxlich commented 6 years ago

this is great, hope it will alleviate issues with missing ReQL features like Math.sqrt

thelinuxlich commented 6 years ago

@atris can you cherry pick it to the 2.4 branch?

lbguilherme commented 6 years ago

This is a problem, because r.js is not always deterministic. Take r.js("Math.random()"). As far as I understand there is the hard assumption that deterministic terms are always deterministic and this assumption is used to avoid transferring documents between primary and secondary replicas and instead run the update function on every replica when the update is atomic. This change might break replication!

thelinuxlich commented 6 years ago

You are right, but then this is the case where we leave part of the responsibility on the user side, like the conflict functions.

thelinuxlich commented 6 years ago

it is just a matter of documenting the edge cases

lbguilherme commented 6 years ago

I'm not sure they compare. If I'm indeed right then a irresponsible user can write an "atomic" update that causes the table replicas do diverge without warning and possible crash the database. This is not ok.

thelinuxlich commented 6 years ago

The irresponsible user should read the docs before fiddling with r.js then.

A irresponsible user can lockdown the entire cluster by writing some kind of fork bomb inside the conflict function for inserts, it's the same for example with Redis Lua scripts. This opens a lot of potential to be dismissed just because of edge cases.

atris commented 6 years ago

I am slightly hesitant to merge since this may open a security leak in the engine. On no account should the engine stop over due to a user error. In the minimum, we should check against these cases if it is possible. Can you investigate that?

asakatida commented 6 years ago

The engine is now isolated to a per query error condition. Are you looking for tests that an exception in query A doesn't affect query B? Just wanted to clarify my next steps.

thelinuxlich commented 6 years ago

This will enable a lot of cool stuff in ReQL :)

atris commented 6 years ago

Yes please,some tests would be good

On Mon, 2 Jul 2018, 01:10 Adam Grandquist, notifications@github.com wrote:

The engine is now isolated to a per query error condition. Are you looking for tests that an exception in query A doesn't affect query B? Just wanted to clarify my next steps.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/RebirthDB/rebirthdb/pull/4#issuecomment-401628314, or mute the thread https://github.com/notifications/unsubscribe-auth/ABpO49s0ux4ImfUP7TsQgQrE8zmjTtTgks5uCSWSgaJpZM4UT7Se .