search

kitsonk / kv-toolbox

Utilities for working with Deno KV 🦕🗝️
https://kview.deno.dev/kv-toolbox
MIT License
66 stars 5 forks source link

not implemented: v8.serialize error in deno deploy #9

Closed inverted-capital closed 6 months ago

inverted-capital commented 6 months ago

I'm seeing an error in deno deploy using v0.16.2: https://github.com/kitsonk/kv-toolbox/blob/740654752a88f2266d0c73501f9867daf70056c1/batched_atomic.ts#L65

I will post a repro once I have a clean one.

inverted-capital commented 6 months ago

Error: Not implemented: v8.serialize at notImplemented (ext:deno_node/_utils.ts:9:9) at serialize (node:v8:59:3) at getByteLength (https://deno.land/x/kv_toolbox@0.16.2/batched_atomic.ts:65:10) at BatchedAtomicOperation.commit (https://deno.land/x/kv_toolbox@0.16.2/batched_atomic.ts:325:28) at set (https://deno.land/x/kv_toolbox@0.16.2/blob.ts:670:31)

inverted-capital commented 6 months ago

Here is a repro: https://dash.deno.com/playground/lazy-bat-30

which loads the page at: https://lazy-bat-30.deno.dev

kitsonk commented 6 months ago

darn... I really should have checked it on Deploy.

I am using the V8 serialize to estimate the byte size of keys and values to figure out when to overflow to another batch... the problem is that it isn't exposed in Deploy at all, which means I need to fallback to some other method.

inverted-capital commented 6 months ago

Call me a butcher, but you could just attempt to write it, and if it errors, half the size and try again 😬

kitsonk commented 6 months ago

No because the whole transaction fails, and there are multiple limits... there is the total payload size and there is the key size, Deno KV calculates each at .commit(), that plus the limits on number of mutations per commit (1000) and number of checks per commit (100). Trying to catch the errors, backoff whatever the offending part of the transaction again, and keep trying until it does fail would just simply not be tenable.

inverted-capital commented 6 months ago

Oh right - clearly you know more about it than me!!

kitsonk commented 6 months ago

I added a byte sized estimator that will work under Deploy that doesn't require V8 serialize. I did some benchmarking too and in most cases the manual estimator is substantially quicker than V8 serialize, so I just replaced it wholesale instead of trying to switch between the two.

inverted-capital commented 6 months ago

Oh wow that's awesome news - thank for this fix and thank you for such a rapid response !!