hibiken / asynq

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

feat: add MinIdleConns to RedisClientOpt struct #891

Open sprabowo opened 3 weeks ago

sprabowo commented 3 weeks ago

context: i want to have min idle conns config available on asynq. asynq uses go-redis@v9 which support MinIdleConns.

changelog:

sprabowo commented 3 weeks ago

kindly review this small change for exposing MinIdleConns redis config @hibiken @kamikazechaser

kamikazechaser commented 1 week ago

@sprabowo You can override the asynq go-redis client and bring your own with MakeRedisClient with all custom options that are not otherwise not exposed.

Perhaps we could document that rather that individually add opts.

sprabowo commented 1 week ago

@kamikazechaser IIRC, should this be handled on app instead of this lib?

i was trying to make my own MustNewClient. it turns out, it does not work.

Screenshot 2024-06-24 at 15 00 32 Screenshot 2024-06-24 at 15 00 01

here are my finding:

  1. we can't override client init func since there's RDB internal pkg call/import, it is not allowed to import could not import github.com/hibiken/asynq/internal/rdb (invalid use of internal package "github.com/hibiken/asynq/internal/rdb").
  2. goredis pkg version will not sync i use asynq v0.22.1 while my goredis package is v7. again i can still use v7 or upgrade but this will not sync

thus i still propose to just add additional config rather re-implement this CMIIW

sprabowo commented 1 week ago
// asynq/asynq.go
package asynq

import (
    "crypto/tls"
    "time"

    "github.com/go-redis/redis/v7"
)

type MustRedisConnOpt interface {
    MustMakeRedisClient() interface{}
}

type MustRedisClientOpt struct {
    Network      string
    Addr         string
    Username     string
    Password     string
    DB           int
    DialTimeout  time.Duration
    ReadTimeout  time.Duration
    WriteTimeout time.Duration
    PoolSize     int
    MinIdleConn  int
    TLSConfig    *tls.Config
}

func (opt MustRedisClientOpt) MustMakeRedisClient() interface{} {
    return redis.NewClient(&redis.Options{
        Network:      opt.Network,
        Addr:         opt.Addr,
        Username:     opt.Username,
        Password:     opt.Password,
        DB:           opt.DB,
        DialTimeout:  opt.DialTimeout,
        WriteTimeout: opt.WriteTimeout,
        PoolSize:     opt.PoolSize,
        MinIdleConns: opt.MinIdleConn,
        TLSConfig:    opt.TLSConfig,
    })
}
// asynq/client.go
package asynq

import (
    "fmt"

    "github.com/go-redis/redis/v7"
    "github.com/hibiken/asynq/internal/rdb"
)

type Client struct {
    rdb *rdb.RDB
}

func MustNewClient(r MustRedisConnOpt) *Client {
    c, ok := r.MustMakeRedisClient().(redis.UniversalClient)
    if !ok {
        panic(fmt.Sprintf("asynq: unsupported RedisConnOpt type %T", r))
    }
    return &Client{rdb: rdb.NewRDB(c)}
}

MustNewClient implementation. this doesn't work

sprabowo commented 2 days ago

any suggestions @kamikazechaser @hibiken? really appreciate it