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(shard): allow domain sharding #860

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

rexxars
Copy link
Member

@rexxars rexxars commented Jul 2, 2024

This introduces the concept of domain sharding. It's a workaround for certain clients that have a network setup that disallows use of HTTP2/3. In applications such as the sanity studio, there may be many open network requests at the same time (listeners, queries, mutations etc) which poses a problem when HTTP1 is used, which has a maximum of 6 connections per hostname.

By spreading requests over a number of hostnames (:projectId.api.r[1-9].sanity.io), we can work around that limitation. Note that this prevents any auth cookie from being attached to the request - if you need auth, you'll have to use the authorization header (through the token parameter).

We use a "global" sharder singleton instance to avoid multiple different clients having separate buckets and ending up with one bucket being "filled up" because of an unshared state.

Had to take a few shortcuts in the tests here - ideally this would only be applied in browser environments, but couldn't find a great way to do so. We can refactor later if we find a workaround, but it shouldn't be a blocker.

Note that this is implemented primarily as a get-it middleware, but because the listen code-path does not use get-it under the hood, it has a separate, manual tracking implementation.

Note: the shard domains may not be up and running yet.

@rexxars rexxars requested review from a team and binoy14 and removed request for a team July 2, 2024 18:17
Copy link

@binoy14 binoy14 left a comment

Choose a reason for hiding this comment

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

Changes makes sense, was not able to test manually but unit tests look good enough. Thanks!

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