Skip to content

fix: on Ctrl+C, stop accepting writes and flush what is already enqueued - #321

Merged
drake-nominal merged 1 commit into
perf/py-onlyfrom
fix/sigint-drain
Aug 26, 2026
Merged

fix: on Ctrl+C, stop accepting writes and flush what is already enqueued#321
drake-nominal merged 1 commit into
perf/py-onlyfrom
fix/sigint-drain

Conversation

@drake-nominal

Copy link
Copy Markdown
Contributor

Stacked on #320. Touches only py-nominal-streaming.

Desired behaviour

On Ctrl+C: no further data can be enqueued, everything already enqueued reaches avro or the
backend, and the process exits once it has.

What it did instead

The handler called cancel, whose contract was to abandon buffered points for a fast exit --
the opposite of the above.

What changes

stop_accepting_writes sets a flag that makes subsequent writes raise
RuntimeError("stream is shutting down"). The SIGINT handler sets it and re-raises
KeyboardInterrupt; the drain then happens in the ordinary close, reached through __exit__ as
the exception unwinds, or through the stream's destructor (added in #320) otherwise.

Refusing first is what makes the drain converge, rather than racing a producer that has not
noticed the interrupt. That matters most for multi-threaded writers: KeyboardInterrupt is
delivered only to the main thread, so worker threads would otherwise keep feeding the buffer.
They now see the RuntimeError instead.

stop_accepting_writes takes &self, not &mut self, on purpose: shutdown begins on whichever
thread caught the signal while others may be inside enqueue, and close needs &mut self,
which would fail to borrow in that situation.

Verification

Real SIGINT sent to a live streaming process, avro file read back and compared against the count
the process reported enqueuing:

enqueued in avro file
single writer 10,442,916 10,442,916 exited in 3.4s
four writer threads 1,498,653 1,498,653 all 4 threads refused after shutdown

Note on the earlier approach

#318 implemented the opposite semantics (abandon the backlog to exit immediately) against the
core crate. It should be closed in favour of this unless a fast-and-lossy teardown is separately
wanted -- and if it is, it belongs behind an explicit call rather than on Ctrl+C.

🤖 Generated with Claude Code

The SIGINT handler called `cancel`, whose contract was to abandon buffered points and exit fast.
The behaviour we want is the opposite: refuse further writes, let everything already enqueued
reach avro or the backend, and exit once it has.

Refusing first is what makes the drain converge. Without it the teardown races a producer loop
that has not noticed the interrupt yet, and on a multi-threaded writer it never notices at all --
KeyboardInterrupt is delivered only to the main thread.

`stop_accepting_writes` takes `&self` rather than `&mut self` deliberately: shutdown starts on
whichever thread caught the signal while other threads may be inside `enqueue`, and `close` needs
`&mut self`, which would fail to borrow in that situation. The handler sets the flag and re-raises
KeyboardInterrupt; the drain then happens in `close`, reached through `__exit__` as the exception
unwinds, or through the stream's destructor if the caller is not using a context manager.

Writes attempted after shutdown begins raise `RuntimeError("stream is shutting down")`.

Verified by sending a real SIGINT to a streaming process:

  single writer:  10,442,916 points enqueued, 10,442,916 in the avro file, exited in 3.4s
  four writers:    1,498,653 points enqueued,  1,498,653 in the avro file, all 4 threads refused

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@drake-nominal
drake-nominal merged commit 676a19b into perf/py-only Aug 26, 2026
3 checks passed
@drake-nominal
drake-nominal deleted the fix/sigint-drain branch August 26, 2026 22:08
@drake-nominal

Copy link
Copy Markdown
Contributor Author

Folded into #320. These are not safe to merge separately: with only the perf commit, Ctrl+C on a multi-threaded writer deadlocks. close() needs &mut self and cannot take the pyclass borrow while worker threads sit inside enqueue holding it, and with no refusal flag those threads never stop writing. Verified by test — the process hung rather than draining.

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.

1 participant