vwkd / denokv-graphql

GraphQL bindings for Deno KV
https://deno.land/x/denokv_graphql
MIT License
11 stars 0 forks source link

Improve consistency #11

Closed vwkd closed 1 year ago

vwkd commented 1 year ago
vwkd commented 1 year ago

Nested queries are now atomically consistent with https://github.com/vwkd/graphql-denokv/commit/1f03750aaf8605d516055851ad7389f58a51039e

But it assumes that writes happen consistently too. Currently, we can only write consistently to one table at a time. Writing nested data is not atomically consistent, because needs to do multiple mutations after each other, set the referenced row, get the id back, and insert the referencing row with that id.

How do we make that consistent?

Even if accepted versionstamp in mutation argument and would do a check, in the failure case it would already have the previously written part of the data. We need to bundle it all data in one mutation call and either write it all or nothing.

vwkd commented 1 year ago

Mutations now use atomic transactions with https://github.com/vwkd/graphql-denokv/commit/bb3eccd91d8f89a577b048c88210425976b21c77

This solves all of the points above. The only remaining point above is exposing the weak concurrency option in queries and consistency of list for large number of entries, which can be done later.

vwkd commented 1 year ago

This isn't quite solved yet. Currently, there's no versionstamp for an assembled row, see https://github.com/denoland/deploy_feedback/issues/422 As a placeholder, a query currently returns the versionstamp of the 'id' field and mutations only check the 'id' field which doesn't ensure consistency yet.

vwkd commented 1 year ago

We can use the 'id' field versionstamp as versionstamp for the whole row, as long as we keep it updated when any field is updated such that it's always the newest versionstamp. The README mentions this now with https://github.com/vwkd/denokv-graphql/commit/c132f1b53f07e8d2ce4cac5d0ab3626e4865716b and field versionstamps are now verified with https://github.com/vwkd/denokv-graphql/commit/6345f7805d05bad455615b00fe82580f9fba9030. This introduces potential for consistency bugs if future additions like https://github.com/vwkd/denokv-graphql/issues/2 forget to check this. Unfortunately it seems the only way to do this.

An alternative was using the largest versionstamp of all fields. But it would require changing the whole resolver chain from reading each field individually in the leaf resolver to reading all fields in the root resolver such that we can compare and get the largest versionstamp. Also, we would need to maintain a patched library until/if Deno implements https://github.com/denoland/deploy_feedback/issues/422.