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

scheduler: tolerate having only one dynamic port available #17619

Merged
merged 1 commit into from
Jun 20, 2023

Conversation

tgross
Copy link
Member

@tgross tgross commented Jun 20, 2023

If the dynamic port range for a node is set so that the min is equal to the max, there's only one port available and this passes config validation. But the scheduler panics when it tries to pick a random port. Only add the randomness when there's more than one to pick from.

Adds a test for the behavior but also adjusts the commentary on a couple of the existing tests that made it seem like this case was already covered if you didn't look too closely.

Fixes: #17585

If the dynamic port range for a node is set so that the min is equal to the max,
there's only one port available and this passes config validation. But the
scheduler panics when it tries to pick a random port. Only add the randomness
when there's more than one to pick from.

Adds a test for the behavior but also adjusts the commentary on a couple of the
existing tests that made it seem like this case was already covered if you
didn't look too closely.

Fixes: #17585
@tgross tgross force-pushed the b-scheduler-panic-1-dynamic-port branch from d87473a to e9e4358 Compare June 20, 2023 15:27
@tgross tgross changed the title network: tolerate having only one dynamic port available scheduler: tolerate having only one dynamic port available Jun 20, 2023
@tgross tgross added this to the 1.6.0 milestone Jun 20, 2023
@tgross tgross marked this pull request as ready for review June 20, 2023 15:57
@tgross tgross requested review from shoenig and angrycub June 20, 2023 15:57
Copy link
Member

@shoenig shoenig left a comment

Choose a reason for hiding this comment

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

LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.3.x backport to 1.3.x release line backport/1.4.x backport to 1.4.x release line backport/1.5.x backport to 1.5.x release line theme/scheduling type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Scheduler panic at evaluation time if there is a client with max_dynamic_port == min_dynamic_port
2 participants