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

=act #15040 ScatterGatherFirstCompleted reply when no routees #18260

Merged
merged 1 commit into from
Aug 18, 2015

Conversation

patriknw
Copy link
Member

No description provided.

@patriknw
Copy link
Member Author

Refs #15040

@akka-ci akka-ci added the validating PR is currently being validated by Jenkins label Aug 18, 2015

"without routees should reply immediately" in {
val probe = TestProbe()
val router = system.actorOf(ScatterGatherFirstCompletedPool(nrOfInstances = 0, within = 5.seconds).props(Props.empty))
Copy link
Member

Choose a reason for hiding this comment

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

I thought that we may consider 0 illegal here, but the case Heiko spoke of can well be triggered by all the initially in-the-pool routees to die, leaving the pool having 0 routees... then the optimisation makes sense, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, i just thought this was an easy way to test it

Copy link
Member

Choose a reason for hiding this comment

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

Okey, sounds good. Making the 0 throw right away during construction probably does not buy much here in terms of safety for users, they'll notice anyway.

Copy link
Member Author

Choose a reason for hiding this comment

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

There might be corner cases with not allowing 0, e.g. could work with 0 + resizer.

Copy link
Member

Choose a reason for hiding this comment

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

I see :) The PR looks good in any case, feel free to merge when you get the time

@ktoso
Copy link
Member

ktoso commented Aug 18, 2015

LGTM

@akka-ci akka-ci added tested PR that was successfully built and tested by Jenkins and removed validating PR is currently being validated by Jenkins labels Aug 18, 2015
@akka-ci
Copy link

akka-ci commented Aug 18, 2015

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://jenkins.akka.io/job/pr-validator-per-commit-jenkins/3444/

patriknw added a commit that referenced this pull request Aug 18, 2015
…eted-patriknw

=act #15040 ScatterGatherFirstCompleted reply when no routees
@patriknw patriknw merged commit 1732e62 into master Aug 18, 2015
@patriknw patriknw deleted the wip-15040-ScatterGatherFirstCompleted-patriknw branch August 18, 2015 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tested PR that was successfully built and tested by Jenkins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants