matching: introduce consistent hashing matcher #14875
Conversation
This introduces a new matcher that allows matching on an input value by computing a hash value and matching if the value % (configured value) is greater than a configured threshold. This is useful in being able to define match criteria that should match for a certain % of input values in a way that is consistent between independent Envoy instances (e.g. it does not rely on a random input). Signed-off-by: Snow Pettersen <snowp@lyft.com>
Signed-off-by: Snow Pettersen <snowp@lyft.com>
|
Will split this into a PR for the changes to the core matching logic (since I want to make similar changes to the other factories) and leave this as just the extension change, hence the draft |
| message ConsistentHashing { | ||
| // The threshold the resulting hash must be over in order for this matcher to evaluate to true. | ||
| // This value must be below the configured modulo value. | ||
| uint32 threshold = 1 [(validate.rules).uint32 = {gt: 0}]; |
There was a problem hiding this comment.
Yeah good point, will remove the validation
| } | ||
|
|
||
| // Otherwise, match if (hash(input) % modulo) > threshold. | ||
| return HashUtil::xxHash64(*input) % modulo_ > threshold_; |
There was a problem hiding this comment.
One interesting implication of the consistent hasher API guarantee that the results are fleet-wide consistent is that if we ever change xxHash64, or xxhash itself internally makes some inconsistent breaking change, the property will be violated during rollouts. FWIW we have this problem today with our affinity load balancers. Not sure how you prefer to handle it (maybe warn in API documentation is enough?) but worth considering.
There was a problem hiding this comment.
I'll add a warning to the docs, not sure what else we could do here besides implementing our own hash that we promise we'll never change (or never update xxhash?).
There was a problem hiding this comment.
Can we model 100% match here given the >? Should it be >=?
| TEST(MatcherTest, EmptyValue) { | ||
| Matcher matcher(10, 100); | ||
|
|
||
| ASSERT_FALSE(matcher.match(absl::nullopt)); |
test/extensions/matching/input_matchers/consistent_hashing/matcher_test.cc
Show resolved
Hide resolved
| @@ -162,3 +162,5 @@ extensions/filters/http/oauth2 @rgs1 @derekargueta @snowp | |||
| /*/extensions/filters/http/kill_request @qqustc @htuch | |||
| # Rate limit expression descriptor | |||
| /*/extensions/rate_limit_descriptors/expr @kyessenov @lizan | |||
| # hash input matcher | |||
| /*/extensions/matching/input_matchers/consistent_hashing @snowp @donyu | |||
Signed-off-by: Snow Pettersen <snowp@lyft.com>
Signed-off-by: Snow Pettersen <snowp@lyft.com>
@htuch We're actually in need of this kind of matching in RouteConfiguration and are using Lua to get it done. |
|
@snowp @htuch do you think it's possible to also add a configurable "seed", this is something we also need, because without a seed a given value would consistently match across every given service using a rule. We're using this kind of hashing for "sticky" deployments based on a unique device identificator header, what happens without a seed is when a value of the header becomes covered by the threshold, then that user would simultaneously be enabled on all sticky deployments on the platform. |
Signed-off-by: Snow Pettersen <snowp@lyft.com>
|
@sschepens Sure, I was going to add that in later as we'll need this as well. Would simply passing through a uint64_t to xxhash be sufficient for your use case? |
|
@snowp yep, i think that would be alright, any string seed could be converted by management servers to a uint64 |
|
@snowp another question, will this be usable from RouteConfiguration or is this part of something else? |
| } | ||
|
|
||
| // Otherwise, match if (hash(input) % modulo) > threshold. | ||
| return HashUtil::xxHash64(*input) % modulo_ > threshold_; |
There was a problem hiding this comment.
Can we model 100% match here given the >? Should it be >=?
| auto message = Config::Utility::translateAnyToFactoryConfig( | ||
| config.typed_config(), ProtobufMessage::getStrictValidationVisitor(), factory); | ||
| auto matcher = factory.createInputMatcher(*message, context); | ||
| ASSERT_NE(nullptr, matcher); |
There was a problem hiding this comment.
Nit: EXPECT for a final assertion (even if fatal).
Signed-off-by: Snow Pettersen <snowp@lyft.com>
Signed-off-by: Snow Pettersen <snowp@lyft.com>
Signed-off-by: Snow Pettersen <snowp@lyft.com>
|
/retest |
|
Retrying Azure Pipelines: |
|
@htuch ptal |
| return false; | ||
| } | ||
|
|
||
| // Otherwise, match if (hash(input) % modulo) > threshold. |
|
|
||
| // The consistent hashing matchers computes a consistent hash from the input and matches if the resulting hash | ||
| // is within the configured threshold. | ||
| // More specifically, this matcher evaluates to true if hash(input) % modulo > threshold. |
Signed-off-by: Snow Pettersen <snowp@lyft.com>
|
@htuch ptal |
|
/lgtm api |
This introduces a new matcher that allows matching on an input value by
computing a hash value and matching if the value % (configured value) is
greater than a configured threshold. This is useful in being able to
define match criteria that should match for a certain % of input values
in a way that is consistent between independent Envoy instances (e.g. it
does not rely on a random input).
Risk Level: Low, new extension
Testing: UTs
Docs Changes: Inline proto docs
Release Notes: n/a
Platform Specific Features: n/a
Fixes #14782