Skip to content

upstream: code clean up of upstream cluster creation#26438

Merged
wbpcode merged 2 commits intoenvoyproxy:mainfrom
wbpcode:dev-minor-opt-transport-factory
Apr 1, 2023
Merged

upstream: code clean up of upstream cluster creation#26438
wbpcode merged 2 commits intoenvoyproxy:mainfrom
wbpcode:dev-minor-opt-transport-factory

Conversation

@wbpcode
Copy link
Copy Markdown
Member

@wbpcode wbpcode commented Mar 29, 2023

Commit Message: upstream: code clean up of upstream cluster creation
Additional Description:

In the previous impl, we will create a stats Scope instance and a TransportSocketFactoryContext instance for every cluster in the ClusterFactoryImplBase::create call.

Both objects will be forwarded to createClusterImpl call, and then be forwarded to createClusterWithConfig call, and then be forwarded to construction method of cluster, and finally be forwarded to construction method of ClusterImplBase.
Both objects finally will be managed by the new created cluster instance.

However, this long forwarding path make it's hard to track the lifetime of these objects or to understand these code.
This patch make a change to create both objects in the construction method of ClusterImplBase directly.

Risk Level: Mid.
Testing: n/a.
Docs Changes: n/a.
Release Notes: n/a.
Platform Specific Features: n/a.

@wbpcode
Copy link
Copy Markdown
Member Author

wbpcode commented Mar 29, 2023

This PR introduced lots of code change but has no any new function. But I still think it make sense in the long term to improve the code readability/maintainability.

@wbpcode
Copy link
Copy Markdown
Member Author

wbpcode commented Mar 29, 2023

/retest

@repokitteh-read-only
Copy link
Copy Markdown

Retrying Azure Pipelines:
Check envoy-presubmit isn't fully completed, but will still attempt retrying.
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #26438 (comment) was created by @wbpcode.

see: more, trace.

Copy link
Copy Markdown
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Nice cleanup! Thank you.

@wbpcode wbpcode merged commit e665d42 into envoyproxy:main Apr 1, 2023
RiverPhillips pushed a commit to RiverPhillips/envoy that referenced this pull request Apr 7, 2023
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.

2 participants