Skip to content

test: Add a LB simulation test to validate selection probabilities#30818

Merged
adisuissa merged 6 commits intoenvoyproxy:mainfrom
tonya11en:pr/lb-sim
Dec 14, 2023
Merged

test: Add a LB simulation test to validate selection probabilities#30818
adisuissa merged 6 commits intoenvoyproxy:mainfrom
tonya11en:pr/lb-sim

Conversation

@tonya11en
Copy link
Copy Markdown
Member

@tonya11en tonya11en commented Nov 10, 2023 •
edited
Loading

Adds a LB simulation test to ensure that any subtle behavioral changes to the Least Request LB will cause a test failure. The test verifies the selection probabilities are as expected in various contexts.

The intent of this test is not to ossify the behavior of the LB, but to flag any potential unintended behavioral changes.

This test was run 10,000 times (using remote executors) to check for flakiness and there was not any observed failure:

 txallen@stormbeard  ~/src/envoy/test/common/upstream   lb-sim  bazel test //test/common/upstream:load_balancer_simulation_test --runs_per_test 10000
INFO: Invocation ID: 994289cb-9172-4db7-9307-30f202507d87
INFO: Analyzed target //test/common/upstream:load_balancer_simulation_test (0 packages loaded, 527 targets configured).
INFO: Found 1 test target...
Target //test/common/upstream:load_balancer_simulation_test up-to-date:
  bazel-bin/test/common/upstream/load_balancer_simulation_test
INFO: Elapsed time: 87.417s, Critical Path: 21.53s
INFO: 10001 processes: 1 internal, 10000 remote.
INFO: Build completed successfully, 10001 total actions

Executed 1 out of 1 test: 1 test passes.

kyessenov and others added 2 commits November 9, 2023 19:39
@kyessenov
Revert "fix: only enable full scan when enable_full_scan is set expli…
ba2fab0 
…citly forleast request lb (envoyproxy#30794)"

This reverts commit e93e556.

Revert "Fix least request lb not fair (envoyproxy#29873)"

This reverts commit 3ea2bc4.

restore api

Signed-off-by: Kuat Yessenov <kuat@google.com>

fix merge

Signed-off-by: Kuat Yessenov <kuat@google.com>
@tonya11en
test: Add a LB simulation test to validate selection probabilities
82c1d78 
Signed-off-by: Tony Allen <tony@allen.gg>
@repokitteh-read-only
Copy link
Copy Markdown

repokitteh-read-only Bot commented Nov 10, 2023

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #30818 was opened by tonya11en.

see: more, trace.

@repokitteh-read-only
Copy link
Copy Markdown

repokitteh-read-only Bot commented Nov 10, 2023

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @adisuissa
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #30818 was opened by tonya11en.

see: more, trace.

@tonya11en tonya11en mentioned this pull request Dec 4, 2023
Merged
@tonya11en
Merge remote-tracking branch 'upstream/main' into lb-sim
1db142d 
Signed-off-by: Tony Allen <tony@allen.gg>
@tonya11en
.
ea9417d 
Signed-off-by: Tony Allen <tony@allen.gg>
@tonya11en tonya11en marked this pull request as ready for review December 11, 2023 18:33
@tonya11en
Copy link
Copy Markdown
Member Author

tonya11en commented Dec 11, 2023

@adisuissa @wbpcode this PR should now be ready for a first pass.

adisuissa
adisuissa reviewed Dec 11, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@adisuissa adisuissa left a comment

Choose a reason for hiding this comment

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

Thanks!
Minor comments, but otherwise LGTM!

Comment thread changelogs/current.yaml Outdated
Comment on lines +162 to +166
- area: upstream
change: |
Added support to use http async client to fetch the credentials from EC2 instance metadata and ECS task metadata providers
instead of libcurl which is deprecated. By default this behavior is disabled. To enable set
``envoy.reloadable_features.use_http_client_to_fetch_aws_credentials`` to true.
Copy link
Copy Markdown
Contributor

@adisuissa adisuissa Dec 11, 2023

Choose a reason for hiding this comment

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

Doesn't seem related to the current PR, should probably be reverted.

Comment thread docs/root/intro/arch_overview/upstream/load_balancing/load_balancers.rst Outdated
Comment thread test/common/upstream/load_balancer_simulation_test.cc Outdated
Comment thread test/common/upstream/load_balancer_simulation_test.cc Outdated
@tonya11en
comments
16d259c 
Signed-off-by: Tony Allen <tony@allen.gg>
@tonya11en
format
400af7f 
Signed-off-by: Tony Allen <tony@allen.gg>
adisuissa
adisuissa approved these changes Dec 13, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@adisuissa adisuissa left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@adisuissa
Copy link
Copy Markdown
Contributor

adisuissa commented Dec 14, 2023

/retest

@adisuissa adisuissa merged commit 6d5d755 into envoyproxy:main Dec 14, 2023
@tonya11en tonya11en deleted the pr/lb-sim branch December 14, 2023 21:27
@wbpcode wbpcode mentioned this pull request Dec 21, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adisuissa adisuissa adisuissa approved these changes

Assignees

@adisuissa adisuissa

Labels

api

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@tonya11en @adisuissa @kyessenov