C-MOVE receive path: images can be silently lost, partial transfers succeed, and other robustness improvements

[medihack]

Summary

The C-MOVE download path (worker → PACS → receiver container → FileTransmit → worker) correlates incoming C-STOREs with the requesting worker via a {calling_ae}\{StudyInstanceUID} topic and a pre-fetched list of SOPInstanceUIDs. The overall approach is sound, but several details can lead to silently incomplete transfers or stalled workers. This issue collects what I found while reading the code, plus proposed changes. Happy to split it into separate issues or open a PR for the first items.

Relevant code:

• adit/core/utils/dicom_operator.py: _fetch_images_with_c_move, _consume_from_receiver
• adit/core/management/commands/receiver.py: _send_files
• adit/core/utils/store_scp.py: StoreScp._handle_store
• adit/core/utils/file_transmit.py: FileTransmitServer, FileTransmitClient

Current behaviour (for reference)

1. The worker runs IMAGE-level C-FIND(s) to collect all SOPInstanceUIDs of the requested study/series.
2. The worker starts a consumer thread that connects to FileTransmitServer and subscribes to the topic {source.ae_title}\{StudyInstanceUID}.
3. The worker sends the C-MOVE with RECEIVER_AE_TITLE as destination.
4. The receiver's StoreScp writes each C-STORE to a temp file, returns 0x0000 to the PACS immediately, and queues the path.
5. _send_files reads the file, builds the topic from the association's calling AE title and the dataset's StudyInstanceUID, sends the file to every session subscribed to that topic (sequentially), then always deletes it (finally: os.unlink).
6. The worker removes received SOPInstanceUIDs from its list, ignores duplicates, and unsubscribes once the list is empty. After the final C-MOVE-RSP it waits at most C_MOVE_DOWNLOAD_TIMEOUT (30 s) since the last image.

Problems

1. Race between subscribe and C-MOVE → images silently dropped

_fetch_images_with_c_move submits _consume_from_receiver to a thread pool and immediately calls send_c_move. Nothing waits until the TCP connection to the receiver is established and the session is registered in FileTransmitServer._sessions; the protocol has no subscribe acknowledgement. If the first C-STORE arrives before that, publish_file finds no matching session and _send_files deletes the file.

Impact: the images are gone from ADIT's perspective. See 2 for why this does not fail the task.

2. Partially received studies count as success

check_images_received raises RetriableDicomError only if no image was received (remaining_image_uids == image_uids). Any other shortfall is logged as a warning and the transfer task completes. There is also no targeted retry, although the missing SOPInstanceUIDs are known and fetch_image (IMAGE-level C-MOVE) already exists.

3. Publish-then-delete without acknowledgement, no spool

The receiver has no notion of "not yet delivered". Any file that cannot be delivered right now (no subscriber, subscriber disconnected, write error mid-transfer) is deleted. Combined with 1 this makes the loss window larger than just the initial race.

4. Routing topic depends on the PACS's calling AE title

Worker side: f"{self.server.ae_title}\\{study_uid}" (configured value). Receiver side: event.assoc.remote["ae_title"] (what the PACS actually sends). PACS clusters or setups with a separate sending AE title (Q/R answered by PACS_QR, C-STOREs sent by PACS_SEND) will never match. Every image is dropped with no error visible to the user, only the worker's timeout warning.

Since SOPInstanceUIDs are globally unique, the AE title adds little to the routing key.

5. Head-of-line blocking in the receiver

_send_files is a single loop, and publish_file awaits session.send_file for each session sequentially with drain() backpressure. One slow or stalled worker stalls delivery to all topics while the PACS keeps sending. Also, if sending to one session raises, later sessions on the same topic do not receive the file.

6. Timeouts (to verify)

• The worker's inactivity timeout only starts after the final C-MOVE-RSP. Until then the only guard is dimse_timeout (default 60 s) on the move association. A PACS that sends no Pending responses while transferring a large study would trigger a timeout plus stamina retry although images are flowing.
• C_MOVE_DOWNLOAD_TIMEOUT of 30 s after the final RSP is fine as long as the receiver is not backlogged (see 5).

7. Hard dependency on IMAGE-level C-FIND

Some PACS disable IMAGE-level C-FIND, silently cap the number of matches, or are very slow at it. Currently the SOPInstanceUID list is mandatory, so C-MOVE downloads from such a PACS either fail or are incomplete.

Proposed changes

Roughly ordered by benefit/effort.

• Subscribe handshake. FileTransmitServer acknowledges the subscription (one line) after the session is registered. FileTransmitClient.subscribe signals a threading.Event; _fetch_images_with_c_move waits for it before send_c_move. Fixes 1.
• Short spool with replay. Files without a subscriber (or whose delivery failed) stay in the receiver's temp dir under their topic for a configurable TTL (e.g. 60–120 s) instead of being deleted. On subscribe, pending files for that topic are replayed first. Disk usage is bounded by TTL × PACS throughput. Fixes 3, hardens 1, survives short worker hiccups.
• Retry missing instances, then fail. After the move finishes, re-fetch remaining_image_uids with IMAGE-level C-MOVE (N attempts). If still incomplete, raise (RetriableDicomError or a hard failure, configurable) instead of a warning. Fixes 2.
• Route on StudyInstanceUID only. Drop the calling AE title from the topic, or make it an optional per-server filter (e.g. a list of accepted calling AE titles). In any case, log at WARNING/ERROR when a file arrives for a topic without subscribers. Fixes 4.
• Per-session send queue. publish_file only enqueues; each session has its own sender task. A stalled session no longer blocks others, and failures are isolated per session. (_sessions should also become an instance attribute rather than a class-level mutable default.) Fixes 5.
• Timer semantics. Reset the move-side inactivity guard on incoming images as well (raise dimse_timeout for the move association, or let the consumer thread report progress), so a slow PACS without Pending responses is not aborted while images are flowing. Addresses 6.
• Make the SOPInstanceUID list optional. Add image_level_find_support to DicomServer (like the existing *_move_support flags). Without it: accept everything matching Study (and Series) UID, dedupe by SOPInstanceUID from the received dataset, and determine completeness via the final C-MOVE-RSP counters (NumberOfCompletedSuboperations vs. received count, NumberOfRemainingSuboperations from the first Pending RSP when available) plus the inactivity timeout. Retry on mismatch is then SERIES-level. Addresses 7.

Non-goals

• Coalescing multiple workers requesting the same study. The current duplicate-tolerant design (both subscribe, both dedupe, unsubscribe independently) is simple and works. Not worth the complexity without a persistent spool.
• Delaying the C-STORE-RSP to the PACS until delivery. Given the known quirks with failure statuses on some PACS (see the GE/Synapse comments in dimse_connector.py), returning success immediately and ensuring completeness on the worker side (item 3) seems safer.
• MoveOriginatorMessageID / MoveOriginatorApplicationEntityTitle (already noted in KNOWLEDGE.md). Optional in the standard and not needed, since correlation on UIDs works with every PACS.