Skip to content

Commit

Permalink
bug: Fix SETINFO ensuring it is set-and-forget (#2915)
Browse files Browse the repository at this point in the history
* Exexcute set-info without validation

* Fix tests

* Remove spaces from runtime.Version

* fix typo

* Send setinfo after auth

* Add pipline

* fix golangci

* revert fixing typo

* support sentinel
  • Loading branch information
ofekshenawa committed Feb 20, 2024
1 parent 99527f0 commit 5da49b1
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 13 deletions.
2 changes: 1 addition & 1 deletion commands.go
Expand Up @@ -309,7 +309,7 @@ func (c statefulCmdable) ClientSetInfo(ctx context.Context, info LibraryInfo) *S

var cmd *StatusCmd
if info.LibName != nil {
libName := fmt.Sprintf("go-redis(%s,%s)", *info.LibName, runtime.Version())
libName := fmt.Sprintf("go-redis(%s,%s)", *info.LibName, internal.ReplaceSpaces(runtime.Version()))
cmd = NewStatusCmd(ctx, "client", "setinfo", "LIB-NAME", libName)
} else {
cmd = NewStatusCmd(ctx, "client", "setinfo", "LIB-VER", *info.LibVer)
Expand Down
4 changes: 2 additions & 2 deletions commands_test.go
Expand Up @@ -2105,7 +2105,7 @@ var _ = Describe("Commands", func() {

logEntries, err := client.ACLLog(ctx, 10).Result()
Expect(err).NotTo(HaveOccurred())
Expect(len(logEntries)).To(Equal(1))
Expect(len(logEntries)).To(Equal(4))

for _, entry := range logEntries {
Expect(entry.Reason).To(Equal("command"))
Expand All @@ -2121,7 +2121,7 @@ var _ = Describe("Commands", func() {

limitedLogEntries, err := client.ACLLog(ctx, 2).Result()
Expect(err).NotTo(HaveOccurred())
Expect(len(limitedLogEntries)).To(Equal(1))
Expect(len(limitedLogEntries)).To(Equal(2))
})

It("should ACL LOG RESET", Label("NonRedisEnterprise"), func() {
Expand Down
20 changes: 20 additions & 0 deletions internal/util.go
Expand Up @@ -2,6 +2,7 @@ package internal

import (
"context"
"strings"
"time"

"github.com/redis/go-redis/v9/internal/util"
Expand Down Expand Up @@ -44,3 +45,22 @@ func isLower(s string) bool {
}
return true
}

func ReplaceSpaces(s string) string {
// Pre-allocate a builder with the same length as s to minimize allocations.
// This is a basic optimization; adjust the initial size based on your use case.
var builder strings.Builder
builder.Grow(len(s))

for _, char := range s {
if char == ' ' {
// Replace space with a hyphen.
builder.WriteRune('-')
} else {
// Copy the character as-is.
builder.WriteRune(char)
}
}

return builder.String()
}
22 changes: 12 additions & 10 deletions redis.go
Expand Up @@ -334,22 +334,24 @@ func (c *baseClient) initConn(ctx context.Context, cn *pool.Conn) error {
pipe.ClientSetName(ctx, c.opt.ClientName)
}

if !c.opt.DisableIndentity {
libName := ""
libVer := Version()
if c.opt.IdentitySuffix != "" {
libName = c.opt.IdentitySuffix
}
pipe.ClientSetInfo(ctx, WithLibraryName(libName))
pipe.ClientSetInfo(ctx, WithLibraryVersion(libVer))
}

return nil
})
if err != nil {
return err
}

if !c.opt.DisableIndentity {
libName := ""
libVer := Version()
if c.opt.IdentitySuffix != "" {
libName = c.opt.IdentitySuffix
}
p := conn.Pipeline()
p.ClientSetInfo(ctx, WithLibraryName(libName))
p.ClientSetInfo(ctx, WithLibraryVersion(libVer))
_, _ = p.Exec(ctx)
}

if c.opt.OnConnect != nil {
return c.opt.OnConnect(ctx, conn)
}
Expand Down
6 changes: 6 additions & 0 deletions sentinel.go
Expand Up @@ -153,6 +153,9 @@ func (opt *FailoverOptions) sentinelOptions(addr string) *Options {
ConnMaxLifetime: opt.ConnMaxLifetime,

TLSConfig: opt.TLSConfig,

DisableIndentity: opt.DisableIndentity,
IdentitySuffix: opt.IdentitySuffix,
}
}

Expand Down Expand Up @@ -190,6 +193,9 @@ func (opt *FailoverOptions) clusterOptions() *ClusterOptions {
ConnMaxLifetime: opt.ConnMaxLifetime,

TLSConfig: opt.TLSConfig,

DisableIndentity: opt.DisableIndentity,
IdentitySuffix: opt.IdentitySuffix,
}
}

Expand Down

0 comments on commit 5da49b1

Please sign in to comment.