Skip to content

StepVerifier fix For Fusion and request after onNext#1982

Merged
simonbasle merged 1 commit into
reactor:3.2.xfrom
OlegDokuka:bugfix/fusion_verification
Dec 5, 2019
Merged

StepVerifier fix For Fusion and request after onNext#1982
simonbasle merged 1 commit into
reactor:3.2.xfrom
OlegDokuka:bugfix/fusion_verification

Conversation

@OlegDokuka
Copy link
Copy Markdown
Contributor

@OlegDokuka OlegDokuka commented Dec 4, 2019

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
Copy Markdown
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
Copy Markdown
Contributor

simonbasle commented Dec 5, 2019

@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
Copy Markdown
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
Copy Markdown
Contributor Author

OlegDokuka commented Dec 5, 2019

@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
Copy Markdown

reactorbot commented Dec 5, 2019

@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

1 more reviewer

@simonbasle simonbasle simonbasle approved these changes

Reviewers whose approvals may not affect merge requirements

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