Make zone load balancing logic depend on local_cluster host distribution. by RomanDzhabarov · Pull Request #174 · envoyproxy/envoy

RomanDzhabarov · 2016-10-26T20:27:39Z

Documentation around runtime control/stats/general overview are going to be on a separate commit.

I was running simulation tests from load_balancer_simulation_test.cc, results look very good.

mattklein123 · 2016-10-27T16:24:27Z

source/common/upstream/load_balancer_impl.cc


-  // If number of hosts in a local zone big enough then route all requests to the same zone.
-  if (local_zone_healthy_hosts.size() * number_of_zones >= host_set_.healthyHosts().size()) {
+  std::vector<uint64_t> local_percentage =


Use a stack allocated array with the right size, rework the functions above to take the array and populate it if you want to do that. You can't allocate things like vectors in this code path.

Or just do the pre-calculation work now. I would actually prefer you just do that.

i made stack alloc for now, i want make sure things works good in prod

mattklein123 · 2016-10-27T16:26:43Z

source/common/upstream/load_balancer_impl.cc

+  // At this point we should route cross zone as we cannot route to the local zone.
+  stats_.zone_routing_no_sampled_.inc();
+
+  std::vector<uint64_t> capacity_left;


stack allocated array

mattklein123 · 2016-10-27T16:27:49Z

source/common/upstream/load_balancer_impl.cc

+  stats_.zone_routing_no_sampled_.inc();
+
+  std::vector<uint64_t> capacity_left;
+  // Local zone does not have additional capacity (we already routed what we could), but


More comments. Don't understand what is happening here.

…ts." This reverts commit ab35059. Conflicts: test/common/upstream/load_balancer_simulation_test.cc

ccaraman · 2016-10-31T18:26:41Z

source/common/upstream/load_balancer_impl.cc

+  if (total_hosts != 0) {
+    size_t i = 0;
+    for (const auto& zone_hosts : hosts_per_zone) {
+      ret[i++] = 10000ULL * zone_hosts.size() / total_hosts;


Please add comment explaining the 10,000ULL.

yep, good point, will add more details into load_balancer_impl.h. Done.

ccaraman · 2016-10-31T18:47:53Z

source/common/upstream/load_balancer_impl.cc

+  // Now we need to figure out how much traffic we can route cross zone and to which exact zone
+  // we should route. Percentage of requests routed cross zone to a specific zone should be
+  // proportional to the residual capacity upstream zone has.
+  //


nit: Needed?

mattklein123 · 2016-10-31T19:41:52Z

include/envoy/upstream/upstream.h

  COUNTER(update_failure)                                                                          \
  COUNTER(zone_cluster_too_small)                                                                  \
-  COUNTER(zone_over_percentage)                                                                    \
+  COUNTER(zone_routing_all_directly)                                                               \


nit: can you prefix all of the lb ones with "lb_" and then reorder this list.

yep, will do for consistency with panic threshold stats

mattklein123 · 2016-10-31T19:46:23Z

source/common/upstream/load_balancer_impl.cc

-    stats_.zone_over_percentage_.inc();
-    return local_zone_healthy_hosts;
+  uint64_t local_percentage[number_of_zones];
+  calculateZonePercentage(local_host_set_->healthyHostsPerZone(), local_percentage);


ASSERT here also that local_host_set_->healthyHostsPerZone().size() == host_set_.healthyHostsPerZone().size()

mattklein123 · 2016-10-31T19:55:16Z

source/common/upstream/load_balancer_impl.h

+   * In this case we'll route requests to hosts no matter if they are healthy or not.
+   */
+  bool isGlobalPanic(const HostSet& host_set);
+  const std::vector<HostPtr>& tryChooseLocalZoneHosts();


nit: add comment or newline before this line

put comments.

mattklein123 · 2016-10-31T19:56:12Z

source/common/upstream/load_balancer_impl.cc

+  // Local zone (index 0) does not have residual capacity as we have routed all we could.
+  residual_capacity[0] = 0;
+  for (size_t i = 1; i < number_of_zones; ++i) {
+    // Only route to the zones that have additional capacity.


still having trouble quickly understanding this logic. More comments, and potentially an example.

more comments added

mattklein123 · 2016-10-31T19:56:57Z

source/common/upstream/load_balancer_impl.cc

-  // Not zone routed requests will be distributed between all hosts and hence
-  // we need to route only fraction of req_percent_to_route to the local zone.
-  double actual_routing_ratio = (ratio_to_route - zone_host_ratio) / (1 - zone_host_ratio);
+  // Random sampling to select specific zone for cross zone traffic based on the additional


more comments. What type of number does residual_capacity[number_of_zones - 1] have in it. What range?

more comments added

mattklein123 · 2016-10-31T21:21:42Z

source/common/upstream/load_balancer_impl.cc

+  // sampled value is, we accumulate values in residual capacity. This is what it will look like:
+  // residual_capacity: 0 10000 15000
+  // Now to find zone to route (bucket) we could simply iterate over residual_capacity searching
+  // where


nit: can you fix line wrapping in this comment block. Keep to 100 col flow unless you need a new paragraph.

:( fix format broke this... fixing

mattklein123

lg after nit

* Add Dockerfile for the C++ SDK. Signed-off-by: John Plevyak <jplevyak@gmail.com>

…#114) (envoyproxy#174) Previously, the update callback was called only when the secret was received for the first time or when its value changed. This meant that if the same secret (e.g. trusted CA) was used in multiple resources, then resources using it but configured after the secret was already received, remained unconfigured until the secret's value changed. The missing callback should have resulted in transport factories stuck in the "not ready" state, however, because of an incorrect code, the available secret was processed like inlined validation context, and only rules from the "secret" part of the validation context were applied, leading to a complete bypass of rules from the "default" part. Signed-off-by: Piotr Sikora <piotrsikora@google.com> Co-authored-by: Oliver Liu <yonggangl@google.com> Co-authored-by: Oliver Liu <yonggangl@google.com>

…verprovisioning_factor zh-translation: /intro/arch_overview/upstream/load_balancing/overprovisioning.rst

**Commit Message**: This pull request includes updates to dependencies and configuration files for better security and compatibility. Envoy Gateway dependency is kept to the latest, whose tag is a weird v0.5.0-rc2, and because of that Trivy and Dependabot tries to "downgrade" to the latest tagged version. This fixes that by adding EG to the ignore list of dependency as well as add the latest EG CVE to the trivyignore file. Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>

**Commit Message**: This fixes the placement of `ignore` section in dependabot.yml. **Related Issues/PRs (if applicable)**: Follow up on #174 Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>

RomanDzhabarov added 3 commits October 26, 2016 13:22

initial change to balancing logic.

db76822

more requests, fix number of hosts.

db1df90

Add more tests around load balancing.

c83b5af

mattklein123 reviewed Oct 27, 2016

View reviewed changes

RomanDzhabarov added 4 commits October 27, 2016 14:55

comments.

fb39d7a

Enable tests, it takes around 25sec to complete all those tests.

ab35059

make 3% as we do not run through that many requests.

c7710ed

Revert "Enable tests, it takes around 25sec to complete all those tes…

82f2696

…ts." This reverts commit ab35059. Conflicts: test/common/upstream/load_balancer_simulation_test.cc

ccaraman reviewed Oct 31, 2016

View reviewed changes

more comments on multiplier and method.

879445d

ccaraman reviewed Oct 31, 2016

View reviewed changes

fix wording.

c756172

mattklein123 reviewed Oct 31, 2016

View reviewed changes

RomanDzhabarov added 3 commits October 31, 2016 14:08

comments.

f431d66

sort.

193f215

fix_format.

e49231a

mattklein123 reviewed Oct 31, 2016

View reviewed changes

mattklein123 approved these changes Oct 31, 2016

View reviewed changes

line wrapping.

90067cf

RomanDzhabarov merged commit d4d2393 into master Oct 31, 2016

RomanDzhabarov deleted the use_local_cluster branch November 1, 2016 02:46

jplevyak referenced this pull request in jplevyak/envoy Sep 17, 2019

Wasm dockerfile (istio#174)

d647c8f

* Add Dockerfile for the C++ SDK. Signed-off-by: John Plevyak <jplevyak@gmail.com>

jplevyak referenced this pull request in istio/envoy Sep 17, 2019

Wasm dockerfile (#174) (#108)

f13bebe

* Add Dockerfile for the C++ SDK. Signed-off-by: John Plevyak <jplevyak@gmail.com>

PiotrSikora pushed a commit to PiotrSikora/envoy that referenced this pull request Oct 10, 2019

Wasm dockerfile (envoyproxy#174)

0e5c253

* Add Dockerfile for the C++ SDK. Signed-off-by: John Plevyak <jplevyak@gmail.com>

wolfguoliang pushed a commit to wolfguoliang/envoy that referenced this pull request Jan 23, 2021

Merge pull request envoyproxy#174 from webup/intro/arch/upstream/lb/o…

99d4988

…verprovisioning_factor zh-translation: /intro/arch_overview/upstream/load_balancing/overprovisioning.rst

Conversation

RomanDzhabarov commented Oct 26, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RomanDzhabarov Oct 31, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ccaraman Oct 31, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RomanDzhabarov Oct 31, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mattklein123 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

RomanDzhabarov Oct 31, 2016 •

edited

Loading

ccaraman Oct 31, 2016 •

edited

Loading

RomanDzhabarov Oct 31, 2016 •

edited

Loading