HarperFast / HarperFast/harper

Table background work (expiration timer, delete/cleanup callbacks) not disposed when a database's stores are closed

Open
#1,811 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

## 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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.