Skip to content

Conversation

@Whoops
Copy link
Contributor

@Whoops Whoops commented Jul 7, 2025

Summary of changes

Asana Ticket: 🍎💳 Resolve API Run Queue Too High Alerts

@Whoops Whoops force-pushed the wh-run-queue-rework branch from ea8b4ac to be41fd4 Compare July 7, 2025 20:53
@Whoops Whoops requested review from a team and lemald and removed request for a team July 7, 2025 20:57
Comment on lines +6 to +7
This check always returns healthy as we don't want to kill tasks based on the run queue length.
Instead it logs the maximum run queue length across all schedulers for monitoring purposes.
Copy link
Member

Choose a reason for hiding this comment

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

suggestion (non-blocking): if this isn't going to be used as a "real" health check anymore, does it make sense to maybe split this out and make it its own GenServer living somewhere in the supervision tree? The only issue is that I'm not sure where exactly that "somewhere" would be, especially in the context of the API umbrella app setup.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I both agree, but didn't feel like it was worth the overhead. I can easily be convinced otherwise. The rest of the metrics we collect come from a reporter built into ehmon. We don't currently have another way of surfacing metrics so it'd mean building a way of scheduling this to run on some interval, which just seemed like more code than it was worth for this one metric.

defp pid_log(other) do
inspect(other)
defp max_queue_length do
Enum.max(:erlang.statistics(:run_queue_lengths))
Copy link
Member

Choose a reason for hiding this comment

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

nitpick (non-blocking): this could be a good pipeline operator candidate

Copy link
Member

@lemald lemald left a comment

Choose a reason for hiding this comment

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

A couple of non-blocking things but overall LGTM.

@Whoops Whoops merged commit 2566c74 into master Jul 8, 2025
43 checks passed
@Whoops Whoops deleted the wh-run-queue-rework branch July 8, 2025 13:59
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.

3 participants