Handle Unversioned Current and Ramping Version #7244
Merged
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
## What changed? <!-- Describe what has changed in this PR --> - Updated deployment entity wf's API's (naming) for versioning-3.1. Removed those which will not be required. - Added functionality for RegisterTaskQueue + DescribeVersion - Added functional tests for verifying the above two work. - UserData stuff has been removed right now since there are pending discussions ## Why? <!-- Tell your future self why have you made these changes --> - Versioning-3.1 ## How did you test it? <!-- How have you verified this change? Tested locally? Added a unit test? Checked in staging env? --> - Functional tests have been added. Note, the tests added are exact replicas (in terms of their core functionality) with the existing tests under `deployment_test.go` ## Potential risks <!-- Assuming the worst case, what can be broken when deploying this change to production? --> - None, feature branch. ## Documentation <!-- Have you made sure this change doesn't falsify anything currently stated in `docs/`? If significant new behavior is added, have you described that in `docs/`? --> ## Is hotfix candidate? <!-- Is this PR a hotfix candidate or does it require a notification to be sent to the broader community? (Yes/No) --> - No
## What changed? <!-- Describe what has changed in this PR --> Using new Deployment Options fields sent by SDK in Versioning 3 functionality. Old fields are still used when new fields are absent. Implementation did not change, both new and old fields sent in polls and task responses are still converted to old `Deployment` object and used as before. Later, code will be refactored to change the `Deployment` usages to `DeploymentVersion`. Also added new fields to replace `Deployment` with `DeploymentVersion` fields in internal protos where needed. Matching<->History communication happens via these new fields, only new internal fields are written but both new and old fields are read. ## Why? <!-- Tell your future self why have you made these changes --> Incorporating latest renames in Versioning APIs. ## How did you test it? <!-- How have you verified this change? Tested locally? Added a unit test? Checked in staging env? --> Existing tests changed to use new fields (or both old and new depending on the test). ## Potential risks <!-- Assuming the worst case, what can be broken when deploying this change to production? --> None. ## Documentation <!-- Have you made sure this change doesn't falsify anything currently stated in `docs/`? If significant new behavior is added, have you described that in `docs/`? --> None yet. ## Is hotfix candidate? <!-- Is this PR a hotfix candidate or does it require a notification to be sent to the broader community? (Yes/No) --> No.
## What changed? <!-- Describe what has changed in this PR --> - DescribeWorkerDeployment API for versioning-3.1 For this API to work, I realized that the worker deployment workflow has to have information about the various deployment versions it encapsulates. Now that version deployment workflows won't have a `deploymentName` present in their workflowID (for now), partial matching workflow ID's with the received deployment name to target version workflows was not feasible. Another option was to store the mapping from "versions" to "the local state of a version workflow". However, this would have made the deployment workflow arguments large in size since in theory, we could have many deployment versions each with large sized local states. The option I ended up going with was storing the version in a deployment workflow during task-queue registration. During a `DescribeWorkerDeployment` call, we would call `Describe` on each stored version and return the aggregated info to the user. A version will be removed from a worker deployment when it becomes "scavenged" (to be implemented) Potential future improvements include adding a caching mechanism. ## Why? <!-- Tell your future self why have you made these changes --> - Versioning-3.1 ## How did you test it? <!-- How have you verified this change? Tested locally? Added a unit test? Checked in staging env? --> - Added a functional test + more tests to follow ## Potential risks <!-- Assuming the worst case, what can be broken when deploying this change to production? --> - None, going inside a feature. ## Documentation <!-- Have you made sure this change doesn't falsify anything currently stated in `docs/`? If significant new behavior is added, have you described that in `docs/`? --> ## Is hotfix candidate? <!-- Is this PR a hotfix candidate or does it require a notification to be sent to the broader community? (Yes/No) --> - No
## What changed? <!-- Describe what has changed in this PR --> Matching to Worker Deployment Ramping Version and its percentage when routing tasks. User-facing APIs are not implemented yet. The implementation also supports ramping from a Deployment Version to unversioned workers. ## Why? <!-- Tell your future self why have you made these changes --> Part of Worker Versioning features. ## How did you test it? <!-- How have you verified this change? Tested locally? Added a unit test? Checked in staging env? --> Added unit and functional tests. ## Potential risks <!-- Assuming the worst case, what can be broken when deploying this change to production? --> None. ## Documentation <!-- Have you made sure this change doesn't falsify anything currently stated in `docs/`? If significant new behavior is added, have you described that in `docs/`? --> None. ## Is hotfix candidate? <!-- Is this PR a hotfix candidate or does it require a notification to be sent to the broader community? (Yes/No) --> No --------- Co-authored-by: Shivam Saraf <shivam.saraf@temporal.io>
## What changed? <!-- Describe what has changed in this PR --> - Fix flaky `DescribeWorkerDeployment`. The flake was caused because I forgot to pass in the right parameters when creating version entity workflows. Moreover, recent proto changes meant that my assertions also had to be updated and have done so :) - Other breaking unit tests in the versioning-3.1 repo ## Why? <!-- Tell your future self why have you made these changes --> - don't like seeing red markers after CI is done running ## How did you test it? <!-- How have you verified this change? Tested locally? Added a unit test? Checked in staging env? --> - Ran unit and functional tests ## Potential risks <!-- Assuming the worst case, what can be broken when deploying this change to production? --> - None, going to a feature. ## Documentation <!-- Have you made sure this change doesn't falsify anything currently stated in `docs/`? If significant new behavior is added, have you described that in `docs/`? --> ## Is hotfix candidate? <!-- Is this PR a hotfix candidate or does it require a notification to be sent to the broader community? (Yes/No) -->
## What changed? <!-- Describe what has changed in this PR --> - title ## Why? <!-- Tell your future self why have you made these changes --> - versioning-3.1 ## How did you test it? <!-- How have you verified this change? Tested locally? Added a unit test? Checked in staging env? --> - existing suite of tests + added functional tests ## Potential risks <!-- Assuming the worst case, what can be broken when deploying this change to production? --> - None, feature branch ## Documentation <!-- Have you made sure this change doesn't falsify anything currently stated in `docs/`? If significant new behavior is added, have you described that in `docs/`? --> ## Is hotfix candidate? <!-- Is this PR a hotfix candidate or does it require a notification to be sent to the broader community? (Yes/No) --> - No
## What changed? <!-- Describe what has changed in this PR --> - ListWorkerDeployments API - Also removed presence of locks when adding a version to a worker-deployment workflow. Causes deadlocks and we don't need them. ## Why? <!-- Tell your future self why have you made these changes --> - Versioning-3.1 ## How did you test it? <!-- How have you verified this change? Tested locally? Added a unit test? Checked in staging env? --> - Added functional tests (happy-path) - Existing suite ## Potential risks <!-- Assuming the worst case, what can be broken when deploying this change to production? --> - None, feature ## Documentation <!-- Have you made sure this change doesn't falsify anything currently stated in `docs/`? If significant new behavior is added, have you described that in `docs/`? --> ## Is hotfix candidate? <!-- Is this PR a hotfix candidate or does it require a notification to be sent to the broader community? (Yes/No) --> - No
## What changed? Add DrainageStatus child workflow to worker deployment system ## Why? To periodically update the version workflow with results from visibility. ## How did you test it? Functional tests. Currently broken (see todo comment in code) ## Potential risks <!-- Assuming the worst case, what can be broken when deploying this change to production? --> ## Documentation <!-- Have you made sure this change doesn't falsify anything currently stated in `docs/`? If significant new behavior is added, have you described that in `docs/`? --> ## Is hotfix candidate? <!-- Is this PR a hotfix candidate or does it require a notification to be sent to the broader community? (Yes/No) --> --------- Co-authored-by: Shivam Saraf <shivam.saraf@temporal.io>
…, fix comments (#7178) ## What changed? <!-- Describe what has changed in this PR --> ## Why? <!-- Tell your future self why have you made these changes --> ## How did you test it? <!-- How have you verified this change? Tested locally? Added a unit test? Checked in staging env? --> ## Potential risks <!-- Assuming the worst case, what can be broken when deploying this change to production? --> ## Documentation <!-- Have you made sure this change doesn't falsify anything currently stated in `docs/`? If significant new behavior is added, have you described that in `docs/`? --> ## Is hotfix candidate? <!-- Is this PR a hotfix candidate or does it require a notification to be sent to the broader community? (Yes/No) -->
## What changed? <!-- Describe what has changed in this PR --> - Ramping PR for versioning-3.1 - now, if a ramping version becomes current, the ramp will be unset. ## Why? <!-- Tell your future self why have you made these changes --> - Versioning-3.1 ## How did you test it? <!-- How have you verified this change? Tested locally? Added a unit test? Checked in staging env? --> - Added more functional tests - Existing suite ## Potential risks <!-- Assuming the worst case, what can be broken when deploying this change to production? --> - None, going to feature. ## Documentation <!-- Have you made sure this change doesn't falsify anything currently stated in `docs/`? If significant new behavior is added, have you described that in `docs/`? --> ## Is hotfix candidate? <!-- Is this PR a hotfix candidate or does it require a notification to be sent to the broader community? (Yes/No) --> No
## What changed? <!-- Describe what has changed in this PR --> - Deployment BuildId's are no longer unique across a namespace. The combination of <DeploymentName, BuildID> will be. - Constraints for not allowing "/" and "__" in deploymentName and buildID respectively - We want the APIs to leave open the possibility of using a custom version id instead of deployment_name/build_id to set current or ramping. Also we want to accept the "__unversioned__" string without having to accep an "unversioned version" . To support this, refactored APIs to accept version strings. Refactored internal code to use build id string for build id only, and version string for fully-qualified version string only. ## Why? <!-- Tell your future self why have you made these changes --> - Versioning-3.1 ## How did you test it? <!-- How have you verified this change? Tested locally? Added a unit test? Checked in staging env? --> - Existing suite of tests ## Potential risks <!-- Assuming the worst case, what can be broken when deploying this change to production? --> ## Documentation <!-- Have you made sure this change doesn't falsify anything currently stated in `docs/`? If significant new behavior is added, have you described that in `docs/`? --> ## Is hotfix candidate? <!-- Is this PR a hotfix candidate or does it require a notification to be sent to the broader community? (Yes/No) --> --------- Co-authored-by: Carly de Frondeville <carly.defrondeville@temporal.io>
## What changed? <!-- Describe what has changed in this PR --> ## Why? <!-- Tell your future self why have you made these changes --> ## How did you test it? <!-- How have you verified this change? Tested locally? Added a unit test? Checked in staging env? --> ## Potential risks <!-- Assuming the worst case, what can be broken when deploying this change to production? --> ## Documentation <!-- Have you made sure this change doesn't falsify anything currently stated in `docs/`? If significant new behavior is added, have you described that in `docs/`? --> ## Is hotfix candidate? <!-- Is this PR a hotfix candidate or does it require a notification to be sent to the broader community? (Yes/No) --> --------- Co-authored-by: Shivam <57200924+Shivs11@users.noreply.github.com>
## What changed? <!-- Describe what has changed in this PR --> Updated code to use latest API changes in which Deployment Versions are represented by string fields rather than structs. `WorkerDeploymentVersion` proto message still exists but only for the internal APIs. ## Why? <!-- Tell your future self why have you made these changes --> See temporalio/api#547. ## How did you test it? <!-- How have you verified this change? Tested locally? Added a unit test? Checked in staging env? --> ## Potential risks <!-- Assuming the worst case, what can be broken when deploying this change to production? --> ## Documentation <!-- Have you made sure this change doesn't falsify anything currently stated in `docs/`? If significant new behavior is added, have you described that in `docs/`? --> ## Is hotfix candidate? <!-- Is this PR a hotfix candidate or does it require a notification to be sent to the broader community? (Yes/No) --> --------- Co-authored-by: Carly de Frondeville <carly.defrondeville@temporal.io>
## What changed? <!-- Describe what has changed in this PR --> ## Why? <!-- Tell your future self why have you made these changes --> ## How did you test it? <!-- How have you verified this change? Tested locally? Added a unit test? Checked in staging env? --> ## Potential risks <!-- Assuming the worst case, what can be broken when deploying this change to production? --> ## Documentation <!-- Have you made sure this change doesn't falsify anything currently stated in `docs/`? If significant new behavior is added, have you described that in `docs/`? --> ## Is hotfix candidate? <!-- Is this PR a hotfix candidate or does it require a notification to be sent to the broader community? (Yes/No) --> --------- Co-authored-by: Shivam <57200924+Shivs11@users.noreply.github.com> Co-authored-by: Shahab Tajik <shahab@temporal.io> Co-authored-by: Shivam Saraf <shivam.saraf@temporal.io> Co-authored-by: ShahabT <shahab.tajik@temporal.io>
## What changed? <!-- Describe what has changed in this PR --> ## Why? <!-- Tell your future self why have you made these changes --> ## How did you test it? <!-- How have you verified this change? Tested locally? Added a unit test? Checked in staging env? --> ## Potential risks <!-- Assuming the worst case, what can be broken when deploying this change to production? --> ## Documentation <!-- Have you made sure this change doesn't falsify anything currently stated in `docs/`? If significant new behavior is added, have you described that in `docs/`? --> ## Is hotfix candidate? <!-- Is this PR a hotfix candidate or does it require a notification to be sent to the broader community? (Yes/No) -->
## What changed? <!-- Describe what has changed in this PR --> - added poller presence (called MissingTaskQueue in code) checks when a version starts to ramp or wants to be set as current - happy path tests for delete version have been added - happy path tests for poller presence checks have been added ## Why? <!-- Tell your future self why have you made these changes --> - versioning-3.1 ## How did you test it? <!-- How have you verified this change? Tested locally? Added a unit test? Checked in staging env? --> - NO TESTS have been added - just an initial prototype for review. ## Potential risks <!-- Assuming the worst case, what can be broken when deploying this change to production? --> ## Documentation <!-- Have you made sure this change doesn't falsify anything currently stated in `docs/`? If significant new behavior is added, have you described that in `docs/`? --> ## Is hotfix candidate? <!-- Is this PR a hotfix candidate or does it require a notification to be sent to the broader community? (Yes/No) --> --------- Co-authored-by: Carly de Frondeville <carly.defrondeville@temporal.io>
… wf (#7240) ## What changed? <!-- Describe what has changed in this PR --> ## Why? <!-- Tell your future self why have you made these changes --> ## How did you test it? <!-- How have you verified this change? Tested locally? Added a unit test? Checked in staging env? --> ## Potential risks <!-- Assuming the worst case, what can be broken when deploying this change to production? --> ## Documentation <!-- Have you made sure this change doesn't falsify anything currently stated in `docs/`? If significant new behavior is added, have you described that in `docs/`? --> ## Is hotfix candidate? <!-- Is this PR a hotfix candidate or does it require a notification to be sent to the broader community? (Yes/No) -->
…kQueueFamilyData (#7234) ## What changed? 1. Pass Task Queue Types to `CheckIfTaskQueuesHavePollers` activity so that we check pollers for only the task queue types that are registered in that version. 2. Remove `DeploymentVersionData` from the per-type map in `TaskQueueFamilyData`. `DeploymentVersionData` stores the same info as `VersionLocalState`, so repeating it num_task_queue times inside the VersionLocalState was a waste of space, and kind of confusing. I spoke with Shahab yesterday about how `TaskQueueFamilyData` should contain information that is specific to each task queue type within that version. So far, the only data that is specific to the version + task queue tuple is `first_poller_time`, so I put that in a new `TaskQueueVersionData` struct and put it in `TaskQueueFamilyData`. ## Why? See explanation above. ## How did you test it? Tested that the existing Versioning 3.1 functional tests still work, including DeleteVersion with poller presence. ## Potential risks <!-- Assuming the worst case, what can be broken when deploying this change to production? --> ## Documentation <!-- Have you made sure this change doesn't falsify anything currently stated in `docs/`? If significant new behavior is added, have you described that in `docs/`? --> ## Is hotfix candidate? <!-- Is this PR a hotfix candidate or does it require a notification to be sent to the broader community? (Yes/No) -->
… wf (#7240) ## What changed? <!-- Describe what has changed in this PR --> ## Why? <!-- Tell your future self why have you made these changes --> ## How did you test it? <!-- How have you verified this change? Tested locally? Added a unit test? Checked in staging env? --> ## Potential risks <!-- Assuming the worst case, what can be broken when deploying this change to production? --> ## Documentation <!-- Have you made sure this change doesn't falsify anything currently stated in `docs/`? If significant new behavior is added, have you described that in `docs/`? --> ## Is hotfix candidate? <!-- Is this PR a hotfix candidate or does it require a notification to be sent to the broader community? (Yes/No) -->
…eploymentVersionForWorkflowID
carlydf
commented
Feb 5, 2025
carlydf
commented
Feb 5, 2025
| // If the new current is unversioned, and it was previously ramping, we need to tell | ||
| // all the task queues with unversioned ramp that they no longer have unversioned ramp. | ||
| // The task queues with unversioned ramp are the task queues of the previous current version. | ||
| // TODO (Carly): Should we ban people from changing the task queues in the current version while they have an unversioned ramp? |
There was a problem hiding this comment.
reasoning for banning this is that we use the current version's task queues to know what task queues the unversioned ramp is set on. We need that list to be able to unset the unversioned ramp.
There was a problem hiding this comment.
task queues can only be added to the current version, right? so the list will only get larger so you'd still unset all the unversioned ramps from the TQs who got the unversioned ramp.
There was a problem hiding this comment.
If someone does ignoreMissingTaskQueues = true then they can do whatever they want though right? including removing task queues from the current version?
There was a problem hiding this comment.
We would be unable to tell those task queues "you are done with unversioned ramp"
There was a problem hiding this comment.
Other option is to maintain a separate list inside the DeploymentLocalState of the task queues that we set unversioned ramp on ...
374d486 to
8213b09
Compare
ShahabT
approved these changes
Feb 5, 2025
ShahabT
added a commit
that referenced
this pull request
Feb 5, 2025
## What changed? - The initial current version of a worker deployment is now `"__unversioned__"` instead of `""` - When the ramping version of a worker deployment becomes unversioned, the deployment tells all the task queues in the current version that they now have an unversioned ramp. - If the unversioned ramp is promoted to current version, or if the unversioned ramping version is unset, the deployment tells all the task queues of the previous current version that they no longer have an unversioned ramp. ## Why? <!-- Tell your future self why have you made these changes --> ## How did you test it? - Made sure all existing WorkerDeploymentSuite and DeploymentVersionSuite tests pass - Wrote new tests in WorkerDeploymentSuite for the specific edge cases that this code handles: - `TestSetCurrentVersion_Unversioned_NoRamp`: Test that when current version changes from versioned -> unversioned, the task queues become unversioned - `TestSetCurrentVersion_Unversioned_PromoteUnversionedRamp`: Test that when the current version changes from versioned -> unversioned and unversioned was previously ramping, the task queues become unversioned with no more unversioned ramp - `TestSetRampingVersion_Unversioned_UnversionedCurrent`: Test that this fails with "Ramping version __unversioned__ is already current" error - `TestSetRampingVersion_Unversioned_VersionedCurrent`: Test that the ramping version of the current version's task queues becomes unversioned ## Potential risks <!-- Assuming the worst case, what can be broken when deploying this change to production? --> ## Documentation <!-- Have you made sure this change doesn't falsify anything currently stated in `docs/`? If significant new behavior is added, have you described that in `docs/`? --> ## Is hotfix candidate? <!-- Is this PR a hotfix candidate or does it require a notification to be sent to the broader community? (Yes/No) --> --------- Co-authored-by: Shivam <57200924+shivs11@users.noreply.github.com> Co-authored-by: Shahab Tajik <shahab@temporal.io> Co-authored-by: Shivam Saraf <shivam.saraf@temporal.io> Co-authored-by: ShahabT <shahab.tajik@temporal.io>
ShahabT
added a commit
that referenced
this pull request
Feb 6, 2025
## What changed? - The initial current version of a worker deployment is now `"__unversioned__"` instead of `""` - When the ramping version of a worker deployment becomes unversioned, the deployment tells all the task queues in the current version that they now have an unversioned ramp. - If the unversioned ramp is promoted to current version, or if the unversioned ramping version is unset, the deployment tells all the task queues of the previous current version that they no longer have an unversioned ramp. ## Why? <!-- Tell your future self why have you made these changes --> ## How did you test it? - Made sure all existing WorkerDeploymentSuite and DeploymentVersionSuite tests pass - Wrote new tests in WorkerDeploymentSuite for the specific edge cases that this code handles: - `TestSetCurrentVersion_Unversioned_NoRamp`: Test that when current version changes from versioned -> unversioned, the task queues become unversioned - `TestSetCurrentVersion_Unversioned_PromoteUnversionedRamp`: Test that when the current version changes from versioned -> unversioned and unversioned was previously ramping, the task queues become unversioned with no more unversioned ramp - `TestSetRampingVersion_Unversioned_UnversionedCurrent`: Test that this fails with "Ramping version __unversioned__ is already current" error - `TestSetRampingVersion_Unversioned_VersionedCurrent`: Test that the ramping version of the current version's task queues becomes unversioned ## Potential risks <!-- Assuming the worst case, what can be broken when deploying this change to production? --> ## Documentation <!-- Have you made sure this change doesn't falsify anything currently stated in `docs/`? If significant new behavior is added, have you described that in `docs/`? --> ## Is hotfix candidate? <!-- Is this PR a hotfix candidate or does it require a notification to be sent to the broader community? (Yes/No) --> --------- Co-authored-by: Shivam <57200924+shivs11@users.noreply.github.com> Co-authored-by: Shahab Tajik <shahab@temporal.io> Co-authored-by: Shivam Saraf <shivam.saraf@temporal.io> Co-authored-by: ShahabT <shahab.tajik@temporal.io>
ShahabT
added a commit
that referenced
this pull request
Feb 6, 2025
## What changed? - The initial current version of a worker deployment is now `"__unversioned__"` instead of `""` - When the ramping version of a worker deployment becomes unversioned, the deployment tells all the task queues in the current version that they now have an unversioned ramp. - If the unversioned ramp is promoted to current version, or if the unversioned ramping version is unset, the deployment tells all the task queues of the previous current version that they no longer have an unversioned ramp. ## Why? <!-- Tell your future self why have you made these changes --> ## How did you test it? - Made sure all existing WorkerDeploymentSuite and DeploymentVersionSuite tests pass - Wrote new tests in WorkerDeploymentSuite for the specific edge cases that this code handles: - `TestSetCurrentVersion_Unversioned_NoRamp`: Test that when current version changes from versioned -> unversioned, the task queues become unversioned - `TestSetCurrentVersion_Unversioned_PromoteUnversionedRamp`: Test that when the current version changes from versioned -> unversioned and unversioned was previously ramping, the task queues become unversioned with no more unversioned ramp - `TestSetRampingVersion_Unversioned_UnversionedCurrent`: Test that this fails with "Ramping version __unversioned__ is already current" error - `TestSetRampingVersion_Unversioned_VersionedCurrent`: Test that the ramping version of the current version's task queues becomes unversioned ## Potential risks <!-- Assuming the worst case, what can be broken when deploying this change to production? --> ## Documentation <!-- Have you made sure this change doesn't falsify anything currently stated in `docs/`? If significant new behavior is added, have you described that in `docs/`? --> ## Is hotfix candidate? <!-- Is this PR a hotfix candidate or does it require a notification to be sent to the broader community? (Yes/No) --> --------- Co-authored-by: Shivam <57200924+shivs11@users.noreply.github.com> Co-authored-by: Shahab Tajik <shahab@temporal.io> Co-authored-by: Shivam Saraf <shivam.saraf@temporal.io> Co-authored-by: ShahabT <shahab.tajik@temporal.io>
ShahabT
added a commit
that referenced
this pull request
Feb 6, 2025
## What changed? - The initial current version of a worker deployment is now `"__unversioned__"` instead of `""` - When the ramping version of a worker deployment becomes unversioned, the deployment tells all the task queues in the current version that they now have an unversioned ramp. - If the unversioned ramp is promoted to current version, or if the unversioned ramping version is unset, the deployment tells all the task queues of the previous current version that they no longer have an unversioned ramp. ## Why? <!-- Tell your future self why have you made these changes --> ## How did you test it? - Made sure all existing WorkerDeploymentSuite and DeploymentVersionSuite tests pass - Wrote new tests in WorkerDeploymentSuite for the specific edge cases that this code handles: - `TestSetCurrentVersion_Unversioned_NoRamp`: Test that when current version changes from versioned -> unversioned, the task queues become unversioned - `TestSetCurrentVersion_Unversioned_PromoteUnversionedRamp`: Test that when the current version changes from versioned -> unversioned and unversioned was previously ramping, the task queues become unversioned with no more unversioned ramp - `TestSetRampingVersion_Unversioned_UnversionedCurrent`: Test that this fails with "Ramping version __unversioned__ is already current" error - `TestSetRampingVersion_Unversioned_VersionedCurrent`: Test that the ramping version of the current version's task queues becomes unversioned ## Potential risks <!-- Assuming the worst case, what can be broken when deploying this change to production? --> ## Documentation <!-- Have you made sure this change doesn't falsify anything currently stated in `docs/`? If significant new behavior is added, have you described that in `docs/`? --> ## Is hotfix candidate? <!-- Is this PR a hotfix candidate or does it require a notification to be sent to the broader community? (Yes/No) --> --------- Co-authored-by: Shivam <57200924+shivs11@users.noreply.github.com> Co-authored-by: Shahab Tajik <shahab@temporal.io> Co-authored-by: Shivam Saraf <shivam.saraf@temporal.io> Co-authored-by: ShahabT <shahab.tajik@temporal.io>
ShahabT
added a commit
that referenced
this pull request
Feb 6, 2025
## What changed? - The initial current version of a worker deployment is now `"__unversioned__"` instead of `""` - When the ramping version of a worker deployment becomes unversioned, the deployment tells all the task queues in the current version that they now have an unversioned ramp. - If the unversioned ramp is promoted to current version, or if the unversioned ramping version is unset, the deployment tells all the task queues of the previous current version that they no longer have an unversioned ramp. ## Why? <!-- Tell your future self why have you made these changes --> ## How did you test it? - Made sure all existing WorkerDeploymentSuite and DeploymentVersionSuite tests pass - Wrote new tests in WorkerDeploymentSuite for the specific edge cases that this code handles: - `TestSetCurrentVersion_Unversioned_NoRamp`: Test that when current version changes from versioned -> unversioned, the task queues become unversioned - `TestSetCurrentVersion_Unversioned_PromoteUnversionedRamp`: Test that when the current version changes from versioned -> unversioned and unversioned was previously ramping, the task queues become unversioned with no more unversioned ramp - `TestSetRampingVersion_Unversioned_UnversionedCurrent`: Test that this fails with "Ramping version __unversioned__ is already current" error - `TestSetRampingVersion_Unversioned_VersionedCurrent`: Test that the ramping version of the current version's task queues becomes unversioned ## Potential risks <!-- Assuming the worst case, what can be broken when deploying this change to production? --> ## Documentation <!-- Have you made sure this change doesn't falsify anything currently stated in `docs/`? If significant new behavior is added, have you described that in `docs/`? --> ## Is hotfix candidate? <!-- Is this PR a hotfix candidate or does it require a notification to be sent to the broader community? (Yes/No) --> --------- Co-authored-by: Shivam <57200924+shivs11@users.noreply.github.com> Co-authored-by: Shahab Tajik <shahab@temporal.io> Co-authored-by: Shivam Saraf <shivam.saraf@temporal.io> Co-authored-by: ShahabT <shahab.tajik@temporal.io>
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?
"__unversioned__"instead of""Why?
How did you test it?
TestSetCurrentVersion_Unversioned_NoRamp: Test that when current version changes from versioned -> unversioned, the task queues become unversionedTestSetCurrentVersion_Unversioned_PromoteUnversionedRamp: Test that when the current version changes from versioned -> unversioned and unversioned was previously ramping, the task queues become unversioned with no more unversioned rampTestSetRampingVersion_Unversioned_UnversionedCurrent: Test that this fails with "Ramping version unversioned is already current" errorTestSetRampingVersion_Unversioned_VersionedCurrent: Test that the ramping version of the current version's task queues becomes unversionedPotential risks
Documentation
Is hotfix candidate?