WIP: dedicated pool dialers to vtorc and to throttler functions, no change in "concurrency"#15560
Closed
shlomi-noach wants to merge 8 commits intovitessio:mainfrom
Closed
WIP: dedicated pool dialers to vtorc and to throttler functions, no change in "concurrency"#15560shlomi-noach wants to merge 8 commits intovitessio:mainfrom
vtorc and to throttler functions, no change in "concurrency"#15560shlomi-noach wants to merge 8 commits intovitessio:mainfrom
Conversation
Signed-off-by: Manan Gupta <manan@planetscale.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
shlomi-noach
added
Type: Bug
Component: VTOrc
Contributor
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
vitess-bot
bot
added
NeedsBackportReason
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Comment on lines
+106
to
+116
| // rpcClientMap maps an address to a tmc | ||
| type rpcClientMap map[string](chan *tmc) | ||
|
|
||
| // grpcClient implements both dialer and poolDialer. | ||
| type grpcClient struct { | ||
| // This cache of connections is to maximize QPS for ExecuteFetchAs{Dba,App}, | ||
| // CheckThrottler and FullStatus. Note we'll keep the clients open and close them upon Close() only. | ||
| // But that's OK because usually the tasks that use them are one-purpose only. | ||
| // The map is protected by the mutex. | ||
| mu sync.Mutex | ||
| rpcClientMap map[string]chan *tmc | ||
| mu sync.Mutex | ||
| rpcClientMaps map[DialPoolGroup]rpcClientMap |
Member
There was a problem hiding this comment.
DialPoolGroupThrottler and DialPoolGroupVTOrc do not need a channel and should only open One connection per tablet address.
better to have a different type for them and leave the other concurrent one seperate.
Contributor
Author
There was a problem hiding this comment.
Is it OK if I still make the code idential for all of them, but use channel size 1 for VTOrc and Throttler?
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Closed
5 tasks
Contributor
Author
|
Yeah I messed up something here. I'm gonna create yet another final draft PR, where I keep all the existing logic, and add bespoke logic for the non-default dial groups (vtorc, throttler). |
5 tasks
Contributor
Author
|
Closing in favor of #15562 |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Description
One final alternative, this is similar to #15559, but while #15559 remvoes the so-called "concurrency" element, this PR preserves it. In this PR:
vtorc; another, dedicated to the tablet throttler.dialPoolfunction returns aninvalidator, much like Fix for FullStatus gRPC connection pooling #15520. However, onlyFullStatusandCheckThrottlerchoose to invoke it. When they do, they do not affect any other pool dialer.Related Issue(s)
#15563
Checklist
Deployment Notes