Skip to content

Revise implementation of RedisMessageListenerContainer #2256

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

Closed
wants to merge 3 commits into from
Closed

Conversation

mp911de
Copy link
Member

@mp911de mp911de commented Feb 9, 2022

RedisMessageListenerContainer is now reimplemented using non-blocking synchronization guards and state management to simplify its maintenances. Additionally, listener registration and subscription setup through the start() method waits until the listener subscription is confirmed by the Redis server. The synchronization removes potential race conditions that could happen by concurrent access to blocking Redis connectors in which the registration state was guessed and not awaited.

Closes #964

RedisMessageListenerContainer is now reimplemented using non-blocking synchronization guards and a state management to simplify its maintenances. Additionally, listener registration and subscription setup through the start() method awaits until the listener subscription is confirmed by the Redis server. The synchronization removes potential race conditions that could happen by concurrent access to blocking Redis connectors in which the registration state was guessed and not awaited.
christophstrobl pushed a commit that referenced this pull request Feb 17, 2022
RedisMessageListenerContainer is now reimplemented using non-blocking synchronization guards and a state management to simplify its maintenances. Additionally, listener registration and subscription setup through the start() method awaits until the listener subscription is confirmed by the Redis server. The synchronization removes potential race conditions that could happen by concurrent access to blocking Redis connectors in which the registration state was guessed and not awaited.

Resolves: #964
Original Pull Request: #2256
christophstrobl pushed a commit that referenced this pull request Feb 17, 2022
christophstrobl added a commit that referenced this pull request Feb 17, 2022
Error when afterPropertiesSet is called more than once. Update documentation and error messages.
Original Pull Request: #2256
christophstrobl pushed a commit that referenced this pull request Feb 17, 2022
RedisMessageListenerContainer is now reimplemented using non-blocking synchronization guards and a state management to simplify its maintenances. Additionally, listener registration and subscription setup through the start() method awaits until the listener subscription is confirmed by the Redis server. The synchronization removes potential race conditions that could happen by concurrent access to blocking Redis connectors in which the registration state was guessed and not awaited.

Resolves: #964
Original Pull Request: #2256
christophstrobl pushed a commit that referenced this pull request Feb 17, 2022
christophstrobl added a commit that referenced this pull request Feb 17, 2022
Error when afterPropertiesSet is called more than once. Update documentation and error messages.
Original Pull Request: #2256
@christophstrobl
Copy link
Member

Merged to main development line and fw ported to 3.0.x.

mp911de added a commit that referenced this pull request Feb 17, 2022
Allow Jedis to exist pub/sub loop gracefully. Await unsubscribe. Reduce retry backoff to allow from broken connection recovery. Do not use pooling to avoid using broken connections.

See #2256
mp911de added a commit that referenced this pull request Feb 17, 2022
Allow Jedis to exist pub/sub loop gracefully. Await unsubscribe. Reduce retry backoff to allow from broken connection recovery. Do not use pooling to avoid using broken connections.

See #2256
jebeaudet added a commit to jebeaudet/spring-data-redis that referenced this pull request Apr 11, 2022
RedisMessageListenerContainer relies on 2 threads for subscription when patterns and channels topics are present. With Jedis, since the subscription thread blocks while listening for messages, an additional thread is used to subscribe to patterns while the subscription threads subscribe to channels and block.

There were some race conditions between those two threads that could corrupt the Jedis stream since operations are not synchronized in JedisSubscription. A lock on the JedisSubscription instance has been added  to enforce that operations on the Jedis stream cannot be affected by a concurrent thread.

Additionaly, there were no error handling and retry mechanism on the pattern subscription thread. Multiple conditions could trigger an unexpected behavior here, exceptions were not handled and logged to stderr with no notice. Also, if the connection was not subscribed after 3 tries, the thread would exit silently with no log. Defensive measure have been added to retry redis connection failures and the subscription will now retry indefinitely, unless canceled on shutdown and on the main subscription thread errors.

Fixes spring-projects#964 for versions before spring-projects#2256 was introduced.
jebeaudet added a commit to jebeaudet/spring-data-redis that referenced this pull request Apr 11, 2022
RedisMessageListenerContainer relies on 2 threads for subscription when patterns and channels topics are present. With Jedis, since the subscription thread blocks while listening for messages, an additional thread is used to subscribe to patterns while the subscription threads subscribe to channels and block.

There were some race conditions between those two threads that could corrupt the Jedis stream since operations are not synchronized in JedisSubscription. A lock on the JedisSubscription instance has been added  to enforce that operations on the Jedis stream cannot be affected by a concurrent thread.

Additionaly, there were no error handling and retry mechanism on the pattern subscription thread. Multiple conditions could trigger an unexpected behavior here, exceptions were not handled and logged to stderr with no notice. Also, if the connection was not subscribed after 3 tries, the thread would exit silently with no log. Defensive measure have been added to retry redis connection failures and the subscription will now retry indefinitely, unless canceled on shutdown and on the main subscription thread errors.

Fixes spring-projects#964 for versions before spring-projects#2256 was introduced.
@mp911de mp911de deleted the issue/964 branch March 21, 2023 09:57
@mp911de mp911de added this to the 3.0 M2 (2022.0.0) milestone Mar 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: waiting-for-triage An issue we've not yet triaged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RedisMessageListenerContainer has race conditions [DATAREDIS-389]
3 participants