omit DescribeVersion API calls for drained versions#229
omit DescribeVersion API calls for drained versions#229
Conversation
Switch GetWorkerDeploymentState from the SDK's Describe() to the gRPC DescribeWorkerDeployment API, which returns full drainage info (including DrainedSince timestamps) in the version summaries. This eliminates per-version DescribeVersion calls just to obtain drainage timestamps. Additionally, gate the remaining DescribeVersion call (for poller checks) on the k8s Deployment existing at 0 replicas. Drained versions whose k8s Deployments have already been deleted no longer trigger any DescribeVersion calls, since there is no action to take for them. For a deployment with 1 current + 99 drained versions (no k8s Deployments), this reduces API calls per reconciliation from ~101 to ~2. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Shivs11
changed the title
| state.RampingSince = &rampingSinceTime | ||
| state.RampLastModifiedAt = &lastRampUpdateTime | ||
| if routingConfig.RampingDeploymentVersion != nil { | ||
| if routingConfig.RampingVersionChangedTime != nil { |
There was a problem hiding this comment.
I know the server would never set a ramping version to be a non-nil without a ramping changed time. However, now, we are directly using the gRPC layer to communicate with the server. The SDK, prior to this PR, was smart enough to convert nil timestamps (if ever present) to time.Time values (nil-safe)
we are now playing with protobuf pointers, and the thought process was to be safe than sorry.
There was a problem hiding this comment.
++ @Shivs11 it's good to be safe, here.
| // stubWorkflowServiceClient implements workflowservice.WorkflowServiceClient, returning | ||
| // a NotFound error for DescribeWorkerDeployment (matching the default stub behavior). |
There was a problem hiding this comment.
It doesn't look like the return from DescribeWorkerDeployment is a NotFound :) Looks like the return is a workflowservice.DescribeWorkerDeploymentResponse to me!
| state.RampingSince = &rampingSinceTime | ||
| state.RampLastModifiedAt = &lastRampUpdateTime | ||
| if routingConfig.RampingDeploymentVersion != nil { | ||
| if routingConfig.RampingVersionChangedTime != nil { |
There was a problem hiding this comment.
++ @Shivs11 it's good to be safe, here.
| version.DeploymentVersion.DeploymentName == routingConfig.CurrentDeploymentVersion.DeploymentName && | ||
| version.DeploymentVersion.BuildId == routingConfig.CurrentDeploymentVersion.BuildId { | ||
| versionInfo.Status = temporaliov1alpha1.VersionStatusCurrent | ||
| } else if routingConfig.RampingVersion != nil && | ||
| version.Version.DeploymentName == routingConfig.RampingVersion.DeploymentName && | ||
| version.Version.BuildID == routingConfig.RampingVersion.BuildID { | ||
| } else if routingConfig.RampingDeploymentVersion != nil && | ||
| version.DeploymentVersion.DeploymentName == routingConfig.RampingDeploymentVersion.DeploymentName && | ||
| version.DeploymentVersion.BuildId == routingConfig.RampingDeploymentVersion.BuildId { |
There was a problem hiding this comment.
is version.DeploymentVersion guaranteed to be non-nil? If not, let's add a nil-guard somewhere to this code.
2 participants
Summary
GetWorkerDeploymentStatefrom the SDK'sDescribe()to the gRPCDescribeWorkerDeploymentAPI, which returns fullDrainageInfo(includingDrainedSincetimestamps) in the version summariesDescribeVersioncalls for obtaining drainage timestamps — read them directly from the gRPC summaryDescribeVersioncall (for poller checks) on the k8s Deployment existing at 0 replicas; skip entirely if the k8s Deployment has been deletedTest plan
go test ./internal/temporal/...)go test ./internal/tests/...)DrainedSinceis correctly populated from the gRPC summaryNoTaskQueuesHaveVersionedPollerstill works for drained versions with k8s Deployments at 0 replicasDescribeVersioncalls🤖 Generated with Claude Code