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

Add system properties for enabling SamplingAllocationStrategy #1454

Merged
merged 2 commits into from
Jan 7, 2021

Conversation

violetagg
Copy link
Member

reactor.netty.pool.getPermitsSamplingRate and
reactor.netty.pool.returnPermitsSamplingRate are exposed
in order to be able to enable Reactor Pool's SamplingAllocationStrategy

"reactor.netty.pool.getPermitsSamplingRate" and
"reactor.netty.pool.returnPermitsSamplingRate" are exposed
in order to be able to enable Reactor Pool's SamplingAllocationStrategy
@violetagg violetagg added this to the 0.9.16.RELEASE milestone Jan 7, 2021
DEFAULT_POOL_GET_PERMITS_SAMPLING_RATE,
DEFAULT_POOL_RETURN_PERMITS_SAMPLING_RATE));
}
else {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would consider at least logging a warning if (some) of the values are set but out of 0-1 bounds (eg. somebody sets sampling rate at 50 thinking it's 50 percent). Negative can probably be silently ignored, and 0 (defaults) totally can be silently ignored, but positives above 1.0 should definitely trigger some warning. That said, maybe that check should be done in a static final initializer block upon loading the system property ?

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

@violetagg violetagg merged commit 6c3ab1c into 0.9.x Jan 7, 2021
@violetagg violetagg deleted the sampling-allocation-strategy branch January 7, 2021 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants