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

multi-cluster weighted routing rule #247

Closed
mattklein123 opened this issue Nov 29, 2016 · 11 comments · Fixed by #361
Closed

multi-cluster weighted routing rule #247

mattklein123 opened this issue Nov 29, 2016 · 11 comments · Fixed by #361
Labels
enhancement Feature requests. Not bugs or questions.
Milestone

Comments

@mattklein123
Copy link
Member

Right now it's clumsy if the user wants to have a routing rule that matches and then sends traffic to a number of upstream clusters on a % basis. We should support this directly both via static percentages as well as runtime override of the percentages.

The rule might look something like:

{
...
"weighted_clusters:" [
  { "name": "cluster1", "weight": 33 },
  { "name": "cluster2", "weight": 33 },
  { "name": "cluster3", "weight": 33 },
]
}

Need to also specify what the rule would look like with runtime overrides. Open so to suggestions here.

cc @rshriram

@mattklein123
Copy link
Member Author

See here for more history on this issue: #218

@rshriram
Copy link
Member

@mattklein123
some suggestions:
Having a separate block for cluster weights could lead to cases where there are more clusters defined in the route filter block, but users forget to mention that specific cluster in the weighted cluster block. In this case, Envoy would end up having to return a GATEWAY_NOT_FOUND, causing more confusion on the developer's behalf.

Instead, an alternative way would be to add a weight field to the route filter. For example

{
  "prefix": "...",
  "cluster": "...",
  "weight" : [integer >= 0],
  "timeout_ms": "...",
  "runtime": "{ route.clusterName.weight }",
  ...
}

These weights are basically integers greater than or equal to 0. This is similar to nginx's concept of weights. By default, if a weight is not provided, it would be set to 1. In the code, the weight per cluster can be calculated as a proportion of the sum of all weights.. (myweight/sum_weights).

So, in your example, traffic would automatically be split 1/3rd per cluster across 3 clusters.

Another example: with 3 route filters like

{
 path: "/foo/v1"
 weight: 10
},
{
 path: "/foo/v2"
 weight: 3
},
{
 path: "/foo/v3"
}

v1's weight would be 10/14, v2's weight would be 3/14 and v3's would be 1/14. The advantage of this syntax is that Envoy does not have to validate that the sum of weights add up to 100. The user's configuration tasks become simpler.

Thoughts?

@mattklein123
Copy link
Member Author

I really do not want to apply weights across routes, at least at the top level. IMO It's extremely confusing as it breaks the top down matching semantics that are most commonly used for a routing table.

So as I see it there are 2 options:

  1. Do what I originally proposed above (have a multi-cluster target option)
  2. Define some new "grouped" route type of thing. Where you can group multiple routing rules into an aggregate rule such that only 1 will ever match based on weights.

@rshriram
Copy link
Member

Apologies. I misunderstood where the multi-cluster block was going to reside. Did you mean that it replace the cluster key in the route block with weighted_clusters? If so, then I totally agree.

My example was incorrect. Here is a refined one. Is this what you were suggesting?

{
 path: "/foo",
 weighted_clusters: {
  "name": "foo-v1", weight: 5,
  "name": "foo-v2", weight: 3
 }
},
{
 path: "/baz"
 Cluster:  baz 
},
{
 path: "/bar",
 Cluster: bar
}

@mattklein123
Copy link
Member Author

Yes, that is what I'm suggesting. The remaining thing would be to determine how the runtime keys are defined such that there can be traffic shifting in the group using runtime.

@rshriram
Copy link
Member

rshriram commented Nov 30, 2016 via email

@mattklein123 mattklein123 added this to the 1.2.0 milestone Dec 1, 2016
@mattklein123 mattklein123 added the enhancement Feature requests. Not bugs or questions. label Dec 1, 2016
@mattklein123
Copy link
Member Author

Sounds like @rshriram is going to do this one.

@rshriram
Copy link
Member

rshriram commented Jan 5, 2017

@mattklein123
Another thing I realized/noticed now is how the you specified the weights in the weighted cluster.

Today, to split traffic between three versions (33/33/33), I would have to specify the weights (in runtime) as 33, 66, (and none for the last). However, the syntax you proposed above (weights: 33, 33, 33 out of 100) is different. Is that a typo or was it intentional?

While its possible to translate from this static specification to the right internal representation (i.e., 33, 66, ), it creates ambiguity because when I use the runtime, I would still have to use the old model (33, 66, none).

So, the correct syntax for weighted clusters would be something like this:

{
 "path": "/foo",
 "weighted_clusters": {
  "runtime_key_root" : "weightedfoo",
  "clusters" : [
  { "name": "foo-trial-1", "weight": 33},
  {"name": "foo-trial-2", "weight": 66},
  {"name": "foo-default"}
  ]
}

such that, if I were to use a runtime, there would be parity in configuration in both static config and runtime configs. For e.g., /path/to/runtime/weightedfoo/foo-trial-1 ->33.

If the intuitive way of specifying weights (33,33,33) is needed, then we need to support this for runtimes as well. That would require pre-processing the data read by Runtime, to transform these weights into a monotonically increasing format. Thoughts?

@mattklein123
Copy link
Member Author

The intuitive weights should be used. It is not required that runtime be modified. The basic flow looks like this:

  1. Check if weighted routing rule
  2. Take stable ID and manually modulo by 100
  3. Iterate through each weight until you find the right cluster to route to (and handle bogus runtime value somehow).

@rshriram
Copy link
Member

rshriram commented Jan 5, 2017

I think the three steps are straightforward as long as there is no runtime, i.e., we can support intuitive way of specifying weights (33, 33, 33) and internally, they can be converted into (33, 66, 100).

The ambiguity (not problem) is that when the user decides to use the runtime later on, she would have to obey the internal format. For example, say, she wants to tweak the weights such that it is 70% to v1, 20 % to v2 and 10% to v3, using the runtime. Since the runtime does not support the intuitive format (70/20/10), she would have to specify [70, 90, <catchall>] in the runtime paths (/foo/weightedfoo/foo-default = 70, /foo/weightedfoo/foo-trial-1=90)

In order to avoid this inconsistency in weight specification, I was proposing that we maintain the same format in both static config and the runtime. i.e., x, x+y, x+y+z, <catchall>. One way of avoiding this would be to pre-process the runtime values for routing (after loading a snapshot), such that we can allow intuitive format in runtime for weighted clusters alone. Not sure if this is feasible.

@mattklein123
Copy link
Member Author

None of the above is necessary. Sorry this is becoming too complicated, I will discuss with @ccaraman and she will help you offline.

rshriram pushed a commit to rshriram/envoy that referenced this issue Oct 30, 2018
* build: HttpTemplate as separate target

* fix context build
PiotrSikora added a commit to PiotrSikora/envoy that referenced this issue Aug 2, 2020
…y#247)

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

Co-authored-by: Piotr Sikora <piotrsikora@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature requests. Not bugs or questions.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants