Skip to content

impl(pubsub): pipe publish timeout to concurrent batch actor - #6830

Open
suzmue wants to merge 1 commit into
googleapis:mainfrom
suzmue:pubsub-publish-retry-policy
Open

suzmue wants to merge 1 commit into
googleapis:mainfrom
suzmue:pubsub-publish-retry-policy

Conversation

@suzmue

@suzmue suzmue commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Extract the total timeout duration from the configured retry policy in BasePublisher and forward it through PublisherPartialBuilder and Dispatcher to ConcurrentBatchActor in preparation for publish request hedging. Calculating it this way means we may be off from the actual total_timeout by a small amount (the time between creating the state and querying total_timeout).

This is used by hedging to make sure that the timeout for a hedged request does not exceed the total timeout of the original request.

For #6776

@suzmue
suzmue requested a review from a team as a code owner September 15, 2026 22:38
@product-auto-label product-auto-label Bot added the api: pubsub Issues related to the Pub/Sub API. label Sep 15, 2026
@suzmue suzmue changed the title feat(pubsub): pipe publish timeout to concurrent batch actor impl(pubsub): pipe publish timeout to concurrent batch actor Sep 15, 2026
@suzmue
suzmue force-pushed the pubsub-publish-retry-policy branch from 1cdc3af to a471e18 Compare September 15, 2026 22:38

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a total_timeout configuration of type Option<std::time::Duration> to the Pub/Sub publisher. It extracts the remaining time from the builder's retry policy during BasePublisher initialization and propagates this timeout through the PublisherPartialBuilder, Dispatcher, and ConcurrentBatchActor. Additionally, unit tests are added to verify the default, custom, and attempt-limit-only timeout behaviors. There are no review comments to address, and the changes conform to the repository's guidelines.

@codecov

codecov Bot commented Sep 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.18%. Comparing base (0c2c082) to head (5686b2c).

Additional details and impacted files
@@           Coverage Diff            @@
##             main    #6830    +/-   ##
========================================
  Coverage   97.18%   97.18%            
========================================
  Files         330      330            
  Lines      110390   110567   +177     
========================================
+ Hits       107282   107457   +175     
- Misses       3108     3110     +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@suzmue
suzmue force-pushed the pubsub-publish-retry-policy branch from d6025e2 to 4899c14 Compare September 15, 2026 23:33
@suzmue
suzmue marked this pull request as draft September 16, 2026 20:19
@suzmue

suzmue commented Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

I think we will want to include the whole retry policy, it turns out we want to resolve responses with non-transient errors, so we want the policy to find out which ones are considered non-transient.

@suzmue
suzmue force-pushed the pubsub-publish-retry-policy branch 3 times, most recently from 3ee70a8 to 7aafcb9 Compare September 18, 2026 22:53
@suzmue

suzmue commented Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

I think we will want to include the whole retry policy, it turns out we want to resolve responses with non-transient errors, so we want the policy to find out which ones are considered non-transient.

NVM we don't need it. just saving total_timeout for now.

@suzmue
suzmue force-pushed the pubsub-publish-retry-policy branch from 7aafcb9 to a8d560a Compare September 18, 2026 23:11
Forward the configured publish timeout from BasePublisher through
PublisherPartialBuilder and Dispatcher to ConcurrentBatchActor.
This enables publisher request hedging to clamp hedged attempt timeouts
to min(remaining_total_time, 10s).
@suzmue
suzmue force-pushed the pubsub-publish-retry-policy branch from a8d560a to 5686b2c Compare September 18, 2026 23:12
@suzmue
suzmue marked this pull request as ready for review September 18, 2026 23:12
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.

1 participant