Google Cloud Pub/Sub gRPC: eager-pull stages, subscribe field-leak fi…#1621
Draft
hanishi wants to merge 1 commit into
Draft
Google Cloud Pub/Sub gRPC: eager-pull stages, subscribe field-leak fi…#1621hanishi wants to merge 1 commit into
hanishi wants to merge 1 commit into
Conversation
…x, and Subscriber resource * Bug 1: autoExtendAckDeadlines now tracks on receipt via EagerPullTrackingStage, not on push downstream. Prevents deadline expiry on messages buffered behind saturated mapAsync. * Bug 2: subsequent StreamingPullRequests built from defaultInstance, clearing clientId / maxOutstandingMessages / maxOutstandingBytes that the server rejects on non-initial requests. * Bug 3: FlowControlGateStage rewritten with the eager-pull pattern so permits are acquired on receipt; outstandingCount now reflects actual delivery. Pre-sized internal ArrayDeque to FlowControl.maxOutstandingMessages (clamped to 65536) to skip resize doublings during initial fill. * AckDeadlineExtender: caller-owned tracking map and ticker that survive RestartSource re-materializations (matches Google MessageDispatcher lifecycle). * Subscriber resource (GooglePubSub.subscriber): bundles subscribe + restart + autoExtend + optional flowControl with correct composition by construction. * ExampleApp.subscribeAutoExtend uses the new Subscriber API.
5929a64 to
6139a72
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes three correctness gaps in the
google-cloud-pub-sub-grpcsubscriber pipeline (two introduced by #1494, one latent since 2019 and surfaced by it), and adds a high-levelSubscriberresource so users get a correctsubscriber by construction.
Closes #1620
What this changes
EagerPullTrackingStagereplaces the demand-driven.maptracker insideautoExtendAckDeadlines. Tracking now fires on receipt from the gRPC adapter, so messages buffered behind a slowmapAsyncno longer hit theirack deadline before being tracked. (Bug 1)
StreamingPullRequest.defaultInstancefor subsequent keepalive requests insubscribe(...). The Pub/Sub server forbidsclientId,maxOutstandingMessages, andmaxOutstandingByteson subsequent requests; theprevious code only cleared
subscriptionandstreamAckDeadlineSeconds, so any caller settingmaxOutstandingMessagesgotINVALID_ARGUMENT~1s after stream start. (Bug 2, latent since3f1a4f7f28from Jan 2019)FlowControlGateStagerewritten with the same eager-pull pattern as bug 1. Permits acquired on receipt rather than on push downstream, sooutstandingCountreflects actual in-flight delivery and the gate's limitfinally bounds what the server sends. Internal
ArrayDequepre-sized tomaxOutstandingMessages(clamped to 65536). (Bug 3)AckDeadlineExtenderis a new caller-owned class holding the tracking map and background ticker. Both surviveRestartSource.withBackoffre-materializations, matchingMessageDispatcher's lifecycle in Google'sofficial client.
GooglePubSub.subscriber(...)is the new high-level entry point. Returns aSubscriberresource that bundlessubscribe+ restart +autoExtendAckDeadlines(extender)+ optionalflowControlGate. Composition iscorrect by construction: restart wraps only the inner subscribe, the extender persists across reconnects,
acknowledge/nacksinks release flow-control permits and recordAckDeadlineDistributioncompletion latenciesautomatically when configured.
Subscriberwrapper is deferred (Java/Scala generated client split needs its own design pass; Java users can still compose the operatorsdirectly or use
AckDeadlineExtender.create(...)).k8s/build-and-push.shhardened with anEXITtrap so script failures no longer leakGkeAuthTest.scala/GkeFullFeatureTest.scalainto the source tree.GkeFullFeatureTestexercising the newSubscriberresource end-to-end against real Pub/Sub via Workload Identity. Asset committed but not run this PR cycle.Verification
AutoExtendAckDeadlinesSpec+SubscriberSpec) pass against stubSubscriberClients. No live infrastructure needed.IntegrationSpectests pass against the Pub/Sub emulator (docker compose up gcloud-pubsub-emulator), including the newSubscriber resourcescenario. No regressions in the 17 pre-existing tests.Subscriber resourcescenario also passed against real GCP (projectpekko-connectors) viagcloud auth application-default login. 35s; recorded ingoogle-cloud-pub-sub-grpc/README.mdtest report table.Notes for reviewers
Subscriberis intended as the primary entry point going forward. The low-level operators (subscribe,autoExtendAckDeadlines,flowControlGate, etc.) stay available for callers who genuinely need a different composition(sharing one extender across multiple subscriptions, splitting the source for fan-out, etc.).
FlowControl.outstandingCountsemantics changed: permits are now acquired on receipt, not on push downstream. Telemetry built on this counter will report higher numbers for the same workload, but those numbers are nowaccurate. Doc updated.
AckDeadline,AckDeadlineExtender, andSubscriberare@ApiMayChange.autoExtendAckDeadlines(subscription, ...)overloads stay intact for back-compat, with a doc note that they're not restart-safe and pointing at theAckDeadlineExtenderform for that case.