8000 Changes to server selection and polling behaviour by DavidMcNallyAlianza · Pull Request #992 · c-ares/c-ares · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Changes to server selection and polling behaviour #992

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Changes to server selection and polling behaviour #992

wants to merge 4 commits into from
+787 −157

Conversation

DavidMcNallyAlianza
Copy link
@DavidMcNallyAlianza DavidMcNallyAlianza commented May 12, 2025 •
8000 edited
Loading

This PR contains the following changes:

  • The addition of configurable rise/fall server behaviour (only mark servers as up or down after a certain number of successes or failures) (this is issue More fine-grained control of server priority list #987)
  • Changes to server selection when all servers are failed, instead of picking the highest priority server at all times, a query will prioritise trying all failed servers at least once.
  • Polling now polls all failed servers that are outside of the timeout rather than just the highest priority one.

DavidMcNallyAlianza
DavidMcNallyAlianza commented May 12, 2025
View reviewed changes
DavidMcNallyAlianza
DavidMcNallyAlianza commented May 12, 2025
View reviewed changes
thomaswhiteway
thomaswhiteway reviewed May 12, 2025
View reviewed changes
@DavidMcNallyAlianza DavidMcNallyAlianza marked this pull request as ready for review May 13, 2025 08:11
@bradh352
Copy link
Member

I will review this weekend.

I did see a comment related to configuration, its ok to add members to the end of the ares_options as long as they aren't members that require allocation/deallocation since ares_destroy_options doesn't take the optmask. I think the comment found was more aspirational than practical and predates me being a maintainer ... we've added lots of options to ares_options since then.

We have a ticket to redesign the whole bringup / settings, I'd rather not add helper functions to set new settings since it will make more work for that in the future.

@DavidMcNallyAlianza
Copy link
Author

Hi @bradh352, I would ideally get this change into the next c-ares release, do you know when you will be able to review?

@DavidMcNallyAlianza
Copy link
Author

Hi @bradh352, sorry to bother you again but would it be possible for you to let me know when you will be able review this change?

@bradh352
Copy link
Member
bradh352 commented Jun 2, 2025

Hi David, sorry for the delay. Day job has me crazy lately. I'll try to carve out some time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thomaswhiteway thomaswhiteway thomaswhiteway left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@DavidMcNallyAlianza @bradh352 @thomaswhiteway
0