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

Polish the routing source code #53

Closed
mathetake opened this issue Dec 27, 2024 · 8 comments
Closed

Polish the routing source code #53

mathetake opened this issue Dec 27, 2024 · 8 comments

Comments

@mathetake
Copy link
Member

I added the very naive implementation in #50

// Calculate implements [Router.Calculate].
func (r *router) Calculate(headers map[string]string) (backendName string, outputSchema extprocconfig.VersionedAPISchema, err error) {
var rule *extprocconfig.RouteRule
for i := range r.rules {
_rule := &r.rules[i]
for _, hdr := range _rule.Headers {
v, ok := headers[string(hdr.Name)]
// Currently, we only do the exact matching.
if ok && v == hdr.Value {
rule = _rule
break
}
}
}
if rule == nil {
return "", extprocconfig.VersionedAPISchema{}, errors.New("no matching rule found")
}
backendName, outputSchema = r.selectBackendFromRule(rule)
return
}
func (r *router) selectBackendFromRule(rule *extprocconfig.RouteRule) (backendName string, outputSchema extprocconfig.VersionedAPISchema) {
// Each backend has a weight, so we randomly select depending on the weight.
// This is a pretty naive implementation and can be buggy, so fix it later.
totalWeight := 0
for _, b := range rule.Backends {
totalWeight += b.Weight
}
if totalWeight == 0 {
return rule.Backends[0].Name, rule.Backends[0].OutputSchema
}
selected := r.rng.Intn(totalWeight)
for _, b := range rule.Backends {
if selected < b.Weight {
return b.Name, b.OutputSchema
}
selected -= b.Weight
}
return rule.Backends[0].Name, rule.Backends[0].OutputSchema
}

which I think we should refactor before v0.1.0

@mathetake mathetake added this to the v.0.1.0 milestone Dec 27, 2024
@mathetake mathetake added the help wanted Extra attention is needed label Dec 27, 2024
@theBeginner86
Copy link

Hi @mathetake,
May I help in improving this?

@mathetake
Copy link
Member Author

nah i am sorry but i would rather one of maintainers to take care of this, but thanks for the offer anyways

@liangyuanpeng
Copy link

I'm click into here because the help wanted label.

but i would rather one of maintainers to take care of this

And seems like this probably not need to add the help wanted label (:

@mathetake mathetake removed the help wanted Extra attention is needed label Dec 31, 2024
@mathetake
Copy link
Member Author

fair enough - removed the label

@wengyao04
Copy link
Contributor

@mathetake does any maintainer takes care of this issue ? If not, I can help on it

@mathetake
Copy link
Member Author

Yeah go for it

@mathetake
Copy link
Member Author

ping @wengyao04 any updates?

@wengyao04
Copy link
Contributor

ping @wengyao04 any updates?

I am testing out the ai-gateway within BB for all the integration, since there are lots of change after we switch to open source version, we have to fix our components to fit this change. Sorry I don't have time this week, and pick it up next week.

@mathetake mathetake removed this from the v.0.1.0 milestone Jan 30, 2025
davidxia added a commit to davidxia/ai-gateway that referenced this issue Feb 21, 2025
Right now if there are multiple, unweighted `backendRefs` in a
[AIGatewayRouteRule], the first one is always returned. The correct behavior is
to return a random one since they're essentially all weighted the same.

[AIGatewayRouteRule]: https://envoy-ai-gateway.netlify.app/docs/api/#aigatewayrouterule

contributes to envoyproxy#53

Signed-off-by: David Xia <[email protected]>
davidxia added a commit to davidxia/ai-gateway that referenced this issue Feb 21, 2025
Right now if there are multiple, unweighted `backendRefs` in a
[AIGatewayRouteRule], the first one is always returned. The correct behavior is
to return a random one since they're essentially all weighted the same.

[AIGatewayRouteRule]: https://envoy-ai-gateway.netlify.app/docs/api/#aigatewayrouterule

contributes to envoyproxy#53

Signed-off-by: David Xia <[email protected]>
davidxia added a commit to davidxia/ai-gateway that referenced this issue Feb 21, 2025
Right now if there are multiple, unweighted `backendRefs` in a
[AIGatewayRouteRule], the first one is always returned. The correct behavior is
to return a random one since they're essentially all weighted the same.

[AIGatewayRouteRule]: https://envoy-ai-gateway.netlify.app/docs/api/#aigatewayrouterule

contributes to envoyproxy#53

Signed-off-by: David Xia <[email protected]>
mathetake pushed a commit that referenced this issue Feb 21, 2025
**Commit Message**

Right now if there are multiple, unweighted `backendRefs` in a
[AIGatewayRouteRule], the first one is always returned. The correct
behavior is to return a random one since they're essentially all
weighted the same.

[AIGatewayRouteRule]:
https://envoy-ai-gateway.netlify.app/docs/api/#aigatewayrouterule

**Related Issues/PRs (if applicable)**

contributes to #53

Signed-off-by: David Xia <[email protected]>
mathetake added a commit that referenced this issue Feb 24, 2025
**Commit Message**

Previously, router instance shared the rng among multiple goroutines
which is not a thread safe. This fixes it by having a goroutine local
RNG.


**Related Issues/PRs (if applicable)**

Contributes to #53

---------

Signed-off-by: Takeshi Yoneda <[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

No branches or pull requests

4 participants