ClusterClient support for go-redis

[meximonster]

Hello and thanks for this library. Is there a way to support redis json with redis cluster ?

[Shivam010]

Hello @meximonster, currently go-ReJSON does not support Redis cluster API.

But I think it can be easily implemented, as far as I know, go-redis/redis implementation can be easily updated to handle its ClusterClient for Redis Clusters.

If you are willing to take the issue and implement the support for clusters, I can help.

[jaysonhurd]

Hi I'm wondering the same.... can't use this with a cluster.

[jaysonhurd]

Hello @meximonster, currently go-ReJSON does not support Redis cluster API.

But I think it can be easily implemented, as far as I know, go-redis/redis implementation can be easily updated to handle its ClusterClient for Redis Clusters.

If you are willing to take the issue and implement the support for clusters, I can help.

Hi - I could take a stab on this. We have a real need in a current project to do this in our clustered redis environment. I'd like to know if this is already in progress and if not whether you do this via fork or pull request.

[Shivam010]

Hi @jaysonhurd thank you for showing interest. I don't think @meximonster is working on the issue. So, if you want to work on the issue and implement the feature, feel free to send a PR. And let me know if you need any help.

[jaysonhurd]

@Shivam010 - thanks. I'm working on it now. I will let you know how it goes.

[jaysonhurd]

First round is working using either client or clusterclient. Tomorrow we'll do some more testing and maybe look at the unit tests.

[hirenvadalia]

