lb: unified the lifetime mechanism of worker load lb 2#45170
Conversation
Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com>
|
/retest |
|
/gemini review |
There was a problem hiding this comment.
Pull request overview
This PR aligns several thread-aware/cluster-provided load balancers with the “refresh in place” lifetime model by disabling worker-local LB recreation on host-set changes and ensuring worker-local LBs update their cached view via priority-set member update callbacks.
Changes:
- Set
recreateOnHostChange()tofalsefor several LB factories so worker-local LBs are not recreated on host updates. - Update Redis Cluster and Original DST worker-local LBs to refresh cached topology/host maps via
PrioritySet::addMemberUpdateCb. - Add unit tests validating that existing worker-local LB instances observe updates after member update callbacks fire.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/extensions/clusters/redis/redis_cluster_lb_test.cc | Adds a test ensuring a worker-local Redis LB refreshes its snapshot on member updates (no LB recreation). |
| test/extensions/clusters/original_dst/original_dst_cluster_test.cc | Updates tests for new OriginalDstCluster::LoadBalancer constructor and adds a refresh-on-member-update test. |
| source/extensions/load_balancing_policies/override_host/load_balancer.h | Disables LB recreation on host changes for the override-host LB factory. |
| source/extensions/clusters/reverse_connection/reverse_connection.h | Disables LB recreation on host changes for reverse-connection cluster LB factory. |
| source/extensions/clusters/redis/redis_cluster_lb.h | Refactors Redis worker-local LB to refresh from factory and registers a member-update callback; disables recreation. |
| source/extensions/clusters/redis/redis_cluster_lb.cc | Implements Redis worker-local LB refresh logic and constructs it with priority_set. |
| source/extensions/clusters/original_dst/original_dst_cluster.h | Makes Original DST worker-local LB refresh its cached host map via member-update callback; disables recreation. |
| source/extensions/clusters/dynamic_modules/cluster.cc | Disables LB recreation on host changes for dynamic-modules cluster LB factory. |
| source/extensions/clusters/dynamic_forward_proxy/cluster.h | Disables LB recreation on host changes for dynamic forward proxy cluster LB factory. |
| source/extensions/clusters/composite/cluster.h | Disables LB recreation on host changes for composite cluster LB factory. |
| source/extensions/clusters/aggregate/cluster.h | Disables LB recreation on host changes for aggregate cluster LB factory. |
There was a problem hiding this comment.
Code Review
This pull request updates multiple load balancer implementations to return false for recreateOnHostChange, optimizing performance by avoiding unnecessary re-creations. Specifically, OriginalDstCluster and RedisClusterLoadBalancer now utilize member update callbacks to refresh their internal state. The changes include updates to several cluster extensions and corresponding unit tests. Feedback was provided to improve the safety of the RedisClusterLoadBalancer constructor by avoiding a fragile initialization sequence involving std::move.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: code <wbphub@gmail.com>
|
/retest |
…voy into dev-simplify-lb
Commit Message: lb: unified the lifetime mechanism of worker load lb 2
Additional Description:
Similar to previous #45100 but this PR have updated all left LB.
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]