load_aware_locality: add out-of-band ORCA reporting - #46670
Conversation
Signed-off-by: jukie <10012479+jukie@users.noreply.github.com>
Signed-off-by: jukie <10012479+jukie@users.noreply.github.com>
jukie
changed the title
|
Continued discussion question from #45725 (comment) @paul-r-gall :
Yes, if both this policy and it's endpoint_picking_policy enable OOB each would open their own stream per host. I think documentation around this behavior is adequate vs the complexity of trying to dedupe but if we want that in the future, we could consider adding OOB fields at the cluster scope which would make things easier by having a clear owner. |
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
| @@ -115,8 +116,8 @@ void OrcaOobManager::onHostsAdded(const Upstream::HostVector& hosts) { | |||
| const std::chrono::milliseconds initial_delay(random_.random() % period_ms); | |||
| it->second = std::make_unique<OobSession>(*this, host, initial_delay); | |||
| } | |||
| if (oob_sessions_.size() != prior_size) { | |||
| oob_stats_.active_sessions_.set(oob_sessions_.size()); | |||
| if (oob_sessions_.size() > prior_size) { | |||
| oob_stats_.active_sessions_.add(oob_sessions_.size() - prior_size); | |||
| } | |||
| } | |||
|
|
|||
| @@ -131,8 +132,8 @@ void OrcaOobManager::onHostsRemoved(const Upstream::HostVector& hosts) { | |||
| dispatcher_.deferredDelete(std::move(it->second)); | |||
| oob_sessions_.erase(it); | |||
| } | |||
| if (oob_sessions_.size() != prior_size) { | |||
| oob_stats_.active_sessions_.set(oob_sessions_.size()); | |||
| if (oob_sessions_.size() < prior_size) { | |||
| oob_stats_.active_sessions_.sub(prior_size - oob_sessions_.size()); | |||
| } | |||
| } | |||
|
|
|||
| @@ -141,7 +142,7 @@ void OrcaOobManager::onSessionTerminated(OobSession* session) { | |||
| ASSERT(it != oob_sessions_.end() && it->second.get() == session); | |||
| dispatcher_.deferredDelete(std::move(it->second)); | |||
| oob_sessions_.erase(it); | |||
| oob_stats_.active_sessions_.set(oob_sessions_.size()); | |||
| oob_stats_.active_sessions_.dec(); | |||
There was a problem hiding this comment.
With the previous set(oob_sessions_.size()), each manager overwrote the total with only its own session count, and whichever destructor ran last zeroed the other's live sessions. Not much of an issue on current main but now that OOB is available at both locality and endpoint level, that would actually trigger so this shifts to an increment/decrement logic and I added a test to validate.
|
/retest |
|
/gemini review |
2 participants
Commit Message: implement out-of-band ORCA reporting for load_aware_locality
Additional Description: Follow-up to #45725 to wire up ORCA out-of-band reporting with the additional below changes:
wiptoalphain extensions_metadata.yaml.lb_orca_oob.active_sessionsaccounting in the shared OrcaOobManagerRisk Level: Low - still an alpha marked policy and opt-in only
Testing: Added end-to-end OOB integration test and other coverage
Docs Changes: Yes
Release Notes: Updated existing (still unreleased)
Platform Specific Features: N/A
xRef #43665
xRef #6614