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

Avoid Uni.join().first(unis).toTerminate() with a concurrency limit #831

Conversation

jponge
Copy link
Member

@jponge jponge commented Jan 28, 2022

Also removed dropped exception logging, cancellation on a join effectively means that failures shall be discarded.

@jponge jponge linked an issue Jan 28, 2022 that may be closed by this pull request
@jponge jponge changed the title Avoid Uni.join().first(unis) with a concurrency limit Avoid Uni.join().first(unis).toTerminate() with a concurrency limit Jan 28, 2022
Also removed dropped exception logging, cancellation on a join effectively means that failures shall be discarded.
@jponge jponge force-pushed the feature/uni-join-no-concurrency-limit-on-first-to-terminate branch from 0f08c8b to f7db080 Compare January 28, 2022 20:31
@codecov
Copy link

codecov bot commented Jan 28, 2022

Codecov Report

Merging #831 (f7db080) into main (50dcb78) will increase coverage by 0.15%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main     #831      +/-   ##
============================================
+ Coverage     89.00%   89.16%   +0.15%     
- Complexity     3050     3056       +6     
============================================
  Files           383      383              
  Lines         12175    12169       -6     
  Branches       1541     1544       +3     
============================================
+ Hits          10836    10850      +14     
+ Misses          710      696      -14     
+ Partials        629      623       -6     
Impacted Files Coverage Δ
...c/main/java/io/smallrye/mutiny/groups/UniJoin.java 100.00% <ø> (ø)
...lrye/mutiny/operators/uni/builders/UniJoinAll.java 95.83% <ø> (+5.16%) ⬆️
...ye/mutiny/operators/uni/builders/UniJoinFirst.java 87.87% <0.00%> (+0.92%) ⬆️
...mallrye/mutiny/helpers/queues/MpscLinkedQueue.java 96.77% <0.00%> (-1.62%) ⬇️
...mallrye/mutiny/operators/multi/MultiFlatMapOp.java 83.89% <0.00%> (-1.22%) ⬇️
...erators/multi/builders/SerializedMultiEmitter.java 80.48% <0.00%> (+1.21%) ⬆️
...tiny/operators/multi/MultiBufferWithTimeoutOp.java 74.78% <0.00%> (+1.68%) ⬆️
...ny/operators/uni/builders/UniCreateFromFuture.java 94.64% <0.00%> (+1.78%) ⬆️
...io/smallrye/mutiny/operators/uni/UniMemoizeOp.java 93.20% <0.00%> (+1.94%) ⬆️
... and 6 more

@jponge jponge added this to the 1.4.0 milestone Jan 28, 2022
@jponge jponge added the enhancement New feature or request label Jan 28, 2022
@jponge jponge marked this pull request as ready for review January 28, 2022 20:55
@jponge jponge requested a review from cescoffier January 28, 2022 20:56
@@ -29,6 +28,7 @@ public UniJoinFirst(List<Uni<T>> unis, Mode mode, int concurrency) {
this.unis = unis;
this.mode = mode;
this.concurrency = concurrency;
assert (mode == Mode.FIRST_WITH_ITEM || (mode == Mode.FIRST_TO_EMIT && concurrency == -1)); // Invariant enforced by the caller DSL
Copy link
Member Author

Choose a reason for hiding this comment

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

Note: Maven Surefire enables assertions by default. It's really an invariant worth checking in tests, not at runtime.

@jponge jponge merged commit 34fd96d into smallrye:main Jan 30, 2022
@jponge jponge deleted the feature/uni-join-no-concurrency-limit-on-first-to-terminate branch January 30, 2022 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Uni.join().first(unis).toTerminate() must not support concurrency limit
2 participants