GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — [pipeline-manager] Check memory and storage autoscaling limits in the api-server by xiasongh · Pull Request #7819 · feldera/feldera

Conversation

@xiasongh

@xiasongh xiasongh commented Nov 3, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

POST, PUT, and PATCH /v0/pipelines now refuse a runtime config with memory above the runner's maximum, or with storage autoscaling when the runner does not enable it, with a 400 InvalidRuntimeConfig. A PATCH of deployment_config that resizes a running pipeline refuses memory above the maximum, with a 400 InvalidDeploymentPatch. Before, a resize the runner refused returned 200, went only to the pipeline log, and failed the next start.

Runtime configuration Refused when New api-server flag Helm value
resources.memory_min, memory_max above the maximum --upper-limit-memory-megabyte kubernetesRunner.upperLimitMemoryMegabyte
both storage_min and storage_max storage autoscaling disabled --enable-storage-autoscaling kubernetesRunner.enableStorageAutoscaling

With #7712 (storage size and class allowlists) and #7656 (the resize PATCH checks them), every field that can change while a pipeline runs is checked against the runner's limits. CPU has no runner limit.

  • Behavior change on all installations, in the changelog: memory_min above memory_max is refused, as storage_min above storage_max already is. A memory refusal reads "unsupported memory: ...".
  • Otherwise each new flag is unset by default, and an unset flag is not checked, so the open-source manager and a local install see no other change.
  • Only new values are checked: a stored runtime config still loads.
  • The "Storage limits" docs section becomes "Resource limits", with both rows.

Settings that cannot change while a pipeline runs, such as resources.namespace, service_account_name, or env, are left to the runner, which refuses them at start with a message that names the Helm value.

The Helm side is feldera/cloud#2165, which must merge after this.

Testing: unit tests for each refusal and for parsing the flags as the chart passes them, and endpoint tests for POST, PUT, PATCH of runtime_config (including the deprecated memory_mb_max and an installation without limits), and PATCH of deployment_config. Each check was removed in turn, and a test failed. Platform tests were not run.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
mythical-fred-oss[bot]
mythical-fred-oss Bot reviewed Nov 3, 2026
View reviewed changes
Comment thread crates/pipeline-manager/src/config.rs Resolved

Uh oh!

There was an error while loading. Please reload this page.

mythical-fred-oss[bot]
mythical-fred-oss Bot reviewed Nov 3, 2026
View reviewed changes
Comment thread docs.feldera.com/docs/changelog.md Resolved

Uh oh!

There was an error while loading. Please reload this page.

@mythical-fred-oss

mythical-fred-oss Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

Review summary

What I ran Result
cargo test -p pipeline-manager --lib (runtime_config_limit_requests, runner_settings…, runner_limits…, check_storage…) ✅ 4 passed
Extra QA endpoint cases (below) ✅ all passed
pre-commit run --files <changed> ❌ Clippy type_complexity in config.rs (see inline)
  • Gate failed (high): pre-commit and CI Clippy fail. This blocks merge.
  • Missing (medium): the changelog and the PR description do not say that memory_min > memory_max is now refused on open-source installations too (see inline).
  • Missing tests (medium): the endpoint test checks the new settings only on POST. I ran these extra cases locally and they pass, so please consider adding them:
             (true, "POST", "/pipelines", with_env("e"), 400),
+            (true, "PUT", "/pipelines/p", pipeline("p", json!({"memory_max": "1000M"})), 200), // boundary
+            (true, "PATCH", "/pipelines/p", patch(json!({"memory_max": "1001M"})), 400),
+            (true, "PATCH", "/pipelines/p", patch(json!({"memory_mb_max": 2000})), 400), // legacy field
+            (false, "PATCH", "/pipelines/p", patch(json!({"memory_min": "2G", "memory_max": "1G"})), 400),
+            (true, "PATCH", "/pipelines/p", json!({"runtime_config": {"env": {"A": "1"}}}), 400),
+            (true, "PUT", "/pipelines/p", with_env("p"), 400),
+            (true, "PATCH", "/pipelines/p", json!({"runtime_config": {"env": {}}}), 200), // empty env allowed

