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.

Sign up for GitHub

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

Jump to bottom

StepVerifier fix For Fusion and request after onNext #1982

Merged
merged 1 commit into from
Dec 5, 2019
Merged

StepVerifier fix For Fusion and request after onNext #1982

merged 1 commit into from
Dec 5, 2019

Conversation

OlegDokuka
Copy link
Contributor

Expected

When StepVerifier is used for verification of Publisher behavior in fusion mode as a User, I want to have it act identically to other operators in fusion mode with upstream. For example, let's consider the behavior of FluxPublishOn in ASYNC fusion with UnicastProcessor as the Upstream

Event: unicastProcessor is subscribed by FluxPublishOnSubscriber
Event: FluxPublishOnSubscriber received onSubscribe (let's assume there is no prefetch)
Event: unicastProcessor.onNext(XXX)
Event: FluxPublishOnSubscriber is notified over onNext(null) there is an item in a queue
Event: request(XXX) appeared at FluxPublishOnSubscriber, so it called `drain` method in order to start draining elements from the fused queue

Actual

In case of such events' order StepVerifier will hang until its timeout.

Signed-off-by: Oleh Dokuka shadowgun@i.ua

@OlegDokuka OlegDokuka force-pushed the bugfix/fusion_verification branch from c100331 to 277afd3 Compare December 4, 2019 20:41
simonbasle
simonbasle approved these changes Dec 5, 2019
View reviewed changes
Copy link
Contributor

@simonbasle simonbasle left a comment

Choose a reason for hiding this comment

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

lgtm

@simonbasle
Copy link
Contributor

@OlegDokuka could you try to target 3.2.x for this one instead of master?

@OlegDokuka OlegDokuka changed the base branch from master to 3.2.x December 5, 2019 13:50
@simonbasle
Copy link
Contributor

simonbasle commented Dec 5, 2019 •
edited
Loading

@OlegDokuka you'll probably need to either git rebase -i 3.2.x and drop most commits that were actually from master, or redo the branch from a simple cherry pick

@OlegDokuka
provides events draining in case of request happened after onNext
90af031 
Bug explanation: if we have ASYNC fusion and onNext(null) appears when there is nothing yet requested
(for instance, StepVerifier.create(flux, 0)) then when request appears over `thenRequest(n)`
then the drain loop will NOT be invoked so the actual enqueued element will not be delivered and we will get a hanged test suit

Signed-off-by: Oleh Dokuka <shadowgun@i.ua>
@OlegDokuka OlegDokuka force-pushed the bugfix/fusion_verification branch from 277afd3 to 90af031 Compare December 5, 2019 14:08
@OlegDokuka
Copy link
Contributor Author

@simonbasle fixed

@simonbasle simonbasle added this to the 3.2.14.RELEASE milestone Dec 5, 2019
@simonbasle simonbasle changed the title SpetVerifier Fix For Fusion and request after onNext StepVerifier fix For Fusion and request after onNext Dec 5, 2019
@simonbasle simonbasle merged commit ce40bd0 into reactor:3.2.x Dec 5, 2019
@reactorbot
Copy link

@simonbasle this PR seems to have been merged on a maintenance branch, please ensure the change is merge-forwarded to intermediate maintenance branches and up to master 🙇

simonbasle added a commit that referenced this pull request Dec 5, 2019
@simonbasle
Merge #1982 into 3.3
1628b2e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simonbasle simonbasle simonbasle approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
3.2.14.RELEASE
Development

Successfully merging this pull request may close these issues.
3 participants
@OlegDokuka @simonbasle @reactorbot