android: default to use the system DNS resolver#2511
Merged
Conversation
Rather than c-ares. In our tests, the system DNS resolver consumes less CPU and memory while otherwise performing similarly overall. Signed-off-by: JP Simard <jp@jpsim.com>
Signed-off-by: JP Simard <jp@jpsim.com>
…e-the-system-dns-resolver * origin/main: android: enable forceIPv6 by default (#2510) Signed-off-by: JP Simard <jp@jpsim.com>
Contributor
|
/retest |
Signed-off-by: JP Simard <jp@jpsim.com>
Signed-off-by: JP Simard <jp@jpsim.com>
Contributor
Author
|
/retest |
Augustyniak
approved these changes
Sep 1, 2022
| - iOS: A documentation archive is now included in the GitHub release artifact (:issue: `#2335 <2335>`) | ||
| - api: improved C++ APIs compatibility with Java / Kotlin / Swift (:issue `#2362 <2362>`) | ||
| - api: add option to use the a ``getaddrinfo``-based system DNS resolver instead of c-ares (:issue: `#2419 <2419>`) | ||
| - Android: default to use a ``getaddrinfo``-based system DNS resolver instead of c-ares (:issue: `#2419 <2419>`) |
Contributor
There was a problem hiding this comment.
for whatever reason Android is not capitalized in other occurences in this file. Up to you whether you want to stay consistent or not.
Contributor
Author
There was a problem hiding this comment.
Seems like we should honour brand name capitalization. As someone whose name is frequently mis-capitalized this might be a touchy issue for me 😅.
1 task
colibie
pushed a commit
to colibie/envoy-mobile
that referenced
this pull request
Sep 21, 2022
Description: Rather than c-ares. In our tests, the system DNS resolver consumes less CPU and memory while otherwise performing similarly overall. Risk Level: Medium, although it's performed well in our tests, it's possible that some situations we haven't tested perform worse than c-ares. Users can still disable this in the engine builder though. Testing: Lyft apps & unit tests Docs Changes: Docstrings updated to reflect new default Release Notes: Added Signed-off-by: JP Simard <jp@jpsim.com> Signed-off-by: Chidera Olibie <colibie@google.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description: Rather than c-ares. In our tests, the system DNS resolver consumes less CPU and memory while otherwise performing similarly overall.
Risk Level: Medium, although it's performed well in our tests, it's possible that some situations we haven't tested perform worse than c-ares. Users can still disable this in the engine builder though.
Testing: Lyft apps & unit tests
Docs Changes: Docstrings updated to reflect new default
Release Notes: Added
Signed-off-by: JP Simard jp@jpsim.com