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

Limit the number of concurrent restarts when nuking, or spawn each restart as a separate task #4

Closed
Tracked by #2
Carnagion opened this issue Aug 26, 2023 · 2 comments
Labels
critical Must be worked on as soon as possible enhancement New feature or request lang: rust Relevant to the Rust codebase

Comments

@Carnagion
Copy link
Owner

Carnagion commented Aug 26, 2023

Currently, when executing /registration nuke, every user's registration is restarted concurrently, on the same tokio task (which is spawned by poise and serenity when handling the command).

However, when restarting the registrations of hundreds of users, the high number of incoming and outgoing Discord websocket requests and database connections, combined with the insufficient database connection limit, leads to the operations being performed inconsistently.

This might be solved in one of three ways:

  • Limit the number of concurrent restarts to prevent situations where the database cannot handle the sheer number of connections
  • Perform each restart in a separate tokio task, possibly combined with the above, to introduce some leeway for parallelism and avoid excessive load on one task
  • Restart registrations sequentially rather that concurrently to provide more predictable and consistent behaviour
@Carnagion Carnagion added enhancement New feature or request critical Must be worked on as soon as possible lang: rust Relevant to the Rust codebase labels Aug 26, 2023
@Carnagion Carnagion changed the title Limit the number of concurrent restarts when nuking, or spawn each restart as a separate tokio task Limit the number of concurrent restarts when nuking, or spawn each restart as a separate task Aug 26, 2023
@Carnagion
Copy link
Owner Author

As a temporary solution, the number of concurrent restarts has been limited to 10 - its original value before I removed the limit. See 68e5331.

A more permanent and well-thought out solution should be worked on and tested as soon as possible.

@Carnagion
Copy link
Owner Author

Limiting the number of concurrent restarts has shown itself to be a good enough solution for now, as it was able to nuke upwards of 50 users under 12 minutes.

Since registration nukes are usually rare, one-time events, I believe this will work as a fairly permanent solution. In the unlikely case that a future nuke involves hundreds of students or more, it might be worth implementing one of the other solutions from #2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
critical Must be worked on as soon as possible enhancement New feature or request lang: rust Relevant to the Rust codebase
Projects
None yet
Development

No branches or pull requests

1 participant