registry-client: cancelled/failed downloads leak hypercore blocks in corestore (never cleared)

[a-mium]

Summary

@qvac/registry-client leaks hypercore blocks into the local corestore whenever a download is cancelled or fails. The blocks a download pulls into the RocksDB-backed corestore are only ever cleared on the success path; the catch (error) path closes the core and re-throws without clearing them. Because the leaked blocks are logically live (not tombstoned), RocksDB compaction never reclaims them, so the corestore grows without bound across distinct cancelled model loads.

Root cause

File: packages/registry-server/client/lib/client.js

Both downloadModel and downloadBlob download a hypercore block range into the corestore, stream it out, and on success call _clearBlobBlocks(core, blockStart, blockEnd) — which does core.clear(start, end, { diff: true }) + core.compact() — to free the cached blocks.

_clearBlobBlocks is only reached on success paths:

• downloadModel: the outputFile branch (lib/client.js:361) and the stream cleanup on 'end' (lib/client.js:373).
• downloadBlob: the outputFile branch (lib/client.js:495) and the stream cleanup on 'end' (lib/client.js:507).

The asymmetry is in the error paths. Both catch (error) blocks close blobs and core and re-throw, but never call _clearBlobBlocks:

• downloadModel catch: lib/client.js:400-419 — closes blobs/core, throw error, no clear.
• downloadBlob catch: lib/client.js:527-542 — closes blobs/core, throw error, no clear.

So when a load is cancelled mid-download (the abort signal in _streamBlobToFile rejects with Download cancelled, lib/client.js:620/626) or fails after exhausting retries, the partially-downloaded blocks are left live in the corestore forever.

Note: the block range (blockStart/blockEnd) is currently computed with const inside the try (lib/client.js:340-341 and 471-472), so it is not even in scope in the catch — the declarations need hoisting for the catch to clear the right range.

Impact

Unbounded corestore disk growth across distinct cancelled/failed model loads. The assembled output file in the models dir is cleaned up correctly by a separate clearCache path — only the corestore blocks leak, so it is easy to miss (the visible models cache stays empty while the on-disk db dir keeps growing).

Empirical measurements

Measured against a running service on the downstream @qvac/sdk:

• Cancelling three distinct models mid-download added +74 MB, +36 MB, +59 MB to the corestore db dir — monotonic — while the models cache dir stayed empty.
• Cancelling the same model repeatedly did not grow monotonically: it re-fetches the same content-addressed block range, which churns and later compacts. This is consistent with the root cause — the leak is per distinct block range that is never cleared.

Fix

In the catch (error) of both downloadModel and downloadBlob, clear the partial block range before closing the core (guarded for the case where the range was not yet computed), symmetric with the success paths. PR to follow.

[victorchimakanu]

Verified against the current source. In both downloadModel and downloadBlob, _clearBlobBlocks runs only on the success paths, at lib/client.js lines 361 and 373 for downloadModel and lines 495 and 507 for downloadBlob. Both catch blocks close blobs and core and rethrow without clearing, and blockStart and blockEnd are declared inside the try at lines 340 and 471, so the catch cannot reach them as written. Your measurements match that read. Each distinct cancelled download leaves a new live block range behind, while repeating the same model reuses the same content-addressed range and later compacts.

One note for the PR (#3219). The outputFile path in downloadModel keeps cached blocks across retries on purpose, so a retry can re-stream cheaply. The comment above the withRetry call spells that out. The catch only runs after retries are exhausted, so clearing there does not break that behavior, but the clear should stay out of beforeRetry. A guard for a failure that happens before the block range is computed is also needed, which your hoisting note already covers.

Flagged to the team for review.