Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix BQ storage stream split #32376

Merged
merged 1 commit into from
Aug 30, 2024
Merged

Conversation

RustedBones
Copy link
Contributor

@RustedBones RustedBones commented Aug 30, 2024

A variable shadowing was introduced here, preventing modification of the stream reader source after splitting.

I don't know the implication of this bug.

A variable shadowing was introduced, preventing modification of the
stream reader source after splitting.
Copy link
Contributor

Assigning reviewers. If you would like to opt out of this review, comment assign to next reviewer:

R: @kennknowles for label java.
R: @shunping for label io.

Available commands:

  • stop reviewer notifications - opt out of the automated review tooling
  • remind me after tests pass - tag the comment author after tests pass
  • waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

The PR bot will only process comments in the main thread (not review comments).

Copy link
Contributor

@Abacn Abacn left a comment

Choose a reason for hiding this comment

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

another bug introduced by seemingly trivial code format cleanup.

@Abacn Abacn merged commit 52ab49e into apache:master Aug 30, 2024
17 checks passed
@Abacn
Copy link
Contributor

Abacn commented Aug 30, 2024

#16721 - to add a cross link to that PR tracking checkframework fix side effects

@RustedBones RustedBones deleted the bq-response-stream branch August 30, 2024 15:03
@RustedBones
Copy link
Contributor Author

Implication of this bug looks to be limited to cancel not being called on the right stream after subsequent split. No implication on data

@Abacn
Copy link
Contributor

Abacn commented Aug 30, 2024

cc: @ahmedabu98 sounds like another place of stale stream leak

@kennknowles
Copy link
Member

Implication of this bug looks to be limited to cancel not being called on the right stream after subsequent split. No implication on data

Does this make it excessively hard to test or can a test be added?

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

Successfully merging this pull request may close these issues.

3 participants