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

RedisCoroutinesCommands calls are locked after calling multi() #1954

Closed
fedigiorgio opened this issue Jan 4, 2022 · 4 comments
Closed

RedisCoroutinesCommands calls are locked after calling multi() #1954

fedigiorgio opened this issue Jan 4, 2022 · 4 comments
Labels
type: task A general task
Milestone

Comments

@fedigiorgio
Copy link

Bug Report

When invoke multi method in RedisCoroutinesCommands all commands callings into its actions are being locked.

Current Behavior

  suspend fun test(connection: StatefulRedisConnection<String, String>) {
        val coroutines = connection.coroutines<String, String>()
        coroutines.multi {
            set("myKey1", "myValue1") // The execution flow is locked here.
            set("myKey2", "myValue2")
        }
   }

Expected behavior/code

Enqueue the next commands callings and continue with the execution flow.

Environment

  • Lettuce version(s): [6.1.5.RELEASE]
  • Redis version: [6.2.3]
@mp911de
Copy link
Collaborator

mp911de commented Jan 5, 2022

Paging @sokomishalov

@sokomishalov
Copy link
Collaborator

@fedigiorgio thanks for the report!
Since coroutines API is built on top of reactive API this hang occurred because the underlying publisher hasn't published any value before exec() execution which is called right after closure execution. So it's a very bad deadlock in this case, unfortunately.
I propose to remove this multi extension only for coroutines API until we figure out how to deal with such transactional cases since this API is still experimental.
An alternative workaround is to change closure's receiver parameter from RedisCoroutinesCommands<K, V> to RedisReactiveCommands<K, V> and let the user decide how to deal with it.

suspend fun test(connection: StatefulRedisConnection<String, String>) {
        val coroutines = connection.coroutines<String, String>()
        coroutines.multi { // it as RedisReactiveCommands<K, V>
            set("myKey1", "myValue1").subsribe() // won't hang up 
            set("myKey2", "myValue2").subscribe() // won't hang up
        }
   }

@mp911de how do you think?

@mp911de
Copy link
Collaborator

mp911de commented Jan 10, 2022

Instead of removing it, I'd vote to deprecate it with a proper note of why transactions aren't a good fit with an imperative coroutine coding style.

@sokomishalov
Copy link
Collaborator

@mp911de sounds reasonable. I've opened #1960

@mp911de mp911de added this to the 6.1.6 milestone Jan 10, 2022
@mp911de mp911de added type: task A general task and removed status: waiting-for-triage labels Jan 10, 2022
mp911de added a commit that referenced this issue Jan 11, 2022
mp911de added a commit that referenced this issue Jan 11, 2022
@mp911de mp911de closed this as completed Jan 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: task A general task
Projects
None yet
Development

No branches or pull requests

3 participants