Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTTP consistent hash routing #496

Merged
merged 7 commits into from
Feb 23, 2017
Merged

HTTP consistent hash routing #496

merged 7 commits into from
Feb 23, 2017

Conversation

mattklein123
Copy link
Member

@mattklein123 mattklein123 commented Feb 17, 2017

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
Copy link
Member Author

@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.

/**
* @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
* exist. In the future we might add addtional support for hashing on origin address, etc.
Copy link
Member

Choose a reason for hiding this comment

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

additional

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

Choose a reason for hiding this comment

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

fix, this needs to go above, add test.

@mattklein123
Copy link
Member Author

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

"header_name": "..."
}

header_name
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

done


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
Copy link
Member

Choose a reason for hiding this comment

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

may be link here for "ketama" hashing?

^^^^^^^^^

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.
Copy link
Member

Choose a reason for hiding this comment

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

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?

@@ -6,6 +6,20 @@
namespace Upstream {

/**
* Context passed to a load balancer to use when choosing a host. Not all load balancers make use
Copy link
Member

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

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

Optional<uint64_t> hash_;
Copy link
Member

Choose a reason for hiding this comment

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

const

// 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);
Copy link
Member

Choose a reason for hiding this comment

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

document runtime param

Copy link
Member Author

Choose a reason for hiding this comment

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

done


uint64_t hashes_per_host = 1;
if (hosts.size() < min_ring_size) {
hashes_per_host = min_ring_size / hosts.size();
Copy link
Member

@RomanDzhabarov RomanDzhabarov Feb 22, 2017

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

NBD

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
Copy link
Member

Choose a reason for hiding this comment

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

s/is governs/governs

Copy link
Member

@RomanDzhabarov RomanDzhabarov left a comment

Choose a reason for hiding this comment

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

LGTM, left one nit

@mattklein123 mattklein123 merged commit c45f817 into master Feb 23, 2017
@mattklein123 mattklein123 deleted the ring_hash_lb branch February 23, 2017 00:35

uint64_t hashes_per_host = 1;
if (hosts.size() < min_ring_size) {
hashes_per_host = min_ring_size / hosts.size();
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

@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.

@@ -983,7 +991,7 @@ const std::string Json::Schema::CLUSTER_SCHEMA(R"EOF(
},
"lb_type" : {
"type" : "string",
"enum" : ["round_robin", "least_request", "random"]
"enum" : ["round_robin", "least_request", "random", "ring_hash"]
Copy link
Contributor

Choose a reason for hiding this comment

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

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).

rshriram pushed a commit to rshriram/envoy that referenced this pull request Oct 30, 2018
* Convert Pilot ip mesh attributes to bytes

* Update comment

* Update after review comment
wolfguoliang pushed a commit to wolfguoliang/envoy that referenced this pull request Jan 23, 2021
…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 <[email protected]>

* Update docs/root/configuration/http/http_filters/grpc_http1_reverse_bridge_filter.rst

body update with "响应体"

Co-authored-by: zhiguo-lu <[email protected]>

* Update docs/root/configuration/http/http_filters/grpc_http1_reverse_bridge_filter.rst

Co-authored-by: zhiguo-lu <[email protected]>

* updated

* updated

Co-authored-by: zhiguo-lu <[email protected]>
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.

Support for consistent hashing lb
4 participants