Repository navigation
fix: demand accounting in MultiBufferWithTimeoutOp - #2218
Conversation
|
@jponge isn't there something odd in the tests? We always get more than we requested from the subscriber. |
|
Hum the demand should match lists, not necessarily the number of elements in the list, no? |
|
Oh good catch, I should not have tried to port the other PR tests, and |
- buffer items when demand exhausted instead of BackPressureFailure (finite size) - preserve BackPressureFailure for unbounded size (every(Duration) API) - drain pending buffer on new demand via request() - fix double terminal signal in checkedComplete() - add deterministic and concurrency tests Closes: smallrye#2211 Supersedes: smallrye#2212
|
@ozangunalp updated |
|
@jponge Sorry this took me soo long. I've been writing a couple of tests for this operator, regarding completion and requests, trying to understand how we should handle this section of the Javadoc: This current version fails most of these new tests. To be fair, a couple of these tests fail on the current main as well. I'll push the tests onto this branch. |
|
@ozangunalp I've added a possible fix with drain-loop (yet another one) and some extra tests. |
| private List<T> current; | ||
| private volatile ScheduledFuture<?> task; | ||
|
|
||
| private final ArrayDeque<List<T>> readyBatches = new ArrayDeque<>(); |
There was a problem hiding this comment.
In theory, we should not need more than one list, right?
There was a problem hiding this comment.
There's a matrix of list size / timers where in-flight items require multiple buckets.
There was a problem hiding this comment.
Thank you, @jponge. The operator is already very complex, to be honest, I can no longer follow it. Tests seem reasonable, and they pass now 🚀
2 participants
Closes: #2211
Supersedes: #2212