denoland / deploy_feedback

For reporting issues with Deno Deploy
https://deno.com/deploy
74 stars 5 forks source link

[KV Feedback]: Increase atomic mutation and check limit #418

Closed vwkd closed 1 year ago

vwkd commented 1 year ago

🔍

Type of feedback

General feedback

Description

Currently, atomic mutations and checks are each limited to 10. This is very low and easily hit in a non-trivial atomic mutation.

The reason is likely to ensure performance. But it should be up to the user to decide if decreased performance is acceptable instead of an arbitrarily hard API limit that makes non-trivial atomic mutations impossible.

A workaround to chain multiple API calls besides being less performant and error prone can only increase the limit to 20 atomic mutations. 10 in the first call due to max 10 mutation limit, 10 in the second call due to max 10 check limit, since the checks are needed to ensure consistency with all previous mutations.

This makes it currently impossible to do any non-trivial atomic mutations with Deno KV.

Steps to reproduce (if applicable)

  const db = await Deno.openKv(":memory:");
  await db.atomic()
    .set(["Book", "1", "id"], "1")
    .set(["Book", "1", "title"], "Shadows of Eternity")
    .set(["Book", "1", "authors", "11"], undefined)
    .set(["Book", "1", "authors", "12"], undefined)
    .set(["Book", "2", "id"], "2")
    .set(["Book", "2", "title"], "Whispers of the Forgotten")
    .set(["Book", "2", "authors", "11"], undefined)
    .set(["Book", "2", "authors", "12"], undefined)
    .set(["Author", "11", "id"], "11")
    .set(["Author", "11", "name"], "Victoria Nightshade")
    .set(["Author", "11", "books", "1"], undefined)
    .set(["Author", "11", "books", "2"], undefined)
    .set(["Author", "12", "id"], "12")
    .set(["Author", "12", "name"], "Sebastian Duskwood")
    .set(["Author", "12", "books", "1"], undefined)
    .set(["Author", "12", "books", "2"], undefined)
    .commit();
// error: TypeError: too many mutations (max 10)

Expected behavior (if applicable)

no error

Possible solution (if applicable)

Limited workaround to increase atomic mutations from 10 to 20, where the second call needs to add consistency checks for every mutation from the first call.

  const db = await Deno.openKv(":memory:");
  const { versionstamp } = await db.atomic()
    .set(["Book", "1", "id"], "1")
    .set(["Book", "1", "title"], "Shadows of Eternity")
    .set(["Book", "1", "authors", "11"], undefined)
    .set(["Book", "1", "authors", "12"], undefined)
    .set(["Book", "2", "id"], "2")
    .set(["Book", "2", "title"], "Whispers of the Forgotten")
    .set(["Book", "2", "authors", "11"], undefined)
    .set(["Book", "2", "authors", "12"], undefined)
    .commit();

  await db.atomic()
    .check({ key: ["Author", "11", "id"], versionstamp })
    .check({ key: ["Author", "11", "name"], versionstamp })
    .check({ key: ["Author", "11", "books", "1"], versionstamp })
    .check({ key: ["Author", "11", "books", "2"], versionstamp })
    .check({ key: ["Author", "12", "id"], versionstamp })
    .check({ key: ["Author", "12", "name"], versionstamp })
    .check({ key: ["Author", "12", "books", "1"], versionstamp })
    .check({ key: ["Author", "12", "books", "2"], versionstamp })
    .set(["Author", "11", "id"], "11")
    .set(["Author", "11", "name"], "Victoria Nightshade")
    .set(["Author", "11", "books", "1"], undefined)
    .set(["Author", "11", "books", "2"], undefined)
    .set(["Author", "12", "id"], "12")
    .set(["Author", "12", "name"], "Sebastian Duskwood")
    .set(["Author", "12", "books", "1"], undefined)
    .set(["Author", "12", "books", "2"], undefined)
    .commit();

Additional context

No response

igorzi commented 1 year ago

Fixed in https://github.com/denoland/deno/pull/20126.