HarperFast / HarperFast/harper

Reads served from a partially-built index return 200 with missing rows: isIndexing diverges across threads

Open
#2,537 1 comment 0 reactions 1 assignee Claimed by @kriszyp View on GitHub
Dominant language
JavaScript
Stars
89
Forks
10
Avg merge
2d 6h
Merged PRs (30d)
200

Description

## Summary

The `isIndexing` guard that makes a rebuilding index return `503 "… is not indexed yet"` instead of
serving partial results is **not consistent across threads in the same process**. On a live 5.2.7
cluster, for the same table and the same attribute at the same moment:

- worker threads had `indices..isIndexing === true` → queries correctly threw `IndexRebuildingError`;
- the main thread (and the operations thread) had `isIndexing === false` → queries used the
**partially-built** index and returned **200 with rows silently missing**.

The guard exists precisely to prevent the second behaviour, so wherever the flag doesn't propagate,
a rebuilding index becomes a silent-wrong-results surface.

## Evidence

Against a table whose index was ~9% built, on a thread with `isIndexing === false` — one record,
two lookups:

```
search_by_value matchUrl="" -> [] # 200 OK, row missing
search_by_hash id="" -> [{ id, matchUrl }] # the record is present
```

The record exists; its indexed lookup says it doesn't. No error, no warning, no indication the index
is incomplete.

Sampling both reachable inspector threads on **all 15 nodes** of the cluster gave a completely
uniform split — main thread `isIndexing: false`, worker thread `isIndexing: true`, on every node.
So this is a systematic difference between thread roles, not a race that happened once.

## Where it comes from

`resources/databases.ts` propagates the flag to newly-opened dbis with:

```js
if (attributeDescriptor?.indexingPID) dbi.isIndexing = true;
```

and the failure paths additionally re-stamp both the attribute's dbi and the currently-active one:

```js
attribute.dbi.isIndexing = true;
const activeDbi = Table.indices[attribute.name];
if (activeDbi) activeDbi.isIndexing = true;
```

but the *trigger* path sets it only on the dbi it just opened. A thread that opened the table before
the descriptor gained `indexingPID`, or that holds a different dbi object than the one stamped, keeps
`isIndexing === false` and will happily read through a partial index.

Note the guard is also consulted only at the point a condition is *executed* as the driving index
lookup (`resources/search.ts`, the `!index || index.isIndexing || …` branch). `estimateCondition`
does not consult it at all, so the planner will happily choose a lead condition whose index the
executor is about to refuse.

## Impact

Any read against a rebuilding index can return incomplete results with a 200, non-deterministically
depending on which thread serves the request. Normally this is bounded by the rebuild window; when
the rebuild is stuck it is unbounded — see the companion issue on interrupted backfills never
resuming under PID 1, which is what made this observable for hours rather than minutes.

## Suggested direction

- Derive `isIndexing` from the persisted descriptor at read time (or re-stamp every dbi in
`Table.indices` on the schema-change signal) so no thread can hold a stale `false`.
- Have `estimateCondition` treat an `isIndexing` index the way it treats a missing one, so the planner
and the executor agree on which indexes are usable.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.