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

Is Redis actually transactional? #2071

Open
artursouza opened this issue Sep 13, 2022 · 20 comments
Open

Is Redis actually transactional? #2071

artursouza opened this issue Sep 13, 2022 · 20 comments
Labels
pinned Issue does not get stale question Further information is requested

Comments

@artursouza
Copy link
Member

Our docs report Redis state store as "Transactional" because it allows to send multiple Update/Delete operations at once and to be executed "together". But does it actually have ACID guarantees?

https://docs.dapr.io/reference/components-reference/supported-state-stores/

redis/go-redis#1602

@artursouza artursouza added the question Further information is requested label Sep 13, 2022
@DanielYWoo
Copy link

I am fine with no supporting ACID transaction. Most likely we don't really use Redis for that kind of job, but it would be great if we can clarify that in the doc.

@DanielYWoo
Copy link

From Redis document https://redis.io/docs/manual/transactions

Errors happening after EXEC instead are not handled in a special way: all the other commands will be executed even if some command fails during the transaction.

@dapr-bot
Copy link
Collaborator

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged (pinned, good first issue, help wanted or triaged/resolved) or other activity occurs. Thank you for your contributions.

@dapr-bot dapr-bot added the stale label Oct 23, 2022
@dapr-bot
Copy link
Collaborator

This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as pinned, good first issue, help wanted or triaged/resolved. Thank you for your contributions.

@artursouza artursouza added pinned Issue does not get stale and removed stale labels Jan 23, 2023
@artursouza artursouza reopened this Jan 23, 2023
@berndverst
Copy link
Member

berndverst commented Feb 27, 2023

Quoting from https://redis.io/docs/manual/transactions

It's important to note that even when a command fails, all the other commands in the queue are processed – Redis will not stop the processing of commands.

Redis does not support rollbacks of transactions since supporting rollbacks would have a significant impact on the simplicity and performance of Redis.

Redis is not transactional. It only offers a guaranteed to be sequential batch of operations where it simply will skip commands that fail. If commands are syntactically wrong the batch will never run - but syntactically yet logically incorrect commands resulting in failures - or any other form of failures - will simply be skipped.

With that in mind I think we would do our users a disservice to allow it to be used as an actor state store. I recommend we deprecate the transaction support and remove it. Thoughts @artursouza @yaron2 @ItalyPaleAle?

If the reason to keep Redis around is the local developer experience (we can discuss that in another issue) note that sqlite supports transactions. We should make sqlite the default for state store I think. We can keep Redis for PubSub, configuration store etc

@DanielYWoo
Copy link

Or you can keep Redis but with limited support. e.g., report an error if developers try to use the transactional batch interface.

@berndverst
Copy link
Member

berndverst commented Mar 1, 2023

Or you can keep Redis but with limited support. e.g., report an error if developers try to use the transactional batch interface.

We would do exactly that for a period of 1 or 2 releases while we deprecate the support I think!

We'd keep Redis locally for PubSub and other things - just not as state store and use SQLite for that instead. We should remove transaction support (and hence actor state store support) for Redis.

[one of the maintainers]

@yaron2
Copy link
Member

yaron2 commented Mar 1, 2023

Removing transaction support for Redis will leave a great many actor users without a suitable alternative. Before we go to such an extreme I suggest we:

  1. Make sure if this is really an issue for how actors operate. If not, then we can continue using the current method for actors and simply disable the transactional feature
  2. Make sure we have transactional alternatives in place (ie. AWS and GCP state stores) for actors to give all affected users a path forward

@berndverst
Copy link
Member

Removing transaction support for Redis will leave a great many actor users without a suitable alternative. Before we go to such an extreme I suggest we:

  1. Make sure if this is really an issue for how actors operate. If not, then we can continue using the current method for actors and simply disable the transactional feature

  2. Make sure we have transactional alternatives in place (ie. AWS and GCP state stores) for actors to give all affected users a path forward

For (1) that would be a runtime issue however not Redis specific. If actors can work without transaction support, then no problem here at all!

For (2) we do have other OSS components and offerings that are offered by each cloud provider as managed services (Postgres, MySQL for example). And there are more proprietary services too - so we should be good there.

@berndverst
Copy link
Member

As discussed with @yaron2 we are adding a warning to the Redis state store not to use Redis as actor state store in production at this time.

We will continue to investigate whether we can implement transaction guarantees (not likely) or will officially deprecate the transaction and hence actor state store support.

#2638

@MargaretKrutikova
Copy link

MargaretKrutikova commented Mar 16, 2023

We are now quite invested in using Redis state store with Dapr, and when we started prototyping with Dapr more than half a year ago Redis was the default recommended actor state store (I think still is in the docs) and it also fits our needs perfectly since it is light-weight and offers great performance for our needs. We are not using any of the transaction APIs available, would you still advice against using Redis as actor state store? Would could be a suitable alternative in this case? Supported state stores don't seem to offer any other alternative that would have the characteristics of Redis that our application requires.

Of course, if Dapr needs full transactional support to guarantee correctness of the actor runtime, we have to switch. From the previous comments:

Make sure if this is really an issue for how actors operate. If not, then we can continue using the current method for actors and simply disable the transactional feature

For (1) that would be a runtime issue however not Redis specific. If actors can work without transaction support, then no problem here at all!

Do you have any more insight on this? I would really appreciate any input here, since we are moving towards going live with the system we have built with Dapr soon and so far are quite happy with how things work 🙂

@ItalyPaleAle
Copy link
Contributor

We are not using any of the transaction APIs available

Using Actors does use the transactional capabilities under the hood, especially for things like reminders.

