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

chore: pip performance is improved by ipv4 over ipv6 precedence #14057

Merged
merged 1 commit into from
Oct 4, 2022

Conversation

nstng
Copy link
Contributor

@nstng nstng commented Sep 30, 2022

Signed-off-by: Nils Semmelrock nils.semmelrock@tngtech.com

Summary

There is no ipv6 interface defined on the vms that is able to do external communication. Various tools (also pip) use getaddrinfo for communication with a service. This has per default an ipv6 preference. If ipv6 endpoints are not reachable then this can take much longer than needed.
Here: change to an ipv4 over ipv6 preference.

To reproduce:

  • run time pip3 install pip-install-test with and without this change (can be done without sourcing or restarting)
  • without ipv4 preference:
vagrant@magma-deb:~$ time pip3 install pip-install-test
Collecting pip-install-test
  Using cached pip_install_test-0.5-py3-none-any.whl (1.7 kB)
Installing collected packages: pip-install-test
Successfully installed pip-install-test-0.5

real	0m13.822s
user	0m1.329s
sys	0m0.132s
  • with ipv4 preference:
vagrant@magma-deb:~$ time pip3 install pip-install-test
Collecting pip-install-test
  Using cached pip_install_test-0.5-py3-none-any.whl (1.7 kB)
Installing collected packages: pip-install-test
Successfully installed pip-install-test-0.5

real	0m1.707s
user	0m1.360s
sys	0m0.163s
  • -> real 0m13.822s vs real 0m1.707s

Note: there might be a better way to configure the network interfaces of the VMs - until now I did not find a more clean way.

Test Plan

Make sure no integration test depends on an ipv6 preference:

Additional Information

  • This change is backwards-breaking

Signed-off-by: Nils Semmelrock <nils.semmelrock@tngtech.com>
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines. label Sep 30, 2022
@github-actions
Copy link
Contributor

Thanks for opening a PR! 💯

A couple initial guidelines

Howto

  • Reviews. The "Reviewers" listed for this PR are the Magma maintainers who will shepherd it.
  • Checks. All required CI checks must pass before merge.
  • Merge. Once approved and passing CI checks, use the ready2merge label to indicate the maintainers can merge your PR.

More info

Please take a moment to read through the Magma project's

If this is your first Magma PR, also consider reading

@github-actions github-actions bot added component: agw Access gateway-related issue component: ci All updates on CI (Jenkins/CircleCi/Github Action) labels Sep 30, 2022
@github-actions
Copy link
Contributor

feg-workflow

    2 files  203 suites   40s ⏱️
374 tests 374 ✔️ 0 💤 0
388 runs  388 ✔️ 0 💤 0

Results for commit cad0478.

@github-actions
Copy link
Contributor

dp-workflow

14 tests   14 ✔️  2m 26s ⏱️
  1 suites    0 💤
  1 files      0

Results for commit cad0478.

@github-actions
Copy link
Contributor

agw-workflow

615 tests   611 ✔️  4m 45s ⏱️
    2 suites      4 💤
    2 files        0

Results for commit cad0478.

@nstng nstng self-assigned this Oct 4, 2022
@nstng nstng marked this pull request as ready for review October 4, 2022 06:39
@nstng nstng requested review from a team as code owners October 4, 2022 06:39
@nstng nstng merged commit 93f229e into magma:master Oct 4, 2022
pruthvihebbani pushed a commit to pruthvihebbani/magma that referenced this pull request Oct 10, 2022
…a#14057)

Signed-off-by: Nils Semmelrock <nils.semmelrock@tngtech.com>

Signed-off-by: Nils Semmelrock <nils.semmelrock@tngtech.com>
@nstng nstng linked an issue Oct 10, 2022 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: agw Access gateway-related issue component: ci All updates on CI (Jenkins/CircleCi/Github Action) size/M Denotes a PR that changes 30-99 lines.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make the Bazel local setup more comfortable
3 participants