Skip to content

fix(subscriber): make streaming pull responsive to SIGTERM - #17

Merged
shavonn merged 1 commit into
mainfrom
fix/subscriber-pull-respond-to-sigterm
Jun 15, 2026
Merged

shavonn merged 1 commit into
mainfrom
fix/subscriber-pull-respond-to-sigterm

Conversation

@shavonn

@shavonn shavonn commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Description

Two small fixes to make the Pub/Sub subscriber responsive to SIGTERM in containerized environments.

Motivation

In K8s/Docker, SIGTERM is followed by SIGKILL after terminationGracePeriodSeconds (default 30s). Two paths in the subscriber blocked the shutdown signal from
being acted on in time:

  1. StreamingSubscriber::stream() calls pull(['returnImmediately' => false, ...]), which blocks on a gRPC streaming pull until messages arrive or the
    server-side deadline elapses (often 30–90s). The trait's synchronous pcntl_signal_dispatch() polling could not interrupt this — SIGTERM was only noticed after
    pull() returned, by which time SIGKILL may have already fired.
  2. Both the polling and streaming inner loops processed an entire pull batch before re-checking shouldStop(). With slow handlers and a full batch (up to 100
    messages by default in streaming mode), shutdown could be delayed significantly even when the signal was noticed promptly.

Changes

  • src/Concerns/HandlesSignals.php — call pcntl_async_signals(true) in registerSignalHandlers() so SIGTERM/SIGINT/SIGQUIT handlers fire during the
    blocking pull syscall and set $shouldStopFlag immediately. The existing pcntl_signal_dispatch() calls remain (they become no-ops under async mode but are
    harmless).
  • src/Subscriber/Subscriber.php — add if ($this->shouldStop()) break; after each message in pull()'s loop.
  • src/Subscriber/StreamingSubscriber.php — same per-message check in stream()'s inner loop.

A follow-up PR will address explicit nack-on-shutdown of unprocessed pulled messages (currently they're abandoned and redelivered after the ack-deadline
expires).

Fixes # (issue)

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Deprecation (marks existing API as deprecated)
  • Refactor / internal change (no functional impact)
  • Documentation update
  • CI / tooling change

How Has This Been Tested?

  • Full Pest suite (composer test): 233 passed, 1 skipped, 415 assertions.

  • Updated three streaming-subscriber tests whose stop-after-N-pulls plumbing relied on counting shouldStop() calls — the new inner-loop check increments that
    counter more often, so the threshold bumped from >= 2 to >= 3 with a comment explaining the call cadence. No test removed; behavioral expectations (pull
    called twice, ack/nack expectations) are unchanged.

  • Manual verification of K8s-style shutdown requires a deployed environment; not covered by the unit tests.

  • composer test passes

  • composer analyse (PHPStan / Larastan) passes

  • composer pint shows no formatting changes

  • Added or updated tests covering the change

Test Configuration:

  • PHP version(s): 8.3
  • Laravel version(s): 11.x / 12.x / 13.x (per composer constraints)
  • Pub/Sub target: Mocked (unit tests). Manual K8s verification not performed in this PR.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Deprecation (marks existing API as deprecated)
  • Refactor / internal change (no functional impact)
  • Documentation update
  • CI / tooling change

How Has This Been Tested?

  • Updated three streaming-subscriber tests whose stop-after-N-pulls plumbing relied on counting shouldStop() calls — the new inner-loop check increments that
    counter more often, so the threshold bumped from >= 2 to >= 3 with a comment explaining the call cadence. No test removed; behavioral expectations (pull
    called twice, ack/nack expectations) are unchanged.

  • Manual verification of K8s-style shutdown requires a deployed environment; not covered by the unit tests.

  • composer test passes

  • composer analyse (PHPStan / Larastan) passes

  • composer pint shows no formatting changes

  • Added or updated tests covering the change

Test Configuration:

  • PHP version(s): 8.3
  • Laravel version(s): 11.x / 12.x / 13.x (per composer constraints)
  • Pub/Sub target: Mocked (unit tests). Manual K8s verification not performed in this PR.

Checklist

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Deprecation (marks existing API as deprecated)
  • Refactor / internal change (no functional impact)
  • Documentation update
  • CI / tooling change

How Has This Been Tested?

  • Full Pest suite (composer test): 233 passed, 1 skipped, 415 assertions.

  • Updated three streaming-subscriber tests whose stop-after-N-pulls plumbing relied on counting shouldStop() calls — the new inner-loop check increments that
    counter more often, so the threshold bumped from >= 2 to >= 3 with a comment explaining the call cadence. No test removed; behavioral expectations (pull
    called twice, ack/nack expectations) are unchanged.

  • Manual verification of K8s-style shutdown requires a deployed environment; not covered by the unit tests.

  • composer test passes

  • composer analyse (PHPStan / Larastan) passes

  • composer pint shows no formatting changes

Test Configuration:

  • PHP version(s): 8.3
  • Laravel version(s): 11.x / 12.x / 13.x (per composer constraints)
  • Pub/Sub target: Mocked (unit tests). Manual K8s verification not performed in this PR.

Checklist

  • My code follows the style guidelines of this project (Pint / PSR-12)
  • I have performed a self-review of my code
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing tests pass locally with my changes
  • I have updated the README / docs where relevant
  • My commit messages follow Conventional Commits
  • Any breaking changes are clearly called out above

@shavonn
shavonn merged commit 156cb9c into main Jun 15, 2026
20 checks passed
@shavonn
shavonn deleted the fix/subscriber-pull-respond-to-sigterm branch June 15, 2026 00:20
@github-actions github-actions Bot mentioned this pull request Jun 15, 2026
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