hibiken / asynq

Simple, reliable, and efficient distributed task queue in Go
MIT License
10k stars 714 forks source link

[FEATURE REQUEST] redis client version compatibility #794

Closed Jeremy-Run closed 11 months ago

Jeremy-Run commented 11 months ago

Background: Our project uses asynq-v0.24.0, which was recently upgraded to asynq-v0.24.1 . The client generated by go-redis/v8 used in the project.

After upgrading, I found that this part of the code failed to execute:

// NewClient returns a new Client instance given a redis connection option.
func NewClient(r RedisConnOpt) *Client {
    c, ok := r.MakeRedisClient().(redis.UniversalClient)
    if !ok {
        panic(fmt.Sprintf("asynq: unsupported RedisConnOpt type %T", r))
    }
    return &Client{broker: rdb.NewRDB(c)}
}

The investigation revealed that the cause of this problem is that the UniversalClient of go-redis/v9 is different from the UniversalClient of go-redis/v8.

Suggestion: Can you give us a hint as to what version of redis client should be uploaded? Or be compatible with several new versions of go-redis?

kamikazechaser commented 11 months ago

I don't fully understand your question. Asynq uses go-redis/v9. Afaik, the tests pass and everything works fine.

However if you are reusing a go-redis connection for asynq, you need to set it up correctly.

Jeremy-Run commented 11 months ago

@kamikazechaser Thank you for your reply! Let me improve my question: In our project, this will report an error when using NewClient(r RedisConnOpt). The reason for the error is that r.MakeRedisClient() returns a go-redis/v8 client, which cannot be converted into redis.UniversalClient in go-redis/v9.

kamikazechaser commented 11 months ago

Could you additionally post the code on your end to reproduce this issue.

kamikazechaser commented 11 months ago

Possibly related to #790

Jeremy-Run commented 11 months ago

This panic is inevitable. This is my pseudo-code:

package main

import (
    "github.com/hibiken/asynq"  // v0.24.1 
    "github.com/go-redis/redis/v8"
)

type XRDS struct {
    *redis.Client
}

func (xrds *XRDS) MakeRedisClient() interface{} {
    return xrds
}

func NewRedisClient() *XRDS {
    c := redis.NewFailoverClient(&redis.FailoverOptions{
        MasterName:    "mymaster",
        SentinelAddrs: []string{"x.x.x.x:16379", "x.x.x.x:16379", "x.x.x.x:16379"},
        Password:      "xxxxxx",
        DB:            1,
    })
    return &XRDS{Client: c}
}

func main() {
    _ = asynq.NewClient(NewRedisClient())
}
kamikazechaser commented 11 months ago

"github.com/go-redis/redis/v8"

Use go-redis/v9 with v0.24.1 and let me know if the issue still persists. I see that you are not using the x module which is problematic right now.

Jeremy-Run commented 11 months ago

Yes, we can't go wrong with go-redis/v9. What I want to ask is, does asynq need to support multiple go-redis versions?

kamikazechaser commented 11 months ago

What I want to ask is, does asynq need to support multiple go-redis versions?

No. We want to stick with v9 only. You can track some changes on the sohail/go-update branch which removes the last mention of go-redis/v8 from the x module.

Jeremy-Run commented 11 months ago

Well, thank you for your answer.