fix: Avoid redundant autoscaling start request - #4683
Conversation
| processorName = "new-processor-autoscaling-started-" + randomSuffix | ||
| ) | ||
|
|
||
| resource.Test(t, resource.TestCase{ |
There was a problem hiding this comment.
this tests the STARTED flow from the CREATE call which is not covered in the TestAccStreamProcessor_withOptionsDLQAutoscaling which calls the STARTED flow in the UPDATE call, which is why this was missed.
This edge case bug only affected the from scratch STARTED state generation, this will be fixed separately internally, but this change is still worth pushing to work in prod and also to clean up the redundant autoscaling config being set.
🤖 Augment PR SummarySummary: Fixes redundant autoscaling updates when a stream processor is started. Changes:
Technical Notes: This avoids triggering the backend's separate autoscaling-modification behavior while preserving configured min/max tiers. 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR fixes a regression in stream processor vertical autoscaling where the provider was redundantly re-sending autoscaling configuration on the :startWith request, which MMS/SPM interprets as an additional autoscaling modification. The change ensures autoscaling is persisted only via the preceding Create/PATCH request, and :startWith sends only tier when starting.
Changes:
- Stop sending autoscaling configuration on
StreamsAPI.StartStreamProcessorWithduring both Create and Update start flows. - Add an acceptance test covering the “Create → STARTED with autoscaling configured” path.
- Add a bugfix release note entry.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| internal/service/streamprocessor/resource.go | Removes autoscaling from :startWith payload in Create/Update start logic to avoid redundant autoscaling modifications. |
| internal/service/streamprocessor/resource_test.go | Adds acceptance coverage for creating a processor directly into STARTED with autoscaling configured. |
| .changelog/4683.txt | Documents the bugfix in release notes. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| func TestAccStreamProcessor_withOptionsDLQAutoscalingCreateStarted(t *testing.T) { | ||
| var ( | ||
| projectID, workspaceName = acc.ProjectIDExecutionWithStreamInstance(t) | ||
| _, clusterName = acc.ClusterNameExecution(t, false) | ||
| randomSuffix = acctest.RandString(5) |
|
APIx bot: a message has been sent to Docs Slack channel |
| @@ -0,0 +1,3 @@ | |||
| ```release-note:bug | |||
There was a problem hiding this comment.
Is it safe to say this bug never existed for users (autoscaling has not yet been released)? In that case would avoid this changelog, we can use skip changelog entry for passing the CI check
There was a problem hiding this comment.
yea this never existed before this, it only affected the most recent autoscaling config. Granted I only tested in DEV so it may be a timing issue. But its still good to remove the redundant autoscaling config from being set
There was a problem hiding this comment.
In that case I would remove this changelog entry to avoid confusion on the user side
There was a problem hiding this comment.
okay, this is now removed
| // On the :startWith endpoint, `autoscaling` is TOP-LEVEL (no options wrapper). | ||
| autoscaling, diags := autoscalingFromOptions(ctx, plan.Options) | ||
| if diags.HasError() { | ||
| resp.Diagnostics.Append(diags...) | ||
| return | ||
| } | ||
| startWithOptions.Autoscaling = autoscaling |
There was a problem hiding this comment.
to understand, was this additional property simply redundant, or actually causing a failures? And what would be the change planned on the API side?
There was a problem hiding this comment.
this is redundant, but there appears to be a race condition with the service that the api calls when the autoscaling payload was included. I believe that this 404 returned in the endpoint call is a bug that we will fix. But it's good to also remove the redundant config and fix this error path in the meantime. My understanding that any payload sent the startsWithOptions in this flow will trigger this issue: here is the ticket filed
4dc0953 to
e85b20d
Compare
Summary
Fixes a post-merge issue in the vertical autoscaling support added by #4665.
The provider persists autoscaling through the preceding Create or PATCH request. When a processor is started, it no longer resends the same autoscaling configuration through
:startWith, which MMS/SPM treats as an additional autoscaling modification.Validation
go test ./internal/service/streamprocessor -count=1make verifySTARTEDtest using the local provider build:startWithsent onlytierSTARTEDsuccessfullyFollow-up
The underlying MMS/SPM
:startWithautoscaling path remains a backend issue for direct API clients and is being tracked separately.