ccl/sqlproxyccl: cleanup balancer and connection tracker#79857
ccl/sqlproxyccl: cleanup balancer and connection tracker#79857craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
This change is |
|
re: #79725 (review) TFTR! Previously, andy-kimball (Andy Kimball) wrote…
Done. Previously, andy-kimball (Andy Kimball) wrote…
Done. Good point about the extra allocations and transformations. re: scope of locks, that may not be relevant here since the previous design obtained a snapshot of tenants before copying the individual tenant conns, so we're not holding onto the lock for a long time. Previously, andy-kimball (Andy Kimball) wrote…
Hm, it's currently unclear how the other function will look like yet, so I left this as a TODO in the other PR, but consider this done. We will extract this out once we have a clearer idea. |
jaylim-crl
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @andy-kimball and @jeffswenson)
pkg/ccl/sqlproxyccl/balancer/balancer.go, line 314 at r1 (raw file):
} connMap := b.connTracker.GetConnsMap(tenantID)
One aspect to point out about this design is that we're now obtaining a lock each time we do this (to retrieve *tenantEntry), unlike the previous approach, where we're only obtaining the lock once. That said, these operations should be in the order of nanoseconds, so it should not matter much.
c3a5324 to
246fe83
Compare
This is a follow up to cockroachdb#79725. Previously, there were two issues: 1. Some of the ConnTracker APIs (e.g. GetConns and OnDisconnect) were forcing creation of tenant entries even though that was unnecessary. 2. We were holding onto a large portion of memory (with connections from all tenants). This commit addresses the two issues above: (1) tenant entries are no longer being created when that was unnecessary, and (2) instead of holding onto all the connections at once, we will retrieve the tenant's connections one by one. At the same time, to avoid extra allocations that are solely used during data transformations, we update the ConnTracker API to return a list of connection handles indexed by pod's address directly. Note that we may reintroduce the data transformation in future PRs, depending on how we implement the connection rebalancing logic. That is currently unclear. Release note: None
246fe83 to
4f6b777
Compare
|
TFTR! bors r=JeffSwenson |
|
Build succeeded: |
This is a follow up to #79725. Previously, there were two issues:
creation of tenant entries even though that was unnecessary.
tenants).
This commit addresses the two issues above: (1) tenant entries are no longer
being created when that was unnecessary, and (2) instead of holding onto all
the connections at once, we will retrieve the tenant's connections one by one.
At the same time, to avoid extra allocations that are solely used during data
transformations, we update the ConnTracker API to return a list of connection
handles indexed by pod's address directly.
Note that we may reintroduce the data transformation in future PRs, depending
on how we implement the connection rebalancing logic. That is currently
unclear.
Release note: None