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

Server bottlenecks #825

Merged
merged 2 commits into from
Sep 5, 2024
Merged

Server bottlenecks #825

merged 2 commits into from
Sep 5, 2024

Conversation

scohen
Copy link
Collaborator

@scohen scohen commented Sep 5, 2024

Fixed two bottlenecks in the server.

  1. The mix lock was using a much more contentious build lock rather than its own lock.
  2. Using Task.Supervisor.terminate_child was causing timeouts when bulk operations happen.

Copy link
Collaborator

@zachallaun zachallaun left a comment

Choose a reason for hiding this comment

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

LGTM, made a few optional suggestions.

apps/server/lib/lexical/server/task_queue.ex Outdated Show resolved Hide resolved
Copy link
Collaborator

@zachallaun zachallaun left a comment

Choose a reason for hiding this comment

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

Actually: Looking at the CI failures, 1.13 doesn't support the :mfa attribute in tasks.

When lots of things happen in lexical, it's necessary to kill some
outstanding tasks that won't finish. When lots of things need to be
killed (as in when a lot of files are changed), killing them can
become a bottleneck. This is because we were using terminate_child,
which makes a genserver call through the supervisor and waits for the
pid to exit before it returns. This, in turn would cause the
supervisor to back up and the task queue to die because of timeouts.

Replacing this with `Process.exit` removes the bottleneck and doesn't
seem to have any adverse effects.
Rather than piggyback on the build lock when getting project mix
config, use a different lock type.
@scohen scohen merged commit dd4bd18 into main Sep 5, 2024
12 checks passed
@scohen scohen deleted the cancel-timeouts branch September 5, 2024 18:34
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