Platform tests were not run here (no k8s). Otherwise the logic, the actionable error messages, and the docs look good.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@xiasongh
xiasongh force-pushed the song/api-runner-limits branch from dbc9d3c to e9013fb Compare November 3, 2026 23:48
@xiasongh xiasongh changed the title [pipeline-manager] Check more runner limits in the api-server [pipeline-manager] Check memory and storage autoscaling limits in the api-server Nov 3, 2026
@xiasongh
xiasongh force-pushed the song/api-runner-limits branch from e9013fb to 607d9d1 Compare November 4, 2026 00:10
@xiasongh
xiasongh marked this pull request as ready for review November 4, 2026 00:10
@mythical-fred-oss

mythical-fred-oss Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

LGTM.

What I ran (debug, merge commit):

Check Result
cargo test -p pipeline-manager --lib memory_and_storage_autoscaling runner_limits_parse ✅ 2 passed
cargo test -p pipeline-manager --lib runtime_config_limit_requests (with Postgres) ✅ passed
pre-commit run --files <changed> (clippy, fmt, machete, OpenAPI, docs) ✅ all passed

Gates: tests cover the changed behaviour, nothing flaky (no sleeps, and each case builds its own state), docs and changelog are updated, the memory_min > memory_max behaviour change is in the changelog, no new dependencies, no unsafe. The resize path also matches the merged memory_min/memory_max check that already exists in db/types/deployment.rs, so a one-sided resize can't get past it.

Missing tests (low, optional): at the endpoint level, the storage-autoscaling refusal is only tested for POST, not PUT or PATCH; there's no case for enable_storage_autoscaling = Some(true) accepting both bounds; and the resize has no case for memory_min above the limit. The unit tests cover all three in check_*.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

gz
gz approved these changes Nov 4, 2026
View reviewed changes
@xiasongh
xiasongh force-pushed the song/api-runner-limits branch from 607d9d1 to 17c8b4f Compare November 4, 2026 21:11
@xiasongh
xiasongh enabled auto-merge November 4, 2026 21:11
@xiasongh
xiasongh disabled auto-merge November 4, 2026 21:22
@xiasongh
xiasongh added this pull request to the merge queue Nov 4, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Nov 5, 2026
@xiasongh
xiasongh force-pushed the song/api-runner-limits branch from 17c8b4f to 1b2fb3f Compare November 5, 2026 14:36
@xiasongh
xiasongh enabled auto-merge November 5, 2026 14:37
@xiasongh
[pipeline-manager] Check memory and storage autoscaling limits in the…
3bc6736
… api-server

POST, PUT, and PATCH return 400 InvalidRuntimeConfig for a runtime_config
with memory above the Kubernetes runner's maximum, or with both
storage_min and storage_max when the runner does not enable storage
autoscaling. A PATCH of deployment_config refuses memory above the
maximum. Before, the next start failed, and a refused resize only went
to the pipeline log.

Both flags are unset by default and then not checked. On all
installations, memory_min above memory_max is now refused.
@xiasongh
xiasongh force-pushed the song/api-runner-limits branch from 1b2fb3f to 3bc6736 Compare November 5, 2026 14:40
@xiasongh
xiasongh disabled auto-merge November 5, 2026 14:56
@xiasongh
xiasongh added this pull request to the merge queue Nov 5, 2026
Merged via the queue into main with commit 0dc5c25 Nov 5, 2026
1 check passed

Uh oh!

There was an error while loading. Please reload this page.

@xiasongh
xiasongh deleted the song/api-runner-limits branch November 5, 2026 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mythical-fred-oss mythical-fred-oss[bot] mythical-fred-oss[bot] left review comments
@gz gz gz approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@xiasongh @gz
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.