Cache optimized routing ("PrefixHash" load balancing - i.e. CHWBL) #333
Conversation
api/v1/model_types.go
Outdated
| @@ -144,6 +148,33 @@ type Adapter struct { | |||
| URL string `json:"url"` | |||
| } | |||
|
|
|||
| type LoadBalancing struct { | |||
| Strategy LoadBalancingStrategy `json:"strategy"` | |||
| CHWBL CHWBL `json:"chwbl"` | |||
There was a problem hiding this comment.
nit, prefer to use the full name instead of abbreviations that most people don't understand. It's fine if it's long.
There was a problem hiding this comment.
Changed it to PrefixHash since this is a user-facing API and thats probably easier to understand.
nstogner
changed the title
| var buf bytes.Buffer | ||
| mw := multipart.NewWriter(&buf) | ||
| // Keep the same boundary as the initial request (probably not necessary) | ||
| mw.SetBoundary(boundary) |
There was a problem hiding this comment.
nit: handle error. SetBoundary could fail or cause unexpected behavior
|
|
||
| r.LoadBalancing = model.Spec.LoadBalancing | ||
|
|
||
| if r.LoadBalancing.Strategy == v1.PrefixHashStrategy && r.bodyPayload != nil { |
There was a problem hiding this comment.
Good catch, will push a fix with tests
|
|
||
| type Request struct { | ||
| Body []byte | ||
| bodyPayload map[string]interface{} |
There was a problem hiding this comment.
Thats a big problem, good catch
| ) | ||
|
|
||
| type Request struct { | ||
| Body []byte |
There was a problem hiding this comment.
Would be nice to avoid buffering but clearly out of scope for this PR.
There was a problem hiding this comment.
We always store the body to facilitate retries at the moment. It would probably be nice to be able to disable this.
| // endpoint is found with acceptable load. | ||
| defaultEndpoint = &ep | ||
| } | ||
| if chwblLoadOK(ep.inFlight.Load(), g.totalInFlight.Load(), len(g.endpoints), loadFactor) { |
There was a problem hiding this comment.
nit: this can be done before setting the default endpoint.
There was a problem hiding this comment.
The default endpoint is the endpoint that is able to serve the request (has the adapter) but might not meet the load requirement after all other endpoints have been checked.
Adding comment.
| } | ||
| } | ||
|
|
||
| i++ |
There was a problem hiding this comment.
personal preference: i = (i + 1) % len(g.chwblSortedHashes) // wrap around
| right := len(g.chwblSortedHashes) - 1 | ||
| for left <= right { | ||
| middle := (left + right) / 2 | ||
| if g.chwblSortedHashes[middle] == val { |
There was a problem hiding this comment.
nit: read g.chwblSortedHashes[middle] just once.
There was a problem hiding this comment.
Good recommendation, done
| return true | ||
| } | ||
|
|
||
| avgLoad := float64(totalLoad+1) / float64(n) |
There was a problem hiding this comment.
can you add a comment why +1 here and below
nstogner
changed the title
| Replication int `json:"replication,omitempty"` | ||
| // PrefixCharLength is the number of characters to count when building the prefix to hash. | ||
| // +kubebuilder:validation:Optional | ||
| // +kubebuilder:default=100 |
There was a problem hiding this comment.
Is this ignoring the system prompt when using chat completion?
Implementation of proposal: #314
.spec.loadBalancingfield to ModelPrefixHash(i.e. "Consistent Hashing with Bounded Loads" - CHWBL) load balancing strategyendpointspackage toloadbalancermodelscalerpackage tomodelclientmodelproxyandmessengerand intoapiutilsas a shared library34%improvement in time per generated token using PrefixHash over LeastLoad in specific circumstancesTODO:
PrefixHashthe default strategy in the future if benchmarks look good