Update go-ds-s3 plugin to utilize prefixes for massive rate limit improvements #195
Conversation
This changes the way the go-ds-s3 plugin stores data in s3 in an effort to drastically improve speed. Instead of storing data all in the /ipfs directory (which counts as one prefix), we're storing things in a way so each block gets its own prefix.
|
Thank you for submitting this PR!
Getting other community members to do a review would be great help too on complex PRs (you can ask in the chats/forums). If you are unsure about something, just leave us a comment.
We currently aim to provide initial feedback/triaging within two business days. Please keep an eye on any labelling actions, as these will indicate priorities and status of your contribution. |
|
@obo20 easiest way to get this merged would be to have the constructor take a key transformation function, and have the default function just return |
|
@obo20 : let us know when this ready for review again - thanks! |
|
@obo20 planning to work on this soon again? |
|
@BigLep @whyrusleeping this is ready to go on my end now. |
|
@MichaelMure : are you able to review and get this merged? |
|
I don't think I'll have the time, sorry. |
|
@obo20: core IPFS maintainers aren't planning to take this on as we aren't using this ourselves. We need to figure out a way to setup and denote community membership for this and other datastore repos. We're game to setup others as the maintainers here. Maybe pair up with @v-stickeykeys and review each other's code (@v-stickykeys has #205 open) or ask for other volunteers in the ipfs-operators Slack channel? |
|
@BigLep does your comment mean that even outside of this PR this repo is effectively going to be abandoned unless community maintainers take it over? So far this repo has been updated for build compatibility purposes by core maintainers as IPFS has evolved. Is this something that we should expect to stop? |
|
@obo20 : we had to skip triage last week - apologies for the delay. We can continue to make PRs if there are complex breaking interface changes in things like go-datastore or the go-ipfs plugin interface. However we're not going to be making sure the dependencies are updated to match go-ipfs' (in the event you wanted to build the plugin as a shared-object) or generally work on any S3 particulars since we don't actually use this datastore/run it in production like other contributors here do. Also, historically changes like the recent ones to go-datastore are pretty easy to upstream after reading the release notes so waiting on a PR from a go-ipfs maintainer should not be required (although they'll happily help out if you're stuck and update the relevant release notes). Sound good? |
|
Moving forward, I'll be happy to look at any PRs for this repo and review from a go-ipfs maintainer's perspective (and as someone who has a lot of experience w/ S3), so feel free to assign me to reviews as necessary. It looks like this PR has been superseded by #205, so closing this. |
As mentioned in: #105, S3's rate-limiting is directly related to prefixes instead of files themselves.
Implementing this change will change the rate limits for this plugin
from: 3,500 PUT/COPY/POST/DELETE or 5,500 GET/HEAD requests per second for the entire bucket
to: 3,500 PUT/COPY/POST/DELETE or 5,500 GET/HEAD requests per second per block in the blockstore.
As of right now, this change will mean that previous repos using this plugin will be incompatible with this update. I would very much like to avoid forking this repo into a separately maintained pinata plugin if possible so I'm open to any suggestions on how to best approach this.