Skip to content

Replace the hard-coded oversized-IndexedDB skip with a measured per-store cap - #348

Open
dchaudhari7177 wants to merge 1 commit into
RyanDFIR:mainfrom
dchaudhari7177:feat/340-measured-indexeddb-cap
Open

Replace the hard-coded oversized-IndexedDB skip with a measured per-store cap#348
dchaudhari7177 wants to merge 1 commit into
RyanDFIR:mainfrom
dchaudhari7177:feat/340-measured-indexeddb-cap

Conversation

@dchaudhari7177

Copy link
Copy Markdown
Contributor

Closes #340.

The literal-ID match on the Ghostery store is gone. Iteration carries a per-store record budget instead, and a store that exhausts it is truncated and reported through unparsed.source.

The three decisions

The issue names them explicitly, so here is what I chose and why. All three are cheap to reverse if you would rather have it another way.

1. Metric — record count during iteration, not directory size on disk.
Record volume is the failure mode the issue describes, and it is the thing that actually costs time and memory. Counting during the walk needs no second pass, and no estimate that a freshly-compacted store (small on disk, many records) or a blob-heavy one (large on disk, few records) would throw off in opposite directions. The cap is checked before the record is built, so it bounds the work and not merely the rows kept — iterating a million-record store is the hang, and resolve_indexeddb_blob_refs on each record is the memory.

2. Behaviour — truncate with a note, not a hard skip.
This is the one I feel strongest about, because it makes the threshold much less load-bearing. It is strictly more than the rule it replaces: the Ghostery store currently yields zero records and would now yield the first 100,000. A threshold set too low costs completeness — which is reported — rather than correctness. A hard skip has no such margin.

3. Configurable — module constant, read into an instance attribute.
INDEXEDDB_MAX_RECORDS_PER_STORE = 100_000, alongside SECONDARY_CACHE_DIRS. It is read once into self.indexeddb_max_records_per_store, so a caller with a known-good corpus can raise it, or set it to None to disable the cap entirely, without patching the module. I did not wire it to a CLI flag — that felt like it wanted your call on the surface, and the attribute is enough for a programmatic caller.

On the default of 100,000: this is the number I am least able to defend from measurement, and I want to be straight about that — I do not have the corpus datasets, so I have not measured the record-count distribution of ordinary stores against it. The reasoning is only that it is an order of magnitude below the 1M+ that motivated the original skip and far above any store I would expect to be ordinary. If you have the distribution to hand, it should probably be set from that rather than from my estimate. The truncate-not-skip choice above is what makes an imperfect default survivable.

The budget is per store directory, not per run, so one huge store cannot eat the allowance of the ones after it.

Verification

Five tests driving get_indexeddb against a stubbed WrappedIndexDB. Three fail on main:

FAILED test_a_store_over_the_cap_is_truncated_not_skipped
FAILED test_the_cap_bounds_the_iteration_and_not_only_the_rows_kept
FAILED test_the_truncation_names_the_store_and_how_far_it_got

The other two pin that a store under the cap is still read whole and that a None cap disables truncation. The iteration-bound test asserts against the stub's own iterated counter, so it fails if the cap only filters rows rather than stopping the walk.

Full suite on this branch: 244 passed, 2 skipped, 80 subtests passed. Python 3.12.4.

Not done here

The issue also floats a streamed partial parse. What this does is the simple form of that — first N records plus a truncation note — rather than anything resumable. If you want a store to be readable in pages across runs, that wants a cursor in the output and is a bigger change than this issue.

…tore cap

`get_indexeddb` skipped one specific extension's `.leveldb` by literal directory
name, because that store has over a million records. The protection was
therefore arbitrary: it covered exactly one known-large store, and any other
large store still parsed fully, with the same possible hang or memory blow-up.

The literal-ID match is gone. Iteration now carries a per-store record budget,
and a store that exhausts it is truncated and reported. Three choices behind
that, all reversible:

- Measured on record count during iteration rather than directory size on disk.
  Record volume is the failure mode named in the issue, and counting during the
  walk needs no second pass and no estimate that a compacted or blob-heavy store
  would throw off.
- Truncate with a note rather than a hard skip. This is strictly more than the
  rule it replaces: the store that motivated the original skip yielded nothing,
  and now yields its first records. A threshold set too low then costs
  completeness, which is reported, rather than correctness.
- Configurable. `INDEXEDDB_MAX_RECORDS_PER_STORE` is a module constant, read
  once into an instance attribute so a caller with a known-good corpus can raise
  it, or set it to None to disable the cap, without patching the module.

The cap is checked before the record is built, so it bounds the work rather than
only the rows kept: iterating a million-record store is the hang, and resolving
each record's blob references is the memory.

The budget is per store directory, not per run, so one huge store cannot eat the
allowance of the ones after it.

Truncation is reported through `unparsed.source`, the same channel the
hard-coded skip used, so a capped store is as visible in the run's totals as an
unreadable one.

Five tests, three failing on main.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replace the hard-coded oversized-IndexedDB skip with a general, measured cap

1 participant