-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Report load when rq active is non-zero. #41786
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Xin Zhuang <stevenzzz@google.com>
Signed-off-by: Xin Zhuang <stevenzzz@google.com>
Signed-off-by: Xin Zhuang <stevenzzz@google.com>
|
/assign @adisuissa |
|
/retest |
| // rq_issued is zero(no requests have been issued in this poll | ||
| // window), some load balancer may need rq_active for load balancing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // rq_issued is zero(no requests have been issued in this poll | |
| // window), some load balancer may need rq_active for load balancing. | |
| // rq_issued is zero (no new requests have been issued in this poll | |
| // window). This is needed to report long-lived connections/requests (e.g., when web-sockets are used). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
| scoped_runtime.mergeValues( | ||
| {{"envoy.reloadable_features.report_load_when_rq_active_is_non_zero", "true"}}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will be enabled by default, but it is still ok to enable this here so if a user disables it by default the test will still pass.
Please add a comment stating that when the runtime flag is deprecated this test should be kept, to avoid confusion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, done.
Signed-off-by: Xin Zhuang <stevenzzz@google.com>
Signed-off-by: Xin Zhuang <stevenzzz@google.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks
| // rq_issued is zero(no requests have been issued in this poll | ||
| // window), some load balancer may need rq_active for load balancing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
| scoped_runtime.mergeValues( | ||
| {{"envoy.reloadable_features.report_load_when_rq_active_is_non_zero", "true"}}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
|
/assign-from @envoyproxy/senior-maintainers |
|
@envoyproxy/senior-maintainers assignee is @RyanTheOptimist |
|
Please fix CI issue. |
Signed-off-by: Xin Zhuang <stevenzzz@google.com>
Signed-off-by: Xin Zhuang <stevenzzz@google.com>
|
Thanks, done. |
|
/retest |
Commit Message: If rq_active is non-zero, we should send the locality stats even if rq_issued is zero(no requests have been issued in this poll window), some load balancer may need rq_active for load balancing.
Additional Description:
Risk Level: LOW (use or not completely decided by remote loadbalancer
Testing: unit test
Docs Changes: guarded by runtime
envoy_reloadable_features_report_load_when_rq_active_is_non_zeroRelease Notes: Added runtime guard
envoy.reloadable_features.report_load_when_rq_active_is_non_zero, When enabled, LRS continues to send locality_stats reoprt to config server when there is no request_issued in the poll cycle.Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]