From 3bc09ccae88fd1045acc428b6a14f5a212cb478c Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Sun, 14 Jul 2019 14:01:41 +0530 Subject: [PATCH 1/5] default initial fetch timeout to 15s Signed-off-by: Rama Chavali --- api/envoy/api/v2/core/config_source.proto | 3 +-- .../intro/arch_overview/operations/init.rst | 18 +++++++++++------- docs/root/intro/version_history.rst | 1 + source/common/config/utility.cc | 2 +- test/common/config/utility_test.cc | 2 +- 5 files changed, 15 insertions(+), 11 deletions(-) diff --git a/api/envoy/api/v2/core/config_source.proto b/api/envoy/api/v2/core/config_source.proto index 8b6014dcbf9d3..583c3ec7524ce 100644 --- a/api/envoy/api/v2/core/config_source.proto +++ b/api/envoy/api/v2/core/config_source.proto @@ -112,13 +112,12 @@ message ConfigSource { AggregatedConfigSource ads = 3; } - // Optional initialization timeout. // When this timeout is specified, Envoy will wait no longer than the specified time for first // config response on this xDS subscription during the :ref:`initialization process // `. After reaching the timeout, Envoy will move to the next // initialization phase, even if the first config is not delivered yet. The timer is activated // when the xDS API subscription starts, and is disarmed on first config update or on error. 0 // means no timeout - Envoy will wait indefinitely for the first xDS config (unless another - // timeout applies). Default 0. + // timeout applies). If not specified the default is 15000ms (15 seconds). google.protobuf.Duration initial_fetch_timeout = 4; } diff --git a/docs/root/intro/arch_overview/operations/init.rst b/docs/root/intro/arch_overview/operations/init.rst index 2e05c5a750564..89ed37bc4c0f3 100644 --- a/docs/root/intro/arch_overview/operations/init.rst +++ b/docs/root/intro/arch_overview/operations/init.rst @@ -10,20 +10,24 @@ accepting new connections. * During startup, the :ref:`cluster manager ` goes through a multi-phase initialization where it first initializes static/DNS clusters, then predefined :ref:`EDS ` clusters. Then it initializes - :ref:`CDS ` if applicable, waits for one response (or failure), + :ref:`CDS ` if applicable, waits for one response (or failure) + for a :ref:`bounded period of time `, and does the same primary/secondary initialization of CDS provided clusters. * If clusters use :ref:`active health checking `, Envoy also does a single active health check round. * Once cluster manager initialization is done, :ref:`RDS ` and - :ref:`LDS ` initialize (if applicable). The server - doesn't start accepting connections until there has been at least one response (or failure) for - LDS/RDS requests. -* If LDS itself returns a listener that needs an RDS response, Envoy further waits until an RDS + :ref:`LDS ` initialize (if applicable). The server waits + for a :ref:`bounded period of time ` + for at least one response (or failure) for LDS/RDS requests. After which, it starts accepting connections. +* If LDS itself returns a listener that needs an RDS response, Envoy further waits for + a :ref:`bounded period of time `until an RDS response (or failure) is received. Note that this process takes place on every future listener addition via LDS and is known as :ref:`listener warming `. * After all of the previous steps have taken place, the listeners start accepting new connections. This flow ensures that during hot restart the new process is fully capable of accepting and processing new connections before the draining of the old process begins. -All mentioned "waiting for one response" periods can be limited by setting corresponding -:ref:`initial_fetch_timeout `. +A key design principle of initialization is that an Envoy is always guaranteed to initialize within +:ref:`initial_fetch_timeout `, +with a best effort made to obtain the complete set of xDS configuration within that subject to the +management server availability. diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index 70e8fe3e8c878..4f2fbe56baeeb 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -3,6 +3,7 @@ Version history 1.12.0 (pending) ================ +* config: Default value of :ref:`initial_fetch_timeout ` has been changed from 0s to 15s. This is a change in behaviour in the sense that Envoy initialization always completes within 15s by default. Refer to :ref:`initialization process ` for more details. 1.11.0 (July 11, 2019) ====================== diff --git a/source/common/config/utility.cc b/source/common/config/utility.cc index d22cc0a4cca6f..84dedbcda38f3 100644 --- a/source/common/config/utility.cc +++ b/source/common/config/utility.cc @@ -180,7 +180,7 @@ std::chrono::milliseconds Utility::apiConfigSourceRequestTimeout( std::chrono::milliseconds Utility::configSourceInitialFetchTimeout(const envoy::api::v2::core::ConfigSource& config_source) { return std::chrono::milliseconds( - PROTOBUF_GET_MS_OR_DEFAULT(config_source, initial_fetch_timeout, 0)); + PROTOBUF_GET_MS_OR_DEFAULT(config_source, initial_fetch_timeout, 15000)); } void Utility::translateRdsConfig( diff --git a/test/common/config/utility_test.cc b/test/common/config/utility_test.cc index 30d8fa8b3f3b4..90573d4f7d9b8 100644 --- a/test/common/config/utility_test.cc +++ b/test/common/config/utility_test.cc @@ -55,7 +55,7 @@ TEST(UtilityTest, ApiConfigSourceRequestTimeout) { TEST(UtilityTest, ConfigSourceDefaultInitFetchTimeout) { envoy::api::v2::core::ConfigSource config_source; - EXPECT_EQ(0, Utility::configSourceInitialFetchTimeout(config_source).count()); + EXPECT_EQ(15000, Utility::configSourceInitialFetchTimeout(config_source).count()); } TEST(UtilityTest, ConfigSourceInitFetchTimeout) { From d20d9423608b00e8532d87b409922ea6992f1108 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Sun, 14 Jul 2019 14:19:27 +0530 Subject: [PATCH 2/5] fix format and docs Signed-off-by: Rama Chavali --- docs/root/intro/arch_overview/operations/init.rst | 2 +- docs/root/intro/version_history.rst | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/root/intro/arch_overview/operations/init.rst b/docs/root/intro/arch_overview/operations/init.rst index 89ed37bc4c0f3..4ce245d78f51b 100644 --- a/docs/root/intro/arch_overview/operations/init.rst +++ b/docs/root/intro/arch_overview/operations/init.rst @@ -20,7 +20,7 @@ accepting new connections. for a :ref:`bounded period of time ` for at least one response (or failure) for LDS/RDS requests. After which, it starts accepting connections. * If LDS itself returns a listener that needs an RDS response, Envoy further waits for - a :ref:`bounded period of time `until an RDS + a :ref:`bounded period of time ` until an RDS response (or failure) is received. Note that this process takes place on every future listener addition via LDS and is known as :ref:`listener warming `. * After all of the previous steps have taken place, the listeners start accepting new connections. diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index 4f2fbe56baeeb..b6fb1ce170135 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -3,7 +3,7 @@ Version history 1.12.0 (pending) ================ -* config: Default value of :ref:`initial_fetch_timeout ` has been changed from 0s to 15s. This is a change in behaviour in the sense that Envoy initialization always completes within 15s by default. Refer to :ref:`initialization process ` for more details. +* config: changed the default value of :ref:`initial_fetch_timeout ` from 0s to 15s. This is a change in behaviour in the sense that Envoy initialization always completes within 15s by default. Refer to :ref:`initialization process ` for more details. 1.11.0 (July 11, 2019) ====================== From 8ba8a179155312565cb727542a8b0d3f09f6aa24 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Sun, 14 Jul 2019 15:00:53 +0530 Subject: [PATCH 3/5] fix test case Signed-off-by: Rama Chavali --- test/common/config/subscription_factory_impl_test.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/common/config/subscription_factory_impl_test.cc b/test/common/config/subscription_factory_impl_test.cc index 9c10b7dad2822..c72b02d146083 100644 --- a/test/common/config/subscription_factory_impl_test.cc +++ b/test/common/config/subscription_factory_impl_test.cc @@ -226,7 +226,7 @@ TEST_F(SubscriptionFactoryTest, HttpSubscriptionCustomRequestTimeout) { EXPECT_CALL(cm_, clusters()).WillOnce(Return(cluster_map)); EXPECT_CALL(cluster, info()).Times(2); EXPECT_CALL(*cluster.info_, addedViaApi()); - EXPECT_CALL(dispatcher_, createTimer_(_)); + EXPECT_CALL(dispatcher_, createTimer_(_)).Times(2); EXPECT_CALL(cm_, httpAsyncClientForCluster("static_cluster")); EXPECT_CALL( cm_.async_client_, @@ -246,7 +246,7 @@ TEST_F(SubscriptionFactoryTest, HttpSubscription) { EXPECT_CALL(cm_, clusters()).WillOnce(Return(cluster_map)); EXPECT_CALL(cluster, info()).Times(2); EXPECT_CALL(*cluster.info_, addedViaApi()); - EXPECT_CALL(dispatcher_, createTimer_(_)); + EXPECT_CALL(dispatcher_, createTimer_(_)).Times(2); EXPECT_CALL(cm_, httpAsyncClientForCluster("static_cluster")); EXPECT_CALL(cm_.async_client_, send_(_, _, _)) .WillOnce(Invoke([this](Http::MessagePtr& request, Http::AsyncClient::Callbacks&, @@ -301,7 +301,7 @@ TEST_F(SubscriptionFactoryTest, GrpcSubscription) { return async_client_factory; })); EXPECT_CALL(random_, random()); - EXPECT_CALL(dispatcher_, createTimer_(_)); + EXPECT_CALL(dispatcher_, createTimer_(_)).Times(2); EXPECT_CALL(callbacks_, onConfigUpdateFailed(_)); subscriptionFromConfigSource(config)->start({"static_cluster"}); } From 425074d5a8f14867d580fd64b6a4ac6840495de2 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Tue, 16 Jul 2019 09:45:07 +0530 Subject: [PATCH 4/5] address review comments Signed-off-by: Rama Chavali --- api/envoy/api/v2/core/config_source.proto | 2 +- docs/root/intro/version_history.rst | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/api/envoy/api/v2/core/config_source.proto b/api/envoy/api/v2/core/config_source.proto index 583c3ec7524ce..913f3876115ce 100644 --- a/api/envoy/api/v2/core/config_source.proto +++ b/api/envoy/api/v2/core/config_source.proto @@ -118,6 +118,6 @@ message ConfigSource { // initialization phase, even if the first config is not delivered yet. The timer is activated // when the xDS API subscription starts, and is disarmed on first config update or on error. 0 // means no timeout - Envoy will wait indefinitely for the first xDS config (unless another - // timeout applies). If not specified the default is 15000ms (15 seconds). + // timeout applies). The default is 15s. google.protobuf.Duration initial_fetch_timeout = 4; } diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index b6fb1ce170135..c82938aa39972 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -3,7 +3,7 @@ Version history 1.12.0 (pending) ================ -* config: changed the default value of :ref:`initial_fetch_timeout ` from 0s to 15s. This is a change in behaviour in the sense that Envoy initialization always completes within 15s by default. Refer to :ref:`initialization process ` for more details. +* config: changed the default value of :ref:`initial_fetch_timeout ` from 0s to 15s. This is a change in behaviour in the sense that Envoy will move to the next initialization phase, even if the first config is not delivered in 15s. Refer to :ref:`initialization process ` for more details. 1.11.0 (July 11, 2019) ====================== From 7aa822b2ce71cb7f6a48810925f6c745241f49ca Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Thu, 18 Jul 2019 09:39:46 +0530 Subject: [PATCH 5/5] fix version history conflict Signed-off-by: Rama Chavali --- docs/root/intro/version_history.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index bd159a18d7b3d..99782fae595bf 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -3,9 +3,9 @@ Version history 1.12.0 (pending) ================ -* config: changed the default value of :ref:`initial_fetch_timeout ` from 0s to 15s. This is a change in behaviour in the sense that Envoy will move to the next initialization phase, even if the first config is not delivered in 15s. Refer to :ref:`initialization process ` for more details. * admin: added ability to configure listener :ref:`socket options `. * config: async data access for local and remote data source. +* config: changed the default value of :ref:`initial_fetch_timeout ` from 0s to 15s. This is a change in behaviour in the sense that Envoy will move to the next initialization phase, even if the first config is not delivered in 15s. Refer to :ref:`initialization process ` for more details. * http: added the ability to reject HTTP/1.1 requests with invalid HTTP header values, using the runtime feature `envoy.reloadable_features.strict_header_validation`. 1.11.0 (July 11, 2019)