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

fix(model): bump SessionStartLimit field sizes #2286

Merged
merged 1 commit into from
Oct 15, 2023
Merged

Conversation

vilgotf
Copy link
Member

@vilgotf vilgotf commented Oct 14, 2023

The largest bot is in ~21M guilds, meaning that its session start limit := max(2000, (guild_count / 1000) * 3) is just under u16::MAX (65535), so to safeguard against overflows we should bump the field sizes a step (u16 -> u32).

I also went ahead and bumped the max_concurrency field to u16, size it's plausible that this could exceed 255 by the largest bots.

The [largest bot](https://gist.github.com/advaith1/451dcbca2d7c3503d4f48d63eb918cb0)
is in ~21M guilds, meaning that its session start limit := max(2000, (guild_count / 1000) * 3)
is just under u16::MAX (65535), so to safeguard against overflows we
should bump the field sizes a step (u16 -> u32).
I also went ahead and bumped the `max_concurrency` field to u16, size
it's plausible that this could exceed 255 by the largest bots.
@vilgotf vilgotf requested a review from a team October 14, 2023 10:26
@vilgotf vilgotf added c-model Affects the model crate t-fix Fixes a bug in the library labels Oct 14, 2023
@github-actions github-actions bot added the c-gateway-queue Affects the gateway queue crate label Oct 14, 2023
Copy link
Member

@laralove143 laralove143 left a comment

Choose a reason for hiding this comment

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

thank you for this catch

Copy link
Member

@Gelbpunkt Gelbpunkt left a comment

Choose a reason for hiding this comment

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

Last thing I'm aware of is that the biggest bot has a max concurrency of 256, potentially even higher now. This is a sensible and overdue bump.

@vilgotf vilgotf merged commit b75e644 into next Oct 15, 2023
@vilgotf vilgotf deleted the vilgotf/session-start branch October 15, 2023 05:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c-gateway-queue Affects the gateway queue crate c-model Affects the model crate t-fix Fixes a bug in the library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants