Skip to content

fix: make interrupted GridFS uploads and deletes recoverable - #516

Open
dragoangel wants to merge 1 commit into
zone-eu:masterfrom
dragoangel:gridfs-orphans
Open

dragoangel wants to merge 1 commit into
zone-eu:masterfrom
dragoangel:gridfs-orphans

Conversation

@dragoangel

@dragoangel dragoangel commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Implements #515.

What the problem is

GridFS chunks outlive the files document that names them. files_id is their only
link and it is recorded nowhere else, so once the files document is gone the chunks
are unreachable by any query the code can express. Three paths produce that state: an
upload that never finishes, a delete interrupted between its two non-atomic halves,
and the time-based GC pass itself on the rare message whose upload straddles the water
mark.

That pass does eventually remove them, by age: it deletes files and chunks whose own
_id is below a water mark taken from the oldest queue row, and an orphan's _id
sinks below that mark like anything else. What it cannot do is reason about
reachability, so it cannot tell an orphan from a message that is merely old, and it
can only act once the mark has moved past it. The mark is held back by the oldest
queued delivery, which is itself bounded by maxQueueTime, so collection is late
rather than absent: up to 30 days on the default setting, and indefinitely where
expiry is disabled.

What this changes

Every in-flight upload and delete writes a marker document keyed by files_id into
<gfs>.pending, and removes it once the operation is whole, so no interruption is
invisible. The GC reads those markers and finishes what was started, within minutes
rather than within maxQueueTime, and it acts only on an operation that is known not
to have completed.

The state on the marker is what makes that safe. A file with no chunks means
opposite things depending on which operation was in flight, and a collector that
deleted on sight would destroy a message that had been queued successfully. So the
collector has four outcomes and only three of them delete anything:

  • deleting with a files document still present: the delete stopped partway, finish
    it in the order it would have run in
  • writing with no files document: an upload that never produced one, so its chunks
    are unreachable and go
  • writing with a files document but no delivery naming it: the upload finished but
    the message never reached the queue, so nothing will ever deliver or delete it
  • anything else: a marker whose own removal was lost, and dropping it is the whole of
    the repair

The delete itself is reordered to remove chunks before the files document, so every
interruption leaves either a marker or a complete file and the collector can tell
which.

Both queues store

RemoteQueue is the queue a forked SMTP child stores through, which is the path every
submission arriving over the wire takes; MailQueue is the one behind the HTTP API.
Both write the marker, and both hold it until the message is queued rather than
until the upload finishes: a stored message with no delivery behind it is nobody's,
and only the caller knows when it stops being that. store() hands the upload id back
so the caller can say so, mail-drop clears it after the push, and PUT /store/:id
clears its own, because nothing on that path ever queues anything. That endpoint
answers 500 if the clear fails - unlike the same failure after a push, there is no
delivery to keep the body alive, so 200 would promise durability that will not hold.

How long a marker lives

A marker records when it stops being anybody's rather than when it was written, and
the process holding the operation pushes that moment forward once a minute for as long
as it is still holding it. The collector compares that against the clock and has
nothing to derive - which matters because the process that sweeps is generally not the
process that wrote, and in a split deployment the two do not share a configuration.

The heartbeat is a timer in the very process doing the writing, so it cannot outlive
it. A child killed, out of memory, or thrown out of its event loop takes its timers
with it and the marker goes stale on its own, with no cleanup path having to run
first. A timeout describes what should happen; a missed beat is what did.

It also covers the parts of an operation that write no chunks at all, which a deadline
tied to write progress would not: headers are parsed before the body reaches the
upload stream and are stored separately as file metadata, and after the stream
finishes the message still has to be scanned, have its metadata written and be pushed
to the queue.

The beat only extends, never creates. An operation can be ended by a different process
than the one beating for it, because a forked child hands its cleanup to the master
over IPC, and an upserting beat would raise the marker from the dead there and then
beat for it forever. Finding nothing to extend is also how a beat learns to stop.

The teardown half

An SMTP client that disappears mid-DATA is the producer that needs no crash at all.
smtp-server handles the socket error at session level and, in _onClose, calls
this._dataStream.unpipe() and drops the stream without ending, erroring or
destroying it, so nothing downstream is told anything: the upload stays open,
removeMessage is never called because no delete ever happens, and maildrop.add's
callback never fires.

Erroring the size limiter from onClose is the whole fix, because the error then
takes the same route a size limit or a plugin rejection takes and the existing
handlers unwind the pipeline down to store(), which removes the chunks it had
written. A message that reached the terminating dot is left alone: DATA ends before
the connection closes, so it is complete even if the client never waited for the
response.

The error is named ClientDisconnect rather than only coded, because mail-drop
treats anything whose name ends in Error as a storage failure worth logging and
reporting as NOQUEUE. A client that hung up is neither, and on a busy receiver it is
routine - it is logged at info as an outcome, so the error channel stays useful for
real storage faults.

onClose also records session.connectionClosed, because a connection can be lost
while the smtp:data hooks are still running, before there is an in-flight message
for the teardown to find. Piping a dead stream into a pipeline that is only being
built would suspend it for the life of the process with no way back, so that case
destroys the stream instead.

Existing orphans, and why there is no migration step

The time-based pass stays exactly where it was, as the coarse backstop. It is what
collects anything orphaned before the markers existed, and the chunks it strands
itself on a straddling upload, neither of which any marker describes. So this needs no
operator action on upgrade: the old backlog ages out the way it always has, and
everything new is collected in minutes.

Cost

The two passes do not compete. collectPendingGridfs reads a collection that is empty
whenever nothing is in flight. The heartbeat is one small update a minute per
operation actually in flight. The time-based deletes are _id range scans on an
always-indexed field, and the water mark sits below the oldest queued message, so
nothing a live message owns is ever in range.

Tests

test/mail-queue-gridfs-test.js is new: nineteen cases covering the collector's four
outcomes, the marker lifecycle, the heartbeat, the delete order, and a delete taking
over a stale upload marker. test/remote-queue-gridfs-test.js adds six for the store
path every SMTP submission takes, including which stream failures count as storage
failures and which are outcomes. test/api-store-marker-test.js adds three for the
store-only endpoint, and test/smtp-interface-test.js gains four for the teardown.
All of them fail on the code before this change.

Compatibility

No new configuration. The marker collection is <gfs>.pending, indexed on expires
through indexes.yaml, and nothing outside the process that opened an operation needs
a number to know. An index named mailpending, on created, existed in an earlier
revision of this branch and can be dropped by anyone who deployed from it once every
process is upgraded; a fresh deployment never sees it.

Fixes #515.

@dragoangel
dragoangel force-pushed the gridfs-orphans branch 2 times, most recently from c80c5d4 to e0191e6 Compare September 13, 2026 17:03
@dragoangel

dragoangel commented Sep 13, 2026

Copy link
Copy Markdown
Contributor Author

Hi @andris9 @NickOvt, could you please take a look at this PR and the issue it closes?

GridFS chunks outlive the files document that names them. files_id is
their only link and it is recorded nowhere else, so once the files
document is gone the chunks are unreachable by any query the code can
express. Three paths produce that state: an upload that never finishes,
a delete interrupted between its two non-atomic halves, and the
time-based GC pass itself on a message whose upload straddles its water
mark.

Every in-flight upload and delete now writes a marker document keyed by
files_id and removes it once the operation is whole, so no interruption
is invisible. The GC reads those markers and finishes what was started,
within minutes rather than within maxQueueTime, and acts only on an
operation known not to have completed. The state on the marker is what
makes that safe: a file with no chunks means opposite things depending on
which operation was in flight, and a collector that deleted on sight
would destroy a message that had been queued successfully.

A marker records when it stops being anybody's rather than when it was
written, and the process holding the operation pushes that moment forward
every minute for as long as it is still holding it. The timer lives in
the very process doing the writing, so it cannot outlive it: a child
killed, out of memory, or thrown out of its event loop takes its timers
with it and the marker goes stale with no cleanup path having to run
first. The beat only extends, never creates - an operation can be ended
by a different process than the one beating for it, because a forked
child hands its cleanup to the master over IPC, and an upserting beat
would raise the marker from the dead and then beat for it forever.

Both queues store: RemoteQueue is the one every SMTP submission takes,
MailQueue the one behind the HTTP API, and the marker covers both. It is
held until the message is queued rather than until the upload finishes,
because a stored message with no delivery behind it is nobody's, and
PUT /store/:id clears its own because nothing on that path ever queues
one.

An SMTP client that disappears mid-DATA needs no crash to reach the same
state. smtp-server handles the socket error at session level and, in
_onClose, calls this._dataStream.unpipe() and drops the stream without
ending, erroring or destroying it, so nothing downstream is told
anything: the upload stays open, removeMessage is never called, and
maildrop.add's callback never fires. Erroring the size limiter from
onClose is the whole fix, because the error then takes the route a size
limit or a plugin rejection takes and the existing handlers unwind down
to store(). A message that reached the terminating dot is left alone.

The time-based pass stays where it was, as the coarse backstop for
anything orphaned before the markers existed. No operator action is
needed on upgrade.

Signed-off-by: Dmytro Alieksieiev <1865999+dragoangel@users.noreply.github.com>
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.

GridFS chunks are orphaned on write as well as on delete, and the GC only clears them by age

1 participant