Cleanup: fix deletion hang - #176
Merged
Merged
Conversation
Part 1 in a serious of deletion handling. Deleting a RestateDeployment hangs forever due to conflating is active to mean: * "is a service pointed at this deployment" * "are there active invocations" This change seperates those two facts and considers them differently depending on why the drain is happening (rollout vs deletion). Additionally, a few other issues feel out: 1. The drain requeue deadline was being thrown away do an error policy wrapping the finalizer. 2. A draining version kept its autoscaler. HPA removal has been moved to the point where we actually scale to zero. References #172
darkmuggle
force-pushed
the
bh/clean-up
branch
from
August 10, 2026 16:22
f6b6d54 to
0459f15
Compare
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
pcholakov
self-requested a review
August 10, 2026 16:37
pcholakov
reviewed
Aug 10, 2026
Comment on lines
+29
to
+30
| - **A blocked deletion says what is blocking it.** The `DeploymentInUse` event now names | ||
| each version and its pinned/unpinned invocation counts instead of a generic message. |
Contributor
Author
There was a problem hiding this comment.
After this lands, the next PR is for a richer status including the inflight innovations.
Contributor
|
Thanks for tackling this, @darkmuggle! The biggest thing that is missing for me here is an e2e test – I believe it's time we added some that run in-tree. I'm worried about the cost of querying It's probably worth following up with #174, or at least taking into consideration how that would be solved. We had an external contributor open PR #160; this PR supersedes its data model (splitting the single |
darkmuggle
added a commit
that referenced
this pull request
Aug 10, 2026
- Build the deployment-usage query in Rust rather than sharing one string behind a `WHERE 1 = 0` guard. DataFusion does fold the guard away (EXPLAIN against 1.7.2 shows the unpinned scan disappearing), but the reconcile path should not depend on an optimiser pass across server versions we don't pin. The rollout flavour now scans sys_invocation_status once, with projection [status, pinned_deployment_id] -- no target_service_name decode. - Restore the note explaining the COALESCE/LEFT JOIN pairing, and move the row parsing next to the query it belongs to. - Back off a blocked deletion: retries stretch from 30s to a 5m cap as the wait grows, instead of re-running the deleting query every 30s forever. - Cover the teardown ordering with mocked-apiserver tests: the HPA comes off immediately before the scale to zero, and not while a version is still waiting out its drain deadline (in a rollout or a deletion). - Add e2e tests that run both query flavours against a real Restate server in a container, and parse the rows through the operator's own row struct. Wired into CI, which was not running cargo test at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
darkmuggle
added a commit
that referenced
this pull request
Aug 10, 2026
- Build the deployment-usage query in Rust rather than sharing one string behind a `WHERE 1 = 0` guard. DataFusion does fold the guard away (EXPLAIN against 1.7.2 shows the unpinned scan disappearing), but the reconcile path should not depend on an optimiser pass across server versions we don't pin. The rollout flavour now scans sys_invocation_status once, with projection [status, pinned_deployment_id] -- no target_service_name decode. - Restore the note explaining the COALESCE/LEFT JOIN pairing, and move the row parsing next to the query it belongs to. - Back off a blocked deletion: retries stretch from 30s to a 5m cap as the wait grows, instead of re-running the deleting query every 30s forever. - Cover the teardown ordering with mocked-apiserver tests: the HPA comes off immediately before the scale to zero, and not while a version is still waiting out its drain deadline (in a rollout or a deletion). - Run cargo test in CI, which was not running tests at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
darkmuggle
force-pushed
the
bh/clean-up
branch
from
August 10, 2026 19:03
78306a6 to
7d10d88
Compare
- Build the deployment-usage query in Rust rather than sharing one string behind a `WHERE 1 = 0` guard. DataFusion does fold the guard away (EXPLAIN against 1.7.2 shows the unpinned scan disappearing), but the reconcile path should not depend on an optimiser pass across server versions we don't pin. The rollout flavour now scans sys_invocation_status once, with projection [status, pinned_deployment_id] -- no target_service_name decode. - Restore the note explaining the COALESCE/LEFT JOIN pairing, and move the row parsing next to the query it belongs to. - Back off a blocked deletion: retries stretch from 30s to a 5m cap as the wait grows, instead of re-running the deleting query every 30s forever. - Cover the teardown ordering with mocked-apiserver tests: the HPA comes off immediately before the scale to zero, and not while a version is still waiting out its drain deadline (in a rollout or a deletion). - Run cargo test in CI, which was not running tests at all.
darkmuggle
force-pushed
the
bh/clean-up
branch
from
August 10, 2026 19:04
7d10d88 to
fb57caa
Compare
Contributor
Author
|
@pcholakov new push with the changes. I'll get a separate PR for the E2E tests. I did test the SQL against a local Restate cluster. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
Part 1 in a serious of deletion handling.
Deleting a RestateDeployment hangs forever due to conflating is active to mean:
This change seperates those two facts and considers them differently depending on why the drain is happening (rollout vs deletion).
Additionally, a few other issues feel out:
References #172