-
Notifications
You must be signed in to change notification settings - Fork 992
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
Race condition in RedisPublisher DEMAND.request() and DEMAND.onDataAvailable() #634
Comments
Please learn how to properly format code and logs. The attached Gist is no longer accessible. |
@mp911de ,thank you for reply me,i change my code like following,
Then the application blocked and never get the value back.
i debug the application ,get the dump info :
|
Several points come to my attention:
|
@mp911de
and add log to the reactor inside
and get the fllowing log,
after the 8th print 2786,the application hand up. |
i config the logger to print detail info : |
Thanks a lot. With enough keys in Redis I'm able to reproduce the issue. |
I found the cause of the issue: It's a race condition between It's possible that two concurrent threads, one processing
The fix is to introduce checks in both places:
|
This commit fixes a race condition between two concurrent threads that are calling DEMAND.request(…) and DEMAND.onDataAvailable(…) resulting in registered demand but no further emission. The publishing part now checks whether new demand was registered after the state transition and the requesting thread checks for a state transition after registering demand. If one of the newly introduced conditions yields true, the outstanding signals are processed. There are three key components to create the race: * the command is completed * the requesting thread does not pull because of DEMAND state * the emitting thread completes prematurely The race is possible because the requesting thread calls request(…) in the DEMAND state after the publishing thread determined there was no further demand. The publishing did not check whether demand was registered after its state transition to NO_DEMAND, the requesting code did not check whether a state transition occurred meanwhile so the publisher never completed.
This commit fixes a race condition between two concurrent threads that are calling DEMAND.request(…) and DEMAND.onDataAvailable(…) resulting in registered demand but no further emission. The publishing part now checks whether new demand was registered after the state transition and the requesting thread checks for a state transition after registering demand. If one of the newly introduced conditions yields true, the outstanding signals are processed. There are three key components to create the race: * the command is completed * the requesting thread does not pull because of DEMAND state * the emitting thread completes prematurely The race is possible because the requesting thread calls request(…) in the DEMAND state after the publishing thread determined there was no further demand. The publishing did not check whether demand was registered after its state transition to NO_DEMAND, the requesting code did not check whether a state transition occurred meanwhile so the publisher never completed.
Looks like the change fixed the issue only partially... |
I fixed the issue in <dependency>
<groupId>io.lettuce</groupId>
<artifactId>lettuce-core</artifactId>
<version>5.0.1.BUILD-SNAPSHOT</version>
</dependency>
<repositories>
<repository>
<id>sonatype-nexus-snapshots</id>
<url>http://oss.sonatype.org/content/repositories/snapshots</url>
<snapshots>
<enabled>true</enabled>
</snapshots>
</repository>
</repositories> |
Fix completion where concurrently another thread signals demand setting the state from NO_DEMAND to DEMAND and a concurrent completion occurs. Because completion checks against the current state to perform state change only once, it checks with the old state hence completion never succeeds in that case. Completion now executes in any state as long as the queue is empty.
Fix completion where concurrently another thread signals demand setting the state from NO_DEMAND to DEMAND and a concurrent completion occurs. Because completion checks against the current state to perform state change only once, it checks with the old state hence completion never succeeds in that case. Completion now executes in any state as long as the queue is empty.
RedisPublisher now considers an empty subscription buffer as demand to continue reading from the transport. Reading will prefetch data and ensure the publisher has enough data buffered to satisfy demand from the buffer. Reading from the transport also ensures that command completion is read as the completion does not count into the actual demand.
RedisPublisher now considers an empty subscription buffer as demand to continue reading from the transport. Reading will prefetch data and ensure the publisher has enough data buffered to satisfy demand from the buffer. Reading from the transport also ensures that command completion is read as the completion does not count into the actual demand.
I was already tried 5.0.1.BUILD-SNAPSHOT;unfortunately,the issue still occurs when i commited my show detail logger config, it look like the logging operation can remit the situaction. my OS version is Window Enterprise Server 2008,JDK 9.0 |
Enabling logging changes timing. The fixed issues were caused due to concurrency. Do you know the snapshot version (e.g. The issue was caused by multiple, individual issues. I fixed some of these after my previous |
@mp911de my snapshot version is lettuce-core-5.0.1.BUILD-20171026.194537-22.jar, is the same as you supplied. |
I tried to reproduce the issue after the changes but I'm not able to reproduce the issue anymore with 5.0.1. |
@mp911de
in my sample ,the final output is: |
Lettuce now sends a netty event if auto-read is disabled to stay with config changes within the channel thread. Auto-read state is always set based on whether the demand-aware sink has demand.
Lettuce now sends a netty event if auto-read is disabled to stay with config changes within the channel thread. Auto-read state is always set based on whether the demand-aware sink has demand.
Thanks for the hint with more data. I created a bigger cluster with more data and was able to reproduce and fix the issue. A new build is available via |
Closing as this issue is solved and no longer reproducible. |
I Write very simple code to test lettuce 5.0 redis cluster client ,
it print out three times,then block and never get any info. i am not very good at java,pls help me fix this problem
=========Log data==============================================
十月 24, 2017 4:15:01 下午 reactor.util.Loggers$JdkLogger info
信息: | onSubscribe(reactor.core.publisher.MonoCount$CountSubscriber@20c0a64d)
十月 24, 2017 4:15:01 下午 reactor.util.Loggers$JdkLogger info
信息: | request(unbounded)
cancel
15
5230
15
十月 24, 2017 4:15:01 下午 reactor.util.Loggers$JdkLogger info
信息: | onNext(5230)
十月 24, 2017 4:15:01 下午 reactor.util.Loggers$JdkLogger info
信息: | cancel()
十月 24, 2017 4:15:01 下午 reactor.util.Loggers$JdkLogger info
信息: | onSubscribe(reactor.core.publisher.MonoCount$CountSubscriber@4b213651)
十月 24, 2017 4:15:01 下午 reactor.util.Loggers$JdkLogger info
信息: | request(unbounded)
十月 24, 2017 4:15:01 下午 reactor.util.Loggers$JdkLogger info
信息: | onNext(5230)
十月 24, 2017 4:15:01 下午 reactor.util.Loggers$JdkLogger info
信息: | cancel()
cancel
15
5230
15
十月 24, 2017 4:15:01 下午 reactor.util.Loggers$JdkLogger info
信息: | onSubscribe(reactor.core.publisher.MonoCount$CountSubscriber@4241e0f4)
十月 24, 2017 4:15:01 下午 reactor.util.Loggers$JdkLogger info
信息: | request(unbounded)
十月 24, 2017 4:15:01 下午 reactor.util.Loggers$JdkLogger info
信息: | onNext(5230)
十月 24, 2017 4:15:01 下午 reactor.util.Loggers$JdkLogger info
信息: | cancel()
cancel
17
5230
17
The text was updated successfully, but these errors were encountered: