Copilot commented on code in PR #731:
URL: https://github.com/apache/dubbo-go-pixiu/pull/731#discussion_r2280472859
##########
pkg/filter/llm/proxy/filter.go:
##########
@@ -280,12 +307,14 @@ func (s *Strategy) Execute(executor *RequestExecutor)
(*http.Response, error) {
endpoint.ID, endpoint.Address.GetAddress(),
err, resp.Status)
}
- // 5. If we are here, all retries for the current endpoint are
exhausted.
+ // 6. If we are here, all retries for the current endpoint are
exhausted.
// Get the next endpoint for fallback. The loop will terminate
if it's nil.
+ endpoint.Metadata["LLMUnhealthy"] = "true"
+ endpoint.Metadata["HealthyCheckTime"] =
time.Now().Format(time.RFC3339)
Review Comment:
The health check metadata keys are hardcoded strings repeated throughout the
code. Consider defining constants for "LLMUnhealthy" and "HealthyCheckTime" to
improve maintainability and reduce the risk of typos.
```suggestion
endpoint.Metadata[LLMUnhealthyKey] = "true"
endpoint.Metadata[HealthyCheckTimeKey] =
time.Now().Format(time.RFC3339)
```
##########
pkg/filter/llm/proxy/filter.go:
##########
@@ -237,24 +247,41 @@ func (s *Strategy) Execute(executor *RequestExecutor)
(*http.Response, error) {
err error
)
- // 1. Pick initial endpoint from the cluster
+ // 1. Pick initial endpoint from the cluster based on load balancing.
endpoint := executor.clusterManager.PickEndpoint(executor.clusterName,
executor.hc)
// 2. The main fallback loop. It continues as long as we have a valid
endpoint to try.
for endpoint != nil {
logger.Debugf("[dubbo-go-pixiu] client attempting endpoint [%s:
%v]", endpoint.ID, endpoint.Address.GetAddress())
- // 3. Dynamically load the retry policy for the current endpoint
+ // 3. Check the health of current endpoint,
+ if unhealthy, ok := endpoint.Metadata["LLMUnhealthy"]; ok &&
unhealthy == "true" {
+ // check the health cooldown time
+ if t, ok := endpoint.Metadata["HealthyCheckTime"]; ok {
+ lt, err := time.Parse(time.RFC3339, t)
+ if err == nil && time.Since(lt) <
time.Millisecond*time.Duration(endpoint.LLMMeta.HealthCheckInterval) {
Review Comment:
The time duration calculation
`time.Millisecond*time.Duration(endpoint.LLMMeta.HealthCheckInterval)` is
performed on every health check. Consider pre-calculating this duration or
caching it to avoid repeated multiplication operations.
```suggestion
// Pre-calculate health check cooldown duration for this
endpoint
healthCheckCooldown := time.Millisecond *
time.Duration(endpoint.LLMMeta.HealthCheckInterval)
// 3. Check the health of current endpoint,
if unhealthy, ok := endpoint.Metadata["LLMUnhealthy"]; ok &&
unhealthy == "true" {
// check the health cooldown time
if t, ok := endpoint.Metadata["HealthyCheckTime"]; ok {
lt, err := time.Parse(time.RFC3339, t)
if err == nil && time.Since(lt) <
healthCheckCooldown {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
