AdguardTeam / AdguardTeam/AdGuardHome

biased average upstream response time when optimistic caching is enabled

Abierto
#8,435 3 comentarios 2 reacciones 1 asignado Reclamado por @EugeneOne1 Ver en GitHub
feature request P4: Low
Lenguaje dominante
TypeScript
Estrellas
36.9k
Forks
2.5k
Métricas de merge de PR
Sin PR fusionados en 30 d

Descripción

### 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.

Guía de contribución

Abrir la guía de contribución

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.