@jaysonhurd (Assuming you are going with clusterclient) I would suggest to add support for UniversalClient which can handle all 3 types of client of redis (https://redis.uptrace.dev/guide/universal.html)

[Shivam010]

Hmm, yes looks good @hirenvadalia.

Never worked with it before. But I think using UniversalClient will simplify things for us. @jaysonhurd Can you look into it before starting?

[jaysonhurd]

I'll have a look.  At first glance it may be an issue for us because we use a GSLB to connect so it will think it's a regular client.  Also all our code is done either as standalone or cluster client.  Perhaps this should be added as a 3rd option?

I did get it working with the cluster client yesterday in some preliminary testing.  Today I'll do more and probably put it in one of our services (still in pre-production build status for probably another 3 months) and see if we can get it working there.

Before pushing any PR I have to get approval from Comcast's Open Source department (yes, we actually have one of these).  I've already opened this request. This could take time because apparently they are short staffed.  I will keep you posted.

-Jayson

Feb 15, 2022, 02:54 by @.***:

@jaysonhurd https://github.com/jaysonhurd> (Assuming you are going with clusterclient) I would suggest to add support for UniversalClient which can handle all 3 types of client of redis (> https://redis.uptrace.dev/guide/universal.html> )

— Reply to this email directly, > view it on GitHub https://github.com/nitishm/go-rejson/issues/53#issuecomment-1040072066> , or > unsubscribe https://github.com/notifications/unsubscribe-auth/AG5FLFJEEFGBNMMKKXSPIJLU3IPGBANCNFSM5F4VXZEQ> . Triage notifications on the go with GitHub Mobile for > iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or > Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub> . You are receiving this because you were mentioned.> Message ID: > <nitishm/go-rejson/issues/53/1040072066> @> github> .> com>

[Shivam010]

Hi @jaysonhurd I have checked the code and found that we can directly use the UniversalClient in place of its' default Client because of the interface.

Can you plz check PR #62 and check whether it is working properly with your code? @meximonster Can you also check plz?

[jaysonhurd]

Sure - will check it out.  

However if only the universal client is used it may not work in all situations.  The universal client assumes a standalone connection type if you only pass it one host.  We are using a GSLB for a cluster, which is in fact one host name with 6 behind it. If it uses a standalone connection it won't work ( I know because I already tried it):

https://redis.uptrace.dev/guide/universal.html

"if the number of Addrs is two or more, a ClusterClient is returned." Also - on a side note.  I don't suppose you've ever set up go-redis to connect via TLS have you?  We're having trouble figuring out exactly how the certs get compiled into  the TLSConfig for Redis in go-redis.... -Jayson

Feb 15, 2022, 13:36 by @.***:

Hi > @jaysonhurd https://github.com/jaysonhurd> I have checked the code and found that we can directly use the > UniversalClient https://pkg.go.dev/github.com/go-redis/redis/v8#UniversalClient> in place of its' default > Client> because of the interface.

Can you plz check PR > #62 https://github.com/nitishm/go-rejson/pull/62> and check whether it is working properly with your code?

@meximonster https://github.com/meximonster> Can you also check plz?

— Reply to this email directly, > view it on GitHub https://github.com/nitishm/go-rejson/issues/53#issuecomment-1040767404> , or > unsubscribe https://github.com/notifications/unsubscribe-auth/AG5FLFOX7PSHJLDLUDG4JJLU3K2LXANCNFSM5F4VXZEQ> . Triage notifications on the go with GitHub Mobile for > iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or > Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub> . You are receiving this because you were mentioned.> Message ID: > <nitishm/go-rejson/issues/53/1040767404> @> github> .> com>

[hirenvadalia]

@jaysonhurd trying to understand your use case: if you are using GSLB, then why do you need cluster client, its just standalone client which connects to GSLB and let GSLB do its job. And if you want to use cluster client (where you have to specify all your 6 hosts - which are behind GSLB) then why you need GSLB?

[jaysonhurd]

Checking with my DBA... maybe we will switch back to adding all the hosts in the address field.

I tried connecting as standalone client to the GSLB. Redis moves the connection among nodes depending on failover or which shard the data being requested is on.  As soon as the initial connection moves, any subsequent reads and writes fail with an err that indicates this happened.

Feb 16, 2022, 00:57 by @.***:

@jaysonhurd https://github.com/jaysonhurd> trying to understand your use case: if you are using GSLB, then why do you need cluster client, its just standalone client which connects to GSLB and let GSLB do its job. And if you want to use cluster client (where you have to specify all your 6 hosts - which are behind GSLB) then why you need GSLB?

— Reply to this email directly, > view it on GitHub https://github.com/nitishm/go-rejson/issues/53#issuecomment-1041212769> , or > unsubscribe https://github.com/notifications/unsubscribe-auth/AG5FLFOGKCRRINM3ZSL7FX3U3NKHNANCNFSM5F4VXZEQ> . Triage notifications on the go with GitHub Mobile for > iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or > Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub> . You are receiving this because you were mentioned.> Message ID: > <nitishm/go-rejson/issues/53/1041212769> @> github> .> com>

[Shivam010]

Now, I understood what your problem is, though the use of LB here is unnecessary in my opinion.

And regarding the how the certs get compiled into the TLSConfig for Redis in go-redis: can you elaborate your point? Cause using tls is pretty straightforward just use the TLSConfig field inside go-redis options. e.g

    cert, err := tls.LoadX509KeyPair("cert file path", "key file path")
        if err != nil { ... }

    cnf := &tls.Config{
        ServerName:   "localhost", // your server name
        Certificates: []tls.Certificate{cert},
                // You might need to specify RootCAs - to establish cert authorities
    }

        // with standalone client
        goredis.NewClient(&goredis.Options{
        Addr:      "localhost:6379",
        TLSConfig: cnf,
    })
        // or with universal client
        goredis.NewUniversalClient(&goredis.UniversalOptions{
        Addrs:     []string{"localhost:6379"},
        TLSConfig: cnf,
    })

[Shivam010]

@jaysonhurd @meximonster @hirenvadalia What do you think of this solution: https://github.com/nitishm/go-rejson/pull/62#issuecomment-1042046636 ?

[jaysonhurd]

Maybe we can do a screenshare?  We could set up a time.  I can send you a MS teams link if that works, or we could use Zoom. -Jayson

Feb 16, 2022, 12:08 by @.***:

@jaysonhurd https://github.com/jaysonhurd> > @meximonster https://github.com/meximonster> > @hirenvadalia https://github.com/hirenvadalia What do you think of this solution: > #62 (comment) https://github.com/nitishm/go-rejson/pull/62#issuecomment-1042046636> ?

— Reply to this email directly, > view it on GitHub https://github.com/nitishm/go-rejson/issues/53#issuecomment-1042052071> , or > unsubscribe https://github.com/notifications/unsubscribe-auth/AG5FLFKQJQ5AYW3W7S5XTVDU3PY3VANCNFSM5F4VXZEQ> . Triage notifications on the go with GitHub Mobile for > iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or > Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub> . You are receiving this because you were mentioned.> Message ID: > <nitishm/go-rejson/issues/53/1042052071> @> github> .> com>

[Shivam010]

Ok, let's discuss it in Gophers Slack workspace. I have created a channel for go-rejson there https://gophers.slack.com/archives/C033E3DRC10

[jaysonhurd]

Trying to find it in my personal slack which I just created.  You may need to invite me directly... if you can invite @.*** ? -Jayson

Feb 16, 2022, 12:20 by @.***:

Ok, let's discuss it in Gophers Slack workspace. I have created a channel for go-rejson there > https://gophers.slack.com/archives/C033E3DRC10

— Reply to this email directly, > view it on GitHub https://github.com/nitishm/go-rejson/issues/53#issuecomment-1042068524> , or > unsubscribe https://github.com/notifications/unsubscribe-auth/AG5FLFMHFWV6COVJW2JECGLU3P2HBANCNFSM5F4VXZEQ> . Triage notifications on the go with GitHub Mobile for > iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or > Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub> . You are receiving this because you were mentioned.> Message ID: > <nitishm/go-rejson/issues/53/1042068524> @> github> .> com>

[hirenvadalia]

@jaysonhurd @meximonster @hirenvadalia What do you think of this solution: #62 (comment) ?

+1 for solution @Shivam010 , can we please get it in, i need it for universal client support

[Shivam010]

@jaysonhurd Gophers Slack Invite link https://join.slack.com/t/gophers/shared_invite/zt-11ddswns3-S__IcQcJRsVqlmCDafl5_A

[Shivam010]

Released a new version with the changes: https://github.com/nitishm/go-rejson/releases/tag/v4.1.0