Skip to content

fix(spanner): fix off-by-one in the streaming-query retry attempt count#6086

Open
fornwall wants to merge 1 commit into
googleapis:mainfrom
fornwall:up-14-streaming-retry-state-seeding
Open

fix(spanner): fix off-by-one in the streaming-query retry attempt count#6086
fornwall wants to merge 1 commit into
googleapis:mainfrom
fornwall:up-14-streaming-retry-state-seeding

Conversation

@fornwall

Copy link
Copy Markdown
Contributor

Fix attempt count off-by-one in retries for streaming reads, due to attempt and retry count mixup.

See the doc on RetryPolicy::with_attempt_limit:

    /// This policy decorates an inner policy and limits the total number of
    /// attempts. Note that `on_error()` is not called before the initial
    /// (non-retry) attempt. Therefore, setting the maximum number of attempts
    /// to 0 or 1 results in no retry attempts.

Include also a related minor change to use a constant for default number of attempts instead of hardcoding the attempt limit in the test.

Fix attempt count off-by-one in retries for streaming reads, due to
attempt and retry count mixup.

Include also a related minor change to use a constant for default number
of attempts instead of hardcoding the attempt limit in the test.

Signed-off-by: Fredrik Fornwall <fredrik@fornwall.net>
@fornwall
fornwall requested review from a team as code owners July 17, 2026 12:22
@product-auto-label product-auto-label Bot added the api: spanner Issues related to the Spanner API. label Jul 17, 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 a DEFAULT_ATTEMPT_LIMIT constant to replace hardcoded retry limits in ResultSet. It also corrects the retry attempt count calculation by including the initial attempt (1 + self.retry_count) and updates the corresponding unit tests to dynamically calculate expected backoffs based on the new constant. There are no review comments, and I have no feedback to provide.

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

Labels

api: spanner Issues related to the Spanner API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant