Skip to content

Add details on how Core performs proxy determination - #349

Closed
yashykt wants to merge 6 commits into
grpc:masterfrom
yashykt:AddCoreDetailsToHttpConnectProxySupport
Closed

Add details on how Core performs proxy determination#349
yashykt wants to merge 6 commits into
grpc:masterfrom
yashykt:AddCoreDetailsToHttpConnectProxySupport

Conversation

@yashykt

@yashykt yashykt commented Mar 30, 2023

Copy link
Copy Markdown
Member

Also formats the doc with google's mdformat.

Comment thread A1-http-connect-proxy-support.md
Comment thread A1-http-connect-proxy-support.md Outdated
Comment thread A1-http-connect-proxy-support.md Outdated
Comment thread A1-http-connect-proxy-support.md
Comment thread A1-http-connect-proxy-support.md
name to resolve. Sites that may want to send only external servers through the
proxy can implement their own proxy mapper to do this. We will provide a default
proxy mapper implementation that looks for the existence of the `http_proxy`
environment variable (or equavalent signal). If that signal is present, it will

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

s/equavalent/equivalent/

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not proceeding with changes to this doc anymore

proxy can implement their own proxy mapper to do this. We will provide a default
proxy mapper implementation that looks for the existence of the `http_proxy`
environment variable (or equavalent signal). If that signal is present, it will
do the following: - Ask the resolver to resolve the proxy name instead of the

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

bulleted list formatting is lost in this version. Can you restore it so each line starting with - appears as a separate bullet item?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not proceeding with changes to this doc anymore

addresses, it is impossible to use the normal gRPC client-side per-call load
balancing. It *is* possible to do load balancing on a per-connection basis, but
that may not spread out the load evenly if different clients impose a different
amount of load. (Note that even if we established multiple connections to the

@sanjaypujare sanjaypujare Mar 31, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The wording "...even if we established multiple connections to the..." implies that we currently don't do this which is probably not the intent. Basically with the current implementation (as described in the previous lines 122-124) the gRPC client will create a separate connection to the same proxy when asked to create a channel to the target.

It is important to clarify this point for the use-case we have been discussing and for which this PR has been created to explain the functioning.

So the wording can be something like "Note that the current implementation does establish multiple connections to the proxy, but there is no guarantee that each connection will go to a different backend server behind the proxy".

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not proceeding with changes to this doc anymore

@markdroth

Copy link
Copy Markdown
Member

Rather than modifying a gRFC this old, I think we should just create a new doc in the grpc/grpc repo.

@yashykt

yashykt commented Apr 4, 2023

Copy link
Copy Markdown
Member Author

Alright, moving the new added details to grpc/grpc#32789

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants