on-demand, upstream: on-demand cluster discovery #15523

krnowak · 2021-03-16T17:15:11Z

I'm filing an work-in-progress PR for the feature to gather some initial feedback on the code. Right now I'm working on writing tests.

Commit Message:
This PR adds an on-demand cluster discovery feature to Envoy. The process of the discovery is driven through the on-demand filter. The on-demand filter is updated to also use a per-route configuration to enable ODCDS.

Additional Description:
There is a design document for the feature - https://docs.google.com/document/d/1AqhQnrX_7SS6PAkNwoaA2peCtk9htgwSBTRg1gzFPNw/edit#

I haven't yet implemented checking if CDS is using delta, as ODCDS should not be used together with SotW CDS.

Risk Level:
Medium (new feature)

Testing:
Currently writing tests, will also do some manual testing with cluster_header in RouteAction.

Docs Changes:
TODO (should be both API docs and some usage documentation in on-demand filter, I think).

Release Notes:
New feature: on_demand: On demand discovery of the clusters.

Platform Specific Features:
N/A

[Optional Runtime guard:]

[Optional Fixes #Issue]
Partially addresses #2500 (the clusters part).

[Optional Deprecated:]

[Optional API Considerations:]

repokitteh-read-only · 2021-03-16T17:15:14Z

Hi @krnowak, welcome and thank you for your contribution.

We will try to review your Pull Request as quickly as possible.

In the meantime, please take a look at the contribution guidelines if you have not done so already.

🐱

Caused by: #15523 was opened by krnowak.

see: more, trace.

repokitteh-read-only · 2021-03-16T17:15:19Z

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/envoy/.
API shepherd assignee is @lizan
CC @envoyproxy/api-watchers: FYI only for changes made to api/envoy/.

🐱

Caused by: #15523 was opened by krnowak.

see: more, trace.

mattklein123

Thanks, at a very high level this looks like it's directionally correct. Please let me know when this is ready for real review (comments, tests, etc.).

/wait

api/envoy/extensions/filters/http/on_demand/v3/on_demand.proto

dmitri-d · 2021-03-16T22:04:08Z

source/common/upstream/cluster_manager_impl.cc

+
+  cluster_manager.cdm_.ensureCallbacksAreInstalled();
+
+  auto [handle, discovery_in_progress] = cluster_manager.cdm_.addCallback(name, weak_callback);


This looks like it will miss discovery requests for name initiated in other threads, is this intentional? Could you add a comment then?

Will add more comments, sure.

Note that addCallback will add a callback anyway. But if the discovery for name was already initiated in this thread, then we don't need to do anything else, just add the callback and wait for it to be fired. If the discovery for name was initiated in some other worker thread, then we have no way of knowing that in this worker thread, so we try to initiate the discovery anyway. Whether the discovery will actually be initiated is something that main thread will figure out (see the code where we check pending_cluster_creations_ in main thread).

dmitri-d · 2021-03-16T23:05:26Z

source/extensions/filters/http/on_demand/on_demand_update.cc

+    ProtobufMessage::ValidationVisitor& validation_visitor)
+    : cm_(cm) {
+  if (proto_config.has_odcds_config()) {
+    odcds_ =


This creates an instance of CDS api/subscription in a worker thread, which is different from the rest of xDS that runs on the main thread. I think this is an issue -- there are concurrency (definitely with ADS configured) and synchronisation issues here. I probably missed it -- where is the CDS subscription started?

This is a good question. I'm still in progress of writing the tests, so this will come out soon. But yeah, looks like creating CDS in worker thread calls into cluster manager directly, so looks like I either would need to make it thread safe, or postpone the creation of CDS until I'm in main thread.

Basically I didn't want to start the subscription, because it will result in periodically requesting the management server, which is not what I wanted. I'll have a look what VHDS is doing here.

We did a bit of tracking where this is called from and I have an impression that both createRouteSpecificFilterConfigTyped and createFilterFactoryFromProtoTyped are called from the main thread. And inside those functions we create the CDS, so it ought to be fine. Maybe the factories that those functions return are called from worker threads, aren't they?

Basically I didn't want to start the subscription, because it will result in periodically requesting the management server, which is not what I wanted. I'll have a look what VHDS is doing here.

A subscription is basically a type-specific (i.e. CDS, RDS, VHDS, etc) grpc connection to a management server; in case of ADS all subscriptions are multiplexed over a single grpc connection. The connection needs to be initiated (subscription started) in order to be able to both send and receive xDS updates.

Note that VHDS establishes a separate subscription in addition to the base RDS one and calculates a common state (based on RDS state and VHDS updates). I encourage you to take a closer look at various xDS implementations, as well as muxes. Please also take a look at the docs I linked to earlier.

dmitri-d · 2021-03-17T01:17:21Z

Would it be possible for you to write a brief explanation re: how you plan to handle configuration and starting of on-demand CDS subscriptions? Did you have a chance to read https://blog.envoyproxy.io/envoy-threading-model-a8d44b922310?

markdroth · 2021-03-17T23:42:06Z

api/envoy/extensions/filters/http/on_demand/v3/on_demand.proto

 message OnDemand {
  option (udpa.annotations.versioning).previous_message_type =
      "envoy.config.filter.http.on_demand.v2.OnDemand";
+
+  config.core.v3.ConfigSource odcds_config = 1;


If the only knob here is the presence of a ConfigSource, then how will we configure ODCDS when we're using new-style resource names and don't need a ConfigSource because we're using authority-based routing?

I think the presence of an OCDS PerRouteConfig would indicate we needed to treat the name as on-demand. The meaning of self becomes interesting as it might mean two different things for the filter-level config (received from LDS) and the PerRouteConfig (from RDS).

krnowak · 2021-03-25T20:24:34Z

Hi, updated the PR. I still have a hitch with the integration test failing. Not sure if this is something wrong with my changes in cluster manager or the test itself is wrong. I have asked on slack on how to enable logs during running the integration tests - this hopefully would help me with debugging the issue.

To answer some questions:

@dmitri-d: I wrote a separate class instead of using CdsApi{,Impl} - it shares most of the functionality with CdsApiImpl, but it starts the subscription on first use instead of up-front.

@markdroth: Unfortunately I'm not very well oriented in matters of new-style cluster naming and authority-based routing. I was pointed to the document on the last community meeting, so I'll try to get through it soon.

krnowak · 2021-03-25T23:55:09Z

Got a bit further (it was a new-cluster vs new_cluster mixup). Now the test seems to crash on teardown:

 Caught Segmentation fault, suspect faulting address 0x0
 Backtrace (use tools/stack_decode.py to get line numbers):
 Envoy version: 0/1.18.0-dev/test/DEBUG/BoringSSL
 #0: Envoy::SignalAction::sigHandler() [0x4bc4dac]
 #1: __restore_rt [0x7f0e7a1f4980]
 #2: Envoy::Config::NewGrpcMuxImpl::trySendDiscoveryRequests() [0x44e7cbf]
 #3: Envoy::Config::NewGrpcMuxImpl::updateWatch() [0x44ea82b]
 #4: Envoy::Config::NewGrpcMuxImpl::removeWatch() [0x44eaed4]
 #5: Envoy::Config::NewGrpcMuxImpl::WatchImpl::remove() [0x44f859d]
 #6: Envoy::Config::NewGrpcMuxImpl::WatchImpl::~WatchImpl() [0x44f848a]
 #7: Envoy::Config::NewGrpcMuxImpl::WatchImpl::~WatchImpl() [0x44f850c]
 …
 #10: std::__1::unique_ptr<>::~unique_ptr() [0x2e18149]
 #11: Envoy::Config::GrpcSubscriptionImpl::~GrpcSubscriptionImpl() [0x44c1a55]
 #12: Envoy::Config::GrpcSubscriptionImpl::~GrpcSubscriptionImpl() [0x44c1abc]
 …
 #15: std::__1::unique_ptr<>::~unique_ptr() [0x2dfc229]
 #16: Envoy::Upstream::OdCdsApiImpl::~OdCdsApiImpl() [0x404f8e9]
 #17: Envoy::Upstream::OdCdsApiImpl::~OdCdsApiImpl() [0x404f93c]
 …
 #20: std::__1::__shared_weak_count::__release_shared() [0x60ccd8e]
 #21: Envoy::Extensions::HttpFilters::OnDemand::OnDemandFilterConfig::~OnDemandFilterConfig() [0x4035ed5]
 …
 #34: absl::node_hash_map<>::~node_hash_map() [0x4596fb8]
 #35: Envoy::Router::PerFilterConfigs::~PerFilterConfigs() [0x4592ed5]
 #36: Envoy::Router::VirtualHostImpl::~VirtualHostImpl() [0x45993db]
 #37: Envoy::Router::VirtualHostImpl::~VirtualHostImpl() [0x459949c]
 …
 #40: std::__1::__shared_weak_count::__release_shared() [0x60ccd8e]
 #41: Envoy::Router::RouteMatcher::~RouteMatcher() [0x41bb7f6]
 …
 #44: std::__1::unique_ptr<>::~unique_ptr() [0x41bb6b9]
 #45: Envoy::Router::ConfigImpl::~ConfigImpl() [0x41bb68d]
 #46: Envoy::Router::ConfigImpl::~ConfigImpl() [0x45995ac]
 …
 #49: std::__1::__shared_weak_count::__release_shared() [0x60ccd8e]
 #50: Envoy::Router::StaticRouteConfigProviderImpl::~StaticRouteConfigProviderImpl() [0x41c8cc6]
 #51: Envoy::Router::StaticRouteConfigProviderImpl::~StaticRouteConfigProviderImpl() [0x41c8d3c]
 …
 #54: std::__1::__shared_weak_count::__release_shared() [0x60ccd8e]
 #55: Envoy::Extensions::NetworkFilters::HttpConnectionManager::HttpConnectionManagerConfig::~HttpConnectionManagerConfig() [0x4077847]
 #56: std::__1::__shared_ptr_emplace<>::__on_zero_shared() [0x40871d6]
 #57: std::__1::__shared_weak_count::__release_shared() [0x60ccd8e]
 #58: Envoy::Extensions::NetworkFilters::HttpConnectionManager::HttpConnectionManagerFilterConfigFactory::createFilterFactoryFromProtoTyped()::$_6::~createFilterFactoryFromProtoTyped() [0x4060516]
 …

I tried deferring the destruction to main thread (just to be sure), but it crashes at the same place too. Will investigate.

krnowak · 2021-03-26T23:58:25Z

Fixed the crash (it was caused by bad member ordering in OdCdsApiImpl). Should be ready for a review.

We plan to add an OdCdsApiImpl class next to CdsApiImpl. It's config updated callback functionality would be similar to CdsApiImpl's, but with some extra code. To avoid the duplication of code, we move it to a helper class that will be used by both OdCdsApiImpl and CdsApiImpl. Signed-off-by: Krzesimir Nowak <[email protected]>

Each thread using this functionality will need to instantiate their own OdCdsApi instance and pass it together with a callback to ClusterManager's requestOnDemandClusterDiscovery. Cluster manager will use passed OdCdsApi to perform the discovery. When the discovery is finished, ClusterManager invokes the callbacks in the thread context. OdCdsApiImpl starts the subscription on the first update request. The subsequent request uses the on-demand update functionality of the subscription. That way, subscriptions are only started when actually needed. Signed-off-by: Krzesimir Nowak <[email protected]>

This extends the extension to start the on-demand cluster discovery in case of a missing cluster if such discovery is enabled by specifying the config source for the ODCDS in filter's configuration. The filter also started supporting per-route configuration to allow configuring ODCDS for a specific virtual host or route. Signed-off-by: Krzesimir Nowak <[email protected]>

This sidesteps some of the corner cases in the cluster discovery, and matches the expectations of the ODCDS lifetime. Signed-off-by: Krzesimir Nowak <[email protected]>

This greatly simplifies ODCDS since it does not need to track the discovered names any more. Currently the timeout is set arbitrarily to 5 seconds. Signed-off-by: Krzesimir Nowak <[email protected]>

This is to address an issue raised during a review of the CdsApiHelper refactor. Signed-off-by: Krzesimir Nowak <[email protected]>

Signed-off-by: Krzesimir Nowak <[email protected]>

krnowak · 2021-04-01T20:17:41Z

Now that #15806 is merged, do you expect me to merge the main branch into the PR branch and fix the conflicts?

mattklein123 · 2021-04-01T20:36:36Z

Now that #15806 is merged, do you expect me to merge the main branch into the PR branch and fix the conflicts?

Yes. I don't know how else we would proceed. :)

krnowak · 2021-04-01T20:47:29Z

Now that #15806 is merged, do you expect me to merge the main branch into the PR branch and fix the conflicts?

Yes. I don't know how else we would proceed. :)

Heh, right. Projects I was working on so far allowed rebasing, so I just wanted to be sure. :)

Anyway, I updated the PR. I think I have addressed most (or all) of the initial review issues.

htuch · 2021-04-02T13:50:20Z

source/common/upstream/odcds_api_impl.cc

@@ -0,0 +1,85 @@
+#include "common/upstream/odcds_api_impl.h"


Sorry, I'm a bit late to the party here, but why do we have a different CDS implementation for on-demand vs. regular CDS? In all the other on-demand models (VHDS, SRDS), we reuse the existing machinery and just making it on-demand aware. Here we have diverged CDS implementation. This will probably have implications down the track when we make bug fixes for regular delta CDS but miss out on on OCDS and vice versa.

I would like to see better awareness of on-demand as a horizontal in Envoy that spans across subsystems, rather than doing what is expedient or local to CDS for OCDS here.

CC @mattklein123 @adisuissa @dmitri-d

CC @mattklein123 @adisuissa @dmitri-d

As I said offline, I haven't actually reviewed the code in detail yet, just the interfaces. I will review in more detail today. My expectation is most/all of the code is shared. I'm not sure it is possible to implement on-demand in main CDS due to the SoTW wildcard semantics, but we can see (at a certain point though it may just mean moving the same code split to a different location).

Sorry, I'm a bit late to the party here, but why do we have a different CDS implementation for on-demand vs. regular CDS?

There is not much of the code difference really - most of the code is shared through the CdsApiHelper. The differences between CDS and ODCDS are mostly about when to start subscription (for CDS is sometime at Envoy startup, for ODCDS is on the first request). I suppose I could make CDS on-demand aware, but I'm afraid that would result in awkward code inside it.

In all the other on-demand models (VHDS, SRDS), we reuse the existing machinery and just making it on-demand aware. Here we have diverged CDS implementation. This will probably have implications down the track when we make bug fixes for regular delta CDS but miss out on on OCDS and vice versa.

I tried to move most of the code into the helper class so such bug fixes would most certainly land in a helper class, not in CDS/ODCDS itself.

I would like to see better awareness of on-demand as a horizontal in Envoy that spans across subsystems, rather than doing what is expedient or local to CDS for OCDS here.

CC @mattklein123 @adisuissa @dmitri-d

htuch · 2021-04-02T13:52:59Z

source/common/upstream/cluster_manager_impl.cc

+    // this name, so nothing more left to do here.
+    return std::move(handle);
+  }
+  dispatcher_.post([this, odcds = std::move(odcds), weak_callback, name,


As per my other comment, I think this is tricky cross-thread lifetime related code for on-demand that has already been addressed in both VHDS and SRDS. Re-inventing these patterns here is problematic, we are building guaranteed technical debt.

Sorry to be a downer, I appreciate the work being done here, but I know from experience how hard it was to get things like on-demand VHDS right, we need to have common patterns for deferral and resumption across the on-demand xDS.

CC @mattklein123 @adisuissa @dmitri-d

Yes I agree if we can share this out that would be good.

I had a look and what VHDS is doing and I can see the differences:

I'm "deduplicating" the requests, so 2 worker threads asking for the cluster "foo" will result in one request instead of two.

I'm installing a timeout on the discovery, so the callbacks list won't grow indefinitely in case of cluster "foo" nonexistence.

I guess I can simplify the code, but maybe let's discuss it in the other PR, where I split out the CM changes.

mattklein123

Hi, thanks for the updates. This PR is still too large. For the next set of changes can we:

Split out only the cluster manager changes and tests around those changes.
Save the on-demand filter changes for the next PR.

I think if we do (1) we can more easily evaluate @htuch concerns as well as my high level interface concerns. Thank you!

/wait

mattklein123 · 2021-04-02T18:20:12Z

include/envoy/upstream/cluster_manager.h

+/**
+ * Abstract interface for a On-Demand CDS API provider.
+ */
+class OdCdsApi {
+public:
+  virtual ~OdCdsApi() = default;
+
+  /**
+   * File an on-demand request for a cluster.
+   */
+  virtual void updateOnDemand(const std::string& cluster_name) PURE;
+};
+
+using OdCdsApiPtr = std::unique_ptr<OdCdsApi>;


Somewhat related to @htuch comments, but why is this public interface needed? Conceptually, something asks the cluster manager to fetch a cluster, and gets a callback when complete or failed. I don't think any of this should be exposed "externally" and should be an internal implementation detail.

Since the config source for the on-demand discovery is provided in the per-route config, which makes it a separate object than CM has from cds_config, I need to pass a "CDS" object to CM. It needs to be a public interface, because it's being used in ClusterManager interface. I had two options here: either extend the CdsApi interface with the updateOnDemand function and pass that to CM or create a new interface (OdCdsApi). I went with the latter, because I found the former to be awkward. It's because CdsApi provides initialize, setInitializedCb and versionInfo and for the on-demand discovery I would use none of it.

mattklein123 · 2021-04-02T18:20:50Z

source/common/upstream/cluster_manager_impl.cc

+    // this name, so nothing more left to do here.
+    return std::move(handle);
+  }
+  dispatcher_.post([this, odcds = std::move(odcds), weak_callback, name,


Yes I agree if we can share this out that would be good.

markdroth · 2021-04-05T15:48:43Z

I'd like a bit more info on exactly which problem we're trying to solve here. As I mentioned in this comment thread in the design doc, I think there are two closely related but distinct problems in this space, and it's not clear to me what our strategy is for each of the two. I'm concerned that we may be conflating them here in a way that may limit flexibility.

krnowak · 2021-04-06T20:05:18Z

Hi,

Hi, thanks for the updates. This PR is still too large. For the next set of changes can we:
1. Split out only the cluster manager changes and tests around those changes.

Please see #15857.

2. Save the on-demand filter changes for the next PR.

I suppose I could reuse this PR to add the on-demand filter changes after #15857 settles.

I think if we do (1) we can more easily evaluate @htuch concerns as well as my high level interface concerns. Thank you!

/wait

krnowak · 2021-04-06T20:10:53Z

I'd like a bit more info on exactly which problem we're trying to solve here. As I mentioned in this comment thread in the design doc, I think there are two closely related but distinct problems in this space, and it's not clear to me what our strategy is for each of the two. I'm concerned that we may be conflating them here in a way that may limit flexibility.

What I'm trying to solve here is to enable (or disable) on-demand cluster discovery on connection manager or virtual host or route level, especially together with cluster_header route action.

What I'm not covering in this PR is the case where I statically specify a cluster name for some route and expect it to be discovered at a time of request handling instead of at Envoy startup.

markdroth · 2021-04-06T21:53:39Z

@krnowak Sorry, I don't quite follow. If this will not work for routes with static cluster names, then isn't it only for the cluster_header route action? The only three options for any route action are a cluster name, a WeightedClusters list (which is basically just a list of static cluster names), and cluster_header.

krnowak · 2021-04-07T07:44:52Z

@krnowak Sorry, I don't quite follow. If this will not work for routes with static cluster names, then isn't it only for the cluster_header route action? The only three options for any route action are a cluster name, a WeightedClusters list (which is basically just a list of static cluster names), and cluster_header.

I'd imagine that "static" cluster names (or weighted clusters) would work if they were provided by RDS or VHDS, for example. But honestly, I haven't checked that yet.

I think the reason I have put a boolean flag or a config source inside the route in my first version of the code was to have an access to them when checking the static clusters here - I could basically disable the check and discover the static cluster at a request time. But now that this setting was moved to the extension config I had no clue how to check if the route has the on-demand discovery enabled from that place.

github-actions · 2021-05-07T08:01:12Z

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

github-actions · 2021-05-14T08:07:53Z

This pull request has been automatically closed because it has not had activity in the last 37 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

krnowak · 2021-05-18T06:13:54Z

I'll open a new one with the extension changes after we settle #15857.

This enables a client to send a special resource named * to a server as a way to express an interest in a wildcard subscription. That way the wildcard subscription can coexist with subscriptions to other resources on the same stream. The wildcard resource, like any other resource, can be subscribed to and unsubscribed from at any time. The legacy way of enabling wildcard subscription (by sending an empty initial message) is still supported, but otherwise behaves just like a subscription to *. This behavior is gated behind the enabled-by-default "envoy.restart_features.explicit_wildcard_resource" runtime guard. Risk Level: High, changes in wildcard mode. Testing: As a part of #15523. Also done internally. Docs Changes: Updated version history. Release Notes: xds: implemented the wildcard resource, which can be used by xDS to express an interest in wildcard subscription in a way that allows subscriptions to other resources coexist on the same stream instead of being ignored. This means that the resource name * is reserved. See :ref:wildcard mode description <xds_protocol_resource_hints> and :ref:unsubscribing from resources <xds_protocol_unsubscribing> for details. This behavior is gated behind the envoy.restart_features.explicit_wildcard_resource runtime guard. Platform Specific Features: N/A Runtime guard: envoy.restart_features.explicit_wildcard_resource enabled by default. Signed-off-by: Krzesimir Nowak <[email protected]>

krnowak requested review from htuch and lambdai as code owners March 16, 2021 17:15

repokitteh-read-only bot added the api label Mar 16, 2021

repokitteh-read-only bot assigned lizan Mar 16, 2021

mattklein123 self-assigned this Mar 16, 2021

mattklein123 requested changes Mar 16, 2021

View reviewed changes

api/envoy/extensions/filters/http/on_demand/v3/on_demand.proto Show resolved Hide resolved

repokitteh-read-only bot added the waiting label Mar 16, 2021

dmitri-d reviewed Mar 16, 2021

View reviewed changes

markdroth reviewed Mar 17, 2021

View reviewed changes

lizan removed their assignment Mar 19, 2021

krnowak force-pushed the krnowak/odcds-pr branch from e2bfaaa to 6a852da Compare March 25, 2021 20:13

repokitteh-read-only bot removed the waiting label Mar 25, 2021

krnowak force-pushed the krnowak/odcds-pr branch from 6a852da to 8ccba60 Compare March 25, 2021 23:15

krnowak force-pushed the krnowak/odcds-pr branch from 8ccba60 to 8e6735c Compare March 26, 2021 23:56

krnowak changed the title ~~WIP: on-demand, upstream: on-demand cluster discovery~~ on-demand, upstream: on-demand cluster discovery Mar 27, 2021

krnowak force-pushed the krnowak/odcds-pr branch 5 times, most recently from 92c5505 to 727c613 Compare March 31, 2021 14:04

krnowak added 3 commits March 31, 2021 16:47

krnowak added 4 commits April 1, 2021 22:15

CM: Pass a shared pointer to ODCDS instead of a weak one

1bda75a

This sidesteps some of the corner cases in the cluster discovery, and matches the expectations of the ODCDS lifetime. Signed-off-by: Krzesimir Nowak <[email protected]>

CM: Install a timeout for the discovery process

b43657e

This greatly simplifies ODCDS since it does not need to track the discovered names any more. Currently the timeout is set arbitrarily to 5 seconds. Signed-off-by: Krzesimir Nowak <[email protected]>

CDS: Make name const

805b63c

This is to address an issue raised during a review of the CdsApiHelper refactor. Signed-off-by: Krzesimir Nowak <[email protected]>

on-demand: Handle a case with an empty cluster name

efcf775

Signed-off-by: Krzesimir Nowak <[email protected]>

repokitteh-read-only bot removed the waiting label Apr 1, 2021

Merge remote-tracking branch 'origin/main' into krnowak/odcds-pr

cba9977

htuch reviewed Apr 2, 2021

View reviewed changes

mattklein123 requested changes Apr 2, 2021

View reviewed changes

repokitteh-read-only bot added the waiting label Apr 2, 2021

krnowak mentioned this pull request Apr 6, 2021

CM: Add on-demand cluster discovery functionality #15857

Closed

github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label May 7, 2021

github-actions bot closed this May 14, 2021

htuch reopened this May 14, 2021

htuch closed this May 14, 2021

krnowak mentioned this pull request Jun 7, 2021

Add support for explicit wildcard resource #16855

Merged

krnowak deleted the krnowak/odcds-pr branch October 22, 2021 06:42

adisuissa mentioned this pull request Jan 5, 2023

Clarify that on-demand CDS does not support SotW xDS #24763

Merged


		cluster_manager.cdm_.ensureCallbacksAreInstalled();

		auto [handle, discovery_in_progress] = cluster_manager.cdm_.addCallback(name, weak_callback);

on-demand, upstream: on-demand cluster discovery #15523

on-demand, upstream: on-demand cluster discovery #15523

Conversation

krnowak commented Mar 16, 2021

repokitteh-read-only bot commented Mar 16, 2021

repokitteh-read-only bot commented Mar 16, 2021

mattklein123 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

krnowak Mar 17, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmitri-d Mar 17, 2021 • edited Loading

Choose a reason for hiding this comment

dmitri-d commented Mar 17, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

krnowak commented Mar 25, 2021

krnowak commented Mar 25, 2021

krnowak commented Mar 26, 2021

krnowak commented Apr 1, 2021

mattklein123 commented Apr 1, 2021

krnowak commented Apr 1, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattklein123 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markdroth commented Apr 5, 2021

krnowak commented Apr 6, 2021

krnowak commented Apr 6, 2021

markdroth commented Apr 6, 2021

krnowak commented Apr 7, 2021

github-actions bot commented May 7, 2021

github-actions bot commented May 14, 2021

krnowak commented May 18, 2021

krnowak Mar 17, 2021 •

edited

Loading

dmitri-d Mar 17, 2021 •

edited

Loading