HarperFast / HarperFast/prerender-plugin

Detached sweeps keep run state per-worker, so the guard doesn't guard, the result is unreadable, and the purge's cancel can miss

Abierto
#102 0 comentarios 0 reacciones 0 asignados Ver en GitHub
Lenguaje dominante
JavaScript
Estrellas
0
Forks
0
Merge medio
9 h 39 min
PR fusionados (30 d)
39

Descripción

## Problem

The detached admin sweeps keep their run state in **module scope**:

```js
let running = false;
let lastRun = null;
```

Harper components load per **worker thread**, so this state is per-worker, not per-node. Two
consequences, both observed on a production cluster on the first real use (2026-08-13):

1. **The overlap guard doesn't guard.** `isOrphanSweepRunning()` only knows about the worker answering
the request. A second POST that lands on a different worker reports `alreadyRunning: false` and
starts a *second* concurrent sweep on the same node. Each sweep is a full walk of the target
registry (~1.2M rows there), so this doubles an already-expensive scan.

2. **The result is unreadable.** `getLastOrphanSweep()` is likewise per-worker, so `lastRun` reflects
only what *that* worker last did. Observed: one node returned `{"lastRun": null, "alreadyRunning":
false}` while a sweep was in fact running on another worker, and the completed run's summary was
only ever visible in the log. An operator polling the endpoint for the outcome can wait forever on a
node that has already finished.

For a **destructive** sweep this matters more than it does for a restorative one: the operator has to
read "how many did it delete, and is it still going" before deciding whether to run again — and the
answer they get depends on which worker fielded the request.

## Three call sites now, not one — updated 2026-09-16

The pattern has since been copied into a second destructive sweep, so this is worth fixing once,
properly, rather than per site:

| module | state | destructive | operator polls it for progress |
|---|---|---|---|
| `util/orphanSweep.js` | `running`, `lastRun` | **yes** | yes |
| `util/discoveredPurge.js` (added v0.54.0) | `state`, `cancelRequested` | **yes** | yes — and it also takes a **cancel** |
| `util/reconcile.js` | `running`, `lastRun` | no | no |

`discoveredPurge` is the one that makes this more urgent than when it was filed. It is a paced bulk
delete under an absolute URL prefix, it exposes live progress on `GET /prerender_admin/discovery-purge`
for the operator to watch, and `{ action: 'stop' }` is a **cooperative cancel** — `stopDiscoveredPurge`
sets `cancelRequested` in the worker that received the stop. A cancel that lands on a different worker
than the running purge therefore reports the purge as not running and **does not stop it**. Same defect
as the orphan sweep's guard, but on the control that exists to halt a bulk delete.

`reconcile.js` still has the same shape and still doesn't need urgency: it is periodic and restorative,
a duplicate pass is harmless, and no operator gates a decision on its result. Fix it with the others
because it is the same three lines, not because it is a risk.

## Current workaround

Read the summary from `hdb.log` (`[prerender] orphan sweep ...`) rather than from the API response, and
don't re-POST to a node while one is running. For a purge, confirm from the log that a stop actually
took effect rather than trusting the endpoint's answer.

## Fix

Move both to node-shared state, the way queue state already does it — the coordination shared buffer,
or a small row. Requirements:

- `running` has to be a genuine cross-worker mutex, not an advisory flag, or two workers can still
interleave between check and set.
- `lastRun` (and the purge's live progress) should be readable from any worker on the node.
- The purge's **cancel** has to reach the worker actually running it, not only set a flag locally.
- Whatever holds `running` must not strand it if the worker that set it dies mid-sweep (an expiry, or
ownership tied to something observable), otherwise one crashed sweep locks the node out permanently.

## Not in scope

Cross-NODE coordination. These sweeps are deliberately node-scoped — the in-flight lease check is only
authoritative on the owner — so every node running its own is correct and should stay that way.

Introduced in #100 (v0.48.0); `discoveredPurge` inherited the pattern in #127 (v0.54.0).

Guía de contribución

Abrir la guía de contribución

Línea de trabajo

Start by reading util/orphanSweep.js, util/discoveredPurge.js, and util/reconcile.js, then inspect the existing queue-state coordination buffer or row. Trace the sweep status endpoints and discoveredPurge's stopDiscoveredPurge path. Done means node-shared mutex, progress, results, and cancellation work across workers without stranding state after a worker dies, while coordination remains node-scoped.

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

Evaluación

Stack tecnológico
javascript
Área
api, backend, distributed-systems
Tipo de issue
Error
Dificultad
5/5
Tiempo estimado
Más de una semana
Estado de actividad
Tranquilo
Claridad
Bastante claro
Aptitud para principiantes
45/100

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.