Importing a campaign is a background job read by the worker (#1054) #1104

Merged
claude-bot merged 8 commits from feat/1054-import-in-the-worker into main 2026-10-05 11:58:28 +00:00
Collaborator

Closes #1054 (D281, the owner's answer to Q153). Not to be merged before the independent review reports.

Summary

  • The upload arrives as before, under the same limits and answers, and is kept in media/private/.imports/<job id>.zip. That directory is on the volume the backend and worker share, and nginx already answers 404 under /media/private/.
  • The route answers 202 with a job id. A worker task reads the file and writes the campaign, in one transaction with the job's own "done".
  • Two new routes: GET /api/campaigns/import/jobs/{id} and …/jobs/latest. A job is visible to the person who started it and to an instance admin; anyone else gets the 404 a missing id gets.
  • The form shows that the file is in and being read, polls every 2 s while the page is visible, and ends on the same result or refusal as before. Leaving and coming back finds the job again.
  • The backend's memory is flat during an import. No compose change.

What changes in meaning

  • "Two imports at once, one per person" now holds across the server. It is counted from the job table, and covers a queued file as well as an upload in flight. It was per backend process before, so in effect four.
  • An import and a map decode never overlap. The task takes the map lock, so the worker's peak is the larger of the two, not the sum. That also makes imports run one at a time.
  • data.json may be 24 MB and 1,200,000 values (it was 8 MB and 400,000).

Two mechanisms beyond the plan

No cap alone made a worker survive every file inside the caps, because the caps bound what is parsed and not what the import then builds.

  1. A memory bound on the import. A thread samples the process's memory; once it has grown 400 MB the import is stopped and answers 422, "This file holds more than this server can read in one import." It uses a signal, because two cooperative versions were tried first and let a row-heavy file reach 823 MB and an OOM kill.
  2. worker_max_memory_per_child = 300 MB, a Celery setting for every task. Without it an idle worker ratcheted from 208 to 365 MB after two stopped imports. A child over the mark is replaced after its task ends.

The row-heavy finding (already true on main)

A file inside the old caps could build far more than it parsed:

File at 8 MB / 400,000 values, on main's code Result
80,000 two-field sessions (2.2 MB) a 1 GB container to 946 MB after about 35 minutes
44,000 pages 536 MB, then a 500 (too many query arguments)
57,000 members 423 MB, imported

Any of these would have killed a 512 MB backend. They are now stopped by the memory bound, in words. Counting rows up front is the better answer and is not built here.

Measured

docker run -m 1g, final code, peak above a 99 MB start:

data.json Whole import Outcome
24 MB, one string with an emoji 222 MB 422
24 MB, nested dicts plus a wide string (dearest found) 309 MB 422
24 MB, 92 sessions of transcript with emoji 219 MB imported in 10 s
9.4 MB, 171,000 members stopped at 440 MB 422 in 27 s

Through nginx with a real worker, against main:

main this branch
A real seeded export 201 in 0.36 s 202 in 0.09 s, done at 3.3 s, same result
7.8 MB data.json: backend 381 → 441 MB, and stayed 380 → 384 MB
7.8 MB data.json: worker flat 276 → 334 MB
  • Hostile files at the new caps: worker peaks of 372 to 655 MB of 1 GB, oom_kill 0, no restarts, and the worker back to 166 MB after each.
  • A kill mid-import (the reading process, with 1,042 files stored): the job failed with a sentence at 8.9 s; no campaign, no media, no pending upload.
  • An import and six maps together: both finished; a second person's import waited its turn.

Migration

d1054a2b3c4e adds import_jobs (status as TEXT with a CHECK; no enum). Run by hand against a throwaway Postgres: up, down one, up again, all clean.

Bounds on a job

What Bound Then
An upload that goes silent 5 minutes its row and partial file are removed
Waiting to be read re-queued after 15 minutes; 8 hours failed, 503
Being read 45 minutes (soft limit); 1 hour failed, 500, media discarded
A delivery that finds the job already being read once failed with "The server stopped while it was reading this file…"
An ended job's row 7 days deleted

Changelog

  • changelog.d/1054-import-in-the-worker.changed.md

Test plan

  • Full backend suite, on the tree before a rebase onto rc6 and a test-only change: 1 failed, 8057 passed. The failure was the "one migration head" pin, which moved to the new migration's test. Not re-run at the head commit; CI is the run that counts.
  • At the head commit: tests/export, the watchdog, session hygiene, enqueue, Celery app, orphan sweep and backup tests: 687 passed.
  • Frontend, nine touched files: 338 passed, 1 skipped; eslint 0 errors.
  • Lint clean; version and changelog checks OK; all 16 coverage scripts pass.
  • 50 existing tests that POST an import now read the job's answer through a helper; a fixture runs the task body in the request.
  • Not tested:
    • two backend processes racing for a slot;
    • the time bounds in real time (backdated rows only);
    • a real broker outage, a full disk, a 1 GB upload end to end;
    • a kill of the whole worker container;
    • the new states in a real browser.
  • Full frontend suite, journeys (none imports a campaign), bot and synthetic: CI.

🤖 Generated with Claude Code

Closes #1054 (D281, the owner's answer to Q153). Not to be merged before the independent review reports. ## Summary - **The upload arrives as before**, under the same limits and answers, and is kept in `media/private/.imports/<job id>.zip`. That directory is on the volume the backend and worker share, and nginx already answers 404 under `/media/private/`. - **The route answers 202 with a job id.** A worker task reads the file and writes the campaign, in one transaction with the job's own "done". - **Two new routes:** `GET /api/campaigns/import/jobs/{id}` and `…/jobs/latest`. A job is visible to the person who started it and to an instance admin; anyone else gets the 404 a missing id gets. - **The form** shows that the file is in and being read, polls every 2 s while the page is visible, and ends on the same result or refusal as before. Leaving and coming back finds the job again. - **The backend's memory is flat during an import.** No compose change. ## What changes in meaning - **"Two imports at once, one per person" now holds across the server.** It is counted from the job table, and covers a queued file as well as an upload in flight. It was per backend process before, so in effect four. - **An import and a map decode never overlap.** The task takes the map lock, so the worker's peak is the larger of the two, not the sum. That also makes imports run one at a time. - **`data.json` may be 24 MB and 1,200,000 values** (it was 8 MB and 400,000). ## Two mechanisms beyond the plan No cap alone made a worker survive every file inside the caps, because the caps bound what is parsed and not what the import then builds. 1. **A memory bound on the import.** A thread samples the process's memory; once it has grown 400 MB the import is stopped and answers 422, "This file holds more than this server can read in one import." It uses a signal, because two cooperative versions were tried first and let a row-heavy file reach 823 MB and an OOM kill. 2. **`worker_max_memory_per_child = 300 MB`**, a Celery setting for every task. Without it an idle worker ratcheted from 208 to 365 MB after two stopped imports. A child over the mark is replaced after its task ends. ## The row-heavy finding (already true on main) A file inside the old caps could build far more than it parsed: | File at 8 MB / 400,000 values, on main's code | Result | |---|---| | 80,000 two-field sessions (2.2 MB) | a 1 GB container to 946 MB after about 35 minutes | | 44,000 pages | 536 MB, then a 500 (too many query arguments) | | 57,000 members | 423 MB, imported | Any of these would have killed a 512 MB backend. They are now stopped by the memory bound, in words. Counting rows up front is the better answer and is not built here. ## Measured `docker run -m 1g`, final code, peak above a 99 MB start: | `data.json` | Whole import | Outcome | |---|---|---| | 24 MB, one string with an emoji | 222 MB | 422 | | 24 MB, nested dicts plus a wide string (dearest found) | 309 MB | 422 | | 24 MB, 92 sessions of transcript with emoji | 219 MB | imported in 10 s | | 9.4 MB, 171,000 members | stopped at 440 MB | 422 in 27 s | Through nginx with a real worker, against main: | | main | this branch | |---|---|---| | A real seeded export | 201 in 0.36 s | 202 in 0.09 s, done at 3.3 s, same result | | 7.8 MB `data.json`: backend | 381 → 441 MB, and stayed | 380 → 384 MB | | 7.8 MB `data.json`: worker | flat | 276 → 334 MB | - **Hostile files at the new caps:** worker peaks of 372 to 655 MB of 1 GB, `oom_kill 0`, no restarts, and the worker back to 166 MB after each. - **A kill mid-import** (the reading process, with 1,042 files stored): the job failed with a sentence at 8.9 s; no campaign, no media, no pending upload. - **An import and six maps together:** both finished; a second person's import waited its turn. ## Migration `d1054a2b3c4e` adds `import_jobs` (status as TEXT with a CHECK; no enum). Run by hand against a throwaway Postgres: up, down one, up again, all clean. ## Bounds on a job | What | Bound | Then | |---|---|---| | An upload that goes silent | 5 minutes | its row and partial file are removed | | Waiting to be read | re-queued after 15 minutes; 8 hours | failed, 503 | | Being read | 45 minutes (soft limit); 1 hour | failed, 500, media discarded | | A delivery that finds the job already being read | once | failed with "The server stopped while it was reading this file…" | | An ended job's row | 7 days | deleted | ## Changelog - [x] `changelog.d/1054-import-in-the-worker.changed.md` ## Test plan - [x] **Full backend suite, on the tree before a rebase onto rc6 and a test-only change:** `1 failed, 8057 passed`. The failure was the "one migration head" pin, which moved to the new migration's test. **Not re-run at the head commit**; CI is the run that counts. - [x] At the head commit: `tests/export`, the watchdog, session hygiene, enqueue, Celery app, orphan sweep and backup tests: `687 passed`. - [x] Frontend, nine touched files: `338 passed, 1 skipped`; eslint 0 errors. - [x] Lint clean; version and changelog checks OK; all 16 coverage scripts pass. - [x] 50 existing tests that POST an import now read the job's answer through a helper; a fixture runs the task body in the request. - [ ] **Not tested:** - two backend processes racing for a slot; - the time bounds in real time (backdated rows only); - a real broker outage, a full disk, a 1 GB upload end to end; - a kill of the whole worker container; - the new states in a real browser. - [ ] Full frontend suite, journeys (none imports a campaign), bot and synthetic: CI. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
D281: the worker reads an import's file, so an import answers at once and
finishes in the background. This is the record that joins the request that
takes the upload to the task that reads it, and what the page follows.

One row per import: who started it, where it has got to (receiving, waiting,
working, done, failed: TEXT with a CHECK, not a PG enum), the campaign it
made, the answer the import gives, or the sentence and HTTP status it would
have been refused with. `planned_campaign_id` is the id the new campaign is
to have, written before any file is stored under it, so the files of an
import whose worker died can be found.

Migration d1054a2b3c4e (revises c1007a1b2c3d). Additive; the downgrade drops
the table. Its test runs the real upgrade and downgrade in a scratch schema
and holds the result against the model; the "one head" pin moves to it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An import's data.json was parsed inside the backend (512 MB, 340 to 425 MB
once warm). The backend now receives the upload under the limits and
answers it always had, checks what needs no reading (it is a zip an export
could have made, with a data.json), stores it on the media volume the
worker shares (media/private/.imports/<job id>.zip, which nothing serves
and the media backup leaves out), and queues `read_import`.

The task does what the route did from "read data.json" onward:

- one transaction: the campaign and the job's "done" are one commit, so a
  job that reads "working" has no campaign, and a later delivery or the
  hourly sweep can remove what it stored without asking;
- tried once: a delivery that finds its job "working" knows a worker died on
  it, fails it in words and removes its files and upload, and does not open
  the file again;
- one import at a time, and never beside a map's picture: it takes the map
  lock, and gives its slot back rather than wait for it;
- the walker for the import's maps is queued after the commit, as before.

GET /api/campaigns/import/jobs/{id} follows a job (its starter and an
administrator; a 404 to anyone else) and /import/jobs/latest finds a
person's own again.

The limits (two on a server, one a person) are counted from the table under
an advisory lock, so they hold across backend processes and for a file that
is waiting, not only one arriving. `unstick_lost_tasks` ends a job whose
upload stopped arriving, that waited 8 hours, or that has been "working" for
an hour, queues again one that has waited a quarter of an hour, and removes
uploads nothing will read, including a file with no job.

data.json may now be 24 MB and 1,200,000 values (from 8 MB and 400,000),
measured in a 1 GB container: the dearest parse inside both grows the
process by 309 MB. What an import builds from a file of nothing but rows is
not bounded by either cap (at the old caps, 80,000 two-field sessions took a
1 GB container to 946 MB), so the reading process is watched from a thread
and stopped by a signal when it has grown by 400 MB, with a sentence. A
worker process whose peak passed 300 MB is replaced after its task
(worker_max_memory_per_child): what a large import leaves behind is not
handed back.