Given the discussion above, I would not recommend this for production. It's ok for development, but due to the lack of automatic rollbacks there could be risks with using it in production for actor state store.

Where is your application running and what are your requirements (also in terms of performance)?

@berndverst
Copy link
Member

We are not using any of the transaction APIs available

Using Actors does use the transactional capabilities under the hood, especially for things like reminders.

Given the discussion above, I would not recommend this for production. It's ok for development, but due to the lack of automatic rollbacks there could be risks with using it in production for actor state store.

Where is your application running and what are your requirements (also in terms of performance)?

Anything with the characteristics of Redis is probably not suitable as an actor state store - specifically due to lack of transaction support which is required to support the fundamental promises and concepts of actors.

I advise using any other stable component that has support for actors.

@MargaretKrutikova
Copy link

MargaretKrutikova commented Mar 17, 2023

Thank you for your input!

Where is your application running and what are your requirements (also in terms of performance)?

We are running in k8s and we have a lot of external sources connected to the Dapr system constantly creating new actors and updating the existing ones with reminders in place also triggering state updates very frequently. We have in total maybe around 20 000 actors (created, deleted and updated continuously) and fetching actor state on user requests (we expect to be able to handle over 100 000 users making frequent requests to the system). The updates should propagate through several layers of actors relatively quick to get almost real-time updates. I realize these requirements are pretty vague and don't include any exact numbers on how fast we want the requests to be, but the choice of going with redis was pretty obvious from the start, especially as it is the recommended choice.

Would it be possible to update the docs to make it clear redis is not recommended for state store?

I advise using any other stable component that has support for actors.

What would be a stable component that has support for actors? Since the docs might outdated, would you recommend PostgreSQL?

@mPyKen
Copy link

mPyKen commented Mar 23, 2023

I advise using any other stable component that has support for actors.

Tried with Azure CosmosDB which supports actors, but could not get it to work with workflows. Somehow workflow inputs are not stored when GETing a workflow. I assume the actor numbering via # conflicts with what cosmos db considers valid characters.
This is probably a separate issue, though... Will try another state store

EDIT: It seems to work with CosmosDB, even though the resources cannot be modified or viewed on cosmos.azure.com due to the invalid characters mentioned above. I could not get workflows running on kubernetes with the dotnet SDK initially. Created an issue on that as dapr/dotnet-sdk#1070

EDIT2: one workflow can be run on CosmosDB, but after that, it gets ugly... errors start showing up after the first completed workflow

level=error msg="error updating reminder track: POST https://cosmosdbdaprtest.documents.azure.com:443/dbs/MyDatabase/colls/DaprStateStore/docs\n---- [...] ----\nRESPONSE 412: 412 Precondition Failed\nERROR CODE: PreconditionFailed [...]

@mPyKen
Copy link

mPyKen commented Mar 29, 2023

I advise using any other stable component that has support for actors.

Tried with postgresql and mysql. Getting error empty string is not allowed in set operation (does not happen in redis)

ERRO[0067] asdf: execution failed with a non-recoverable error: empty string is not allowed in set operation  app_id=wfapp scope=dapr.runtime.wfengine type=log ver=1.10.2

Looking at the source code, it looks like neither oracledatabase nor sqlite work:
https://github.com/search?q=org%3Adapr%20%22empty%20string%20is%20not%20allowed%20in%20set%20operation%22&type=code

@berndverst Now I wonder, which stable components do support workflows at this point?
@MargaretKrutikova Did you find any solution?

@MargaretKrutikova
Copy link

We are still transitioning and thinking about going with unmanaged MongoDB cluster that we will setup in our k8s cluster. Haven't started the process yet though.

@ItalyPaleAle
Copy link
Contributor

@mPyKen thanks for sharing these.

Ill open some issues and we’ll get them fixed.

@ItalyPaleAle
Copy link
Contributor

I advise using any other stable component that has support for actors.

Tried with postgresql and mysql. Getting error empty string is not allowed in set operation (does not happen in redis)

ERRO[0067] asdf: execution failed with a non-recoverable error: empty string is not allowed in set operation  app_id=wfapp scope=dapr.runtime.wfengine type=log ver=1.10.2

@mPyKen for this issue, do you know the name of the key that triggered the error? Can you share a few more logs before and after?

CC: @cgillum

@mPyKen
Copy link

mPyKen commented Mar 30, 2023

sorry I already moved on to mongodb which seems to be working. I don't think any keys were logged for that issue...

tmacam added a commit to tmacam/dapr-test-infra that referenced this issue Sep 15, 2023
We are facing some issues using actor with a cosmosDB state store.
This issue might be related to dapr/dapr#6339. We are moving the setup
to use Redis instead of CosmosDB as our state store. This matches
our current longhaul setup.

While this might seem in contradiction with dapr/components-contrib#2071
and dapr/cli#1328, unblocking this issue will allow for
easier and predictable reproductions of our longhaul setup. We might
revisit the use of CosmosDB as a state store in the future.

Signed-off-by: Tiago Alves Macambira <[email protected]>
tmacam added a commit to dapr/test-infra that referenced this issue Sep 19, 2023
We are facing some issues using actor with a cosmosDB state store.
This issue might be related to dapr/dapr#6339. We are moving the setup
to use Redis instead of CosmosDB as our state store. This matches
our current longhaul setup.

While this might seem in contradiction with dapr/components-contrib#2071
and dapr/cli#1328, unblocking this issue will allow for
easier and predictable reproductions of our longhaul setup. We might
revisit the use of CosmosDB as a state store in the future.

Signed-off-by: Tiago Alves Macambira <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pinned Issue does not get stale question Further information is requested
Projects
Status: No status
Development

No branches or pull requests

8 participants