Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

3.x: Change Flowable.groupBy to signal MBE instead of possibly hanging#6740

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
akarnokd merged 5 commits intoReactiveX:3.xfromakarnokd:FlowableGroupByMBE
Dec 6, 2019

Conversation

@akarnokd
Copy link
Member

This PR changes the backpressure behavior ofFlowable.groupBy to signalMissingBackpressureException instead of silently hanging if the produced groups are not ready to be accepted by the downstream.

This can happen if oneflatMaps agroupBy but there are more groups produced than the concurrency level offlatMap. Since replenishment is tied to item consumption from the groups, not consuming them can result in none of the groups receiving any further items and the whole operator hangs.

The following changes have been applied:

  • Removed the queue from the main operator since it will now try to emit directly and not buffer groups.
  • The mainFlowable, lacking a queue, no longer supports operator fusion. Tests checking this property have been removed as well.
  • When a group is drained, consumed items are replenished in batch if possible. Detecting a cancellation will also trigger a replenishment.
  • When a group is pulled (fusion mode), now allpull,isEmpty andclear will trigger replenishment so that other groups can make progress too.
  • Unit tests have been modified to have large enough bufferSize/prefetch amounts to allow them to pass.

Fixes#6641

@akarnokdakarnokd added this to the3.0 milestoneDec 3, 2019
@codecov
Copy link

codecovbot commentedDec 3, 2019
edited
Loading

Codecov Report

Merging#6740 into3.x willdecrease coverage by0.05%.
The diff coverage is94.33%.

Impacted file tree graph

@@             Coverage Diff              @@##                3.x    #6740      +/-   ##============================================- Coverage     98.17%   98.11%   -0.06%+ Complexity     6191     6190       -1============================================  Files           677      677                Lines         44663    44599      -64       Branches       6171     6152      -19     ============================================- Hits          43847    43760      -87- Misses          289      301      +12- Partials        527      538      +11
Impacted FilesCoverage ΔComplexity Δ
.../main/java/io/reactivex/rxjava3/core/Flowable.java100% <ø> (ø)559 <0> (ø)⬇️
...3/internal/operators/flowable/FlowableGroupBy.java96.22% <94.33%> (-0.9%)3 <0> (ø)
.../operators/flowable/FlowableBlockingSubscribe.java93.02% <0%> (-4.66%)10% <0%> (-1%)
...rnal/operators/flowable/FlowableFlatMapSingle.java92.44% <0%> (-2.91%)2% <0%> (ø)
...l/operators/observable/ObservableFlatMapMaybe.java90.14% <0%> (-2.12%)2% <0%> (ø)
...a3/internal/operators/flowable/FlowableCreate.java95.46% <0%> (-1.95%)6% <0%> (ø)
...ernal/operators/flowable/FlowableFromIterable.java95.18% <0%> (-1.61%)5% <0%> (ø)
...operators/observable/ObservableMergeWithMaybe.java99.09% <0%> (-0.91%)2% <0%> (ø)
...a3/internal/operators/flowable/FlowableReplay.java91.97% <0%> (-0.83%)19% <0%> (ø)
... and8 more

Continue to review full report at Codecov.

Legend -Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing data
Powered byCodecov. Last updated98dff6...9cfb5a9. Read thecomment docs.

Copy link
Collaborator

@vanniktechvanniktech left a comment

Choose a reason for hiding this comment

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

Never used that operator and I remember@davidmoten had lots of use cases so let's wait for his approval?

@akarnokd
Copy link
MemberAuthor

Sure.

@akarnokd
Copy link
MemberAuthor

Let's merge this into the release and get feedback from the field.

vanniktech reacted with thumbs up emoji

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

1 more reviewer

@vanniktechvanniktechvanniktech approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

3.0

Development

Successfully merging this pull request may close these issues.

3.x: Change the behavior of Flowable.groupBy to signal MBE if no main requests

2 participants

@akarnokd@vanniktech

[8]ページ先頭

©2009-2025 Movatter.jp