Skip to content

grid: clean up return value #15950

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.

Sign up for GitHub

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

Merged
alyssawilk merged 2 commits into from
Apr 14, 2021
Merged

grid: clean up return value #15950

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
651712e
grid: clean up return value
alyssawilk Apr 13, 2021
ba75582
unfix comment =P
alyssawilk Apr 13, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions source/common/http/conn_pool_grid.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,14 @@ ConnectivityGrid::WrapperCallbacks::ConnectionAttemptCallbacks::ConnectionAttemp
WrapperCallbacks& parent, PoolIterator it)
: parent_(parent), pool_it_(it), cancellable_(nullptr) {}

bool ConnectivityGrid::WrapperCallbacks::ConnectionAttemptCallbacks::newStream() {
ConnectivityGrid::StreamCreationResult
ConnectivityGrid::WrapperCallbacks::ConnectionAttemptCallbacks::newStream() {
auto* cancellable = pool().newStream(parent_.decoder_, *this);
if (cancellable == nullptr) {
return true;
return StreamCreationResult::ImmediateResult;
}
cancellable_ = cancellable;
return false;
return StreamCreationResult::StreamCreationPending;
}

void ConnectivityGrid::WrapperCallbacks::ConnectionAttemptCallbacks::onPoolFailure(
Expand Down Expand Up @@ -82,7 +83,7 @@ void ConnectivityGrid::WrapperCallbacks::deleteThis() {
removeFromList(grid_.wrapped_callbacks_);
}

bool ConnectivityGrid::WrapperCallbacks::newStream() {
ConnectivityGrid::StreamCreationResult ConnectivityGrid::WrapperCallbacks::newStream() {
ENVOY_LOG(trace, "{} pool attempting to create a new stream to host '{}'.",
describePool(**current_), grid_.host_->hostname());
auto attempt = std::make_unique<ConnectionAttemptCallbacks>(*this, current_);
Expand Down Expand Up @@ -137,7 +138,10 @@ bool ConnectivityGrid::WrapperCallbacks::tryAnotherConnection() {
// If there are no other pools to try, return false.
return false;
}
// Create a new connection attempt for the next pool.
// Create a new connection attempt for the next pool. If we reach this point
// return true regardless of if newStream resulted in an immediate result or
// an async call, as either way the attempt will result in success/failure
// callbacks.
current_ = next_pool.value();
newStream();
return true;
Expand Down Expand Up @@ -209,7 +213,7 @@ ConnectionPool::Cancellable* ConnectivityGrid::newStream(Http::ResponseDecoder&
ConnectionPool::Cancellable* ret = wrapped_callback.get();
LinkedList::moveIntoList(std::move(wrapped_callback), wrapped_callbacks_);
// Note that in the case of immediate attempt/failure, newStream will delete this.
if (wrapped_callbacks_.front()->newStream()) {
if (wrapped_callbacks_.front()->newStream() == StreamCreationResult::ImmediateResult) {
// If newStream succeeds, return nullptr as the caller has received their
// callback and does not need a cancellable handle.
return nullptr;
Expand Down
13 changes: 8 additions & 5 deletions source/common/http/conn_pool_grid.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ class ConnectivityGrid : public ConnectionPool::Instance,
std::vector<Http::Protocol> protocols_;
};

enum class StreamCreationResult {
Copy link
Copy Markdown
Contributor

@RyanTheOptimist RyanTheOptimist Apr 13, 2021

Choose a reason for hiding this comment

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

I'm slightly surprised this doesn't exist at the general connection pool layer. But this is super great in any case!

ImmediateResult,
StreamCreationPending,
};

using PoolIterator = std::list<ConnectionPool::InstancePtr>::iterator;

// This is a class which wraps a caller's connection pool callbacks to
Expand All @@ -38,8 +43,7 @@ class ConnectivityGrid : public ConnectionPool::Instance,
public:
ConnectionAttemptCallbacks(WrapperCallbacks& parent, PoolIterator it);

// Returns true if a stream is immediately created, false if it is pending.
bool newStream();
StreamCreationResult newStream();

// ConnectionPool::Callbacks
void onPoolFailure(ConnectionPool::PoolFailureReason reason,
Expand Down Expand Up @@ -67,9 +71,8 @@ class ConnectivityGrid : public ConnectionPool::Instance,
// ConnectionPool::Cancellable
void cancel(Envoy::ConnectionPool::CancelPolicy cancel_policy) override;

// Attempt to create a new stream for pool(). Returns true if the stream has
// been created.
bool newStream();
// Attempt to create a new stream for pool().
StreamCreationResult newStream();

// Removes this from the owning list, deleting it.
void deleteThis();
Expand Down