HarperFast / HarperFast/harper

Skip the component-directory lock in symlinkHarperModule when the harper module links are already valid

Open
#2,376 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
89
Forks
10
Avg merge
2d 6h
Merged PRs (30d)
200

Description

## What

Every non-root component load calls `symlinkHarperModule()`, which unconditionally goes through `Status.primaryStore.tryLock()` on the component directory — even when the `node_modules/harper` (and `node_modules/harperdb`) links are already correct, which is the overwhelmingly common case after the first worker has run.

On a multi-worker startup that means N workers × M app components all serialize on a per-directory cross-thread lock to perform a check that, for all but one of them, finds nothing to do.

[`components/componentLoader.ts:389`](https://github.com/HarperFast/harper/blob/main/components/componentLoader.ts#L389) (call site at [line 665](https://github.com/HarperFast/harper/blob/main/components/componentLoader.ts#L665)).

## Proposed change

Validate first, lock only to repair:

```ts
export function symlinkHarperModule(componentDirectory: string) {
if (harperModulesAreLinked(componentDirectory)) return Promise.resolve();
// ... existing tryLock / repair / unlock path
}
```

where `harperModulesAreLinked()` is a synchronous `lstatSync` + `realpathSync` check against `PACKAGE_ROOT`, and the repair body is extracted to a `repairHarperModuleLinks()` helper. This trades a small amount of synchronous metadata I/O per component for removal of the lock round-trip entirely on the steady-state path.

## Provenance

Written and reviewed as part of the now-closed #2343, which lost to #2314 on the shared startup-liveness fix (merged as `635fbd5387386cd4296ec56e25df8d68931da467`). The lock-free fast path is the one part of #2343 that #2314 does not contain, so it is filed here rather than left to die with the branch. The implementation exists on `fix/keep-workers-alive-until-ready` and can be lifted, but it should be rebuilt against post-#2314 `main` rather than cherry-picked — the surrounding function changed in #2314.

Two review conclusions from that branch, recorded so they are not re-derived:

- Comparing against bare `PACKAGE_ROOT` instead of `realpathSync(PACKAGE_ROOT)` is safe — it is already resolved at its definition in [`utility/packageUtils.js:47`](https://github.com/HarperFast/harper/blob/main/utility/packageUtils.js#L47). No need to pay a second `realpathSync` per check.
- Whether the waiter's lock timeout resolves or rejects is not load-bearing for startup — the sole call site already catches and logs, so neither outcome can fail a worker.

## Not in scope

The contended-repair path itself. A waiter that hits the 10s timeout still continues with the link in whatever state it is in, and a repair owner that dies mid-repair is still not retried by the waiter. #2314 made that path safe (bounded, non-fatal, no cross-thread unlock of a lock this thread never held), but it did not make it recoverable. Stale-owner reclamation needs its own design and is deliberately excluded here.

## Verification this needs

The claim is a contention reduction, and it is currently unmeasured — that is the main reason this is filed rather than merged. Whoever picks it up should land a before/after number for concurrent worker startup with several app components, not just a green test run. If the win doesn't show up, closing this is a fine outcome.

Contributor guide

Open the contributing guide

Research direction

Start in components/componentLoader.ts at symlinkHarperModule() around line 389 and its call site near line 665; review utility/packageUtils.js:47 for PACKAGE_ROOT and the existing lock and repair flow. Rebuild the validation-first path against current main, then measure concurrent worker startup with several app components before and after. Done means the steady-state path avoids the lock and the contention reduction is quantified.

Written by the indexing model from the issue text.

Assessment

Tech stack
nodejs, typescript
Domain
backend, performance
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
62/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.