Skip to content

Publish one Pub/Sub request per flush - #48

Open
BinaryFiddler wants to merge 1 commit into
mainfrom
codex/osprey-pubsub-request-per-flush
Open

BinaryFiddler wants to merge 1 commit into
mainfrom
codex/osprey-pubsub-request-per-flush

Conversation

@BinaryFiddler

@BinaryFiddler BinaryFiddler commented Aug 20, 2026 •

Copy link
Copy Markdown

Summary

AsyncPubSubPublisher already batches in an asyncio queue. With BatchSettings(max_messages=1), the high-level client still created one commit thread and one publish RPC per message.

This change calls the generated GAPIC client once per asyncio flush. The public API and flush timing are unchanged: messages already queued together share a request, without adding a batching delay.

Results

Microbenchmark: real _sync_flush, RPC stubbed at the same boundary, 4,000 × 1.2 KB messages, three runs.

Messages per flush CPU/message, before → after Threads/message
1 74 → 22 µs 1.01 → 0
13 71 → 3.4 µs 1.00 → 0

Production averages about 15.2k attempts/s across these publishers. Most flushes still contain one message, so the steady-state gain is removing per-message wrapper and thread work; bursts batch naturally.

Other fixes

  • stop() drains the queue even if the flush task never starts, retries messages requeued during the drain, and records residual messages as shutdown_dropped.
  • _stopping prevents the Python 3.11 asyncio.wait_for cancellation race from hanging shutdown.
  • Cancelled and Unknown now match the generated client's transient retry set.
  • Adds queue_depth and messages_per_request; publish counters remain per message.

Risk

One RPC now shares an outcome across its batch. Transient failures requeue the batch; permanent failures drop it. Seven-day production data shows ~0.17 RetryError/s, ~0.12 TimeoutError/s, and no permanent error tag.

messages_per_request.count measures logical GAPIC publish calls, not extra wire attempts made by internal retries.

Review follow-ups before merge

  • Fix the byte guard to check the prospective serialized request size. It currently allows a message to push a batch past Pub/Sub's 10 MB request limit.
  • Make the shutdown wording/implementation explicit: the 30-second check stops starting new flushes, but does not bound an in-flight publish or executor wait.
  • Remove or justify partial-response recovery. The API does not document that a short successful response means “prefix accepted, tail rejected.”

Verification and rollout

CI is green for async unit tests, Python quality, integration tests, docs, UI, and Rust. The author also exercised burst, paced, immediate-stop, and missing-topic cases against a Pub/Sub emulator.

After rollout, verify attempt rate is stable, failure rate does not rise, queue depth stays near zero, and shutdown_dropped stays zero. Client pooling remains out of scope.

AsyncPubSubPublisher already batches messages in an asyncio queue, so it
configured the google.cloud.pubsub_v1 wrapper's own batching layer out of
the way with BatchSettings(max_messages=1). That setting makes the wrapper
spawn a commit thread and issue a publish RPC for every single message, no
matter how many the flush loop collected.

Publish each flush as one request through the GAPIC client instead, which
is the layer the wrapper was calling anyway. Measured against
google-cloud-pubsub 2.15.2 with the RPC stubbed at the same boundary in
both versions: 74us -> 22us of client CPU per message for a flush of one
message, 71us -> 3.4us for a flush of 13, and thread creations per message
drop from 1.01 to 0. Batch size stays a product of arrival rate rather
than of an added delay, so publish latency does not regress.

Also fixed while rewriting the flush path:

- stop() could drop the whole queue. The drain lived inside the flush
  task's CancelledError handler, so cancelling a task that had not taken
  its first step skipped it entirely: 500 queued messages, 0 delivered,
  reproduced against a Pub/Sub emulator. The drain now runs in stop(), and
  re-checks the queue instead of sampling qsize() once, which had
  abandoned anything _requeue put back mid-drain.
- stop() could also hang forever: on a cancel landing after queue.get()
  completed, asyncio.wait_for returns the message and consumes the
  CancelledError, leaving the `while True` loop running with stop()
  awaiting it. A _stopping flag now ends the loop.
- The idle path caught the builtin TimeoutError, which on <=3.10 is not
  asyncio.TimeoutError.
- Cancelled and Unknown were missing from the transient set the library
  itself retries publish on.
- A batch stops accumulating at 9MB, keeping requests inside the 10MB
  server limit, and a message oversized on its own is published alone.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@BinaryFiddler
BinaryFiddler force-pushed the codex/osprey-pubsub-request-per-flush branch from 2fb5b33 to 4783cef Compare August 20, 2026 19:28
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