Skip to content

Wire up panic mode subset to receive updates#6221

Merged
htuch merged 5 commits intoenvoyproxy:masterfrom
rgs1:fix-panic-mode
Mar 12, 2019
Merged

Wire up panic mode subset to receive updates#6221
htuch merged 5 commits intoenvoyproxy:masterfrom
rgs1:fix-panic-mode

Conversation

@rgs1
Copy link
Member

@rgs1 rgs1 commented Mar 8, 2019

The initial diff missed handling added/removed hosts, this
handles that.

While at this, lets move the fallback subset creation to the constructor
so that it's consistent with the panic mode subset.

Reported-by: @nezdolik
Signed-off-by: Raul Gutierrez Segales rgs@pinterest.com

The initial diff missed handling added/removed hosts, this
handles that.

Reported-by: @nezdolik
Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>
To make things more consistent between the fallback and the
panic subset, create both at construction time.

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>
@htuch htuch self-assigned this Mar 8, 2019
@@ -186,34 +205,18 @@ SubsetLoadBalancer::LbSubsetEntryPtr SubsetLoadBalancer::findSubset(

void SubsetLoadBalancer::updateFallbackSubset(uint32_t priority, const HostVector& hosts_added,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update name? as this method now updates not only default subset

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm the panic mode subset is tied to the fallback subset.. not sure what a better name would be.

scale_locality_weight_(subsets.scaleLocalityWeight()) {
ASSERT(subsets.isEnabled());

if (fallback_policy_ != envoy::api::v2::Cluster::LbSubsetConfig::NO_FALLBACK) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could be just a personal preference, but would rather use deferred initialisation or private helper method not to make constructor code blow up

htuch
htuch previously approved these changes Mar 11, 2019
Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM


ENVOY_LOG(debug, "subset lb: creating any-endpoint fallback load balancer");
} else {
predicate = std::bind(&SubsetLoadBalancer::hostMatches, this, default_subset_metadata_,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I think a lambda might be clearer here.

Switch to lambda in an unrelated spot too, but it's indirectly
related so might as well be consistent.

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>
Raul Gutierrez Segales added 2 commits March 11, 2019 11:59
Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>
Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>
Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@htuch htuch merged commit 1f9e2fd into envoyproxy:master Mar 12, 2019
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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants