HarperFast / HarperFast/harper
Table background work (expiration timer, delete/cleanup callbacks) not disposed when a database's stores are closed
- Dominant language
- JavaScript
- Stars
- 89
- Forks
- 10
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 200
Description
## Summary
When a database's stores are closed and later reloaded, per-table background work is not torn down. This is pre-existing (it predates the RocksDB backup/restore work) and was surfaced by a cross-model review of that change.
## Details
Two code paths close and reload a loaded database in place:
- `resetDatabases()` in `resources/databases.ts` — invoked on **every schema change** via the ITC schema handler (`server/itc/serverHandlers.js`).
- `closeDatabase()` / `closeLoadedDatabases()` in `resources/databases.ts` — the `restore_backup` flow and job-worker exit.
Neither disposes table-level background work before/after closing the stores. In particular:
- The record-expiration eviction timer — `setInterval(...)` in `runRecordExpirationEviction` (`resources/Table.ts`, ~line 5537, thread 0 only) — is not stored in a handle and never cleared.
- Delete-removal / cleanup callbacks registered against the audit store (`addDeleteRemoval` → `auditStore.addDeleteRemovalCallback`, `resources/Table.ts` ~line 5528) are not deregistered on close.
## Impact
After a close, these timers/callbacks keep referencing the now-closed stores (erroring on the next tick, or silently retaining them); a subsequent reload can register a second set. Bounded, but real — most likely to accumulate on instances with frequent schema changes and tables that use expiration/eviction.
## Proposed fix
Add an idempotent table-disposal method that clears the table's timers/intervals and deregisters its delete/cleanup callbacks, and invoke it before closing the stores. Wire it into both `resetDatabases()` and `closeDatabase()` so both close paths are covered.
## References
- `resources/Table.ts` — `runRecordExpirationEviction` (expiration interval), `addDeleteRemoval` (delete-removal callback)
- `resources/databases.ts` — `closeDatabase`, `closeLoadedDatabases`, `resetDatabases`
_Surfaced via cross-model review (codex) of the RocksDB backup/restore change; filed by Claude._
Contributor guide
Research direction
Start by reading runRecordExpirationEviction and addDeleteRemoval in resources/Table.ts, then trace closeDatabase, closeLoadedDatabases, and resetDatabases in resources/databases.ts. Verify how the ITC schema handler and restore_backup flow reach those close paths. Done means table background timers and delete/cleanup callbacks are disposed idempotently before stores close in both paths, without duplicate work after reload.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript, nodejs, typescript
- Domain
- backend, database
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 62/100