http: moving functions and member variables out of http1 pool #13867
Conversation
…ool. Signed-off-by: Alyssa Wilk <[email protected]>
alyssawilk
force-pushed
the
http_changes
branch
from
903e272 to
3ae4157
Compare
Signed-off-by: Alyssa Wilk <[email protected]>
|
@ggreenway with Snow out can you do first pass? |
| Upstream::Host::CreateConnectionData data = parent_.host()->createConnection( | ||
| parent_.dispatcher(), parent_.socketOptions(), parent_.transportSocketOptions()); | ||
| Upstream::Host::CreateConnectionData data = | ||
| static_cast<Envoy::ConnectionPool::ConnPoolImplBase*>(&parent)->host()->createConnection( |
There was a problem hiding this comment.
What's going on with this cast? Isn't this an upcast, and thus would happen implicitly?
There was a problem hiding this comment.
Annoyingly,
HttpConnPoolImplBase has
Upstream::HostDescriptionConstSharedPtr host() const override { return host_; }
and the base class has
const Upstream::HostConstSharedPtr& host() const { return host_; }
so without the cast it snags the wrong type and complains there's no createConnection function.
There was a problem hiding this comment.
Yuck. Can you add a comment to that effect?
There was a problem hiding this comment.
yeah - will add it to my clean up list too - may be able to do away with it with one of the later refactors!
| Upstream::Host::CreateConnectionData data = parent_.host()->createConnection( | ||
| parent_.dispatcher(), parent_.socketOptions(), parent_.transportSocketOptions()); | ||
| Upstream::Host::CreateConnectionData data = | ||
| static_cast<Envoy::ConnectionPool::ConnPoolImplBase*>(&parent)->host()->createConnection( |
There was a problem hiding this comment.
Annoyingly,
HttpConnPoolImplBase has
Upstream::HostDescriptionConstSharedPtr host() const override { return host_; }
and the base class has
const Upstream::HostConstSharedPtr& host() const { return host_; }
so without the cast it snags the wrong type and complains there's no createConnection function.
Signed-off-by: Alyssa Wilk <[email protected]>
As part of #3431 we need to move the logic from HTTP/1 and HTTP/2 connection pools to the active client, so the new mixed connection pool can just create an active client of the right type and have all the logic in the client, not split between the client and the pool. This removes functions for the HTTP/2 pool not moved in #13867 Risk Level: Medium (data plane refactor) Testing: existing tests pass Docs Changes: n/a Release Notes: n/a Part of #3431 Signed-off-by: Alyssa Wilk <[email protected]>
Signed-off-by: Alyssa Wilk <[email protected]>
As part of #3431 we need to move the logic from HTTP/1 and HTTP/2 connection pools to the active client,
so the new mixed connection pool can just create an active client of the right type.
This removes member variables from the HTTP/1.1 connection pool in preparation for that move.
a follow-up PR will do the same for HTTP/2, then the two classes will be removed in favor of the base http connection
pool being created with an instantiate_active_client_ closure which creates the right type of clients.
The alpn pool will eventually create an active client of the right type based on initial ALPN.
Risk Level: High (data plane refactor, albeit intended no-op)
Testing: existing tests pass
Docs Changes: n/a
Release Notes: n/a