clear drainage status of a rollbacked version workflow #8119
Conversation
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
carlydf
reviewed
Jul 30, 2025
Contributor
carlydf
left a comment
carlydf
left a comment
There was a problem hiding this comment.
Nice job with the repro and the fix!
I know it's annoying, but I really think that we should have solid replay tests for this patch, particularly if it's going into the OSS patch release.
I think it's worth doing the patch test ticket before merging this, so that this can be replay tested
carlydf
approved these changes
Jul 31, 2025
yycptt
pushed a commit
that referenced
this pull request
Aug 5, 2025
- When a deactivated (drained/draining) version gets rollbacked to becoming active again, the drainage information that was associated with it will get cleared. - This is required because when this version were to get deactivated again, we need the code to start the drainage process on it again. Right now, it was not doing that since we were not clearing the drainage information which did not start a fresh drainage tracker for this version! *NOTE*: This change requires a patch to be present. This is because when this scenario is replicated in production, it will result in two new history calls (syncSummary as well as a CAN of the version workflow). This will specifically happen when the rolled-back version gets deactivated eventually, over [here](https://github.com/temporalio/temporal/blob/ada5d0bbee45150888b63a132be8239fd5a87212/service/worker/workerdeployment/version_workflow.go#L546) - note how previously, since the drainage information was not cleared, [this](https://github.com/temporalio/temporal/blob/ada5d0bbee45150888b63a132be8239fd5a87212/service/worker/workerdeployment/version_workflow.go#L261) was never executed!) *How did I verify that this requires a patch*: - Ran the replay test script before the fix introduced in this PR and noticed: <img width="793" height="79" alt="image" src="https://github.com/user-attachments/assets/bb975d26-4ad1-4847-b477-b74c7d2f55e4" /> - Ran the replay test script after the fix introduced in this PR and noticed: <img width="911" height="99" alt="image" src="https://github.com/user-attachments/assets/ad3ff3e1-666a-49b4-bda4-e05f3412d984" /> In other words, the total number of our entity workflows went up after introducing this change and thus requires a patch gate! - we found a bug in the versioning entity workflows and this is an effort to fix this! - [ ] built - [ ] run locally and tested manually - [ ] covered by existing tests - [ ] added new unit test(s) - [x] added new functional test(s) - yes, there is a risk of NDE's happening in production but I mitigated that risk by placing the fix against a version gate.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What changed?
NOTE: This change requires a patch to be present. This is because when this scenario is replicated in production, it will result in two new history calls (syncSummary as well as a CAN of the version workflow). This will specifically happen when the rolled-back version gets deactivated eventually, over here - note how previously, since the drainage information was not cleared, this was never executed!)
How did I verify that this requires a patch:
In other words, the total number of our entity workflows went up after introducing this change and thus requires a patch gate!
Why?
How did you test it?
Potential risks