ccl/sqlproxyccl: include DRAINING pods in the directory cache #79368
Conversation
|
This change is |
jaylim-crl
requested review from
jeffswenson and
andy-kimball
and removed request for
a team
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @jaylim-crl and @jeffswenson)
pkg/ccl/sqlproxyccl/tenant/directory.proto, line 48 at r1 (raw file):
// TenantID is the tenant that owns the pod. uint64 tenant_id = 2 [(gogoproto.customname) = "TenantID"]; // addr is the ip and port combination identifying the tenant pod, (e.g.
NIT: The first letter of these comments should be capitalized, b/c this comment gets written to the generated Go file, where the field is capitalized. Our convention is to make the comment match the generated Go definition, not the protobuf definition.
pkg/ccl/sqlproxyccl/tenant/directory_cache.go, line 197 at r1 (raw file):
// Trigger resumption if there are no RUNNING pods. runningPods := make([]*Pod, 0, len(tenantPods))
You're not actually using runningPods here. Instead, you can iterate through the tenantPods list and keep a foundRunning boolean that you set if you find a RUNNING pod.
Previously, cockroachdb#67452 removed DRAINING pods from the directory cache. This commit adds that back. The connector will now need to filter for RUNNING pods manually before invoking the balancer. This is needed so that we could track DRAINING pods, and wait until 60 seconds has elapsed before transferring connections away from them. To support that, we also update the Pod's proto definition to include a StateTimestamp field to reprevent that timestamp that the state field was last updated. The plan is to have a polling mechanism every X seconds to check DRAINING pods, and use that information to start migrating connections. Release note: None
jaylim-crl
force-pushed
the
jay/220404-cache-draining-pods
branch
from
5c78e85 to
7586717
Compare
There was a problem hiding this comment.
TFTR!
Reviewable status:
pkg/ccl/sqlproxyccl/tenant/directory.proto, line 48 at r1 (raw file):
Previously, andy-kimball (Andy Kimball) wrote…
NIT: The first letter of these comments should be capitalized, b/c this comment gets written to the generated Go file, where the field is capitalized. Our convention is to make the comment match the generated Go definition, not the protobuf definition.
Done.
pkg/ccl/sqlproxyccl/tenant/directory_cache.go, line 197 at r1 (raw file):
Previously, andy-kimball (Andy Kimball) wrote…
You're not actually using
runningPodshere. Instead, you can iterate through thetenantPodslist and keep afoundRunningboolean that you set if you find a RUNNING pod.
Whoops - done.
|
TFTR! bors r=JeffSwenson |
|
Already running a review |
|
Build succeeded: |
Previously, #67452 removed DRAINING pods from the directory cache. This commit
adds that back. The connector will now need to filter for RUNNING pods manually
before invoking the balancer. This is needed so that we could track DRAINING
pods, and wait until 60 seconds has elapsed before transferring connections
away from them. To support that, we also update the Pod's proto definition to
include a StateTimestamp field to reprevent that timestamp that the state field
was last updated.
The plan is to have a polling mechanism every X seconds to check DRAINING pods,
and use that information to start migrating connections.
Release note: None
Jira issue: CRDB-14759