xds: introduce initial_fetch_timeout option to limit initialization time #6048
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ation time Signed-off-by: Marcin Falkowski <marcin.falkowski@allegro.pl>
MarcinFalkowski
changed the title
htuch
reviewed
Feb 26, 2019
Signed-off-by: Marcin Falkowski <marcin.falkowski@allegro.pl>
Signed-off-by: Marcin Falkowski <marcin.falkowski@allegro.pl>
Signed-off-by: Marcin Falkowski <marcin.falkowski@allegro.pl>
MarcinFalkowski
force-pushed
the
initial-fetch-timeout-sq
branch
from
ae25c26 to
e71fc4e
Compare
MarcinFalkowski
changed the title
htuch
reviewed
Mar 11, 2019
|
|
||
| typedef DeltaSubscriptionImpl<envoy::api::v2::ClusterLoadAssignment> DeltaEdsSubscriptionImpl; | ||
|
|
||
| class DeltaSubscriptionImplTest : public testing::Test { |
There was a problem hiding this comment.
I would advocate to factor this into harness/test as done for the other config tests. That way, we can reuse common tests (and write specialized ones where needed).
There was a problem hiding this comment.
I created delta subscription test harness and attached it to the common subscription tests in subscription_impl_test.cc.
htuch
suggested changes
Mar 11, 2019
…ut-sq Signed-off-by: Marcin Falkowski <marcin.falkowski@allegro.pl>
…ut-sq Signed-off-by: Marcin Falkowski <marcin.falkowski@allegro.pl>
Signed-off-by: Marcin Falkowski <marcin.falkowski@allegro.pl>
|
@htuch should I add release notes for this change? |
htuch
suggested changes
Mar 14, 2019
| @@ -33,7 +33,11 @@ class FilesystemSubscriptionTestHarness : public SubscriptionTestHarness { | |||
| api_(Api::createApiForTest(stats_store_)), dispatcher_(api_->allocateDispatcher()), | |||
| subscription_(*dispatcher_, path_, stats_, *api_) {} | |||
|
|
|||
| ~FilesystemSubscriptionTestHarness() { EXPECT_EQ(0, ::unlink(path_.c_str())); } | |||
| ~FilesystemSubscriptionTestHarness() { | |||
| if (::access(path_.c_str(), F_OK) != -1) { | |||
There was a problem hiding this comment.
I added a test, where deliverConfigUpdate is never called. So the config file is not even created, and unlink fails. All other tests call deliverConfigUpdate, so it was not a problem until now.
Signed-off-by: Marcin Falkowski <marcin.falkowski@allegro.pl>
|
Thanks! |
spenceral
added a commit
to spenceral/envoy
that referenced
this pull request
Mar 20, 2019
* master: (59 commits) http fault: add response rate limit injection (envoyproxy#6267) xds: introduce initial_fetch_timeout option to limit initialization time (envoyproxy#6048) test: fix cpuset-threads tests (envoyproxy#6278) server: add an API for registering for notifications for server instance life… (envoyproxy#6254) remove remains of TestBase (envoyproxy#6286) dubbo_proxy: Implement the routing of Dubbo requests (envoyproxy#5973) Revert "stats: add new BoolIndicator stat type (envoyproxy#5813)" (envoyproxy#6280) runtime: codifying runtime guarded features (envoyproxy#6134) mysql_filter: fix integration test flakes (envoyproxy#6272) tls: update BoringSSL to debed9a4 (3683). (envoyproxy#6273) rewrite buffer implementation to eliminate evbuffer dependency (envoyproxy#5441) Remove the dependency from TimeSystem to libevent by using the Event::Scheduler abstraction as a delegate. (envoyproxy#6240) fuzz: fix use of literal in default initialization. (envoyproxy#6268) http: add HCM functionality required for rate limiting (envoyproxy#6242) Disable mysql_integration_test until it is deflaked. (envoyproxy#6250) test: use ipv6_only IPv6 addresses in custom cluster integration tests. (envoyproxy#6260) tracing: If parent span is propagated with empty string, it causes th… (envoyproxy#6263) upstream: fix oss-fuzz issue envoyproxy#11095. (envoyproxy#6220) Wire up panic mode subset to receive updates (envoyproxy#6221) docs: clarify xds docs with warming information (envoyproxy#6236) ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
xDS: introduce initial_fetch_timeout option to limit server initialization time
Signed-off-by: Marcin Falkowski marcin.falkowski@allegro.pl
Description:
Introduce new optional configuration parameter
initial_fetch_timeoutto ConfigSource proto.This parameter can be used to limit time Envoy spend on initialization of CDS, LDS and RDS.
Enabling this timeout prevents Envoy from being stuck in PRE_INITIALIZING or INITIALIZING phase, as described in #5862.
Risk Level: Low (optional feature, disabled by default)
Testing: manual testing, unit tests
Docs Changes: add new parameter
initial_fetch_timeoutto ConfigSource protoRelease Notes: Added
Fixes #5862