Skip to content
This repository has been archived by the owner on Feb 21, 2023. It is now read-only.

multi/exec transactions complete after client-side exceptions #696

Closed
yurymann opened this issue Feb 7, 2020 · 2 comments
Closed

multi/exec transactions complete after client-side exceptions #696

yurymann opened this issue Feb 7, 2020 · 2 comments

Comments

@yurymann
Copy link

yurymann commented Feb 7, 2020

Way to reproduce:

  1. Add a number of commands to a multi/exec pipeline, including one with a None argument.
  2. After calling execute(), aioredis throws TypeError("args must not contain None") for the None argument.
  3. The exception is recorded in the failed command's Future. But the rest of the commands are executed!
  4. As the result, only a part of the multi/exec pipeline is executed.

Expected behaviour: the whole transaction should be either executed completely or discarded completely. Current behaviour results in corrupted data and nasty logical errors in our application.

I guess this line should be conditional on whether any command was processed with an exception.

@seandstewart seandstewart added this to To do in Resuscitation via automation Nov 30, 2020
@seandstewart
Copy link
Collaborator

This is because we're scheduling all of the commands with Futures, so execution order isn't guaranteed. It's a tricky problem which will likely mean we need a new API to do properly.

Ideally we should build a buffer of commands and execute them synchronously, perhaps providing a flush() method to flush the buffer and send to the Redis server manually (See Redis-py's Pipeline impl).

I'm guessing this implementation is also the cause of performance issues (#700), since we're making an untold number of network calls. Even though they're not blocking the client, they're blocking the server.

@seandstewart
Copy link
Collaborator

Hey @yurymann -

I've just merged #891, which re-writes aioredis from the ground up as an asyncio port of redis-py. This means the Pipeline implementation should ensure we don't get into the mixed-commit state you saw on an exception.

Resuscitation automation moved this from To do to Done Mar 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Development

No branches or pull requests

2 participants