nitishm / go-rejson

Golang client for redislabs' ReJSON module with support for multilple redis clients (redigo, go-redis)
MIT License
343 stars 47 forks source link

`redis: client is closed` only with go-redis/v8 + go-rejson/v4 #52

Closed 5HT2 closed 3 years ago

5HT2 commented 3 years ago

Describe the bug I get an error trying to access redis either via go-redis or go-rejoin, once I add go-rejson to my code.

To Reproduce Basically close to the default example. Accessing either the database via either redis.Client or rejson.Handler will cause the redis: client is closed error. Removing rejson.Handler from my setup to look like the second code block fixes my issue (which is not ideal, since I'd like to use go-rejson).

// Causes database to just not be accessible
func SetupDatabase() (*redis.Client, *rejson.Handler) {
    rh := rejson.NewReJSONHandler()

    client := redis.NewClient(&redis.Options{
        Addr:     redisAddr,
        Password: redisPass,
    })
    defer func() {
        if err := client.FlushAll(ctx).Err(); err != nil {
            log.Fatalf("goredis - failed to flush: %v", err)
        } else {
            log.Printf("goredis - flushed")
        }
        if err := client.Close(); err != nil {
            log.Fatalf("goredis - failed to communicate to redis-server: %v", err)
        }
    }()

    rh.SetGoRedisClient(client)
    return client, rh
}
// Works fine
func SetupDatabase() (*redis.Client, *rejson.Handler) {
    client := redis.NewClient(&redis.Options{
        Addr:     redisAddr,
        Password: redisPass,
    })
    return client
}

Expected behavior It works fine

Desktop (please complete the following information):

Additional context It looks like client.PoolStats().Misses is 1 after doing the .FulshAll. Removing the .FlushAll does not fix anything, but it does change Misses to 0. client.PoolStats().TotalConns is always 0 for whatever reason.

I'm running redis-server 6.2.5.

Shivam010 commented 3 years ago

Hello @l1ving, the problem here is in the function SetupDatabase. The deferred function executes when the SetupDatabase returns and hence, the client is closed when you are using it.

Deferred functions are executed when the calling function returns. :link: :link:

You can change your function to func SetupDatabase() (cli *redis.Client, rh *rejson.Handler) and defer the close function after calling SetupDatabase in your program.

func SetupDatabase() (cli *redis.Client, rh *rejson.Handler) {
    rh := rejson.NewReJSONHandler()
    client := redis.NewClient(&redis.Options{
        Addr:     redisAddr,
        Password: redisPass,
    })
    rh.SetGoRedisClient(client)
    return client, rh
}

func program() {
    // ...
    cli, rh := SetupDatabase()
    defer func() {
        // Note: You probably don't wanna do Flush here, as it will clear your redis-server
        if err := cli.Close(); err != nil {
            // TODO: handle error
        }
    }()
    // ...
}
5HT2 commented 3 years ago

Thank you for explaining, much appreciated c:

5HT2 commented 3 years ago

Though, I planned on using cli and rh as global variables in a package, is that not possible with the way defer works here?

Shivam010 commented 3 years ago

Then just defer Close in your main() function just after you have initialized the client (and before starting to use it).

5HT2 commented 3 years ago

Thank you :)