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

loadbalancer-experimental: DefaultLoadBalancer logs settings on startup #3000

Conversation

bryce-anderson
Copy link
Contributor

Motivation:

As DefaultLoadBalancer becomes the norm it can be helpful to track its activation via log messages.

Modifications:

  • Emit a log message on startup that includes the lb description and important settings.
  • Fix a bug where the cancellation penalty was inadvertently used for the error penalty.
  • Cancel the update stream from the OutlierDetector when the lb closes.

Motivation:

As DefaultLoadBalancer becomes the norm it can be helpful to track its
activation via log messages.

Modifications:

- Emit a log message on startup that includes the lb description and
  important settings.
- Fix a bug where the cancellation penalty was inadvertently used for
  the error penalty.
- Cancel the update stream from the OutlierDetector when the lb closes.
Comment on lines +172 to 173
this.outlierDetectorStatusChangeStream = this.outlierDetector.healthStatusChanged().forEach((ignored) ->
sequentialExecutor.execute(() -> sequentialUpdateUsedHosts(usedHosts)));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Previously this was simply discarded which was fine because of the way it's implemented but we might as well cancel it in case that changes in the future.

Copy link
Member

Choose a reason for hiding this comment

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

Good catch. Up to you if you'd like to keep it here or move into a separate PR for visibility.

Comment on lines +172 to 173
this.outlierDetectorStatusChangeStream = this.outlierDetector.healthStatusChanged().forEach((ignored) ->
sequentialExecutor.execute(() -> sequentialUpdateUsedHosts(usedHosts)));
Copy link
Member

Choose a reason for hiding this comment

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

Good catch. Up to you if you'd like to keep it here or move into a separate PR for visibility.

@bryce-anderson bryce-anderson merged commit 72e9511 into apple:main Jul 9, 2024
11 checks passed
@bryce-anderson bryce-anderson deleted the bl_anderson/log-DefaultLoadBalancer-on-startup branch July 9, 2024 23:25
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