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

Implement max_concurrency support when starting shards #2661

Merged
merged 6 commits into from
Jan 16, 2024

Conversation

mkrasnitski
Copy link
Collaborator

Supercedes #2650.

When a session is first kicked off and we need to start many shards, we can use the value of max_concurrency to kick off multiple shards in parallel, and do so in batches every 5 seconds. However, the contents of the batches need to be managed so that we don't accidentally get ratelimited.

Simply put, shards are ratelimited together if their IDs are equivalent modulo max_concurrency. For example, if max_concurrency is 16, then shards 0 and 16 get ratelimited together, and so do 1 and 17, etc. So, we need to make sure that a given batch doesn't include any conflicting shards that might cause the ratelimit to get hit. If we want a batch of maximal size, we need to wait until the queue contains enough shards to create a full batch.

However, we don't always want to wait for a batch to fill up if we know it's not possible for that to happen. This happens when we restart a single shard - in order to avoid waiting the 5 second loop timeout, we need to add a bool field to ShardQueuerMessage::Shard to signal whether we want to add the shard to the queue or not.

@github-actions github-actions bot added model Related to the `model` module. client Related to the `client` module. gateway Related to the `gateway` module. labels Dec 12, 2023
@mkrasnitski mkrasnitski changed the title Implement max concurrency support Implement max_concurrency support when starting shards Dec 12, 2023
Copy link
Member

@GnomedDev GnomedDev left a comment

Choose a reason for hiding this comment

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

I'm yet to test this out on my bot, but here's my first pass review.

src/gateway/bridge/mod.rs Outdated Show resolved Hide resolved
src/gateway/bridge/shard_queuer.rs Show resolved Hide resolved
src/gateway/bridge/shard_queuer.rs Show resolved Hide resolved
src/gateway/bridge/shard_queuer.rs Show resolved Hide resolved
Deduplicate the call to `self.checked_start` by pushing all shards to
the queue instead of just any that failed to start, and by falling
through in case of a timeout.
@arqunis arqunis added enhancement An improvement to Serenity. breaking change The public API is changed, resulting in miscompilations or unexpected new behaviour for users discord feature Related to Discord's functionality. labels Jan 16, 2024
@arqunis arqunis merged commit c9b6766 into serenity-rs:next Jan 16, 2024
22 checks passed
@mkrasnitski mkrasnitski deleted the max-concurrency branch January 16, 2024 16:01
arqunis pushed a commit to arqunis/serenity that referenced this pull request Jan 22, 2024
…#2661)

When a session is first kicked off and we need to start many shards, we
can use the value of `max_concurrency` to kick off multiple shards in
parallel.
GnomedDev pushed a commit that referenced this pull request Feb 9, 2024
When a session is first kicked off and we need to start many shards, we
can use the value of `max_concurrency` to kick off multiple shards in
parallel.
arqunis pushed a commit to arqunis/serenity that referenced this pull request Mar 1, 2024
…#2661)

When a session is first kicked off and we need to start many shards, we
can use the value of `max_concurrency` to kick off multiple shards in
parallel.
GnomedDev pushed a commit that referenced this pull request Mar 10, 2024
When a session is first kicked off and we need to start many shards, we
can use the value of `max_concurrency` to kick off multiple shards in
parallel.
GnomedDev pushed a commit that referenced this pull request Mar 11, 2024
When a session is first kicked off and we need to start many shards, we
can use the value of `max_concurrency` to kick off multiple shards in
parallel.
GnomedDev pushed a commit that referenced this pull request Mar 11, 2024
When a session is first kicked off and we need to start many shards, we
can use the value of `max_concurrency` to kick off multiple shards in
parallel.
GnomedDev pushed a commit to GnomedDev/serenity that referenced this pull request Mar 13, 2024
…#2661)

When a session is first kicked off and we need to start many shards, we
can use the value of `max_concurrency` to kick off multiple shards in
parallel.
GnomedDev pushed a commit that referenced this pull request Mar 13, 2024
When a session is first kicked off and we need to start many shards, we
can use the value of `max_concurrency` to kick off multiple shards in
parallel.
GnomedDev pushed a commit to GnomedDev/serenity that referenced this pull request Mar 19, 2024
…#2661)

When a session is first kicked off and we need to start many shards, we
can use the value of `max_concurrency` to kick off multiple shards in
parallel.
GnomedDev pushed a commit to GnomedDev/serenity that referenced this pull request Mar 19, 2024
…#2661)

When a session is first kicked off and we need to start many shards, we
can use the value of `max_concurrency` to kick off multiple shards in
parallel.
GnomedDev pushed a commit that referenced this pull request Mar 21, 2024
When a session is first kicked off and we need to start many shards, we
can use the value of `max_concurrency` to kick off multiple shards in
parallel.
GnomedDev pushed a commit that referenced this pull request Mar 25, 2024
When a session is first kicked off and we need to start many shards, we
can use the value of `max_concurrency` to kick off multiple shards in
parallel.
GnomedDev pushed a commit that referenced this pull request Mar 29, 2024
When a session is first kicked off and we need to start many shards, we
can use the value of `max_concurrency` to kick off multiple shards in
parallel.
GnomedDev pushed a commit that referenced this pull request Mar 31, 2024
When a session is first kicked off and we need to start many shards, we
can use the value of `max_concurrency` to kick off multiple shards in
parallel.
GnomedDev pushed a commit to GnomedDev/serenity that referenced this pull request Mar 31, 2024
…#2661)

When a session is first kicked off and we need to start many shards, we
can use the value of `max_concurrency` to kick off multiple shards in
parallel.
GnomedDev pushed a commit to GnomedDev/serenity that referenced this pull request Apr 1, 2024
…#2661)

When a session is first kicked off and we need to start many shards, we
can use the value of `max_concurrency` to kick off multiple shards in
parallel.
GnomedDev pushed a commit to GnomedDev/serenity that referenced this pull request May 14, 2024
…#2661)

When a session is first kicked off and we need to start many shards, we
can use the value of `max_concurrency` to kick off multiple shards in
parallel.
GnomedDev pushed a commit that referenced this pull request May 14, 2024
When a session is first kicked off and we need to start many shards, we
can use the value of `max_concurrency` to kick off multiple shards in
parallel.
GnomedDev pushed a commit that referenced this pull request May 23, 2024
When a session is first kicked off and we need to start many shards, we
can use the value of `max_concurrency` to kick off multiple shards in
parallel.
GnomedDev pushed a commit that referenced this pull request May 28, 2024
When a session is first kicked off and we need to start many shards, we
can use the value of `max_concurrency` to kick off multiple shards in
parallel.
GnomedDev pushed a commit to GnomedDev/serenity that referenced this pull request Jun 9, 2024
…#2661)

When a session is first kicked off and we need to start many shards, we
can use the value of `max_concurrency` to kick off multiple shards in
parallel.
GnomedDev pushed a commit that referenced this pull request Jun 22, 2024
When a session is first kicked off and we need to start many shards, we
can use the value of `max_concurrency` to kick off multiple shards in
parallel.
GnomedDev pushed a commit to GnomedDev/serenity that referenced this pull request Jun 22, 2024
…#2661)

When a session is first kicked off and we need to start many shards, we
can use the value of `max_concurrency` to kick off multiple shards in
parallel.
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Jul 29, 2024
…#2661)

When a session is first kicked off and we need to start many shards, we
can use the value of `max_concurrency` to kick off multiple shards in
parallel.
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Jul 30, 2024
…#2661)

When a session is first kicked off and we need to start many shards, we
can use the value of `max_concurrency` to kick off multiple shards in
parallel.
GnomedDev pushed a commit that referenced this pull request Aug 16, 2024
When a session is first kicked off and we need to start many shards, we
can use the value of `max_concurrency` to kick off multiple shards in
parallel.
GnomedDev pushed a commit to GnomedDev/serenity that referenced this pull request Oct 7, 2024
…#2661)

When a session is first kicked off and we need to start many shards, we
can use the value of `max_concurrency` to kick off multiple shards in
parallel.
GnomedDev pushed a commit to GnomedDev/serenity that referenced this pull request Oct 20, 2024
…#2661)

When a session is first kicked off and we need to start many shards, we
can use the value of `max_concurrency` to kick off multiple shards in
parallel.
GnomedDev pushed a commit that referenced this pull request Oct 20, 2024
When a session is first kicked off and we need to start many shards, we
can use the value of `max_concurrency` to kick off multiple shards in
parallel.
GnomedDev pushed a commit to GnomedDev/serenity that referenced this pull request Nov 11, 2024
…#2661)

When a session is first kicked off and we need to start many shards, we
can use the value of `max_concurrency` to kick off multiple shards in
parallel.
GnomedDev pushed a commit that referenced this pull request Nov 13, 2024
When a session is first kicked off and we need to start many shards, we
can use the value of `max_concurrency` to kick off multiple shards in
parallel.
GnomedDev pushed a commit to GnomedDev/serenity that referenced this pull request Nov 15, 2024
…#2661)

When a session is first kicked off and we need to start many shards, we
can use the value of `max_concurrency` to kick off multiple shards in
parallel.
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Dec 8, 2024
…#2661)

When a session is first kicked off and we need to start many shards, we
can use the value of `max_concurrency` to kick off multiple shards in
parallel.
arqunis pushed a commit to arqunis/serenity that referenced this pull request Jan 16, 2025
…#2661)

When a session is first kicked off and we need to start many shards, we
can use the value of `max_concurrency` to kick off multiple shards in
parallel.
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Feb 1, 2025
…#2661)

When a session is first kicked off and we need to start many shards, we
can use the value of `max_concurrency` to kick off multiple shards in
parallel.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change The public API is changed, resulting in miscompilations or unexpected new behaviour for users client Related to the `client` module. discord feature Related to Discord's functionality. enhancement An improvement to Serenity. gateway Related to the `gateway` module. model Related to the `model` module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants