upstream: the cluster provoded load balancing policy could be configured as extension #27512
Conversation
…red as extension Signed-off-by: wbpcode <[email protected]>
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
|
After this PR, only the subset couldnot be configured as extension. |
|
/lgtm api |
| @@ -835,24 +835,35 @@ ClusterManagerImpl::loadCluster(const envoy::config::cluster::v3::Cluster& clust | |||
| auto& lb = new_cluster_pair_or_error->second; | |||
| Cluster& cluster_reference = *new_cluster; | |||
|
|
|||
| const auto cluster_info = cluster_reference.info(); | |||
There was a problem hiding this comment.
nit: const auto&. I assume that keeping a reference to the shared pointer is sufficient here.
There was a problem hiding this comment.
Yeah, you are complete right. Ref is enough here. But the info() will return a copy of the shared pointer rather than a ref.
So, it make no sense to use const auto& here, because we always create a copy of shared pointer. In the previous implementation, these code obviously create lots of unnecessary copies of shared pointer (by calling info() repeatedly), we reduced the copy to just once.
By the way, I think lots of interface design could be improved. For example, in the scenario to return a shared pointer member, lots of interface will return shared_ptr<T> directly, but not all callers alyways need the ownership. using const shared_ptr<T>& would be better. If we have some free time, I think we can re-evaluate these interfaces and create a clear specifications to guide these code.
test/extensions/load_balancing_policies/cluster_provided/integration_test.cc
Outdated
Show resolved
Hide resolved
test/extensions/load_balancing_policies/cluster_provided/integration_test.cc
Show resolved
Hide resolved
Signed-off-by: wbpcode <[email protected]>
…luster-provider-lb
|
friendly ping @adisuissa |
|
@envoyproxy/senior-maintainers assignee is @zuercher |
…red as extension (envoyproxy#27512) * upstream: the cluster provoded load balancing policy could be configured as extension Signed-off-by: wbpcode <[email protected]> * address comments Signed-off-by: wbpcode <[email protected]> --------- Signed-off-by: wbpcode <[email protected]> Signed-off-by: Ryan Eskin <[email protected]>
Commit Message: upstream: the cluster provoded load balancing policy could be configured as extension
Additional Description:
Part of #20634.
Risk Level: low.
Testing: unit/integration.
Docs Changes: n/a.
Release Notes: added.
Platform Specific Features: n/a.