Skip to content

feat(pubsub/v2): add subscriber shutdown options#12829

Merged
hongalex merged 13 commits into
googleapis:mainfrom
hongalex:feat-pubsub-shutdown-options
Sep 25, 2025
Merged

feat(pubsub/v2): add subscriber shutdown options#12829
hongalex merged 13 commits into
googleapis:mainfrom
hongalex:feat-pubsub-shutdown-options

Conversation

@hongalex
Copy link
Copy Markdown
Member

@hongalex hongalex commented Sep 8, 2025

This introduces the ability to tell the client library how you want your messages to behave when shutdown is initiated. You can configure the behavior as ShutdownOptions.Behavior as ShutdownBehaviorWaitForProcessing or ShutdownBehaviorNackImmediately.

In addition, you can specify ShutdownOptions.Timeout to configure how long you want to wait for messages to be processed, or provide a timeout to the nack calls before returning.

@hongalex hongalex requested a review from shollyman as a code owner September 8, 2025 22:19
@hongalex hongalex requested review from a team September 8, 2025 22:19
@product-auto-label product-auto-label Bot added the api: pubsub Issues related to the Pub/Sub API. label Sep 8, 2025
@hongalex hongalex added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Sep 8, 2025
@hongalex hongalex force-pushed the feat-pubsub-shutdown-options branch from 1178bd6 to 1a9e6de Compare September 21, 2025 17:16
@hongalex hongalex removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Sep 22, 2025
Comment thread pubsub/v2/shutdown.go
Comment thread pubsub/v2/shutdown.go
@hongalex hongalex requested a review from feywind September 23, 2025 00:36
feywind
feywind previously approved these changes Sep 25, 2025
Copy link
Copy Markdown

@feywind feywind left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a Go Pub/Sub expert, but this looks like it generally does what it's supposed to :)

Comment thread pubsub/v2/shutdown.go
Copy link
Copy Markdown
Contributor

@shollyman shollyman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use of new context.Background() in iterator looks a bit odd, but LGTM given our discussions.

@hongalex
Copy link
Copy Markdown
Member Author

The use of new context.Background() in iterator looks a bit odd, but LGTM given our discussions.

To capture our offline discussions, a detached context.Background() always existed in the iterator (so in flight messages are not cancellable by the user by default). This matches the default behavior. I needed to introduce a context finally because nackInventory requires us to be able to cancel that request after ShutdownOptions.Timeout duration has passed.

@hongalex hongalex merged commit 14c3887 into googleapis:main Sep 25, 2025
9 checks passed
hongalex pushed a commit that referenced this pull request Sep 25, 2025
🤖 I have created a release *beep* *boop*
---


##
[2.1.0](pubsub/v2/v2.0.1...pubsub/v2/v2.1.0)
(2025-09-25)


### Features

* **pubsub/v2:** Add subscriber shutdown options
([#12829](#12829))
([14c3887](14c3887))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
@hongalex hongalex deleted the feat-pubsub-shutdown-options branch September 29, 2025 21:06
@kurochan
Copy link
Copy Markdown
Contributor

kurochan commented Oct 2, 2025

Hi @hongalex ,
In subscriber.go, I noticed that when Timeout > 0, the code always calls iter.nackInventory, regardless of the value of ShutdownOptions.Behavior.
This seems to mean that even if ShutdownBehaviorWaitForProcessing is set, messages will still be nacked immediately.
Was the intention that Behavior should only matter when Timeout is negative (infinite wait), or should the shutdown logic branch on Behavior as well? Could you clarify the intended interaction between Timeout and Behavior?

@hongalex
Copy link
Copy Markdown
Member Author

hongalex commented Oct 2, 2025

Approved the above PR but responding here for completeness.

This was indeed a bug, the Behavior should have been checked here (thanks for the PR).

The interaction between Behavior/Timeout is this:

  1. Timeout = 0 trumps everything. This tells the library to drop everything on the ground, and shutdown immediately. You don't need to specify a Behavior here.
  2. Timeout <=0 means we disabled the timeout, and are waiting indefinitely for either Behavior to complete. In WaitForProcessing, wait for messages to be acked or nacked and the callback returns (current library behavior). In NackImmediately mode, we Nack all messages at shutdown.
  3. Timeout >= 0 means we just follow the ShutdownBehavior (wait or nack) up to the specified timeout.

@kurochan
Copy link
Copy Markdown
Contributor

kurochan commented Oct 3, 2025

@hongalex Thank you for your kind explanation. I understand.

hongalex pushed a commit that referenced this pull request Oct 3, 2025
Previously, `ShutdownOptions.Behavior` was never respected — when a
positive `Timeout` was set, messages were always nacked. This PR fixes
the shutdown logic so that messages are only nacked if `Behavior` is
explicitly set to `ShutdownBehaviorNackImmediately`. This makes the
`Behavior` field effective as intended.

related to #12829
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: pubsub Issues related to the Pub/Sub API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants