Feature: Redis lock - Add sentinel support

[VuiDJi]

In what area(s)?

/area runtime

What version of Dapr?

1.9.5

Expected Behavior

We've deployed redis with sentinel with 3 nodes.

According to the following documentation https://docs.dapr.io/reference/components-reference/supported-locks/redis-lock/ it's expected that it is possible to use a failover redis with sentinel for lock component.

Actual Behavior

However, dapr sidecar crashes several times until it starts with the following error: time="2023-01-15T15:13:33.408791532Z" level=fatal msg="process component donum-lockstore error: [INIT_COMPONENT_FAILURE]: initialization error occurred for donum-lockstore (lock.redis/v1): [standaloneRedisLock]: InitLockStore error. Replication is not supported" app_id=donum-staging-donum-proxyinput instance=donum-proxyinput-5bffbfcd4b-szx57 scope=dapr.runtime type=log ver=1.9.5

Steps to Reproduce the Problem

Deploy redis+sentinels with 3 nodes:

helm upgrade redis bitnami/redis \
--install \
--namespace $DONUMNAMESPACE --create-namespace \
--set "architecture=replication" \
--set "sentinel.enabled=true" \
--set "sentinel.redisShutdownWaitFailover=false" \
--set "auth.enabled=false" \
--set "auth.sentinel=false" \
--set "pdb.create=true" \
--set "pdb.minAvailable=2" \
--wait

Apply lock.redis component:

apiVersion: dapr.io/v1alpha1
kind: Component
metadata:
  name: donum-lockstore
  namespace: {{ .Release.Namespace | quote }}
spec:
  type: lock.redis
  version: v1
  metadata:
  - name: redisHost
    value: redis:26379
  - name: failover
    value: true
  - name: sentinelMasterName
    value: mymaster
  - name: redisType
    value: cluster

Try to deploy some ASP.NET Core app with dapr sidecar:

    metadata:
      annotations:
        dapr.io/app-id: some-app-name
        dapr.io/app-port: "80"
        dapr.io/config: dapr-config
        dapr.io/enabled: "true"
        dapr.io/http-max-request-size: "1024"
        dapr.io/sidecar-readiness-probe-threshold: "20"

[ItalyPaleAle]

Moved to dapr/components-contrib

@seeflood would you be able to take a look at this please?

[seeflood]

I think it's a mistake in the documentation. Currently the redis component doesn't support redis cluster (including those implemented with sentinel) . I didn't implement the cluster feature because fail-over can lead to lock loss.

@VuiDJi Can you introduce the usage scenario? For example, is it acceptable to lose locks caused by failover? If we are going to support redis cluster, using the redlock algorithm might be a better choice

[rueian]

Hi all, I just came across this issue by chance.

I have a redlock implementation combined with client-side caching that can detect and notify lock losses to clients. The implementation might be useful to you: https://github.com/rueian/rueidis/tree/main/rueidislock

[berndverst]

@seeflood can you please try to improve the documentation for this?

@VuiDJi please provide more details around your expectations here and the use case.

@rueian perhaps you and @seeflood can discuss a potential implementation.

[VuiDJi]

@seeflood It is not acceptable for our cases to lose locks caused by failover. We were going to use lock: 1) In combination with dapr cron to be able to run some regular tasks (e.g. to generate and send daily reports), but so that they were executed only in one pod, even if deployment with multiple replicas was deployed for fault tolerance. 2) To perform some operation, the execution of which must be single-threaded. For example, updating a customer's balance when receiving information (pub/sub) about the creation of a cash flow.

I also read the opinion that using Redis (RedLock) for distributed locks is not a good idea:

Based on the above considerations, in fact, the author of Redis also considered this problem. He proposed a RedLock algorithm. But it is unnecessarily heavyweight and expensive for efficiency-optimization locks, but it is not sufficiently safe for situations in which correctness depends on the lock.

Another flaw about RedLock is its dangerous assumptions about timing and system clocks and it violates safety properties if those assumptions are not met.

On the other hand, if you need locks for correctness, please don’t use Redlock. Instead, please use a proper consensus system such as Zookeeper.

Perhaps it would be better to implement in dapr a component for distributed locks using something like zookeeper? Thanks!

[gcaracuel]

Checking the helm command you use to install Redis looks like what you are doing is using Redis Sentinel (so failover) but not Redis Cluster which are 2 different things. As per Dapr docs when using Redis Sentinel failover needs to be set to true but redisType kept to node.

But as it was mentioned in the thread Redis Cluster nor Redis Sentinel (failover) is supported in the docs even tho documentation show the files to config it, I tried this myself and getting next error:

[standaloneRedisLock]: InitLockStore error. Failover is not supported