-
Notifications
You must be signed in to change notification settings - Fork 60
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a Redis Sentinel backend #175
Open
sebmil-daily
wants to merge
6
commits into
prebid:master
Choose a base branch
from
dailymotion-oss:add_redis_sentinel_backend
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
0f2eca0
Add a Redis Sentinel backend
sebmil-daily 9bca40c
Fix configuration test and remove unused expiration field
sebmil-daily 337ec1f
Merge pull request #1 from dailymotion-oss/add_redis_sentinel_backend
sebmil-daily 742e424
Fix redis_sentinel configuration reading
sebmil-daily 8752139
Merge pull request #2 from dailymotion-oss/add_redis_sentinel_backend
sebmil-daily 541c237
Fix redis_sentinel password usage
sebmil-daily File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,99 @@ | ||
| package backends | ||
|
|
||
| import ( | ||
| "context" | ||
| "crypto/tls" | ||
| "time" | ||
|
|
||
| "github.com/prebid/prebid-cache/config" | ||
| "github.com/prebid/prebid-cache/utils" | ||
| "github.com/redis/go-redis/v9" | ||
| log "github.com/sirupsen/logrus" | ||
| ) | ||
|
|
||
| // RedisSentinelDB is an interface that helps us communicate with an instance of a | ||
| // Redis Sentinel database. Its implementation is intended to use the "github.com/redis/go-redis" | ||
| // client | ||
| type RedisSentinelDB interface { | ||
| Get(ctx context.Context, key string) (string, error) | ||
| Put(ctx context.Context, key string, value string, ttlSeconds int) (bool, error) | ||
| } | ||
|
|
||
| // RedisSentinelDBClient is a wrapper for the Redis client that implements the RedisSentinelDB interface | ||
| type RedisSentinelDBClient struct { | ||
| client *redis.Client | ||
| } | ||
|
|
||
| // Get returns the value associated with the provided `key` parameter | ||
| func (db RedisSentinelDBClient) Get(ctx context.Context, key string) (string, error) { | ||
| return db.client.Get(ctx, key).Result() | ||
| } | ||
|
|
||
| // Put will set 'key' to hold string 'value' if 'key' does not exist in the redis storage. | ||
| // When key already holds a value, no operation is performed. That's the reason this adapter | ||
| // uses the 'github.com/go-redis/redis's library SetNX. SetNX is short for "SET if Not eXists". | ||
| func (db RedisSentinelDBClient) Put(ctx context.Context, key, value string, ttlSeconds int) (bool, error) { | ||
| return db.client.SetNX(ctx, key, value, time.Duration(ttlSeconds)*time.Second).Result() | ||
| } | ||
|
|
||
| // RedisSentinelBackend when initialized will instantiate and configure the Redis client. It implements | ||
| // the Backend interface. | ||
| type RedisSentinelBackend struct { | ||
| cfg config.RedisSentinel | ||
| client RedisDB | ||
| } | ||
|
|
||
| // NewRedisSentinelBackend initializes the Redis Sentinel client and pings to make sure connection was successful | ||
| func NewRedisSentinelBackend(cfg config.RedisSentinel, ctx context.Context) *RedisSentinelBackend { | ||
| options := &redis.FailoverOptions{ | ||
| MasterName: cfg.MasterName, | ||
| SentinelAddrs: cfg.SentinelAddrs, | ||
| SentinelPassword: cfg.Password, | ||
| Password: cfg.Password, | ||
| DB: cfg.Db, | ||
| } | ||
|
|
||
| if cfg.TLS.Enabled { | ||
| options.TLSConfig = &tls.Config{InsecureSkipVerify: cfg.TLS.InsecureSkipVerify} | ||
| } | ||
|
|
||
| client := RedisSentinelDBClient{client: redis.NewFailoverClient(options)} | ||
|
|
||
| _, err := client.client.Ping(ctx).Result() | ||
| if err != nil { | ||
| log.Fatalf("Error creating Redis Sentinel backend: %v", err) | ||
| } | ||
| log.Infof("Connected to Redis Sentinels at %v", cfg.SentinelAddrs) | ||
|
|
||
| return &RedisSentinelBackend{ | ||
| cfg: cfg, | ||
| client: client, | ||
| } | ||
| } | ||
|
|
||
| // Get calls the Redis Sentinel client to return the value associated with the provided `key` | ||
| // parameter and interprets its response. A `Nil` error reply of the Redis client means | ||
| // the `key` does not exist. | ||
| func (b *RedisSentinelBackend) Get(ctx context.Context, key string) (string, error) { | ||
| res, err := b.client.Get(ctx, key) | ||
| if err == redis.Nil { | ||
| err = utils.NewPBCError(utils.KEY_NOT_FOUND) | ||
| } | ||
|
|
||
| return res, err | ||
| } | ||
|
|
||
| // Put writes the `value` under the provided `key` in the Redis Sentinel storage server. Because the backend | ||
| // implementation of Put calls SetNX(item *Item), a `false` return value is interpreted as the data | ||
| // not being written because the `key` already holds a value, and a RecordExistsError is returned | ||
| func (b *RedisSentinelBackend) Put(ctx context.Context, key string, value string, ttlSeconds int) error { | ||
| success, err := b.client.Put(ctx, key, value, ttlSeconds) | ||
| if err != nil && err != redis.Nil { | ||
| return err | ||
| } | ||
| if !success { | ||
| return utils.NewPBCError(utils.RECORD_EXISTS) | ||
| } | ||
|
|
||
| return nil | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,195 @@ | ||
| package backends | ||
|
|
||
| import ( | ||
| "context" | ||
| "errors" | ||
| "testing" | ||
|
|
||
| "github.com/prebid/prebid-cache/utils" | ||
| "github.com/redis/go-redis/v9" | ||
| "github.com/stretchr/testify/assert" | ||
| ) | ||
|
|
||
| func TestRedisSentinelClientGet(t *testing.T) { | ||
| redisSentinelBackend := &RedisSentinelBackend{} | ||
|
|
||
| type testInput struct { | ||
| client RedisDB | ||
| key string | ||
| } | ||
|
|
||
| type testExpectedValues struct { | ||
| value string | ||
| err error | ||
| } | ||
|
|
||
| testCases := []struct { | ||
| desc string | ||
| in testInput | ||
| expected testExpectedValues | ||
| }{ | ||
| { | ||
| desc: "RedisSentinelBackend.Get() throws a redis.Nil error", | ||
| in: testInput{ | ||
| client: FakeRedisClient{ | ||
| Success: false, | ||
| ServerError: redis.Nil, | ||
| }, | ||
| key: "someKeyThatWontBeFound", | ||
| }, | ||
| expected: testExpectedValues{ | ||
| value: "", | ||
| err: utils.NewPBCError(utils.KEY_NOT_FOUND), | ||
| }, | ||
| }, | ||
| { | ||
| desc: "RedisBackend.Get() throws an error different from redis.Nil", | ||
| in: testInput{ | ||
| client: FakeRedisClient{ | ||
| Success: false, | ||
| ServerError: errors.New("some other get error"), | ||
| }, | ||
| key: "someKey", | ||
| }, | ||
| expected: testExpectedValues{ | ||
| value: "", | ||
| err: errors.New("some other get error"), | ||
| }, | ||
| }, | ||
| { | ||
| desc: "RedisBackend.Get() doesn't throw an error", | ||
| in: testInput{ | ||
| client: FakeRedisClient{ | ||
| Success: true, | ||
| StoredData: map[string]string{"defaultKey": "aValue"}, | ||
| }, | ||
| key: "defaultKey", | ||
| }, | ||
| expected: testExpectedValues{ | ||
| value: "aValue", | ||
| err: nil, | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range testCases { | ||
| redisSentinelBackend.client = tt.in.client | ||
|
|
||
| // Run test | ||
| actualValue, actualErr := redisSentinelBackend.Get(context.Background(), tt.in.key) | ||
|
|
||
| // Assertions | ||
| assert.Equal(t, tt.expected.value, actualValue, tt.desc) | ||
| assert.Equal(t, tt.expected.err, actualErr, tt.desc) | ||
| } | ||
| } | ||
|
|
||
| func TestRedisSentinelClientPut(t *testing.T) { | ||
| redisSentinelBackend := &RedisSentinelBackend{} | ||
|
|
||
| type testInput struct { | ||
| redisSentinelClient RedisDB | ||
| key string | ||
| valueToStore string | ||
| ttl int | ||
| } | ||
|
|
||
| type testExpectedValues struct { | ||
| writtenValue string | ||
| redisClientErr error | ||
| } | ||
|
|
||
| testCases := []struct { | ||
| desc string | ||
| in testInput | ||
| expected testExpectedValues | ||
| }{ | ||
| { | ||
| desc: "Try to overwrite already existing key. From redis client documentation, SetNX returns 'false' because no operation is performed", | ||
| in: testInput{ | ||
| redisSentinelClient: FakeRedisClient{ | ||
| Success: false, | ||
| StoredData: map[string]string{"key": "original value"}, | ||
| ServerError: redis.Nil, | ||
| }, | ||
| key: "key", | ||
| valueToStore: "overwrite value", | ||
| ttl: 10, | ||
| }, | ||
| expected: testExpectedValues{ | ||
| redisClientErr: utils.NewPBCError(utils.RECORD_EXISTS), | ||
| writtenValue: "original value", | ||
| }, | ||
| }, | ||
| { | ||
| desc: "When key does not exist, redis.Nil is returned. Other errors should be interpreted as a server side error. Expect error.", | ||
| in: testInput{ | ||
| redisSentinelClient: FakeRedisClient{ | ||
| Success: true, | ||
| StoredData: map[string]string{}, | ||
| ServerError: errors.New("A Redis client side error"), | ||
| }, | ||
| key: "someKey", | ||
| valueToStore: "someValue", | ||
| ttl: 10, | ||
| }, | ||
| expected: testExpectedValues{ | ||
| redisClientErr: errors.New("A Redis client side error"), | ||
| }, | ||
| }, | ||
| { | ||
| desc: "In Redis, a zero ttl value means no expiration. Expect value to be successfully set", | ||
| in: testInput{ | ||
| redisSentinelClient: FakeRedisClient{ | ||
| StoredData: map[string]string{}, | ||
| Success: true, | ||
| ServerError: redis.Nil, | ||
| }, | ||
| key: "defaultKey", | ||
| valueToStore: "aValue", | ||
| ttl: 0, | ||
| }, | ||
| expected: testExpectedValues{ | ||
| writtenValue: "aValue", | ||
| }, | ||
| }, | ||
| { | ||
| desc: "RedisBackend.Put() successful, no need to set defaultTTL because ttl is greater than zero", | ||
| in: testInput{ | ||
| redisSentinelClient: FakeRedisClient{ | ||
| StoredData: map[string]string{}, | ||
| Success: true, | ||
| ServerError: redis.Nil, | ||
| }, | ||
| key: "defaultKey", | ||
| valueToStore: "aValue", | ||
| ttl: 1, | ||
| }, | ||
| expected: testExpectedValues{ | ||
| writtenValue: "aValue", | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range testCases { | ||
| // Assign redis backend client | ||
| redisSentinelBackend.client = tt.in.redisSentinelClient | ||
|
|
||
| // Run test | ||
| actualErr := redisSentinelBackend.Put(context.Background(), tt.in.key, tt.in.valueToStore, tt.in.ttl) | ||
|
|
||
| // Assertions | ||
| assert.Equal(t, tt.expected.redisClientErr, actualErr, tt.desc) | ||
|
|
||
| // Put error | ||
| assert.Equal(t, tt.expected.redisClientErr, actualErr, tt.desc) | ||
|
|
||
| if actualErr == nil || actualErr == utils.NewPBCError(utils.RECORD_EXISTS) { | ||
| // Either a value was inserted successfully or the record already existed. | ||
| // Assert data in the backend | ||
| storage, ok := tt.in.redisSentinelClient.(FakeRedisClient) | ||
| assert.True(t, ok, tt.desc) | ||
| assert.Equal(t, tt.expected.writtenValue, storage.StoredData[tt.in.key], tt.desc) | ||
| } | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even if we consolidate
redis-sentinel.goandredis.go, I think we can keep the configuration logic as is: one for redis sentinel, and the other for redis cluster. I agree with the current approach here, let's just use the same adapter (redis.go) for both regularredisorredis_sentinel.