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

Provide APIs for load balance to be used outside of reverse proxy #1165

Open
Kahbazi opened this issue Jul 26, 2021 · 9 comments
Open

Provide APIs for load balance to be used outside of reverse proxy #1165

Kahbazi opened this issue Jul 26, 2021 · 9 comments
Assignees
Labels
samsp_list Personal tag used when reviewing issues for further discussion Type: Idea This issue is a high-level idea for discussion.
Milestone

Comments

@Kahbazi
Copy link
Collaborator

Kahbazi commented Jul 26, 2021

What should we add or change to make your life better?

Provide a service in order to have the same load balancing policy for a cluster outside reverse proxy middleware.

public interface ILoadBalanceService
{
    DestinationConfig? PickDestination(string cluster);
}

Why is this important to you?

We are using YARP with one cluster and multiple destinations. Most requests are being proxied to the cluster but we also have some routes that need some logic and don't exactly need to be proxied but to make a http call to the cluster. We don't need to send HttpContext.Request to the cluster and also don't want the response to be sent in HttpContext.Response. We only need the destination address of the cluster.

@Kahbazi Kahbazi added the Type: Idea This issue is a high-level idea for discussion. label Jul 26, 2021
@Tratcher
Copy link
Member

It seems odd that we'd only do this for load balancing and not other proxy features like session affinity, passive health checks, retry, etc..

There have been a few issues asking about getting access to the clusters from outside the main pipeline, like #1159. IClusterChangeListener works but is cumbersome. Once you had the cluster, most of the load balancing algorithms are trivial.

Note the concurrency counter is internal so you wouldn't be able to reliably track the load added by these extra requests.

internal AtomicCounter ConcurrencyCounter { get; } = new AtomicCounter();

@Kahbazi
Copy link
Collaborator Author

Kahbazi commented Jul 26, 2021

There have been a few issues asking about getting access to the clusters from outside the main pipeline, like #1159. IClusterChangeListener works but is cumbersome. Once you had the cluster, most of the load balancing algorithms are trivial.

How about adding an interface like IClusterProvider and ProxyConfigManager could implement it by returning the clusters dictionary?

It seems odd that we'd only do this for load balancing and not other proxy features like session affinity, passive health checks, retry, etc..
Note the concurrency counter is internal so you wouldn't be able to reliably track the load added by these extra requests.

I guess it would be nice to be able to leverage all YARP features outside the main pipeline. What do you think about adding this?

Task<HttpResponse> SendAsync(HttpRequest request, string cluster);

This method could first make sure that HttpRequest has an relative path so it could be added to the base address from destination. I think it could also handle session affinity, passive health checks and also load balancing.

@karelz
Copy link
Member

karelz commented Jul 27, 2021

Triage: This is not a direction we plan to evolve the design. Unless we hear from more users, we won't do anything here ... closing for now.

Some workarounds: Load-balancing algorithms are trivial. For starting request -- use HttpClient to call into itself ... ugly/awkward, but will work.

@karelz karelz closed this as completed Jul 27, 2021
@Kahbazi
Copy link
Collaborator Author

Kahbazi commented Jul 28, 2021

@Tratcher Right now all load balancing, session affinity and health check are coupled to HttpContext. In order to prevent possible future breaking changes do you think it worth decoupled them from HttpContext now in case in future there's a plan to provide them outside main pipeline?

@Tratcher
Copy link
Member

That coupling was intentional to enable extensibility based on request fields. Also, I don't see how session affinity would work without that coupling, it requires access to the request and response even for its normal scenarios.

We are considering what it would look like for features like load balancing and health checks to exist independently from YARP, maybe as their own library or HttpClient layer, but we don't anticipate an existing model like YARP to directly consume those.

@Kahbazi
Copy link
Collaborator Author

Kahbazi commented Jul 28, 2021

That coupling was intentional to enable extensibility based on request fields. Also, I don't see how session affinity would work without that coupling, it requires access to the request and response even for its normal scenarios.

I was thinking to use some types like RequestTransformContext but without its HttpContext property!

We are considering what it would look like for features like load balancing and health checks to exist independently from YARP, maybe as their own library or HttpClient layer, but we don't anticipate an existing model like YARP to directly consume those.

I understand, thanks for the explanation.

@samsp-msft
Copy link
Contributor

There have been a few issues asking about getting access to the clusters from outside the main pipeline, like #1159. IClusterChangeListener works but is cumbersome. Once you had the cluster, most of the load balancing algorithms are trivial.

How about adding an interface like IClusterProvider and ProxyConfigManager could implement it by returning the clusters dictionary?

AFAIK ProxyConfigManager is internal, so is not easily accessible. Having a mechanism off the ProxyFeature to be able to get a read-only snapshot of the current clusters would be very useful. (hint)

It seems odd that we'd only do this for load balancing and not other proxy features like session affinity, passive health checks, retry, etc..
Note the concurrency counter is internal so you wouldn't be able to reliably track the load added by these extra requests.

I guess it would be nice to be able to leverage all YARP features outside the main pipeline. What do you think about adding this?

Task<HttpResponse> SendAsync(HttpRequest request, string cluster);

This method could first make sure that HttpRequest has an relative path so it could be added to the base address from destination. I think it could also handle session affinity, passive health checks and also load balancing.

As Karel said above, you can always use HttpClient to make a request to yourself, which would then be proxied to the appropriate destination - in a service meshy like way.

We are considering what it would look like for features like load balancing and health checks to exist independently from YARP, maybe as their own library or HttpClient layer, but we don't anticipate an existing model like YARP to directly consume those.

I understand, thanks for the explanation.

This is intended for service to service calls within a deployment. Rather than going through a service mesh, you'd configure a version of HttpClient with the equivalent of routes and clusters, and it would then be able to do that load balancing etc within the service, saving a hop and latency.

@samsp-msft
Copy link
Contributor

Re-opening for 1.x consideration. There are scenarios such as #1414 where there is a desire to be able to choose which cluster to route a request to. The other is the ability to resolve a cluster to a destination (or httpclient based on clusters config).

@samsp-msft samsp-msft reopened this Dec 8, 2021
@samsp-msft samsp-msft added the samsp_list Personal tag used when reviewing issues for further discussion label Dec 8, 2021
@samsp-msft samsp-msft added this to the Backlog milestone Feb 3, 2022
@samsp-msft
Copy link
Contributor

Some of this will be fixed by #126. Adding to backlog for external use of load balancing features.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
samsp_list Personal tag used when reviewing issues for further discussion Type: Idea This issue is a high-level idea for discussion.
Projects
None yet
Development

No branches or pull requests

4 participants