Tests that import through the route read the job's end as the import's
answer (tests/import_answer.py); the worker's task runs in the request.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The upload is as it was. Then the form says the file is waiting to be read,
or being read (one status line, no percentage: the server has none), asks
after the job every two seconds while its tab is shown, and ends on the
imported banner or the refusal banner exactly as before. A job's own
sentence is shown whatever its status.

Coming back to the form asks for the person's latest import: one under way
is picked up (and New campaign opens on "Import a file"), and one that ended
meanwhile is shown once, which this browser remembers. The campaign list is
told when the campaign exists, not when the upload was taken.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
docs(import): the import as a background job (#1054)
All checks were successful
CI / Bot/backend version sync (pull_request) Successful in 26s
CI / Feedback site tests (pull_request) Successful in 45s
CI / Does this change touch the recording path? (pull_request) Successful in 1m0s
CI / Synthetic session harness (no GPU, no LLM) (pull_request) Has been skipped
CI / Docker image build (pull_request) Successful in 1m11s
CI / Backend lint (ruff) (pull_request) Successful in 1m13s
CI / Summarisation accuracy eval harness (stub provider) (pull_request) Successful in 1m37s
CI / Bot tests and audit (pull_request) Successful in 2m36s
CI / Frontend (shard 2/2) (pull_request) Successful in 11m13s
CI / End-to-end journeys (phone) (pull_request) Successful in 10m25s
CI / End-to-end journeys (keyboard) (pull_request) Successful in 11m17s
CI / End-to-end journeys (desktop) (pull_request) Successful in 11m22s
CI / Frontend (shard 1/2) (pull_request) Successful in 13m4s
CI / Backend migration, tests, and audit (pull_request) Successful in 34m54s
de6d8e1b43
API.md: the 202, the job's route and states, what a job that fails says,
and my latest import. OPERATIONS.md: where a pending upload waits and how it
is removed, what ends an import that cannot finish, the limits as the
server's own, and data.json's bounds with the table they were measured
from. The changelog fragment, and IS-492 on HOME-2 and ADMIN-9 for the
form's waiting state, with the catalogue rebuilt.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Author
Collaborator

Independent review of de6d8e1b: nothing serious, seven robustness gaps. Not to be merged until they are fixed.

Found sound

  • Authorisation: a job is readable by its starter and an instance admin, with the same 404 for a missing id, someone else's job, or one still uploading. Nothing is written before authentication.
  • The pending directory: paths are built from the server-made id only; files are created exclusively and never through a link; the sweep touches only its own files and cannot reach campaigns/.
  • Slots: the lock, the count and the insert are one transaction. A disconnect frees the slot at once; a dead process frees it within 5 minutes.
  • The state machine: the campaign and the job's "done" are one commit; a duplicate delivery does nothing; the planned campaign id is server-made and never from the file.
  • The migration: additive, one head, a real downgrade.
  • Stored sentences: a job's detail is one of the app's own sentences, never an exception's text.

Findings, being fixed on this branch

  1. Low to medium, confirmed by running: the memory stop can be lost. If the signal lands while the main thread is inside an event-loop callback, asyncio swallows it and the watcher never sends again; the import then runs unbounded. Fix: keep sampling and re-send until the stop has taken effect.
  2. Low: a file can be written after the clean-up when the time limit or the memory stop lands during a read on the archive thread. Fix: clean up behind that thread.
  3. Low: deleting an account while its own import is running leaves files behind or blocks the delete for the rest of the import. Fix: refuse the deletion in words while an import is under way.
  4. Low: the import sweep runs first and unguarded in the hourly watchdog, so an error in it skips the other unsticking for that hour.
  5. Low: the sweep discards a working job's files before its own commit, without the "no campaign has this id" check.
  6. Low: a trickled upload holds a slot indefinitely. There is a stall bound per gap but no bound on total time. Fix: a floor on progress that an honest slow upload still clears.
  7. Smaller:
    • a time limit landing inside a broad except reports the wrong sentence;
    • the form polls every 2 s through a persistent server error;
    • maps are not queued when the limit lands during a commit that succeeded;
    • a slot is held for up to 75 minutes when an upload's own commit answer is lost.

Not checked by the review, to be shown on a real worker: what Celery does with a task queued behind one whose child is being recycled for memory, and what the hard time limit does to a task's acknowledgement.

Not run by the review: the test suites, a real worker, Postgres or Celery.

**Independent review of `de6d8e1b`: nothing serious, seven robustness gaps. Not to be merged until they are fixed.** **Found sound** - **Authorisation:** a job is readable by its starter and an instance admin, with the same 404 for a missing id, someone else's job, or one still uploading. Nothing is written before authentication. - **The pending directory:** paths are built from the server-made id only; files are created exclusively and never through a link; the sweep touches only its own files and cannot reach `campaigns/`. - **Slots:** the lock, the count and the insert are one transaction. A disconnect frees the slot at once; a dead process frees it within 5 minutes. - **The state machine:** the campaign and the job's "done" are one commit; a duplicate delivery does nothing; the planned campaign id is server-made and never from the file. - **The migration:** additive, one head, a real downgrade. - **Stored sentences:** a job's `detail` is one of the app's own sentences, never an exception's text. **Findings, being fixed on this branch** 1. **Low to medium, confirmed by running: the memory stop can be lost.** If the signal lands while the main thread is inside an event-loop callback, asyncio swallows it and the watcher never sends again; the import then runs unbounded. Fix: keep sampling and re-send until the stop has taken effect. 2. **Low: a file can be written after the clean-up** when the time limit or the memory stop lands during a read on the archive thread. Fix: clean up behind that thread. 3. **Low: deleting an account while its own import is running** leaves files behind or blocks the delete for the rest of the import. Fix: refuse the deletion in words while an import is under way. 4. **Low: the import sweep runs first and unguarded in the hourly watchdog**, so an error in it skips the other unsticking for that hour. 5. **Low: the sweep discards a working job's files before its own commit**, without the "no campaign has this id" check. 6. **Low: a trickled upload holds a slot indefinitely.** There is a stall bound per gap but no bound on total time. Fix: a floor on progress that an honest slow upload still clears. 7. **Smaller:** - a time limit landing inside a broad `except` reports the wrong sentence; - the form polls every 2 s through a persistent server error; - maps are not queued when the limit lands during a commit that succeeded; - a slot is held for up to 75 minutes when an upload's own commit answer is lost. **Not checked by the review, to be shown on a real worker:** what Celery does with a task queued behind one whose child is being recycled for memory, and what the hard time limit does to a task's acknowledgement. **Not run by the review:** the test suites, a real worker, Postgres or Celery.
The memory stop could be lost. Its signal handler raised wherever the main
thread was; in a callback of the event loop that is not the import's own
step the loop catches that, logs "Exception in callback" and carries on, and
the import ran on unbounded. The handler now raises only in the import's own
step and otherwise asks the loop to cancel the import's task; the watcher
sends the stop again every second while memory is still over the mark; and a
refusal is never rewritten as "too much" because a stop was once sent.

Also:
- the archive thread has finished before an import's files are discarded
  (the discard is queued behind it) and before the map lock is let go;
- an account cannot be deleted while an import it started is waiting or
  being read (409), and a delivery whose job row is gone still removes what
  it stored, by the planned id it holds;
- a failure in the import sweep no longer costs the hourly watchdog its
  other sweeps; the sweep discards a lost job's files after its commit and
  only if no campaign has the id (one function, shared with the task);
- an upload has a floor on progress, 64 KiB/s averaged from its start once
  two minutes in, so a trickle cannot hold a place (408);
- a job ended by the soft time limit says so even when the exception was
  turned into a refusal on the way up (sheet_archive's read-back);
- a delivery that finds its job done always asks for the campaign's maps;
- a "waiting" commit whose answer was lost is looked up before the upload is
  removed, and the import goes on if it landed;
- a task the hard time limit killed is handed back to the queue
  (acks_on_failure_or_timeout=False) instead of acknowledged and dropped, so
  the next delivery ends the job at once rather than the sweep an hour on.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Asking after a job that fails for any reason but a 404 was repeated every
two seconds for as long as the tab was shown. It now waits 2, 4, 8, 16 and
then 30 seconds, goes back to two once answered, and after six failures in a
row stops and says the import could not be checked, that it carries on, and
offers "Try again".

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The upload's floor on progress, the account-deletion refusal, what the soft
and hard time limits do to a job (with what a real worker logged), that the
memory stop is sent again, and that replacing a large worker process loses
and repeats nothing.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Merge remote-tracking branch 'origin/main' into feat/1054-import-in-the-worker
All checks were successful
CI / Does this change touch the recording path? (pull_request) Successful in 16s
CI / Synthetic session harness (no GPU, no LLM) (pull_request) Has been skipped
CI / Bot/backend version sync (pull_request) Successful in 24s
CI / Backend lint (ruff) (pull_request) Successful in 32s
CI / Feedback site tests (pull_request) Successful in 45s
CI / Docker image build (pull_request) Successful in 46s
CI / Summarisation accuracy eval harness (stub provider) (pull_request) Successful in 54s
CI / Bot tests and audit (pull_request) Successful in 2m9s
CI / Frontend (shard 2/2) (pull_request) Successful in 9m55s
CI / Frontend (shard 1/2) (pull_request) Successful in 11m36s
CI / End-to-end journeys (phone) (pull_request) Successful in 11m58s
CI / End-to-end journeys (keyboard) (pull_request) Successful in 13m13s
CI / End-to-end journeys (desktop) (pull_request) Successful in 13m24s
CI / Backend migration, tests, and audit (pull_request) Successful in 35m19s
38bfac15a6
Author
Collaborator

The review's findings are fixed in 38bfac15, each with a test. A run on a real worker found one more defect, also fixed.

Finding Fix
1. The memory stop could be lost The handler raises only in the import's own step. Anywhere else it cancels the task through the loop, so nothing is raised into a callback to be swallowed. The watcher re-sends every second while memory is over the mark
2. A file written after the clean-up The discard runs on the archive thread, behind whatever it is doing. The map lock is not released while a read is running
3. Deleting an account mid-import Refused with a 409 and a sentence while a job is waiting or working. Files are discarded even when the job's row is gone
4. The sweep unguarded in the watchdog Wrapped; a failure is logged and the other sweeps run
5. The sweep discarding before its commit Discards after the commit, with the "no campaign has this id" check
6. A trickled upload After 120 s the average rate must be at least 64 KiB/s, or the upload ends with a 408. A gigabyte on a 1 Mbit/s uplink still passes
7. Smaller The time limit keeps its sentence; the form backs off (2 s to 30 s) and stops after six failures with "Try again"; maps are queued on every done path; a lost answer to the "waiting" commit keeps the file and goes on

On a real worker

One worker process, so "queued behind" is literal.

  • A child replaced for memory: after a 48 MP map decode, and after a 24 MB import, three quick tasks queued behind each ran exactly once, in the new process. The first waited 0.4 to 2.8 s. Celery logs nothing about the replacement itself; the process number goes up.
  • The soft time limit (shortened to 10 s for the run): the job failed with the "took longer" sentence; no campaign, no files, no upload left.
  • The hard time limit: a defect. As first built, a task killed by the hard limit was acknowledged and not redelivered. The job stayed "working" with 2,371 files in place until the hourly sweep.
    • Fix: acks_on_failure_or_timeout=False on the import task.
    • Re-run: two deliveries, the job failed exactly once, nothing left behind, and the two tasks behind it ran once each.

Still open, and stated

  • The sheet evaluator swallows the soft time limit (two broad except Exception clauses that return null). An import interrupted there carries on, and the hard limit ends it two minutes later.
  • Large JPEGs inside an import are now shrunk (#1103) on the archive thread, and that counts towards the 400 MB bound. An import with many large JPEGs was not measured against it.
  • The Profile page shows its generic failure for the new 409.
  • The merge commit of main has no Co-Authored-By line.

Ran

  • Full backend suite at 38bfac15: 8416 passed, 14 skipped, 107 deselected.
  • Lint clean; version and changelog checks OK.
  • Frontend, ten touched files, on the commit before the merge of main (which touched no frontend file): 368 passed, 1 skipped; eslint 0 errors.

Not run

  • An end-to-end pass through nginx after the fixes: the trickled upload, the memory stop and the deletion refusal are unit-tested only.
  • The hard limit at its real 47 minutes.
  • The stop landing inside a real database callback (reproduced with a stand-in callback only).
  • The full frontend suite, journeys, bot and synthetic: CI.
**The review's findings are fixed in `38bfac15`, each with a test. A run on a real worker found one more defect, also fixed.** | Finding | Fix | |---|---| | 1. The memory stop could be lost | The handler raises only in the import's own step. Anywhere else it cancels the task through the loop, so nothing is raised into a callback to be swallowed. The watcher re-sends every second while memory is over the mark | | 2. A file written after the clean-up | The discard runs on the archive thread, behind whatever it is doing. The map lock is not released while a read is running | | 3. Deleting an account mid-import | Refused with a 409 and a sentence while a job is waiting or working. Files are discarded even when the job's row is gone | | 4. The sweep unguarded in the watchdog | Wrapped; a failure is logged and the other sweeps run | | 5. The sweep discarding before its commit | Discards after the commit, with the "no campaign has this id" check | | 6. A trickled upload | After 120 s the average rate must be at least 64 KiB/s, or the upload ends with a 408. A gigabyte on a 1 Mbit/s uplink still passes | | 7. Smaller | The time limit keeps its sentence; the form backs off (2 s to 30 s) and stops after six failures with "Try again"; maps are queued on every done path; a lost answer to the "waiting" commit keeps the file and goes on | ## On a real worker One worker process, so "queued behind" is literal. - **A child replaced for memory:** after a 48 MP map decode, and after a 24 MB import, three quick tasks queued behind each ran exactly once, in the new process. The first waited 0.4 to 2.8 s. Celery logs nothing about the replacement itself; the process number goes up. - **The soft time limit** (shortened to 10 s for the run): the job failed with the "took longer" sentence; no campaign, no files, no upload left. - **The hard time limit: a defect.** As first built, a task killed by the hard limit was acknowledged and not redelivered. The job stayed "working" with 2,371 files in place until the hourly sweep. - Fix: `acks_on_failure_or_timeout=False` on the import task. - Re-run: two deliveries, the job failed exactly once, nothing left behind, and the two tasks behind it ran once each. ## Still open, and stated - **The sheet evaluator swallows the soft time limit** (two broad `except Exception` clauses that return null). An import interrupted there carries on, and the hard limit ends it two minutes later. - **Large JPEGs inside an import are now shrunk (#1103) on the archive thread**, and that counts towards the 400 MB bound. An import with many large JPEGs was not measured against it. - **The Profile page shows its generic failure** for the new 409. - **The merge commit of main has no `Co-Authored-By` line.** ## Ran - **Full backend suite at `38bfac15`:** `8416 passed, 14 skipped, 107 deselected`. - Lint clean; version and changelog checks OK. - Frontend, ten touched files, on the commit before the merge of main (which touched no frontend file): `368 passed, 1 skipped`; eslint 0 errors. ## Not run - An end-to-end pass through nginx after the fixes: the trickled upload, the memory stop and the deletion refusal are unit-tested only. - The hard limit at its real 47 minutes. - The stop landing inside a real database callback (reproduced with a stand-in callback only). - The full frontend suite, journeys, bot and synthetic: CI.
claude-bot deleted branch feat/1054-import-in-the-worker 2026-10-05 11:58:29 +00:00
rbrooks referenced this pull request from a commit 2026-10-05 11:58:48 +00:00
Sign in to join this conversation.
No description provided.