The metrics refresh time might be much larger than the refreshMetricsInterval

[spacewander]

In the current implementation, the pod metrics get refreshed for each refreshMetricsInterval + the time of refreshMetrics:
https://github.com/kubernetes-sigs/llm-instance-gateway/blob/918960cf7f4b2d53d912e6a08b7378f20d20bef7/pkg/ext-proc/backend/provider.go#L82-L87

As the fetchMetricsTimeout is 5s, when the refresh is aborted because of the timeout that happened in one of the requests, the time of refreshMetrics will be 5s. This time is much larger than the refreshMetricsInterval. As a result, stale info would be used for seconds.

One of the solution may be using a separate refresh loop per pod, so a timed-out pod metrics refresh request would not affect other requests. And we may drop the pod which can't provide up-to-date metrics.

[spacewander]

Friendly ping @liu-cong @ahg-g
What do you think about that?

[liu-cong]

Hi @spacewander , thanks for raising this! I think using a separate refresh loop per pod makes sense.

[spacewander]

@liu-cong
I rethought the per-pod solution. As the pod is added dynamically, per-pod fetch with a fixed refresh interval can't guarantee the each pod's fetched metrics are in the same time range. So, I just ensure the fetch time is within the time interval without changing the process: #207

[liu-cong]

per-pod fetch with a fixed refresh interval can't guarantee the each pod's fetched metrics are in the same time range.

Right, but this is fine? Can you elaborate why this is bad?

[danehans]

@spacewander when you have a moment, please respond to @liu-cong's question.

[kfswain]

/assign spacewander

[spacewander]

@spacewander when you have a moment, please respond to @liu-cong's question.

I have discussed this with @liu-cong in #207 (comment).

And now we agree that this consideration won't be a problem anymore.