HTTP consistent hash routing by mattklein123 · Pull Request #496 · envoyproxy/envoy

mattklein123 · 2017-02-17T23:00:41Z

Although the ring hashing load balancer ("ketama") was primarily implemented
for the upcoming redis support, as a bonus this commit adds HTTP consistent
hash routing as well, since it's pretty simple and it's been requested several
times.

fixes #197

Although the ring hashing load balancer ("ketama") was primarily implemented for the upcoming redis support, as a bonus this commit adds HTTP consistent hash routing as well, since it's pretty simple and it's been requested several times.

mattklein123 · 2017-02-17T23:01:16Z

@lyft/network-team this is missing docs but in case you want to start reviewing the code changes are done. Will report back when the docs are done.

junr03 · 2017-02-17T23:09:52Z

include/envoy/router/router.h

+
+  /**
+   * @return Optional<uint64_t> an optional hash value to route on given a set of HTTP headers.
+   *         A hash value might not be returned if for example the specified HTTP header does not


mattklein123 · 2017-02-17T23:54:10Z

source/common/upstream/ring_hash_lb.cc

+                                        const std::vector<HostPtr>& hosts) {
+  log_trace("ring hash: building ring");
+  if (hosts.empty()) {
+    ring_.clear();


fix, this needs to go above, add test.

mattklein123 · 2017-02-22T04:31:28Z

@lyft/network-team I added docs. This is done and ready for full review.

RomanDzhabarov · 2017-02-22T17:43:00Z

docs/configuration/http_conn_man/route_config/route.rst

+     "header_name": "..."
+   }
+
+header_name


I'd cover case on what happens if a given header is not present in the request

RomanDzhabarov · 2017-02-22T17:45:27Z

docs/intro/arch_overview/load_balancing.rst

+
+The ring/modulo hash load balancer implements consistent hashing to upstream hosts. The algorithm is
+based on mapping all hosts onto a circle such that any host set changes only affect 1/N requests.
+This technique is also commonly known as "ketama" hashing. The consistent hashing load balancer is


may be link here for "ketama" hashing?

RomanDzhabarov · 2017-02-22T17:55:39Z

docs/intro/arch_overview/load_balancing.rst

+^^^^^^^^^
+
+The ring/modulo hash load balancer implements consistent hashing to upstream hosts. The algorithm is
+based on mapping all hosts onto a circle such that any host set changes only affect 1/N requests.


i think this is not 100% correct, specifically it's only single addition or removal of the host from host set will affect 1/N requests, but not any host set changes.
rephrase it a bit here?

RomanDzhabarov · 2017-02-22T17:57:33Z

include/envoy/upstream/load_balancer.h


 namespace Upstream {

+/**


nit: of all or any context information

RomanDzhabarov · 2017-02-22T18:02:05Z

source/common/router/router.h

+    // Upstream::LoadBalancerContext
+    const Optional<uint64_t>& hashKey() const override { return hash_; }
+
+    Optional<uint64_t> hash_;


RomanDzhabarov · 2017-02-22T18:23:08Z

source/common/upstream/ring_hash_lb.cc

+  //       standpoint and duplicates the regeneration computation. In the future we might want
+  //       to generate the rings centrally and then just RCU them out to each thread. This is
+  //       sufficient for getting started.
+  uint64_t min_ring_size = runtime.snapshot().getInteger("upstream.ring_hash.min_ring_size", 1024);


document runtime param

RomanDzhabarov · 2017-02-22T18:28:43Z

source/common/upstream/ring_hash_lb.cc

+
+  uint64_t hashes_per_host = 1;
+  if (hosts.size() < min_ring_size) {
+    hashes_per_host = min_ring_size / hosts.size();


a bit simply, hashes_per_host = (min_ring_size + hosts.size() - 1) / hosts.size(); (and remove if below)

I just spent a few minutes trying to reason about whether this is correct or not and I couldn't easily do it, so I'm going to leave the if statement. It's easy to understand.

RomanDzhabarov · 2017-02-22T21:59:14Z

docs/intro/arch_overview/load_balancing.rst

+mechanism is to hash via :ref:`HTTP header values <config_http_conn_man_route_table_hash_policy>` in
+the :ref:`HTTP router filter <arch_overview_http_routing>`. The default minimum ring size is
+specified in :ref:`runtime <config_cluster_manager_cluster_runtime_ring_hash>`. The minimum ring
+size is governs the replication factor for each host in the ring. For example, if the minimum ring


s/is governs/governs

RomanDzhabarov

LGTM, left one nit

PiotrSikora · 2017-02-25T09:45:55Z

source/common/upstream/ring_hash_lb.cc

+
+  uint64_t hashes_per_host = 1;
+  if (hosts.size() < min_ring_size) {
+    hashes_per_host = min_ring_size / hosts.size();


This should be hashes_per_host = 160 (or hashes_per_host = 160 * weight) in order to keep it consistent (and interoperable) with other implementations of ketama.

Note: This will consume quite a bit of memory with huge clusters.

@PiotrSikora since this was already submitted, can you open a GH issue and we can discuss there? It's not clear to me that we have to explicitly support 160 points per server (I'm not a hashing expert but that seems arbitrary to me). Another option is to have a different type that we expose in the config called "ketama" which uses the same underlying code but does 160 points per server.

PiotrSikora · 2017-02-25T09:48:23Z

source/common/json/config_schemas.cc

      "lb_type" : {
        "type" : "string",
-        "enum" : ["round_robin", "least_request", "random"]
+        "enum" : ["round_robin", "least_request", "random", "ring_hash"]


This should be exposed as "ketama" (or at the very least "consistent_hash"), otherwise it's not obvious what the underlying mapping represents.

Also, we might want to add other consistent hashes in the future (like the less memory-hungry "jump" algorithm).

* Convert Pilot ip mesh attributes to bytes * Update comment * Update after review comment

…reverse_bridge_filter.rst (envoyproxy#496) * update docs/root/configuration/http/http_filters/grpc_http1_reverse_bridge_filter.rst * update grpc_http1_reverse_bridge_filter.rst * update translation of "arch overview & reference" * Update docs/root/configuration/http/http_filters/grpc_http1_reverse_bridge_filter.rst Co-authored-by: zhiguo-lu <70930322+zhiguo-lu@users.noreply.github.com> * Update docs/root/configuration/http/http_filters/grpc_http1_reverse_bridge_filter.rst body update with "响应体" Co-authored-by: zhiguo-lu <70930322+zhiguo-lu@users.noreply.github.com> * Update docs/root/configuration/http/http_filters/grpc_http1_reverse_bridge_filter.rst Co-authored-by: zhiguo-lu <70930322+zhiguo-lu@users.noreply.github.com> * updated * updated Co-authored-by: zhiguo-lu <70930322+zhiguo-lu@users.noreply.github.com>

HTTP consistent hash routing

0ad568c

Although the ring hashing load balancer ("ketama") was primarily implemented for the upcoming redis support, as a bonus this commit adds HTTP consistent hash routing as well, since it's pretty simple and it's been requested several times.

junr03 reviewed Feb 17, 2017

View reviewed changes

build fix

8bc5661

mattklein123 commented Feb 17, 2017

View reviewed changes

mattklein123 added 3 commits February 21, 2017 14:28

Merge branch 'master' into ring_hash_lb

c9a6e51

fix

87041d2

docs

a2761c9

RomanDzhabarov reviewed Feb 22, 2017

View reviewed changes

include/envoy/upstream/load_balancer.h

namespace Upstream {

/**

Copy link
Copy Markdown

Member

RomanDzhabarov Feb 22, 2017

Choose a reason for hiding this comment

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

nit: of all or any context information

RomanDzhabarov reviewed Feb 22, 2017

View reviewed changes

comments

da53644

RomanDzhabarov reviewed Feb 22, 2017

View reviewed changes

RomanDzhabarov approved these changes Feb 22, 2017

View reviewed changes

Update load_balancing.rst

e006b28

mattklein123 merged commit c45f817 into master Feb 23, 2017

mattklein123 deleted the ring_hash_lb branch February 23, 2017 00:35

PiotrSikora reviewed Feb 25, 2017

View reviewed changes

rshriram pushed a commit to rshriram/envoy that referenced this pull request Oct 30, 2018

Convert Pilot mesh ip attributes to BYTES (envoyproxy#496)

0e8ab14

* Convert Pilot ip mesh attributes to bytes * Update comment * Update after review comment

Conversation

mattklein123 commented Feb 17, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mattklein123 commented Feb 17, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mattklein123 commented Feb 22, 2017

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 Feb 22, 2017 • 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!

RomanDzhabarov left a comment

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mattklein123 commented Feb 17, 2017 •

edited

Loading

RomanDzhabarov Feb 22, 2017 •

edited

Loading