Skip to content

Fold a processor identity into the workflow cache key #109

Description

@conradbzura

Description

cache_key identifies the producing processor only by its version number, never by which processor it is. Two processors that claim the same (file, artifact_kind) pair at equal processor_version therefore derive an identical key and read back each other's artifacts as cache hits — a wrong answer rather than a miss.

Nothing collides today, but only by accident of the registry rather than by anything the key does. TabixIntervalProcessor and BamIndexProcessor both already declare processor_version = 2; they stay apart solely because their supported_formats are disjoint (BED/bigBed/VCF/GFF/GFF3/GTF/NarrowPeak/BroadPeak versus BAM/SAM).

PR #108 turns that latent flaw into a loaded one. Re-typing bedpe and bigInteract to minted cfdb: terms means no processor claims them, so /index returns 404 and the incorrect .tbi artifacts built for them before the re-typing are unreachable — but still cached. A paired-interval processor is both the natural next step (ENCODE-SUPPLEMENT.md promises tileset endpoints) and precisely the case that would collide with those artifacts.

A live key from the current pipeline:

encode/ENCFF732YBO/index/6fccbb438a046075cb438f84d0defe8d-v2

Nothing in it says TabixIntervalProcessor. A BedpeProcessor at version 2 producing an index for a re-typed file derives exactly the key the tabix processor used for that file, and serves a first-mate-only index as if it were correct — the original defect resurrected through a side door, and harder to spot the second time because the artifact arrives as a cache hit rather than a fresh build.

Expected Behavior

Two processors claiming the same file and artifact kind derive different cache keys regardless of their version numbers, so one can never read back the other's artifacts.

Root Cause

src/cfdb/workflows/keys.py builds the key from five components, none of which identifies the processor:

return (
    f"{normalize_dcc(dcc)}/"
    f"{normalize_local_id(local_id)}/"
    f"{artifact_kind.value}/"
    f"{normalize_md5(md5)}-v{processor_version}"
)

BaseProcessor.cache_key_for in src/cfdb/workflows/processors/base.py is the single call site and passes self.processor_version without type(self). The docstring claims bumping the version "invalidates cached outputs for the corresponding processor without affecting other processors' artifacts" — true only while each (file, artifact_kind) pair has one processor.

Fold the processor's class name or a registry-assigned id into the key. Prefer a registry-assigned id if class renames should not invalidate the cache; prefer the class name if the simplicity is worth the coupling.

Post-merge cleanup

Changing the key invalidates every cached artifact by construction — every lookup becomes a miss against the new scheme — so the stale-artifact purge belongs with this migration rather than as separate work.

Two populations to clear:

  1. Orphaned tabix artifacts for the re-typed formats. Files whose file_format.id is now cfdb:bedpe or cfdb:biginteract but which carry data/index artifacts from when they were typed BED/bigBed. Measured against a full local sync, 1,892 .bedpe and 252 bigInteract files predate PR Ingest ENCODE annotation files during sync — Closes #94 #108 in the experiment corpus, which bounds the affected set. These are unreachable today and are exactly the payload a colliding key would resurrect.
  2. Everything keyed under the old scheme, once the new key ships, since nothing will read it again.

Sequencing matters: do this before any paired-interval processor lands. Afterwards the collision is live and the stale artifacts are servable.

Expected Outcome

  • cache_key includes a processor identity, and two processors at equal processor_version claiming the same file and artifact kind derive different keys.
  • A test pins that property, so it is enforced rather than incidental.
  • The Warning: block on cache_key added in PR Ingest ENCODE annotation files during sync — Closes #94 #108 is removed or narrowed to whatever remains true.
  • Orphaned artifacts for the re-typed paired-interval files are purged, and the deploy that ships the key change accounts for a fully cold cache.

Activity

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

Metadata

Metadata

Assignees

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