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

BaseProcessor back-pressure fix backport from #1282 #1343

Merged
merged 1 commit into from
Mar 16, 2020

Conversation

danielkec
Copy link
Contributor

@danielkec danielkec commented Feb 3, 2020

There is a problem with BaseProcessor request count:
For example if map operator is requested like this:

request(2);
request(2);
request(2);

his publisher will receive:

request(2);
request(4);
request(6);

That can lead to flooding subscriber if proper back-pressure is applied

Signed-off-by: Daniel Kec daniel.kec@oracle.com

Signed-off-by: Daniel Kec <daniel.kec@oracle.com>
@danielkec danielkec self-assigned this Feb 3, 2020
Copy link

@olotenko olotenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've seen this, but did not raise an issue, because it is not apparent if it can be triggered outside a compliance test. Eg if all known uses of BaseProcessor ensure subscription is available before the downstream subscribes, the issue with request counter does not manifest itself.
beebd46#diff-ca92013a99c748b99a9e08660b9dea17R108 - just don't call downstream onSubscribe until you have received onSubscribe from upstream. Then you don't need any of the machinery to track requests that have arrived before there was anyone to drain the requests to.

@barchetta barchetta added this to the 1.4.2 milestone Feb 3, 2020
@danielkec danielkec added reactive Reactive streams and related components 1.x Issues for 1.x version branch labels Mar 2, 2020
@barchetta barchetta modified the milestones: 1.4.2, 1.4.3 Mar 4, 2020
@danielkec
Copy link
Contributor Author

@romain-grecourt Can I ask for review pls, it seems to be severe bug enough to be backported

@danielkec danielkec merged commit 480249a into helidon-io:helidon-1.x Mar 16, 2020
@danielkec danielkec deleted the backpressure-fix branch July 3, 2020 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.x Issues for 1.x version branch reactive Reactive streams and related components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants