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.

Sign up for GitHub

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

Jump to bottom

Query: Fix (*Stream).receive infinite loop when target response Unimplemented error (#4676) #4681

Merged
merged 1 commit into from
Sep 19, 2021
Merged

Query: Fix (*Stream).receive infinite loop when target response Unimplemented error (#4676) #4681

merged 1 commit into from
Sep 19, 2021

Conversation

hanjm
Copy link
Member

@hanjm hanjm commented Sep 19, 2021 •
edited
Loading

Fix #4676.
This PR base on #4680.

grpc doc say https://github.com/grpc/grpc-go/blob/v1.40.0/stream.go#L123

	// RecvMsg blocks until it receives a message into m or the stream is
	// done. It returns io.EOF when the stream completes successfully. On
	// any other error, the stream is aborted and the error contains the RPC
	// status.
  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

Changes

if gRPC bid-stream response error, return.

Verification

test in #4676

@hanjm
Query: Fix (*exemplarsStream).receive/(*metricMetadataStream).receive…
3abfe9f 
…/(*targetsStreamStream).receive infinite loop when target response Unimplemented error (thanos-io#4676)

Signed-off-by: hanjm <[email protected]>
@hanjm
Copy link
Member Author

hanjm commented Sep 19, 2021 •
edited
Loading

Hi @yeya24 @bwplotka @hitanshu-mehta , This is a import fix to compatible with old store/sidecar, could you like to help review? thank you.

@hanjm hanjm changed the title Query: Fix (*StreamStream).receive infinite loop when target response Unimplemented error (#4676) Query: Fix (*Stream).receive infinite loop when target response Unimplemented error (#4676) Sep 19, 2021
@hanjm hanjm mentioned this pull request Sep 19, 2021
Closed
yeya24
yeya24 approved these changes Sep 19, 2021
View reviewed changes
Copy link
Contributor

@yeya24 yeya24 left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for fixing this. @hanjm

@yeya24 yeya24 merged commit 2d4d140 into thanos-io:main Sep 19, 2021
@yeya24 yeya24 mentioned this pull request Sep 20, 2021
Closed
2 tasks
@bwplotka
Copy link
Member

Thanks a lot!

bwplotka pushed a commit that referenced this pull request Sep 20, 2021
@hanjm @bwplotka
Query: Fix (*exemplarsStream).receive/(*metricMetadataStream).receive…
c2294b4 
…/(*targetsStreamStream).receive infinite loop when target response Unimplemented error (#4676) (#4681)

Signed-off-by: hanjm <[email protected]>
@bwplotka bwplotka mentioned this pull request Sep 20, 2021
Merged
kakkoyun pushed a commit that referenced this pull request Sep 20, 2021
@bwplotka @hanjm @yeya24
Cut 0.23.0-rc.1 and cherry picked 3 critical commits from main. (#4684)
2bf7e1c 
* Sidecar: Fix process external label on promethues v2.28+ use units.Bytes config type (#4657)

* Sidecar: Fix process external label when promethues v2.28+ use units.Bytes config type (#4656)

Signed-off-by: hanjm <[email protected]>

* E2E: Upgrade prometheus image version

Signed-off-by: hanjm <[email protected]>

* upgrade Prometheus dependency version to v2.30.0 (#4669)

* upgrade Prometheus dependency version to v2.30.0

Signed-off-by: Ben Ye <[email protected]>

* fix unit test

Signed-off-by: Ben Ye <[email protected]>
# Conflicts:
#	go.mod
#	go.sum

* Query: Fix (*exemplarsStream).receive/(*metricMetadataStream).receive/(*targetsStreamStream).receive infinite loop when target response Unimplemented error (#4676) (#4681)

Signed-off-by: hanjm <[email protected]>

* Cut 0.23.0-rc.1

Signed-off-by: Bartlomiej Plotka <[email protected]>

Co-authored-by: Jimmiehan <[email protected]>
Co-authored-by: Ben Ye <[email protected]>
bwplotka added a commit that referenced this pull request Oct 6, 2021
@bwplotka @hanjm
@yeya24 @matej-g
Pull release-0.23 into main (#4738)
ff801b1 
* Cut release 0.23.0-rc.0 (#4625)

Signed-off-by: Bartlomiej Plotka <[email protected]>

* Updated version.

Signed-off-by: Bartlomiej Plotka <[email protected]>

* Cut 0.23.0-rc.1 and cherry picked 3 critical commits from main. (#4684)

* Sidecar: Fix process external label on promethues v2.28+ use units.Bytes config type (#4657)

* Sidecar: Fix process external label when promethues v2.28+ use units.Bytes config type (#4656)

Signed-off-by: hanjm <[email protected]>

* E2E: Upgrade prometheus image version

Signed-off-by: hanjm <[email protected]>

* upgrade Prometheus dependency version to v2.30.0 (#4669)

* upgrade Prometheus dependency version to v2.30.0

Signed-off-by: Ben Ye <[email protected]>

* fix unit test

Signed-off-by: Ben Ye <[email protected]>
# Conflicts:
#	go.mod
#	go.sum

* Query: Fix (*exemplarsStream).receive/(*metricMetadataStream).receive/(*targetsStreamStream).receive infinite loop when target response Unimplemented error (#4676) (#4681)

Signed-off-by: hanjm <[email protected]>

* Cut 0.23.0-rc.1

Signed-off-by: Bartlomiej Plotka <[email protected]>

Co-authored-by: Jimmiehan <[email protected]>
Co-authored-by: Ben Ye <[email protected]>

* Cut 0.23.0 release. (#4697)

* Endpointset: Do not use info client to obtain metadata (for now) (#4714)

* Do not use info client to obtain metadata

Signed-off-by: Matej Gera <[email protected]>

* Update CHANGELOG.

Signed-off-by: Matej Gera <[email protected]>

* Comment out client.info usage

Signed-off-by: Matej Gera <[email protected]>

* Fix lint error

Signed-off-by: Matej Gera <[email protected]>

* Cutting 0.23.1 (#4718)

Signed-off-by: Bartlomiej Plotka <[email protected]>

* Moved tutorials Thanos versions to 0.23.1

Signed-off-by: Bartlomiej Plotka <[email protected]>

* Added volounteer for shepharding, fixed VERSION.

Signed-off-by: Bartlomiej Plotka <[email protected]>

Co-authored-by: Jimmiehan <[email protected]>
Co-authored-by: Ben Ye <[email protected]>
Co-authored-by: Matej Gera <[email protected]>
someshkoli pushed a commit to someshkoli/thanos that referenced this pull request Nov 7, 2021
@hanjm @someshkoli
Query: Fix (*exemplarsStream).receive/(*metricMetadataStream).receive…
02f7af3 
…/(*targetsStreamStream).receive infinite loop when target response Unimplemented error (thanos-io#4676) (thanos-io#4681)

Signed-off-by: hanjm <[email protected]>
@hanjm hanjm mentioned this pull request Mar 31, 2022
Closed
sthaha added a commit to sthaha/thanos that referenced this pull request Apr 12, 2022
@sthaha
Bug 2071479: Thanos Querier high CPU and memory usage till OOM
18e05ab 
This cherry-picks upstream patch that fixes the bug

Query: Fix (*exemplarsStream).receive/(*metricMetadataStream).receive/(*targetsStreamStream).receive
  infinite loop when target response Unimplemented error (thanos-io#4676) (thanos-io#4681)

See:
  - thanos-io#4676 (comment)
  - thanos-io#4681

Signed-off-by: hanjm <[email protected]>
(cherry picked from commit 2d4d140)

Signed-off-by: Sunil Thaha <[email protected]>
sthaha added a commit to sthaha/thanos that referenced this pull request Apr 12, 2022
@sthaha
Bug 2074413: Thanos Querier high CPU and memory usage till OOM
9997d02 
This cherry-picks upstream patch that fixes the bug

Query: Fix (*exemplarsStream).receive/(*metricMetadataStream).receive/(*targetsStreamStream).receive
  infinite loop when target response Unimplemented error (thanos-io#4676) (thanos-io#4681)

See:
  - thanos-io#4676 (comment)
  - thanos-io#4681

Signed-off-by: hanjm <[email protected]>
(cherry picked from commit 2d4d140)

Signed-off-by: Sunil Thaha <[email protected]>
@sthaha sthaha mentioned this pull request Apr 12, 2022
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yeya24 yeya24 yeya24 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Exemplar: Can not set partial response.
3 participants
@hanjm @bwplotka @yeya24