Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add dynamic config flag to disable worker versioning #4222

Merged

Conversation

bergundy
Copy link
Member

The feature is not ready yet and once it is, we'll still need a safe way to roll it out, hence this feature flag.

@bergundy bergundy requested a review from a team as a code owner April 25, 2023 03:44
@bergundy bergundy merged commit e5372ec into temporalio:master Apr 25, 2023
10 checks passed
@bergundy bergundy deleted the worker-versioning-dynamic-config-flag branch April 25, 2023 05:06
@@ -115,6 +115,8 @@ const (
DeadlockInterval = "system.deadlock.Interval"
// How many extra goroutines can be created per root.
DeadlockMaxWorkersPerRoot = "system.deadlock.MaxWorkersPerRoot"
// Enable / disable worker versioning.
EnableWorkerVersioning = "system.workerVersioning.enable"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be something like frontend.workerVersioningData since it's controlling the frontend rpcs that read/write versioning data. we'll have another dynamic config for the matching behavior

@@ -117,4 +117,5 @@ var (
errBatchOpsMaxWorkflowExecutionCount = serviceerror.NewInvalidArgument("Workflow executions count exceeded.")

errUpdateWorkflowExecutionAPINotAllowed = serviceerror.NewPermissionDenied("UpdateWorkflowExecution operation is disabled on this namespace.", "")
errWorkerVersioningNotAllowed = serviceerror.NewPermissionDenied("Worker versioning operations is disabled on this namespace.", "")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
errWorkerVersioningNotAllowed = serviceerror.NewPermissionDenied("Worker versioning operations is disabled on this namespace.", "")
errWorkerVersioningNotAllowed = serviceerror.NewPermissionDenied("Worker versioning operations are disabled on this namespace.", "")

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I already merged this as I thought we'd use a single system-wide flag to control this behavior.
I still believe a single flag is preferable from the operator perspective.
I can fix the grammer in a separate PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already had a situation where I was very glad that we had separate controls for batch api and batch workers, for example. I can imagine similar situations with this feature, e.g. if we need to do some schema migration we might want to block updates but allow workflows to continue running. I'd rather do the more flexible thing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I'll change in a follow up PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants