Skip to content

Experimental onBackpressureBufferWithCapacity #1916

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

Conversation

benjchristensen
Copy link
Member

Manual merge of #1899

  • changed Func0 to Action
  • added Beta markers

srvaroa and others added 4 commits November 21, 2014 17:05
The operator takes an optional capacity for the buffer and a callback to
be triggered when the buffer fills up.
@@ -15,23 +15,46 @@
*/
package rx.internal.operators;

import java.nio.BufferOverflowException;
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure about the use of this exception. I think it should be MissingBackpressureException like we use elsewhere.

@benjchristensen
Copy link
Member Author

Updated to use MissingBackpressureException.

@benjchristensen
Copy link
Member Author

Should we change Action0 to Action1<T> so that the function receive each item it is dropping so the function can choose to inspect, log or otherwise do something? On one hand it would provide flexibility for anything. On the other it could easily be abused and easily result in blocking IO (even just logging) and kill a system right when backpressure is trying to kick in by dropping data.

@benjchristensen
Copy link
Member Author

Should we change Action0 to Action1 so that the function receive each item it is dropping

Nevermind. We only call the action onOverflow once, not every item overflowed.

@benjchristensen
Copy link
Member Author

This seems good enough to include in an @Beta form so am merging to include in 1.0.2. We can iterate further while it's in beta if needed, but it seems okay as is.

benjchristensen added a commit that referenced this pull request Dec 2, 2014
…ureBufferWithCapacity

Experimental onBackpressureBufferWithCapacity
@benjchristensen benjchristensen merged commit c8c272e into ReactiveX:1.x Dec 2, 2014
@benjchristensen benjchristensen deleted the experimentalOnBackpressureBufferWithCapacity branch December 2, 2014 05:59
@benjchristensen benjchristensen restored the experimentalOnBackpressureBufferWithCapacity branch December 2, 2014 06:19
@srvaroa
Copy link
Contributor

srvaroa commented Dec 2, 2014

Hi @benjchristensen

Re. readability of ensureCapacity I agree, thanks for fixing that one + the exception.

Re. Action1 vs. Action0 I'm leaning towards keeping Action0 because the risks seem much clearer than the hypothetical benefits of seeing the dropped item. But I don't have a strong opinion, up to you (we could always add another overload, and warn in the javadoc).

I've been taking a look at the test failures but I can't see a clear cause for the deadlock (which also seem to happen on tests that as far as I can see should not use the code added here). I do reproduce it locally anyway so I'll keep looking into it and submit the fix, hopefully soon.

@benjchristensen benjchristensen deleted the experimentalOnBackpressureBufferWithCapacity branch June 11, 2015 21:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants