Skip to content

feat: Modularize SystemStatus to allow custom backpressure mechanisms#3529

Open
janbuchar wants to merge 3 commits intomasterfrom
modularize-system-status
Open

feat: Modularize SystemStatus to allow custom backpressure mechanisms#3529
janbuchar wants to merge 3 commits intomasterfrom
modularize-system-status

Conversation

@janbuchar
Copy link
Copy Markdown
Contributor

@janbuchar janbuchar commented Mar 26, 2026

  • The autoscaling pipeline was hardcoded to four resource types with no way to add new overload signals
  • I extracted a LoadSignal interface and SnapshotStore with fromInterval/fromEvent factories, rewired the four built-in resources as signal implementations, and made SystemStatus iterate over all signals in one loop. New signals like navigation timeouts can now be passed in via systemStatusOptions.loadSignals with zero changes to the core.
  • It should be fully backwards-compatible
  • This should make implementing feat: add network-aware navigation-timeout backpressure #3476 way more straightorward - no more building a parallel concurrency-control system with unforeseeable interactions with AutoscaledPool
  • Also relevant to Scale down when a high failure rate is detected #3305

@janbuchar janbuchar added adhoc Ad-hoc unplanned task added during the sprint. t-tooling Issues with this label are in the ownership of the tooling team. labels Mar 26, 2026
@github-actions github-actions bot added this to the 137th sprint - Tooling team milestone Mar 26, 2026
@github-actions github-actions bot added the tested Temporary label used only programatically for some analytics. label Mar 26, 2026
Copy link
Copy Markdown
Contributor

@nikitachapovskii-dev nikitachapovskii-dev 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 so much for addressing this problem, I really appreciate it!

Comment on lines +255 to +257
if (BUILTIN_SIGNAL_NAMES.has(signal.name)) {
(result as any)[signal.name] = info;
} else {
Copy link
Copy Markdown
Contributor

@nikitachapovskii-dev nikitachapovskii-dev Mar 27, 2026

Choose a reason for hiding this comment

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

I’m a bit puzzled about loadSignals.
If I understand it correctly, SystemStatus now evaluates both:
the built-in signals coming from Snapshotter and
user-provided signals passed via systemStatusOptions.loadSignals
And if a user provides a custom signal named e.g. memInfo, cpuInfo, it will overwrite the corresponding built-in field in the resulting SystemInfo?

Is that expected, or should the built-in signals remain reserved?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Well, I think we should let users overwrite the default memory/cpu/... signals if they know what they're doing. If everyone agrees, I guess let's sure that that's really the case and document it properly.

Copy link
Copy Markdown
Contributor

@l2ysho l2ysho left a comment

Choose a reason for hiding this comment

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

should we cover new loadSignals option in tests? 🤔

@janbuchar
Copy link
Copy Markdown
Contributor Author

janbuchar commented Mar 27, 2026

should we cover new loadSignals option in tests? 🤔

Thanks @l2ysho, added two basic ones to autoscaled pool test suite.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

adhoc Ad-hoc unplanned task added during the sprint. t-tooling Issues with this label are in the ownership of the tooling team. tested Temporary label used only programatically for some analytics.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants