Repository navigation
Follow-ups from the review of #118 (concurrent database starts) #134
Description
These came out of reviewing #118. Each one was checked against main. None of them is a regression that blocks merge: they are either already on main or new code that works but could be better. Listed most important first.
Should fix
1. gemdb file.py starts the stone without assertSafeToStart's guards
src/cli.ts (file mode, startstone) checks nothing before it starts the stone: not the engine version against the extent, NFS, space limits or room for the extent. #118 added these guards to the GemDB Shell. File mode is the one way to start the stone that still skips them. This was already the case on main.
Impact: after an engine pin moves, every existing database is orphaned (see CLAUDE.md). A terminal user then gets a bare GemStone 4045 login failure instead of the "delete this directory" message. A stone is also left running that the window would have refused to start. Every pin move will cause this for anyone who runs files from a terminal.
2. Stone start/wait logic is implemented twice: TypeScript and generated bash
The stone state parsing, Startup/Shutdown handling, the lock wait and the timeouts live both in processes.ts/lock.ts and in the wrapper cli.ts generates. #118 added about 65 lines of bash to that. The copies already differ: the lock wait is 10 s in bash and 60–150 s with progress tracking in TypeScript, bash has no guards (item 1), and they handle an external database differently (item 3).
Suggestion: have file mode run ensureProcesses through the Node bundle that is already staged (for example gemdb-shell.js --ensure-stone). Keep a bash fast path for "stone at OK" so the common case doesn't pay for Node startup. This would also fix items 1 and 3.
3. External database: the wrapper waits at Startup, the window and Shell don't
The wrapper's external branch now rejects Shutdown and waits on Startup. lifecycle.ts (ensureRunning's external branch) and cliMain.ts still accept any listed stone. The [Unreleased] CHANGELOG entry says GemDB waits, which is only true for GemDB's own database. While fixing this, also pull the two copies of the external-is-down check into one assertExternalDatabaseUp().
Impact: when the administrator's stone is starting or recovering, the window or Shell login fails with a GemStone error while gemdb file.py waits. This is rare.
4. The no-lock overlap integration test can pass without an overlap
rivalStartsNetldi (src/__integration__/fixtures/processContender.ts) gives up silently after 20 s. The test in processes.test.ts never asserts that the two startnetldi runs overlapped, which side lost, or the raw side's exit code. If it doesn't overlap, the test still passes, so a return of #89 could pass CI. It should fail on timeout, and assert startedNetldi and the exit codes.
5. A stone lock carrying this process's own pid is never treated as stale
stoneLockIsStale returns !isAlive(owner), and isAlive(process.pid) is always true. A lock left by a crash, whose pid this process happens to get again (devcontainers, deterministic pids), blocks every start for a minute. #118 (ea64653) put the lock path back in that error message. The real fix is to track whether this process actually holds the stone lock, as the setup lock already does with heldHere. An own-pid lock we don't hold is then stale. The wording "This window…" is also wrong when the process is the GemDB Shell. This was already on main.
6. A stone that vanishes while we wait is reported as "stopped while it was starting"
waitForStoneReady (and the wrapper's wait_for_stone) say this even when this call didn't start the stone. For example, a stopping stone's last moments read Startup (measured). It would be better to loop back and start the stone, or at least word it as a stop. The outcome matches main (no automatic start), only the message differs, and running it again works.
Nits
- Wrapper and a killed stone:
stone_statusignores gslist's exit code, so akilledrow is treated as up for one invocation (-ccleans it, and the next run starts the stone). Treatkilledas not listed.stoneStatecould also mapkilledtoabsent. The extension is unaffected, becauselistProcessesdiscards non-zero gslist output. - Wrong log line after a declined force-stop: with the stone at
Shutdown,runStoplogs "GemDB is still running.". It should checkstoneStateforstopping. The log line is the same onmain, and the status bar is correct. $STATUSin the generated wrapper: it holds the stone's gslist status, then topaz's exit-status file. Rename the first toSTONE_STATUS.- CHANGELOG:
[Unreleased]has three### Fixedheadings. Merge them when the release is cut. ensureListenerandensureProcesses: their locked flows overlap. Consider merging them only if they change again, since their wait semantics differ.
🤖 Generated with Claude Code
Activity