HarperFast / HarperFast/harper-pro

An empty http-worker list latches worker placement to NaN, permanently stopping all subscription placement

Open Beginner friendly
#836 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
3
Forks
0
Avg merge
1d 21h
Merged PRs (30d)
80

Description

`placeSubscription` (and its two equivalents on `main`) round-robins worker placement with:

```js
nextWorkerIndex = nextWorkerIndex % httpWorkers.length; // wrap around as necessary
worker = httpWorkers[nextWorkerIndex++];
```

When `httpWorkers` is empty, `0 % 0` is `NaN`. `NaN++` leaves `nextWorkerIndex` as `NaN`, and it **never recovers**: every later call computes `NaN % n === NaN` and returns `httpWorkers[NaN]`, which is `undefined`. So a single placement attempt during a window with no http workers permanently disables subscription placement for the life of the process — including for every database whose workers are up by then.

```
$ node -e "let i=0; const w=[]; i = i % w.length; const p=w[i++]; console.log({i,p});
const w2=[{}]; i = i % w2.length; console.log({i, p: w2[i++]});"
{ i: NaN, p: undefined } # empty worker list
{ i: NaN, p: undefined } # workers exist now — still NaN
```

The empty case is expected rather than exceptional: the call site immediately below logs `No http workers available to subscribe to node`, so it is a handled state, not an assertion. After the latch that warning keeps firing with a message that is no longer true (workers *are* available), which is what makes this hard to recognise from logs.

## Where

Pre-existing on `main`, at two sites in `replication/subscriptionManager.ts` — around the reconcile placement and the subscribe placement (`nextWorkerIndex` is declared once in `startOnMainThread` and shared by both). Found while reviewing harper-pro#822, which consolidates one of them into `placeSubscription`; that PR does not introduce or change the arithmetic.

## Impact

Replication placement stops for the process. Nothing binds a new subscription, so a node that hits the window during startup or after a worker restart can sit with no inbound replication while `cluster_status` shows the node healthy. A restart clears it.

## Follow-up steps

1. Guard the empty list before the modulo, in **both** places:
```js
if (httpWorkers.length === 0) return undefined;
nextWorkerIndex = nextWorkerIndex % httpWorkers.length;
```
(Returning `undefined` is what the caller already handles — it logs and leaves the entry unplaced for the next reconcile.)
2. Add a unit case to `unitTests/replication/` asserting that a placement call with an empty worker list, followed by one with a non-empty list, returns a real worker — that is the regression, and it fails today.
3. Worth a quick grep for the same `% length` shape elsewhere in the replication placement paths.

🤖 Filed by Claude Opus 5 on behalf of Kris.

Contributor guide

Open the contributing guide

Research direction

Start in replication/subscriptionManager.ts at the reconcile and subscribe placement sites, then inspect how startOnMainThread shares nextWorkerIndex. Add a regression case under unitTests/replication/ that places with an empty worker list and then a non-empty list; done means the later call returns a real worker. Grep replication placement paths for other modulo-by-length cases.

Written by the indexing model from the issue text.

Assessment

Tech stack
node.js, typescript
Domain
databases, distributed-systems
Issue type
Bug
Difficulty
2/5
Estimated time
Half a day
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
88/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.