Skip to content

Conversation

@alexandre-daubois
Copy link
Member

This would help better understand optimal scaling parameters per application

cc @soyuka

@alexandre-daubois alexandre-daubois force-pushed the log-on-scale branch 2 times, most recently from 7c6b6d7 to a4b0f31 Compare June 27, 2025 11:40
@alexandre-daubois
Copy link
Member Author

PR updated with your suggestions. Thanks!

scaling.go Outdated
// convert threads to inactive if they have been idle for too long
if thread.state.is(stateReady) && waitTime > maxThreadIdleTime.Milliseconds() {
logger.LogAttrs(context.Background(), slog.LevelDebug, "auto-converting thread to inactive", slog.Int("threadIndex", thread.threadIndex))
logger.LogAttrs(context.Background(), slog.LevelInfo, "downscaling thread", slog.Int("thread", thread.threadIndex), slog.Int64("wait_time", waitTime), slog.Int("num_threads", len(autoScaledThreads)-1))
Copy link
Member

Choose a reason for hiding this comment

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

Nit: shouldn't we log after having done the work? (so the -1 will not be necessary)

Copy link
Member Author

Choose a reason for hiding this comment

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

We can! I chose the way that brings the smallest diff but totally fine with it

Copy link
Member

Choose a reason for hiding this comment

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

Let's do that then.

Copy link
Member

Choose a reason for hiding this comment

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

(Logging can be slow, depending on the log backend, let's do the actual work first)

Copy link
Contributor

@soyuka soyuka left a comment

Choose a reason for hiding this comment

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

that's awesome!

@dunglas dunglas merged commit 995c829 into php:main Jun 30, 2025
43 checks passed
@dunglas
Copy link
Member

dunglas commented Jun 30, 2025

Thanks!

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