Skip to content
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

support redis sentinel #5645

Closed
wkloucek opened this issue Feb 24, 2023 · 6 comments · Fixed by #5737
Closed

support redis sentinel #5645

wkloucek opened this issue Feb 24, 2023 · 6 comments · Fixed by #5737

Comments

@wkloucek
Copy link
Contributor

Is your feature request related to a problem? Please describe.

I'd like to use Redis sentinel as a cache.

Describe the solution you'd like

oCIS must allow the admin to configure the Mastername Option in Reva:

diff --git a/pkg/storage/cache/cache.go b/pkg/storage/cache/cache.go
index 07842c32..0b4f595d 100644
--- a/pkg/storage/cache/cache.go
+++ b/pkg/storage/cache/cache.go
@@ -29,6 +29,7 @@ import (
        provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
        natsjs "github.com/go-micro/plugins/v4/store/nats-js"
        "github.com/go-micro/plugins/v4/store/redis"
+       redisopt "github.com/go-redis/redis/v8"
        "github.com/nats-io/nats.go"
        microetcd "github.com/owncloud/ocis/v2/ocis-pkg/store/etcd"
        microstore "go-micro.dev/v4/store"
@@ -173,6 +174,11 @@ func getStore(store string, nodes []string, database, table string, ttl time.Dur
                        microstore.Database(database),
                        microstore.Table(table),
                        microstore.Nodes(nodes...),
+                       microstore.Option(
+                               redis.WithRedisOptions(redisopt.UniversalOptions{
+                                       MasterName: "mymaster",
+                               }),
+                       ),
                ) // only the first node is taken into account
        case "memory":
                return microstore.NewStore(

Background see: https://github.com/redis/go-redis/blob/6ec458549ed0808ec57c303df9c922aba9520e14/universal.go#L214-L219

Describe alternatives you've considered

Additional context

#5137 -> cache down, means oCIS dead. Therfore I need a way to provide a HA Redis like with sentinel.

@wkloucek
Copy link
Contributor Author

wkloucek commented Mar 6, 2023

@butonic The current implementation doesn't look finished.

https://github.com/search?q=repo%3Aowncloud%2Focis%20%20%27redis&type=code

and

Store string `yaml:"store" env:"OCIS_CACHE_STORE_TYPE;GATEWAY_CACHE_STORE_TYPE;GATEWAY_CACHE_STORE" desc:"Store implementation for the cache. Valid values are \"memory\" (default), \"redis\", and \"etcd\"."`

show some discrepancies.

When I run storage-users and gateway with redis sentinel, I have stacktraces in the gateway service

@wkloucek
Copy link
Contributor Author

wkloucek commented Mar 22, 2023

I was missing the master name in my setup. `OCIS_CACHE_STORE_ADDRESS: redis.ocis-redis.svc.cluster.local:26379/mymaster" is working now.

@aduffeck will care for the missing "redis-sentinel" documentation option in the gateway service 👍

@wkloucek
Copy link
Contributor Author

@butonic The REVA ocs service als has a cache, that supports Redis but no Redis Sentinel:

https://github.com/owncloud/ocis/blob/master/services/frontend/pkg/config/config.go#L126

Is that a cache one should use in a distributed setup? If so we should also allow Redis Sentinal there.

@butonic
Copy link
Member

butonic commented Mar 22, 2023

The ocs cache defaults to an in memory cache with a TTL of 0. We should allow redis-sentinel as I expect a lot of shares...

urgh but it is not implemented with a go micro store interface but reva/pkg/cache

In the gateway and storage providers we are using the go micro store, but it cannot get multiple keys with a single request. I haven't found a good way to request multiple keys with the micro api, yet.

@butonic
Copy link
Member

butonic commented Mar 22, 2023

related: cs3org/reva#3742

@wkloucek
Copy link
Contributor Author

I could use redis-sentinal, so I'm closing this, because looks fulfilled

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants