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

2.x UnicastProcessor fail-fast support #5226

Merged

Conversation

mostroverkhov
Copy link
Contributor

@mostroverkhov mostroverkhov commented Mar 24, 2017

This PR adds support for fail-fast behavior to UnicastProcessor with methods UnicastProcessor<T> create(boolean delayError), UnicastProcessor<T> create(int capacityHint, Runnable onTerminated, boolean delayError). Relates to #5165, #5217

@CheckReturnValue
@Experimental
public static <T> UnicastProcessor<T> create(int capacityHint, Runnable onCancelled, boolean delayError) {
return new UnicastProcessor<T>(capacityHint, onCancelled,delayError);
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: newline between onCancelled,delayError

Copy link
Member

Choose a reason for hiding this comment

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

Space.

return new UnicastProcessor<T>(capacityHint, onCancelled,delayError);
}

/**
* Creates an UnicastProcessor with the given capacity hint.
* @param capacityHint the capacity hint for the internal, unbounded queue
* @since 2.0
*/
UnicastProcessor(int capacityHint) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this one really necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

could you elaborate a bit?

@codecov
Copy link

codecov bot commented Mar 24, 2017

Codecov Report

Merging #5226 into 2.x will increase coverage by 0.06%.
The diff coverage is 93.75%.

Impacted file tree graph

@@             Coverage Diff              @@
##                2.x    #5226      +/-   ##
============================================
+ Coverage     96.01%   96.07%   +0.06%     
- Complexity     5747     5758      +11     
============================================
  Files           628      628              
  Lines         41085    41099      +14     
  Branches       5698     5703       +5     
============================================
+ Hits          39446    39485      +39     
+ Misses          657      642      -15     
+ Partials        982      972      -10
Impacted Files Coverage Δ Complexity Δ
...java/io/reactivex/processors/UnicastProcessor.java 95.9% <93.75%> (-0.91%) 64 <6> (+7)
...a/io/reactivex/processors/SerializedProcessor.java 91.48% <0%> (-6.39%) 27% <0%> (-1%)
.../internal/operators/maybe/MaybeTakeUntilMaybe.java 93.87% <0%> (-6.13%) 2% <0%> (ø)
.../operators/observable/ObservableFlatMapSingle.java 91.04% <0%> (-4.48%) 2% <0%> (ø)
...ernal/operators/maybe/MaybeTakeUntilPublisher.java 96% <0%> (-4%) 2% <0%> (ø)
...rnal/operators/completable/CompletableTimeout.java 94% <0%> (-4%) 2% <0%> (ø)
...ternal/operators/completable/CompletableCache.java 96.96% <0%> (-3.04%) 23% <0%> (-1%)
...x/internal/operators/maybe/MaybeSwitchIfEmpty.java 97.22% <0%> (-2.78%) 2% <0%> (ø)
...activex/internal/observers/QueueDrainObserver.java 61.53% <0%> (-2.57%) 12% <0%> (-1%)
...va/io/reactivex/internal/queue/SpscArrayQueue.java 97.61% <0%> (-2.39%) 22% <0%> (-1%)
... and 40 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 59dc7a3...d7409bb. Read the comment docs.

Copy link
Member

@akarnokd akarnokd left a comment

Choose a reason for hiding this comment

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

One of the new package-private constructors is unnecessary.

* @param delayError deliver pending onNext events before onError
* @since 2.0.8 - experimental
*/
UnicastProcessor(int capacityHint, boolean delayError) {
Copy link
Member

Choose a reason for hiding this comment

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

This is unnecessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

3 args constructor has non-null onTerminate check; should i move this check to factory?

Copy link
Member

Choose a reason for hiding this comment

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

Could you move that check into the appropriate create method(s)?

@akarnokd akarnokd changed the title [2.x] UnicastProcessor fail-fast support 2.x UnicastProcessor fail-fast support Mar 24, 2017
@akarnokd akarnokd added this to the 2.1 milestone Mar 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants