Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: batch endpoint check items in parallel #170

Merged
merged 1 commit into from
Jul 17, 2023
Merged

feat: batch endpoint check items in parallel #170

merged 1 commit into from
Jul 17, 2023

Conversation

olizilla
Copy link
Contributor

Switch to checking all the items in parallel, and let the platform decide the concurrency.

Drops the time to check 1000 items by an order of magnitude

  • cold kv from ~180s to ~10s
  • warm kv from ~50s to ~5s
time curl "http://127.0.0.1:8787/" --json @denylist.post.1000.json
[]
________________________________________________________
Executed in    4.36 secs      fish           external

...for obvious reason of not doing each one sequentially.

License: MIT

switch to checking all the items in parallel, and let the platform decide the concurrency.

Drops the unwarm time to check 1000 items from ~180s to ~10s

Drops the warm time to check 100o items from ~50s to 5s

```sh
❯ time curl "http://127.0.0.1:8787/" --json @denylist.post.1000.json
[]
________________________________________________________
Executed in    4.36 secs      fish           external
```

...for obvious reason  of not doing each one sequentially.

License: MIT
Signed-off-by: Oli Evans <[email protected]>
@@ -35,7 +35,7 @@ routes = [
{ pattern = "denylist-staging.dag.haus/*", zone_id = "f2f8a5b1c557202c6e3d0ce0e98e4c8e" }
]
kv_namespaces = [
{ binding = "DENYLIST", id = "f4eb0eca32e14e28b643604a82e00cb3" }
{ binding = "DENYLIST", id = "f4eb0eca32e14e28b643604a82e00cb3", preview_id = "f4eb0eca32e14e28b643604a82e00cb3" }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this lets try out changes against the staging kv with

wrangler dev --remote --env staging

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh nice, did not know this

Comment on lines +11 to +12
const hash = await sha256.encode(uint8arrays.fromString(`${cid}/`))
return uint8arrays.toString(hash, 'hex')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unrelated simplification that was bugging me

@olizilla
Copy link
Contributor Author

tested on staging

cold

time curl "https://denylist-staging.dag.haus/" --json @denylist.post.1000.json
[]
________________________________________________________
Executed in   30.55 secs      fish           external

warm

time curl "https://denylist-staging.dag.haus/" --json @denylist.post.1000.json
[]
________________________________________________________
Executed in    5.75 secs      fish           external

Copy link
Contributor

@vasco-santos vasco-santos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
One thing that kept me thinking, should we get rate limits for this API to avoid abuses leading into rate limits?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants