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

feat: Add support for new redis command CLIENT NO-TOUCH #2551

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

ktsivkov
Copy link
Contributor

Issue: #2519

@ktsivkov
Copy link
Contributor Author

@monkey92t hey, could you please review this one?

@monkey92t
Copy link
Collaborator

Based on my understanding, this is a client state, not a regular command. It labels a network connection, and we should figure out how to handle similar commands.

@monkey92t monkey92t mentioned this pull request Apr 23, 2023
@ktsivkov
Copy link
Contributor Author

ktsivkov commented Apr 24, 2023

@monkey92t @SoulPancake
Some brainstorming, based on my limited understanding we need to..

  • be able to "tag" connections (multiple conn* handle pool and cluster?)
    i.e.
type baseClient struct {
	...
	connPool pool.Pooler // This here
  • after executing (such labaling commands), we need to send all followup commands to this exact connection until they are disabled again
  • find a way to handle connection drops (if a labeled connection is lost, we need to be able to handle it)

Needs some more thought but what comes to my mind on the fly is either one of those...

  • We could use some kind of a Builder interface
// ClientNoTouch /or any other labeling command/ Creates and tags a new connection and returns it for use
cl := redis.ClientNoTouch()
cl.Get(...)
cl.Close()
  • We could use sync.map to create a map of the connections and their labels, such as client no touch
  • We could introduce / or re-use / a feature to group connections that are initiated with such pre-executed commands redis.CreateGroup(ctx, &GroupConfig{ClientNoTouch:true})

@chayim chayim requested a review from ofekshenawa July 26, 2023 06:50
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

4 participants