Redis: v7: breaking changes you would like to see

[vmihailenco]

I am planning v7 release which should support context.Context and which will be incompatible with v6 in one or another way. So if you have in mind some other incompatible changes that make sense - you are welcome to add them here.

One such change I've just made is changing ZAdd from ZAdd(key string, members ...Z) *IntCmd to ZAdd(key string, members ...*Z) *IntCmd. It saves 1 allocations and is detected by compiler - so why not.

[FlamingTree]

will pool related operation(net.Dial etc.) support context.Context？so we can record related time consumed.

[vmihailenco]

@FlamingTree do you mean passing context to Dialer? Yes, it is possible - see https://github.com/go-redis/redis/pull/1046/files

[pierrre]

Proper support for Go modules.
Currently my go.mod uses the +incompatible suffix.
If you want to keep the backward compatibility with older versions of Go (without modules support), you may want to move all the code to a /v7 sub-directory (I don't know if it's a good practice, but it was suggested in the official doc).

[vmihailenco]

@pierrre My main concern is that so far I don't have a single go-mod compatible import in the project I am working on (which has ~50 imports). They are either v0.9999 or incompatible.

[pierrre]

@vmihailenco in project that is using Go modules, you can import dependencies that don't do anything special to support Go modules.

I'm currently migrating all the projects at my company:

• if a dependency is supporting Go modules, then it's fine
• if a dependency is following semver for version v1 or v0, there is nothing special to do
• if a dependency is following semver for version v2+ (such as go-redis), then I need to use the +incompatible suffix
• if a dependency only has a "master" branch, then I'm managing it with v0.0.0 + commit ID

The only issue with go-redis, it that the "update all dependencies to the latest version" command keeps reverting go-redis to v6.15.2 instead of 6.15.3, and I don't understand why...

[vmihailenco]

Try GOPROXY=direct go get github.com/go-redis/redis

[pierrre]

GOPROXY=direct go get github.com/go-redis/[email protected]+incompatible
Result in go.mod: github.com/go-redis/redis v6.15.3+incompatible

go list -m -u all
Result: github.com/go-redis/redis v6.15.3+incompatible [v6.15.2+incompatible] (It wants to downgrade ??????)

GOPROXY=direct go get github.com/go-redis/redis
Result in go.mod: github.com/go-redis/redis v6.15.2+incompatible

[FlamingTree]

@FlamingTree do you mean passing context to Dialer? Yes, it is possible - see https://github.com/go-redis/redis/pull/1046/files

I mean it's better to have hook point on specific operation, like getConn from pool, make new connection, etc. Just like stdlib's net/http/httptrace.

[zikaeroh]

For what it's worth, Go at tip (so what will be 1.13) doesn't have the downgrade issue, probably due to golang/go#30634 and a few other module fixes. My gotip list -m -u all:

github.com/go-redis/redis v6.15.2+incompatible [v6.15.3+incompatible]

EDIT: This might actually be a bug in and of itself; I don't think module-aware projects should have +incompatible versions.

[vmihailenco]

I mean it's better to have hook point on specific operation, like getConn from pool, make new connection, etc. Just like stdlib's net/http/httptrace.

I'd like to support something like this at some point, but there is nothing backwards incompatible so this change can wait until later.

[nanasi880]

I want you to call internal.Logf with context.
Because, I injected custom logger to context.
Logger is mutable by HTTP request metadata. Not a static. ( e.g. logger have HTTP request resource path, logger adding log to parent log by resource path )

[vmihailenco]

Because, I injected custom logger to context.

@nanasi880 do you have an example of relatively popular lib that is doing this? I wonder if there are any existing conventions here.

[nanasi880]

@vmihailenco
thanks for reply.

I wonder if there are any existing conventions here.

I think so modern library is better to propagate context.
Library do not have to use context.
But, if library logic is long running or create multiple goroutine, It is better to handling ctx.Done()

do you have an example of relatively popular lib that is doing this?

I known only a little.

It is appengine/log package
https://github.com/golang/appengine/blob/master/log/api.go#L18

this package is required appengine's context.
appengine log package usage.

import (
    "context"
    "google.golang.org/appengine"
    "google.golang.org/appengine/log"
)

func httpHandler(w http.ResponseWriter, r *http.Request) {
    var ctx context.Context = appengine.NewContext(r) // create context. returned context is capture HTTP request.
    log.Debugf(ctx, "hi") // output to stackdriver : "hi"

    // invalid call
    // this will panic
    // first parameter is must be appengine's context, and context must have valid HTTP request.
    log.Debugf(context.Background(), "Oops")
}

appengine's log is flush to stackdriver.
stackdriver will grouping log by HTTP resource path and trace id.
for that, logging library need metadata from context.

If, internal.Logf call with context. I can logging which HTTP request is the problem.

like this

func httpHandler(w http.ResponseWriter, r *http.Request) {
    ctx := appengine.NewContext(r)

    // if problem happen, internal.Logf can call with ctx
    err := redisClient.Set(ctx, "my-key", 100, -1).Err()
}

[dcormier]

I'd like to see #616 done, somehow.

[Me1onRind]

Because, I injected custom logger to context.

@nanasi880 do you have an example of relatively popular lib that is doing this? I wonder if there are any existing conventions here.

https://github.com/jinzhu/gorm support logger hook. Provide a non-authoritative idea, My company's php redis lib only logging command and key , time consuming, and error.

[catkins]

Is v7 likely to come out in the next few months? starting a new project and considering whether to start out on the beta versions or to stick with stable for now?

[catkins]

:wave: Hi @vmihailenco same question as above – I notice that v7 is still in beta, and I was updating some shared libraries we use, and was weighing up with whether to hop on the v7 train or stick to v6.

[vmihailenco]

Just tagged v7.0.0 - enjoy :)

[catkins]

Perfect timing! Thanks @vmihailenco 👏