Skip to content

Add wait_for_pending_sends to drain concurrent enqueues#1018

Draft
mensfeld wants to merge 3 commits into
mainfrom
feat/concurrent-send-drain
Draft

Add wait_for_pending_sends to drain concurrent enqueues#1018
mensfeld wants to merge 3 commits into
mainfrom
feat/concurrent-send-drain

Conversation

@mensfeld

Copy link
Copy Markdown
Collaborator

ShoryukenConcurrentSendAdapter enqueues by scheduling the SQS send on a background future and returning immediately. With no way to wait for those futures, jobs enqueued shortly before the process exits could be silently dropped - the send never runs.

The adapter now tracks in-flight send futures (removing each as it resolves, so the set does not grow unbounded) and exposes wait_for_pending_sends(timeout = nil), which blocks until they all finish and returns false if the optional timeout elapses first. Call it from a shutdown hook to flush pending sends.

Coverage:

  • integration spec that slows the real SQS send via a client middleware, asserts the send is still in-flight right after perform_later, drains, and confirms the job reached SQS
  • unit specs that the drain blocks until an async send completes, honors a timeout, returns true when nothing is pending, and stops tracking resolved sends

ShoryukenConcurrentSendAdapter enqueues by scheduling the SQS send on a
background future and returning immediately. With no way to wait for
those futures, jobs enqueued shortly before the process exits could be
silently dropped - the send never runs.

The adapter now tracks in-flight send futures (removing each as it
resolves, so the set does not grow unbounded) and exposes
wait_for_pending_sends(timeout = nil), which blocks until they all
finish and returns false if the optional timeout elapses first. Call it
from a shutdown hook to flush pending sends.

Coverage:
- integration spec that slows the real SQS send via a client middleware,
  asserts the send is still in-flight right after perform_later, drains,
  and confirms the job reached SQS
- unit specs that the drain blocks until an async send completes, honors
  a timeout, returns true when nothing is pending, and stops tracking
  resolved sends
@mensfeld mensfeld self-assigned this Jun 14, 2026
@mensfeld mensfeld added the bug label Jun 14, 2026
@coderabbitai

coderabbitai Bot commented Jun 14, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: dde6d32f-c5ec-4c2e-8d99-9deafeab16f9

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/concurrent-send-drain

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

mensfeld added a commit that referenced this pull request Jun 22, 2026
The ShoryukenConcurrentSendAdapter enqueues by scheduling the SQS send on
a background future and returning immediately, so jobs enqueued shortly
before the process exits (e.g. by a worker during processing) could be
silently dropped. PR #1018 added ShoryukenConcurrentSendAdapter#wait_for_pending_sends
to drain them, but nothing called it during Shoryuken's own shutdown.

Launcher#stop and #stop! now drain the configured ActiveJob adapter after
the executor has shut down (so all worker-initiated sends have been
issued), bounded by Shoryuken.options[:timeout]. The call is duck-typed
and guarded: it is a no-op unless ActiveJob is loaded and its adapter
responds to wait_for_pending_sends, and any error is swallowed so a
draining hiccup can't break shutdown.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant