Skip to content

_ShardLease.load holds every shard of a target in memory at once — the forecast leg's #126 #269

Closed

Description

@Polichinel

The defect

_ShardLease.load materialises every shard of a target before decoding any of them:

def load(self):
    """Fetch (by pinned id), verify, curate — return the PRODUCT ``(frame, headers)``."""
    shard_bytes = {
        name: self.store.download(file_id)
        for name, file_id in self.shard_file_ids.items()
    }
    frame, headers = track_a_source.frames_for_target(self.manifest, shard_bytes)

The dict comprehension completes before frames_for_target is called, so all 36 monthly
shards are resident simultaneously, and the decoded frame is built on top of them.

This compounds with the manager: CRAFDPostProcessorManager holds self._historical_frame
— 28,421,738 rows over 64,742 cells for the CRAF'd delivery — from _read through the whole
of _save. Peak is the sum: historical frame + every shard's bytes + the decoded frame.

Measured

First un_crafd delivery attempt, 2026-08-13 (views-crafdapi#44). Same code, same data,
two runs:

Run Machine state Outcome
17:43 15 GB already in use, swap exhausted (1.8 of 2.0 GB) died in the shard download
18:08 memory freed, nothing else changed completed, staged cleanly

dmesg shows the kernel OOM-killing a 14.6 GB process minutes later. No code changed
between the two runs.

For scale on the consumer side: views-crafdapi's preflight assembles the same run and peaks
at 4.9 GB RSS — and it streams. This delivery path does not.

The repo has met this before, on the other leg

#126 — "un_fao: retire the pandas historical path — deliver frame-native end-to-end
(fixes run-0 OOM)"
— and #131, which recorded the result as "OOM gone".

That fixed the historical leg. The forecast shard leg has the same
everything-resident shape and was never revisited. CRAF'd's historical is already
frame-native (the manager logs "Historical is frame-native"), which is precisely why the
remaining pressure is the shard dict rather than the frame.

This is a design change, not a patch

track_a_source.frames_for_target(manifest, shard_bytes) takes the whole mapping, so
streaming means changing that signature or introducing an iterator between them. Worth
scoping deliberately rather than reaching for the smallest edit.

A working precedent for the shape exists on the consumer side —
views-crafdapi/src/views_crafdapi/preflight.py::_assembly_gate walks the same run
month-by-month, releasing each month's state before the next, and completes on the full
production run in 4.9 GB. Not code to import (different repo, different purpose), but the
traversal it uses is the one this path needs.

Acceptance

  • A target's shards are decoded incrementally; the whole run's bytes are never resident at once.
  • The historical frame is released, or not held, across the forecast delivery phase.
  • Measured peak RSS for a full 3-target / 36-month / 64,742-cell delivery is recorded in
    the issue, so the next person has a number rather than an anecdote.
  • un_fao delivery still completes — this path is shared, and un_fao: retire the pandas historical path — deliver frame-native end-to-end (fixes run-0 OOM) #126's fix must not regress.

Cross-references

Filed from the views-crafdapi seat. No change was made in this repo. Per this repo's
convention ("The FILE is the claim; the line number is not"), this issue names symbols.

Activity

  1. added 2 commits that reference this issue on Aug 14, 2026
  2. Polichinel commented on Aug 14, 2026

    @Polichinel
    CollaboratorAuthor

    Fixed in #271 (70f25f0) — but measured first, and the diagnosis in this issue turned out to be one of three roughly equal causes.

    What the measurement showed

    Driving the real TargetLease.load with producer-format .tap.zip shards, tracemalloc and peak RSS agreeing within 2%, at 36 shards x 20,000 cells x 200 samples:

    before after
    peak, tracemalloc 3.06x the delivered frame 1.13x
    peak, RSS delta 3.02x 1.06x

    The three thirds: the raw bytes held together (what this issue names), the decoded per-shard frames held for the stack, and the np.concatenate copy allocated while those were alive. Fixing only the dict comprehension would have reached about 2x. The ratio held at 12, 24 and 36 shards, so it is the shape and not the scale.

    At production scale — 64,742 cells x 36 months x ~1000 samples — peak falls from about 26.6 GB to 9.8 GB per target.

    Two false starts, recorded because both produced confident wrong numbers: my first harness had the fake store return pre-built objects, so the download allocation fell outside the measurement and peak read 2.04x. The obvious correction, bytes(blob), allocates nothing — bytes(b) is b when b is already bytes, and so is b"" + b. It took a bytearray to get a real copy.

    One correction to this issue

    The class is TargetLease, not _ShardLease.

    Your second acceptance criterion is not met, and is filed rather than closed over

    This issue lists "the historical frame is released, or not held, across the forecast delivery phase." #271 does not do that. It is now #273, with the measurement attached: by its declared dimensions (64,742 cells x 438 months = 28,356,996 rows) the historical frame is about 108 MB at one float32 column — ~1.2% of a single forecast target's frame. Real, small, and worth knowing before anyone spends a day on it.

    What review caught, since you will care about the third one

    The rewrite's core shipped unverified at first: nothing in the suite assembled more than one shard. test_wire_fixture.py never touches the assembly, and the end-to-end byte-parity fixture has a single shard, so the new slot arithmetic never ran at a position above zero. There is now a three-shard test with distinct values, months and units asserting equality with np.concatenate in manifest order, mutation-proven against a reversed slot, an off-by-one, and unwritten identifiers.

    Also fixed before merge: a KeyError raised anywhere inside store.download would have been reported as "manifest lists shard X but its bytes were not provided" — blaming the manifest for a store fault, with the traceback discarded. That is C-99's substitution one layer up. The lookup and the download are now separated, and a store KeyError raises a named error chained to its cause.

    Register C-101 carries the full measurement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions