From bf560ca613ffdcc4e2175ce6dd7aa37f87f578d6 Mon Sep 17 00:00:00 2001 From: aaron-ai Date: Wed, 25 Dec 2019 15:16:54 +0800 Subject: [PATCH 1/7] Delta CDS: add on-demand cds support Signed-off-by: aaron-ai --- include/envoy/config/grpc_mux.h | 10 ++++ include/envoy/config/subscription.h | 6 +++ include/envoy/upstream/cluster_manager.h | 14 ++++++ .../common/config/delta_subscription_impl.cc | 5 ++ .../common/config/delta_subscription_impl.h | 1 + source/common/config/grpc_mux_impl.h | 9 ++++ source/common/config/new_grpc_mux_impl.cc | 48 ++++++++++++++----- source/common/config/new_grpc_mux_impl.h | 12 ++++- source/common/config/watch_map.cc | 11 +++++ source/common/config/watch_map.h | 3 ++ source/common/upstream/cds_api_impl.h | 8 ++++ source/common/upstream/cluster_manager_impl.h | 8 ++++ .../config/delta_subscription_impl_test.cc | 6 +++ test/mocks/config/mocks.h | 3 ++ test/mocks/upstream/mocks.h | 2 + 15 files changed, 131 insertions(+), 15 deletions(-) diff --git a/include/envoy/config/grpc_mux.h b/include/envoy/config/grpc_mux.h index 8d65e28d58faf..60088ad059d9a 100644 --- a/include/envoy/config/grpc_mux.h +++ b/include/envoy/config/grpc_mux.h @@ -125,6 +125,16 @@ class GrpcMux { const std::set& resources, SubscriptionCallbacks& callbacks, std::chrono::milliseconds init_fetch_timeout) PURE; + + /** + * The only difference between addToWatch() and addOrUpdateWatch() is that the 'resources' here + * means the *extra* resources we interested in. + */ + virtual Watch* addToWatch(const std::string& type_url, Watch* watch, + const std::set& resources, + SubscriptionCallbacks& callbacks, + std::chrono::milliseconds init_fetch_timeout) PURE; + virtual void removeWatch(const std::string& type_url, Watch* watch) PURE; /** diff --git a/include/envoy/config/subscription.h b/include/envoy/config/subscription.h index 4dabe6eb003fb..4efe03c70425c 100644 --- a/include/envoy/config/subscription.h +++ b/include/envoy/config/subscription.h @@ -90,6 +90,12 @@ class Subscription { * be passed to std::set_difference, which must be given sorted collections. */ virtual void updateResourceInterest(const std::set& update_to_these_names) PURE; + + /** + * Add the resources to fetch. + * @param resources vector of resource names to fetch. + */ + virtual void addToResourceInterest(const std::set&){}; }; using SubscriptionPtr = std::unique_ptr; diff --git a/include/envoy/upstream/cluster_manager.h b/include/envoy/upstream/cluster_manager.h index 047bf2aafd481..6c620863d762f 100644 --- a/include/envoy/upstream/cluster_manager.h +++ b/include/envoy/upstream/cluster_manager.h @@ -227,6 +227,10 @@ class ClusterManager { * @return Config::SubscriptionFactory& the subscription factory. */ virtual Config::SubscriptionFactory& subscriptionFactory() PURE; + + virtual void updateClusterInterest(const std::set& update_to_these_names) PURE; + + virtual void addToClusterInterest(const std::set& add_these_names) PURE; }; using ClusterManagerPtr = std::unique_ptr; @@ -253,6 +257,16 @@ class CdsApi { * @return std::string last accepted version from fetch. */ virtual const std::string versionInfo() const PURE; + + /** + * Update watch set of cluster resources interested. + */ + virtual void updateClusterInterest(const std::set& update_to_these_names) PURE; + + /** + * Add watch set of cluster resources interested. + */ + virtual void addToClusterInterest(const std::set& add_these_names) PURE; }; using CdsApiPtr = std::unique_ptr; diff --git a/source/common/config/delta_subscription_impl.cc b/source/common/config/delta_subscription_impl.cc index b788b08887dc8..5320f7a71ba0a 100644 --- a/source/common/config/delta_subscription_impl.cc +++ b/source/common/config/delta_subscription_impl.cc @@ -41,6 +41,11 @@ void DeltaSubscriptionImpl::updateResourceInterest( stats_.update_attempt_.inc(); } +void DeltaSubscriptionImpl::addToResourceInterest(const std::set& add_these_names) { + watch_ = context_->addToWatch(type_url_, watch_, add_these_names, *this, init_fetch_timeout_); + stats_.update_attempt_.inc(); +} + // Config::SubscriptionCallbacks void DeltaSubscriptionImpl::onConfigUpdate( const Protobuf::RepeatedPtrField& resources, diff --git a/source/common/config/delta_subscription_impl.h b/source/common/config/delta_subscription_impl.h index 1b30df7b5ad00..f4549735d5029 100644 --- a/source/common/config/delta_subscription_impl.h +++ b/source/common/config/delta_subscription_impl.h @@ -45,6 +45,7 @@ class DeltaSubscriptionImpl : public Subscription, public SubscriptionCallbacks void start(const std::set& resource_names) override; void updateResourceInterest(const std::set& update_to_these_names) override; + void addToResourceInterest(const std::set& add_these_names) override; // Config::SubscriptionCallbacks (all pass through to callbacks_!) void onConfigUpdate(const Protobuf::RepeatedPtrField& resources, diff --git a/source/common/config/grpc_mux_impl.h b/source/common/config/grpc_mux_impl.h index 0781eb0cf9a13..6932db8f73916 100644 --- a/source/common/config/grpc_mux_impl.h +++ b/source/common/config/grpc_mux_impl.h @@ -50,6 +50,11 @@ class GrpcMuxImpl : public GrpcMux, SubscriptionCallbacks&, std::chrono::milliseconds) override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } + + Watch* addToWatch(const std::string&, Watch*, const std::set&, + SubscriptionCallbacks&, std::chrono::milliseconds) override { + NOT_IMPLEMENTED_GCOVR_EXCL_LINE; + } void removeWatch(const std::string&, Watch*) override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } void handleDiscoveryResponse( @@ -156,6 +161,10 @@ class NullGrpcMuxImpl : public GrpcMux, SubscriptionCallbacks&, std::chrono::milliseconds) override { throw EnvoyException("ADS must be configured to support an ADS config source"); } + Watch* addToWatch(const std::string&, Watch*, const std::set&, + SubscriptionCallbacks&, std::chrono::milliseconds) override { + throw EnvoyException("ADS must be configured to support an ADS config source"); + } void removeWatch(const std::string&, Watch*) override { throw EnvoyException("ADS must be configured to support an ADS config source"); } diff --git a/source/common/config/new_grpc_mux_impl.cc b/source/common/config/new_grpc_mux_impl.cc index 76989a45a19df..0284f32c42b99 100644 --- a/source/common/config/new_grpc_mux_impl.cc +++ b/source/common/config/new_grpc_mux_impl.cc @@ -30,11 +30,24 @@ Watch* NewGrpcMuxImpl::addOrUpdateWatch(const std::string& type_url, Watch* watc SubscriptionCallbacks& callbacks, std::chrono::milliseconds init_fetch_timeout) { if (watch == nullptr) { - return addWatch(type_url, resources, callbacks, init_fetch_timeout); - } else { - updateWatch(type_url, watch, resources); - return watch; + watch = addWatch(type_url, callbacks, init_fetch_timeout); } + // updateWatch() queues a discovery request if any of 'resources' are not yet subscribed. + updateWatch(type_url, watch, resources); + return watch; +} + +Watch* NewGrpcMuxImpl::addToWatch(const std::string& type_url, Watch* watch, + const std::set& resources, + SubscriptionCallbacks& callbacks, + std::chrono::milliseconds init_fetch_timeout) { + if (watch == nullptr) { + watch = addWatch(type_url, callbacks, init_fetch_timeout); + } + // addToWatch() queues a discovery request for any of *extra* 'resources' we are not yet + // subscribed. + addToWatch(type_url, watch, resources); + return watch; } void NewGrpcMuxImpl::removeWatch(const std::string& type_url, Watch* watch) { @@ -62,9 +75,8 @@ void NewGrpcMuxImpl::onDiscoveryResponse( message->system_version_info()); auto sub = subscriptions_.find(message->type_url()); if (sub == subscriptions_.end()) { - ENVOY_LOG(warn, - "Dropping received DeltaDiscoveryResponse (with version {}) for non-existent " - "subscription {}.", + ENVOY_LOG(warn, "Dropping received DeltaDiscoveryResponse (with version {}) for non-existent " + "subscription {}.", message->system_version_info(), message->type_url()); return; } @@ -124,22 +136,32 @@ GrpcMuxWatchPtr NewGrpcMuxImpl::subscribe(const std::string&, const std::set& resources, - SubscriptionCallbacks& callbacks, +Watch* NewGrpcMuxImpl::addWatch(const std::string& type_url, SubscriptionCallbacks& callbacks, std::chrono::milliseconds init_fetch_timeout) { auto entry = subscriptions_.find(type_url); if (entry == subscriptions_.end()) { // We don't yet have a subscription for type_url! Make one! addSubscription(type_url, init_fetch_timeout); - return addWatch(type_url, resources, callbacks, init_fetch_timeout); + return addWatch(type_url, callbacks, init_fetch_timeout); } - Watch* watch = entry->second->watch_map_.addWatch(callbacks); - // updateWatch() queues a discovery request if any of 'resources' are not yet subscribed. - updateWatch(type_url, watch, resources); return watch; } +void NewGrpcMuxImpl::addToWatch(const std::string& type_url, Watch* watch, + const std::set& resources) { + ASSERT(watch != nullptr); + auto sub = subscriptions_.find(type_url); + RELEASE_ASSERT(sub != subscriptions_.end(), + fmt::format("Watch of {} has no subscription to update.", type_url)); + auto added_removed = sub->second->watch_map_.addToWatchInterest(watch, resources); + sub->second->sub_state_.updateSubscriptionInterest(added_removed.added_, added_removed.removed_); + // Tell the server about our change in interest, if any. + if (sub->second->sub_state_.subscriptionUpdatePending()) { + trySendDiscoveryRequests(); + } +} + // Updates the list of resource names watched by the given watch. If an added name is new across // the whole subscription, or if a removed name has no other watch interested in it, then the // subscription will enqueue and attempt to send an appropriate discovery request. diff --git a/source/common/config/new_grpc_mux_impl.h b/source/common/config/new_grpc_mux_impl.h index b600c2e1e2f01..05705b8773cd8 100644 --- a/source/common/config/new_grpc_mux_impl.h +++ b/source/common/config/new_grpc_mux_impl.h @@ -37,6 +37,11 @@ class NewGrpcMuxImpl Watch* addOrUpdateWatch(const std::string& type_url, Watch* watch, const std::set& resources, SubscriptionCallbacks& callbacks, std::chrono::milliseconds init_fetch_timeout) override; + + Watch* addToWatch(const std::string& type_url, Watch* watch, + const std::set& resources, SubscriptionCallbacks& callbacks, + std::chrono::milliseconds init_fetch_timeout) override; + void removeWatch(const std::string& type_url, Watch* watch) override; // TODO(fredlas) PR #8478 will remove this. @@ -81,8 +86,11 @@ class NewGrpcMuxImpl } private: - Watch* addWatch(const std::string& type_url, const std::set& resources, - SubscriptionCallbacks& callbacks, std::chrono::milliseconds init_fetch_timeout); + Watch* addWatch(const std::string& type_url, SubscriptionCallbacks& callbacks, + std::chrono::milliseconds init_fetch_timeout); + + void addToWatch(const std::string& type_url, Watch* watch, + const std::set& resources); // Updates the list of resource names watched by the given watch. If an added name is new across // the whole subscription, or if a removed name has no other watch interested in it, then the diff --git a/source/common/config/watch_map.cc b/source/common/config/watch_map.cc index 257f42e4ccae2..d9d911fd0ac48 100644 --- a/source/common/config/watch_map.cc +++ b/source/common/config/watch_map.cc @@ -42,6 +42,17 @@ AddedRemoved WatchMap::updateWatchInterest(Watch* watch, findRemovals(newly_removed_from_watch, watch)); } +AddedRemoved WatchMap::addToWatchInterest(Watch* watch, + const std::set& add_these_names) { + std::vector newly_added_to_watch(add_these_names.begin(), add_these_names.end()); + std::vector newly_removed_from_watch; + watch->resource_names_.insert(add_these_names.begin(), add_these_names.end()); + std::set additions = add_these_names; + + return AddedRemoved(findAdditions(newly_added_to_watch, watch), + findRemovals(newly_removed_from_watch, watch)); +} + absl::flat_hash_set WatchMap::watchesInterestedIn(const std::string& resource_name) { absl::flat_hash_set ret = wildcard_watches_; const auto watches_interested = watch_interest_.find(resource_name); diff --git a/source/common/config/watch_map.h b/source/common/config/watch_map.h index 36bcf23f88ea1..ae9d4836257fe 100644 --- a/source/common/config/watch_map.h +++ b/source/common/config/watch_map.h @@ -73,6 +73,9 @@ class WatchMap : public SubscriptionCallbacks, public Logger::Loggable& update_to_these_names); + // Adds the extra set of resource names that the given watch should watch. + AddedRemoved addToWatchInterest(Watch* watch, const std::set& add_these_names); + // Expects that the watch to be removed has already had all of its resource names removed via // updateWatchInterest(). void removeWatch(Watch* watch); diff --git a/source/common/upstream/cds_api_impl.h b/source/common/upstream/cds_api_impl.h index 6653fffeb1a4b..0df7163ffcf05 100644 --- a/source/common/upstream/cds_api_impl.h +++ b/source/common/upstream/cds_api_impl.h @@ -35,6 +35,14 @@ class CdsApiImpl : public CdsApi, } const std::string versionInfo() const override { return system_version_info_; } + void updateClusterInterest(const std::set& update_to_these_names) override { + subscription_->updateResourceInterest(update_to_these_names); + } + + void addToClusterInterest(const std::set& add_these_names) override { + subscription_->addToResourceInterest(add_these_names); + } + private: // Config::SubscriptionCallbacks void onConfigUpdate(const Protobuf::RepeatedPtrField& resources, diff --git a/source/common/upstream/cluster_manager_impl.h b/source/common/upstream/cluster_manager_impl.h index f2cddab327974..6e6d9b30aed51 100644 --- a/source/common/upstream/cluster_manager_impl.h +++ b/source/common/upstream/cluster_manager_impl.h @@ -243,6 +243,14 @@ class ClusterManagerImpl : public ClusterManager, Logger::Loggable& update_to_these_names) override { + cds_api_->updateClusterInterest(update_to_these_names); + } + + void addToClusterInterest(const std::set& add_these_names) override { + cds_api_->addToClusterInterest(add_these_names); + } + protected: virtual void postThreadLocalDrainConnections(const Cluster& cluster, const HostVector& hosts_removed); diff --git a/test/common/config/delta_subscription_impl_test.cc b/test/common/config/delta_subscription_impl_test.cc index 3da5dcc593ace..34bf5a5f0025d 100644 --- a/test/common/config/delta_subscription_impl_test.cc +++ b/test/common/config/delta_subscription_impl_test.cc @@ -56,6 +56,12 @@ TEST_F(DeltaSubscriptionImplTest, PauseHoldsRequest) { subscription_->resume(); } +TEST_F(DeltaSubscriptionImplTest, AddResourceCauseRequest) { + startSubscription({}); + expectSendMessage({"name1", "name2"}, {}, Grpc::Status::WellKnownGrpcStatus::Ok, "", {}); + subscription_->addToResourceInterest({"name1", "name2"}); +} + TEST_F(DeltaSubscriptionImplTest, ResponseCausesAck) { startSubscription({"name1"}); deliverConfigUpdate({"name1"}, "someversion", true); diff --git a/test/mocks/config/mocks.h b/test/mocks/config/mocks.h index 882ebafd3d02a..46d4a341b8cc3 100644 --- a/test/mocks/config/mocks.h +++ b/test/mocks/config/mocks.h @@ -99,6 +99,9 @@ class MockGrpcMux : public GrpcMux { MOCK_METHOD(Watch*, addOrUpdateWatch, (const std::string& type_url, Watch* watch, const std::set& resources, SubscriptionCallbacks& callbacks, std::chrono::milliseconds init_fetch_timeout)); + MOCK_METHOD(Watch*, addToWatch, + (const std::string& type_url, Watch* watch, const std::set& resources, + SubscriptionCallbacks& callbacks, std::chrono::milliseconds init_fetch_timeout)); MOCK_METHOD(void, removeWatch, (const std::string& type_url, Watch* watch)); }; diff --git a/test/mocks/upstream/mocks.h b/test/mocks/upstream/mocks.h index 9a8ca01b00f8e..704d60a74b71b 100644 --- a/test/mocks/upstream/mocks.h +++ b/test/mocks/upstream/mocks.h @@ -330,6 +330,7 @@ class MockClusterManager : public ClusterManager { MOCK_METHOD(ClusterUpdateCallbacksHandle*, addThreadLocalClusterUpdateCallbacks_, (ClusterUpdateCallbacks & callbacks)); MOCK_METHOD(Config::SubscriptionFactory&, subscriptionFactory, ()); + MOCK_METHOD(void, addToClusterInterest, (const std::set&)); NiceMock conn_pool_; NiceMock async_client_; @@ -385,6 +386,7 @@ class MockCdsApi : public CdsApi { MOCK_METHOD(void, initialize, ()); MOCK_METHOD(void, setInitializedCb, (std::function callback)); MOCK_METHOD(const std::string, versionInfo, (), (const)); + MOCK_METHOD(void, addToClusterInterest, (const std::set&)); std::function initialized_callback_; }; From 1452becd84ccfb4bdb3702a9e05e0190db180af6 Mon Sep 17 00:00:00 2001 From: aaron-ai Date: Thu, 9 Jan 2020 14:49:08 +0800 Subject: [PATCH 2/7] Format code Signed-off-by: aaron-ai --- source/common/config/new_grpc_mux_impl.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/source/common/config/new_grpc_mux_impl.cc b/source/common/config/new_grpc_mux_impl.cc index 0284f32c42b99..a22001874cf04 100644 --- a/source/common/config/new_grpc_mux_impl.cc +++ b/source/common/config/new_grpc_mux_impl.cc @@ -75,8 +75,9 @@ void NewGrpcMuxImpl::onDiscoveryResponse( message->system_version_info()); auto sub = subscriptions_.find(message->type_url()); if (sub == subscriptions_.end()) { - ENVOY_LOG(warn, "Dropping received DeltaDiscoveryResponse (with version {}) for non-existent " - "subscription {}.", + ENVOY_LOG(warn, + "Dropping received DeltaDiscoveryResponse (with version {}) for non-existent " + "subscription {}.", message->system_version_info(), message->type_url()); return; } From acdbeaa959ee49b012b9567a218f9494abdad784 Mon Sep 17 00:00:00 2001 From: aaron-ai Date: Sat, 11 Jan 2020 18:43:45 +0800 Subject: [PATCH 3/7] Add document for on-demand CDS Signed-off-by: aaron-ai --- .../upstream/cluster_manager/cds.rst | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/docs/root/configuration/upstream/cluster_manager/cds.rst b/docs/root/configuration/upstream/cluster_manager/cds.rst index dcea74d79710c..4a6a82bbf6197 100644 --- a/docs/root/configuration/upstream/cluster_manager/cds.rst +++ b/docs/root/configuration/upstream/cluster_manager/cds.rst @@ -18,3 +18,40 @@ Statistics ---------- CDS has a :ref:`statistics ` tree rooted at *cluster_manager.cds.* + +On-demand CDS +----------------------------------- + +Similar to VHDS on-demand feature in terms of hosts, the on-demand CDS API is an additional API +that Envoy will call to dynamically fetch upstream clusters which Envoy interested in spontaneously. + +By default in CDS, all cluster configurations are sent to every Envoy instance in the mesh. The +delta CDS provides the ability that the control panel can send incremental CDS to the Envoy instance, +but Envoy instance can not feedback upstream clusters it interested in spontaneously, in other +words, Envoy instance only can receive the CDS passively. + +In order to fix this issue, on-demand CDS uses the delta xDS protocol to allow a cluster configuration +to be subscribed to and the necessary cluster configuration to be requested as needed. Instead +of sending all cluster configuration or cluster configuration the Envoy instance aren't interested +in, using on-demand CDS will allow an Envoy instance to subscribe and unsubscribe from a list of +cluster configurations stored internally in the xDS management server. The xDS management server +will monitor the list and use it to filter the configuration sent to an individual Envoy instance +to only contain the subscribed cluster configurations. + +Subscribing to resources +^^^^^^^^^^^^^^^^^^^^^^^^ + +On-demand CDS allows resources to be :ref:`subscribed ` to using +a :ref:`DeltaDiscoveryRequest ` +with the :ref:`type_url ` set to +`type.googleapis.com/envoy.api.v2.Cluster` and +:ref:`resource_names_subscribe ` +set to a list of cluster resource names for which it would like configuration. + +Typical use case +^^^^^^^^^^^^^^^^ + +Sometimes, there will be a large amount of broker instances provided for +`Apache RocketMQ `_ to produce/consume messages. Perhaps the size of +SToW CDS configurations will be more than 1GB, so it is not practical to deliver the SToW CDS from the +control panel every time, which will cause huge overhead. In this case, on-demand CDS is essential. \ No newline at end of file From 5342488e1e1852741b60153df8fec3c601cd95fb Mon Sep 17 00:00:00 2001 From: aaron-ai Date: Mon, 13 Jan 2020 14:25:45 +0800 Subject: [PATCH 4/7] Fix typo Signed-off-by: aaron-ai --- docs/root/configuration/upstream/cluster_manager/cds.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/root/configuration/upstream/cluster_manager/cds.rst b/docs/root/configuration/upstream/cluster_manager/cds.rst index 4a6a82bbf6197..8397788dbfb03 100644 --- a/docs/root/configuration/upstream/cluster_manager/cds.rst +++ b/docs/root/configuration/upstream/cluster_manager/cds.rst @@ -20,7 +20,7 @@ Statistics CDS has a :ref:`statistics ` tree rooted at *cluster_manager.cds.* On-demand CDS ------------------------------------ +------------- Similar to VHDS on-demand feature in terms of hosts, the on-demand CDS API is an additional API that Envoy will call to dynamically fetch upstream clusters which Envoy interested in spontaneously. From 0df0b7bc60155a94bc3e6782869a36df6f73dc0b Mon Sep 17 00:00:00 2001 From: aaron-ai Date: Wed, 15 Jan 2020 09:55:17 +0800 Subject: [PATCH 5/7] Update comments Signed-off-by: aaron-ai --- docs/root/configuration/upstream/cluster_manager/cds.rst | 8 ++++---- include/envoy/config/grpc_mux.h | 3 +-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/docs/root/configuration/upstream/cluster_manager/cds.rst b/docs/root/configuration/upstream/cluster_manager/cds.rst index 8397788dbfb03..6b22f58d630a3 100644 --- a/docs/root/configuration/upstream/cluster_manager/cds.rst +++ b/docs/root/configuration/upstream/cluster_manager/cds.rst @@ -26,9 +26,9 @@ Similar to VHDS on-demand feature in terms of hosts, the on-demand CDS API is an that Envoy will call to dynamically fetch upstream clusters which Envoy interested in spontaneously. By default in CDS, all cluster configurations are sent to every Envoy instance in the mesh. The -delta CDS provides the ability that the control panel can send incremental CDS to the Envoy instance, -but Envoy instance can not feedback upstream clusters it interested in spontaneously, in other -words, Envoy instance only can receive the CDS passively. +delta CDS provides the ability that the xDS management server can send incremental CDS to the Envoy +instance, but Envoy instance can not feedback upstream clusters it interested in spontaneously, in +other words, Envoy instance only can receive the CDS passively. In order to fix this issue, on-demand CDS uses the delta xDS protocol to allow a cluster configuration to be subscribed to and the necessary cluster configuration to be requested as needed. Instead @@ -54,4 +54,4 @@ Typical use case Sometimes, there will be a large amount of broker instances provided for `Apache RocketMQ `_ to produce/consume messages. Perhaps the size of SToW CDS configurations will be more than 1GB, so it is not practical to deliver the SToW CDS from the -control panel every time, which will cause huge overhead. In this case, on-demand CDS is essential. \ No newline at end of file +management server every time, which will cause huge overhead. In this case, on-demand CDS is essential. \ No newline at end of file diff --git a/include/envoy/config/grpc_mux.h b/include/envoy/config/grpc_mux.h index 60088ad059d9a..a4fb28bfb7e33 100644 --- a/include/envoy/config/grpc_mux.h +++ b/include/envoy/config/grpc_mux.h @@ -127,8 +127,7 @@ class GrpcMux { std::chrono::milliseconds init_fetch_timeout) PURE; /** - * The only difference between addToWatch() and addOrUpdateWatch() is that the 'resources' here - * means the *extra* resources we interested in. + * Adds additional resources to the watch. Unlike addOrUpdateWatch, it never removes resources. */ virtual Watch* addToWatch(const std::string& type_url, Watch* watch, const std::set& resources, From f0b4c761280c1fda669592fbed650c800bbb1d8d Mon Sep 17 00:00:00 2001 From: aaron-ai Date: Wed, 15 Jan 2020 14:23:13 +0800 Subject: [PATCH 6/7] Remove useless code Signed-off-by: aaron-ai --- include/envoy/upstream/cluster_manager.h | 2 -- source/common/config/watch_map.cc | 6 ++---- source/common/upstream/cluster_manager_impl.h | 4 ---- 3 files changed, 2 insertions(+), 10 deletions(-) diff --git a/include/envoy/upstream/cluster_manager.h b/include/envoy/upstream/cluster_manager.h index 6c620863d762f..e75c1344cfc09 100644 --- a/include/envoy/upstream/cluster_manager.h +++ b/include/envoy/upstream/cluster_manager.h @@ -228,8 +228,6 @@ class ClusterManager { */ virtual Config::SubscriptionFactory& subscriptionFactory() PURE; - virtual void updateClusterInterest(const std::set& update_to_these_names) PURE; - virtual void addToClusterInterest(const std::set& add_these_names) PURE; }; diff --git a/source/common/config/watch_map.cc b/source/common/config/watch_map.cc index d9d911fd0ac48..bd02799b6ee91 100644 --- a/source/common/config/watch_map.cc +++ b/source/common/config/watch_map.cc @@ -45,12 +45,10 @@ AddedRemoved WatchMap::updateWatchInterest(Watch* watch, AddedRemoved WatchMap::addToWatchInterest(Watch* watch, const std::set& add_these_names) { std::vector newly_added_to_watch(add_these_names.begin(), add_these_names.end()); - std::vector newly_removed_from_watch; watch->resource_names_.insert(add_these_names.begin(), add_these_names.end()); - std::set additions = add_these_names; + std::set removals; - return AddedRemoved(findAdditions(newly_added_to_watch, watch), - findRemovals(newly_removed_from_watch, watch)); + return AddedRemoved(findAdditions(newly_added_to_watch, watch), std::move(removals)); } absl::flat_hash_set WatchMap::watchesInterestedIn(const std::string& resource_name) { diff --git a/source/common/upstream/cluster_manager_impl.h b/source/common/upstream/cluster_manager_impl.h index 6e6d9b30aed51..99a942eeac794 100644 --- a/source/common/upstream/cluster_manager_impl.h +++ b/source/common/upstream/cluster_manager_impl.h @@ -243,10 +243,6 @@ class ClusterManagerImpl : public ClusterManager, Logger::Loggable& update_to_these_names) override { - cds_api_->updateClusterInterest(update_to_these_names); - } - void addToClusterInterest(const std::set& add_these_names) override { cds_api_->addToClusterInterest(add_these_names); } From 05ba93a48e781e9c5813c0e12a462c5ecc32d673 Mon Sep 17 00:00:00 2001 From: aaron-ai Date: Wed, 15 Jan 2020 15:04:48 +0800 Subject: [PATCH 7/7] Remove useless code Signed-off-by: aaron-ai --- include/envoy/upstream/cluster_manager.h | 5 ----- source/common/upstream/cds_api_impl.h | 4 ---- 2 files changed, 9 deletions(-) diff --git a/include/envoy/upstream/cluster_manager.h b/include/envoy/upstream/cluster_manager.h index e75c1344cfc09..94ec77d81ec08 100644 --- a/include/envoy/upstream/cluster_manager.h +++ b/include/envoy/upstream/cluster_manager.h @@ -256,11 +256,6 @@ class CdsApi { */ virtual const std::string versionInfo() const PURE; - /** - * Update watch set of cluster resources interested. - */ - virtual void updateClusterInterest(const std::set& update_to_these_names) PURE; - /** * Add watch set of cluster resources interested. */ diff --git a/source/common/upstream/cds_api_impl.h b/source/common/upstream/cds_api_impl.h index 0df7163ffcf05..eab0bb818679c 100644 --- a/source/common/upstream/cds_api_impl.h +++ b/source/common/upstream/cds_api_impl.h @@ -35,10 +35,6 @@ class CdsApiImpl : public CdsApi, } const std::string versionInfo() const override { return system_version_info_; } - void updateClusterInterest(const std::set& update_to_these_names) override { - subscription_->updateResourceInterest(update_to_these_names); - } - void addToClusterInterest(const std::set& add_these_names) override { subscription_->addToResourceInterest(add_these_names); }