-
Notifications
You must be signed in to change notification settings - Fork 844
Fixes Issue #6321 caused when proxy.config.http.no_dns_just_forward_to_parent #6332
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
Conversation
proxy/ParentSelection.cc
Outdated
| // findParent() is called early in the transaction so, use that | ||
| // query result and clear the flag. | ||
| if (result->parent_exists_result && result->result == PARENT_SPECIFIED) { | ||
| result->parent_exists_result = false; |
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.
Why are we resetting the flag here? if findParent is called 3 times instead of 2 times, are we going to see the problem again?
proxy/ParentSelection.cc
Outdated
| result->reset(); | ||
| // when proxy.config.http.no_dns_just_forward_to_parent is enabled | ||
| // findParent() is called early in the transaction so, use that | ||
| // query result and clear the flag. |
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.
Can we make ParentExists() not to change any structures instead of using a flag?
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.
+1, IMHO ParentExists() should be an idempotent method
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.
@vmamidi an d @jvgutierrez I see your point. I'll do some more work on this.
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.
@vmamidi and @jvgutierrez I've modified parentExists() to just search for an available parent returning true/false as is found by iterating through the parent lists. Later in the transaction where the result is actually used, findParent() is called which looks up an available parent using the strategy algorithm. parentExists() just needs to return true/false on whether there is an available parent and is not concerned with the selection algorithm. parentExists() is now idempotent which fixes the strict round robin selection issue.
…ward_to_parent is enabled. When this configuration variable is enabled, a parent selection strategies findParent() function is called twice on each transaction resulting in unexpected results such as every other parent is only used in a strict round robin strategy.
e535f1a to
470f59d
Compare
|
Cherry-picked to v9.0.x branch. |
|
This has merge conflicts. |
|
@zwoop I'll have a look. |
Fixes #6321 when proxy.config.http.no_dns_just_forward_to_parent is enabled.
When this configuration variable is enabled, a parent selection strategies findParent() function is called twice on each transaction resulting in unexpected results such as every other parent is only used in a strict round robin strategy.