[Regression on closed #463] TtlCache.invalidate() is still lost when it lands during an in-flight load, and the spec that covers it asserts nothing

Abierto Apto para principiantes
  • ¿Algo no está bien?
#739 0 comentarios 0 reacciones 0 asignados Ver en GitHub

Los mantenedores suelen responder en 1 día

Nadie ha tomado este issue todavía.

Evaluación

Dificultad
2/5
Tiempo estimado
1-3 horas
Aptitud para principiantes
85/100
Tipo de issue
Error
Claridad
Bien especificado
Estado de actividad
Activo
Stack tecnológico
typescript
Área
backend, testing

Línea de trabajo

Lee src/analytics/ttl-cache.ts y el caso de invalidación durante una carga en curso en src/analytics/ttl-cache.spec.ts. Reproduce la condición de carrera iniciando una carga controlada, invalidando antes de que se resuelva y luego llamando a getOrLoad con un loader diferente y sin una segunda invalidación. Actualiza la caché para que ese resultado no se almacene, y haz que la spec demuestre que se ejecuta el loader nuevo y se devuelve su valor.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

bug testing

Location — src/analytics/ttl-cache.ts:11 (invalidate) and src/analytics/ttl-cache.ts:15-33 (getOrLoad, specifically the write at line 26 this.entry = { data, expiresAt: Date.now() + this.ttlMs };). Consumer: src/analytics/analytics.service.ts:91 (invalidatePlatformSummary, the @OnEvent(ANALYTICS_PLATFORM_INVALIDATE_EVENT) handler).

Problem — The lost-invalidation race closed as #463 is present again. invalidate() clears this.entry but has no way to tell an already-running loader that its result is stale, and the loader unconditionally publishes its result into this.entry when it resolves:

invalidate(): void {
  this.entry = null;
}

this.inflight = (async () => {
  try {
    const data = await loader();
    this.entry = { data, expiresAt: Date.now() + this.ttlMs };   // overwrites the invalidation
    return data;
  } finally {
    this.inflight = null;
  }
})();

There is no generation counter, epoch, or "invalidated while loading" flag. So if a bounty is created or paid (which emits ANALYTICS_PLATFORM_INVALIDATE_EVENT) while loadPlatformSummary() is awaiting its three Postgres aggregates, the invalidation is silently discarded and the pre-write snapshot is cached for a full TTL (ANALYTICS_PLATFORM_SUMMARY_TTL_MS, default 60s).

Why it matters — The invalidation exists precisely so the public homepage numbers update immediately after a bounty is funded or paid. The race window is the duration of the aggregate query, which is the window in which bounty writes are most likely (the write that emits the event is concurrent with homepage traffic by definition). The result is GET /analytics/platform serving totalBounties / totalPaidOut that are provably behind committed data for up to a minute, with no way for a client to force a refresh.

The test that is supposed to cover this does not: src/analytics/ttl-cache.spec.ts:55 ('handles invalidate while load is in flight') invalidates during the load, then at line 73 calls cache.invalidate() a second time before the next getOrLoad, so the assertion that the loader runs again passes regardless of whether the first, in-flight invalidation had any effect. Its own comment ("Invalidate should have cleared entry, or subsequent call fetches if expired/invalidated") concedes the test does not distinguish the two. That vacuous test is why #463 could be closed with the bug intact.

Reproduction / failure path

  1. In a unit test: const c = new TtlCache<string>(60000); start getOrLoad with a loader you control; call c.invalidate(); resolve the loader with 'stale'.
  2. Without a second invalidate(), call c.getOrLoad(loaderReturning('fresh')).
  3. It resolves to 'stale' and the second loader is never invoked. Expected: 'fresh'.
  4. End to end: issue GET /analytics/platform on a cold cache and, while the query is in flight, create and pay a bounty. The next 60s of homepage responses omit it.

Suggested fix

  • Add a monotonic private generation = 0; increment it in invalidate(); capture its value when the load starts and only assign this.entry if the captured value still equals the current generation (otherwise return the data to the waiting callers but do not cache it).
  • Rewrite the spec so the in-flight case is asserted directly: invalidate during the load, resolve the loader, then call getOrLoad with a different loader and assert the second loader ran and its value was returned — with no extra invalidate() call in between.
Lenguaje dominante
TypeScript
Estrellas
2
Forks
90
Merge medio
31 min
PR fusionados (30 d)
20

Preparar el entorno

  • Incluye un Dockerfile o un archivo de Docker Compose
  • Sin plantilla de pull request
  • Sin guía de contribución

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de MergeFi/backend

Todos los issues de MergeFi/backend

Issues similares

Más issues de TypeScript

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.