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

cluster_manager: Make ClusterEntry a class instead of a struct #17616

Merged
merged 4 commits into from
Aug 10, 2021

Conversation

RyanTheOptimist
Copy link
Contributor

cluster_manager: Make
Upstream::ClusterManagerImpl::ThreadLocalClusterManagerImpl::ClusterEntry a class instead of a struct since it inherits from the class Upstream::ThreadLocalCluster. Made all members and two helper methods private. Private members required moving two small pieces of logic from ClusterManagerImpl::ThreadLocalClusterManagerImpl into helper methods of ClusterEntry, but the behavior is completely unchanged.

Also noticed that a method 'connPool' should have had an 'http' prefix to be parallel with the 'tcpConnPool' method. Finally, after doing this rename noticed that there were two methods named httpConnPool and two named tcpConnPool, but with different signatures. Renamed two of these to tcpConnPoolImpl and httpConnPoolImpl to make them easier to distinguish and make their intent clear.

Risk Level: Low - No behavior change
Testing: Existing tests because no behavior changed
Docs Changes: N/A
Release Notes: N/A
Platform Specific Features: N/A

Signed-off-by: Ryan Hamilton <rch@google.com>
Signed-off-by: Ryan Hamilton <rch@google.com>
Signed-off-by: Ryan Hamilton <rch@google.com>
Signed-off-by: Ryan Hamilton <rch@google.com>
@RyanTheOptimist
Copy link
Contributor Author

/assign @DavidSchinazi

Copy link
Contributor

@DavidSchinazi DavidSchinazi 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!

@RyanTheOptimist
Copy link
Contributor Author

/assign @alyssawilk

Copy link
Contributor

@alyssawilk alyssawilk left a comment

Choose a reason for hiding this comment

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

Nice clean up!

@alyssawilk alyssawilk merged commit b57827c into envoyproxy:main Aug 10, 2021
mpuncel added a commit to mpuncel/envoy that referenced this pull request Aug 11, 2021
* main: (687 commits)
  ci: set build debug information from env (envoyproxy#17635)
  ext_authz: do the authentication even the direct response is set (envoyproxy#17546)
  upstream: various cleanups in connection pool code (envoyproxy#17644)
  owners: promote Dmitry to maintainer (envoyproxy#17642)
  quiche: client session supports creating bidi stream (envoyproxy#17543)
  Update HTTP/2 METADATA documentation. (envoyproxy#17637)
  ext_proc: Check validity of the :status header (envoyproxy#17596)
  test: add ASSERT indicating that gRPC stream has not been started yet (envoyproxy#17614)
  ensure that the inline cookie header will be folded correctly  (envoyproxy#17560)
  cluster_manager: Make ClusterEntry a class instead of a struct (envoyproxy#17616)
  owners: make Raúl a Thrift senior extension maintainer (envoyproxy#17641)
  quiche: update QUICHE dependency (envoyproxy#17618)
  Delete mock for removed RouteEntry::perFilterConfig() method (envoyproxy#17623)
  REPO_LAYOUT.md: fix outdated link (envoyproxy#17626)
  hcm: forbid use of detection extensions with use_remote_addr/xff_num_trusted_hops (envoyproxy#17558)
  thrift proxy: add request shadowing support (envoyproxy#17544)
  ext_proc: Ensure that timer is always cancelled (envoyproxy#17569)
  Proposal: Add CachePolicy interface to allow for custom cache behavior (envoyproxy#17362)
  proto: fix verify to point at v3 only (envoyproxy#17622)
  api: move generic matcher proto to its own package (envoyproxy#17096)
  ...
mpuncel added a commit to mpuncel/envoy that referenced this pull request Aug 16, 2021
* main: (687 commits)
  ci: set build debug information from env (envoyproxy#17635)
  ext_authz: do the authentication even the direct response is set (envoyproxy#17546)
  upstream: various cleanups in connection pool code (envoyproxy#17644)
  owners: promote Dmitry to maintainer (envoyproxy#17642)
  quiche: client session supports creating bidi stream (envoyproxy#17543)
  Update HTTP/2 METADATA documentation. (envoyproxy#17637)
  ext_proc: Check validity of the :status header (envoyproxy#17596)
  test: add ASSERT indicating that gRPC stream has not been started yet (envoyproxy#17614)
  ensure that the inline cookie header will be folded correctly  (envoyproxy#17560)
  cluster_manager: Make ClusterEntry a class instead of a struct (envoyproxy#17616)
  owners: make Raúl a Thrift senior extension maintainer (envoyproxy#17641)
  quiche: update QUICHE dependency (envoyproxy#17618)
  Delete mock for removed RouteEntry::perFilterConfig() method (envoyproxy#17623)
  REPO_LAYOUT.md: fix outdated link (envoyproxy#17626)
  hcm: forbid use of detection extensions with use_remote_addr/xff_num_trusted_hops (envoyproxy#17558)
  thrift proxy: add request shadowing support (envoyproxy#17544)
  ext_proc: Ensure that timer is always cancelled (envoyproxy#17569)
  Proposal: Add CachePolicy interface to allow for custom cache behavior (envoyproxy#17362)
  proto: fix verify to point at v3 only (envoyproxy#17622)
  api: move generic matcher proto to its own package (envoyproxy#17096)
  ...

Signed-off-by: Michael Puncel <mpuncel@squareup.com>
leyao-daily pushed a commit to leyao-daily/envoy that referenced this pull request Sep 30, 2021
…proxy#17616)

cluster_manager: Make
Upstream::ClusterManagerImpl::ThreadLocalClusterManagerImpl::ClusterEntry a class instead of a struct since it inherits from the class Upstream::ThreadLocalCluster. Made all members and two helper methods private. Private members required moving two small pieces of logic from ClusterManagerImpl::ThreadLocalClusterManagerImpl into helper methods of ClusterEntry, but the behavior is completely unchanged.

Also noticed that a method 'connPool' should have had an 'http' prefix to be parallel with the 'tcpConnPool' method. Finally, after doing this rename noticed that there were two methods named httpConnPool and two named tcpConnPool, but with different signatures. Renamed two of these to tcpConnPoolImpl and httpConnPoolImpl to make them easier to distinguish and make their intent clear.

Risk Level: Low - No behavior change
Testing: Existing tests because no behavior changed
Docs Changes: N/A
Release Notes: N/A
Platform Specific Features: N/A

Signed-off-by: Ryan Hamilton <rch@google.com>
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