AdguardTeam / AdguardTeam/AdGuardHome
biased average upstream response time when optimistic caching is enabled
- Linguagem predominante
- TypeScript
- Estrelas
- 36.9k
- Forks
- 2.5k
- Métricas de merge de PRs
- Nenhum PR com merge em 30d
Descrição
### Prerequisites
- [x] I have checked the [Wiki](https://github.com/AdguardTeam/AdGuardHome/wiki) and [Discussions](https://github.com/AdguardTeam/AdGuardHome/discussions/categories/q-a) and found no answer
- [x] I have searched other issues and found no duplicates
- [x] I want to report a bug and not [ask a question or ask for help](https://github.com/AdguardTeam/AdGuardHome/discussions/categories/q-a)
- [x] I have set up AdGuard Home correctly and [configured clients to use it](https://github.com/AdguardTeam/AdGuardHome/wiki/Clients). (Use the [Discussions](https://github.com/AdguardTeam/AdGuardHome/discussions/categories/q-a) for help with installing and configuring clients.)
### Platform (OS and CPU architecture)
Linux, ARMv7
### Installation
Custom package (OpenWrt, HomeAssistant, etc; please mention in the description)
### Setup
On one machine
### AdGuard Home version
0.107.74
### Action
I enabled optimistic caching in AGH.
### Expected result
I expected that "Average upstream response time for the last 24 hours" on my AGH dashboard (i.e., existing functionality) would calculate an unbiased average that represents all DNS query results used by AGH.
### Actual result
The response times are abnormally too high (i.e., too slow) by the order of ballpark 2x to 4x.
### Additional information and/or screenshots
When AGH processes a request that is a cache hit, it performs a normal DNS lookup upstream: these are the only kinds of requests that are used to calculate the average. This works fine when optimistic caching is disabled in the configuration.
When optimistic caching is enabled and AGH processes an optimistic cache hit, the AGH server returns an answer immediately while starting a background thread to refresh the stats. However, the results of this DNS query are not used to calculate the "average upstream response time."
I'm not familiar with your codebase or go, but the issue may be here involving the cloned context.
```go
if dctxCache.optimistic && expired {
go p.shortFlighter.resolveOnce(minCtxClone, key, p.logger)
}
```
And maybe here because `IsCached` is true.
```go
func (u *unit) add(e *Entry) {
for _, s := range e.UpstreamStats {
if s.IsCached || s.Error != nil {
continue
}
// ... record upstream response time
}
}
```
The reason that the AGH dashboard shows strangely high averages is because the two scenarios involve different populations of hostnames.
- Optimistic caching hits tend to be hostnames that are common both on AGH's LAN but also universally common. For example: `google.com`, `windowsupdate.com`, `itunes.apple.com`. The upstream has these cached too, so upstream processes them fast
- AGH cache misses tend to be rarer hostnames like `duokao.net` and `dwr.virginia.gov`. Upstream probably does not have these cached, so it responds slower.
The statistics calculated with current behavior neither serves to answer either of these useful questions:
- How fast are upstreams?
- How long are downstreams waiting for AGH to resolve queries?
Therefore, the current statistics are more harmful than helpful.
Guia de contribuição
Avaliação
Esta issue ainda não foi avaliada.