Skip to content

feat(pubsub): add enabled option to HedgingOptions (post-merge adjustments) - #9654

Draft
feywind wants to merge 1 commit into
pubsub/hedging-system-testsfrom
pubsub/hedging-post-merge-adjustments
Draft

feywind wants to merge 1 commit into
pubsub/hedging-system-testsfrom
pubsub/hedging-post-merge-adjustments

Conversation

@feywind

@feywind feywind commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

Overview

This is a tentative standalone (unstacked) catch-all PR on top of Stack #9467 (#9462-#9466, #9635-#9636) for post-merge adjustments to Publish Hedging in @google-cloud/pubsub.


Summary of Changes in This PR

  • HedgingOptions.enabled (handwritten/pubsub/src/publisher/hedging.ts, handwritten/pubsub/src/default-options.ts):
    • Adds enabled?: boolean to HedgingOptions and defaultOptions.publish.hedging.enabled = true, aligning with the cross-language hedging settings surface (enabled: true, hedgeDelay: 1000ms (1s), maxTokens: 50, refillRatio: 0.1 with bounds 100ms..10s, 1..250, 0.001..0.2).
    • Returning undefined from HedgingTokenBucket.validateAndResolveHedgingOptions when hedging.enabled === false allows callers to explicitly disable hedging via {enabled: false}.
  • Unit & System Tests (handwritten/pubsub/test/publisher/hedging.ts, handwritten/pubsub/system-test/hedging.test.ts):
    • Added unit and system test assertions verifying hedging: {enabled: false} disables hedging and hedging: {} defaults enabled to true.

@product-auto-label product-auto-label Bot added the api: pubsub Issues related to the Pub/Sub API. label Oct 16, 2026

@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 an enabled flag to the publish hedging options, allowing users to explicitly enable or disable hedging. It updates the default options, the HedgingOptions and ResolvedHedgingOptions interfaces, and the validation logic to respect this flag. It also adds corresponding unit and system tests. The feedback suggests making the enabled property in ResolvedHedgingOptions required rather than optional, as it is always returned as true when the options are successfully resolved.

* Validated and defaulted hedging configuration.
*/
export interface ResolvedHedgingOptions {
readonly enabled?: boolean;

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.

medium

Since validateAndResolveHedgingOptions always returns { enabled: true, ... } when hedging is enabled, and returns undefined otherwise, the enabled property in ResolvedHedgingOptions will always be present and true. It should be defined as a required boolean (readonly enabled: boolean;) rather than optional to avoid unnecessary null/undefined checks for consumers of ResolvedHedgingOptions.

Suggested change
readonly enabled?: boolean;
readonly enabled: boolean;

@feywind
feywind force-pushed the pubsub/hedging-system-tests branch from c7afb42 to 0ad47ac Compare October 19, 2026 21:57
@feywind
feywind force-pushed the pubsub/hedging-post-merge-adjustments branch from 8f26d64 to 1bfd5e3 Compare October 19, 2026 21:58

This branch has not been deployed

No deployments
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