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

fix: query flight rpc connection leaking #13956

Merged
merged 13 commits into from Dec 14, 2023
Merged

fix: query flight rpc connection leaking #13956

merged 13 commits into from Dec 14, 2023

Conversation

dantengsky
Copy link
Member

@dantengsky dantengsky commented Dec 8, 2023 •
edited

I hereby agree to the terms of the CLA available at: https://databend.rs/dev/policies/cla/

Summary

fix rpc connection leak:

  • use the current runtime while building FlightExchange::Receiver in flight_client

  • introduce WatchNotify
    which provides the similar functionality of Notify, excepts that it allows notify_waiters be called before Notified future is instantiated or being polled, without losing wakeups.
    It replaced the Notify used in flight_client, and ExecutorTasksQueue, PipelineExecutor and RangeJoinState

  • Closes #issue

This change is Reviewable

@github-actions github-actions bot added the pr-bugfix this PR patches a bug in codebase label Dec 8, 2023
@dantengsky dantengsky added the ci-benchmark Benchmark: run all test label Dec 8, 2023
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Dec 8, 2023

Docker Image for PR

  • tag: pr-13956-c3d54d5

note: this image tag is only available for internal use,
please check the internal doc for more details.

@github-actions GitHub Actions

This comment was marked as outdated.

Sign in to view
@dantengsky dantengsky added ci-benchmark Benchmark: run all test and removed ci-benchmark Benchmark: run all test labels Dec 8, 2023
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Dec 8, 2023

Docker Image for PR

  • tag: pr-13956-d65dc65

note: this image tag is only available for internal use,
please check the internal doc for more details.

@github-actions GitHub Actions

This comment was marked as outdated.

Sign in to view
@dantengsky dantengsky added ci-benchmark Benchmark: run all test and removed ci-benchmark Benchmark: run all test labels Dec 8, 2023
@dantengsky dantengsky changed the title fix: use current rt fix: use current rt to create FlightExchange Dec 8, 2023
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Dec 8, 2023

Docker Image for PR

  • tag: pr-13956-e0446f2

note: this image tag is only available for internal use,
please check the internal doc for more details.

@dantengsky dantengsky changed the title fix: use current rt to create FlightExchange fix: use current rt to create FlightExchange::Receiver Dec 8, 2023
@github-actions GitHub Actions

This comment was marked as outdated.

Sign in to view
@dantengsky dantengsky added ci-benchmark Benchmark: run all test and removed ci-benchmark Benchmark: run all test labels Dec 8, 2023
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Dec 8, 2023

Docker Image for PR

  • tag: pr-13956-e0446f2

note: this image tag is only available for internal use,
please check the internal doc for more details.

@github-actions GitHub Actions

This comment was marked as outdated.

Sign in to view
@dantengsky dantengsky added ci-benchmark Benchmark: run all test and removed ci-benchmark Benchmark: run all test labels Dec 8, 2023
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Dec 8, 2023

Docker Image for PR

  • tag: pr-13956-0a06647

note: this image tag is only available for internal use,
please check the internal doc for more details.

@github-actions GitHub Actions

This comment was marked as outdated.

Sign in to view
@dantengsky dantengsky added ci-benchmark Benchmark: run all test and removed ci-benchmark Benchmark: run all test labels Dec 8, 2023
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Dec 8, 2023

Docker Image for PR

  • tag: pr-13956-5051d7c

note: this image tag is only available for internal use,
please check the internal doc for more details.

@github-actions GitHub Actions

This comment was marked as outdated.

Sign in to view
@dantengsky
fix: create FlightReceiver using current rt
6a0dbe6
@dantengsky
fix: use notify_one in Flightreceiver::close
801f24f 
instead of `notify_waiters`, so that the task spawned in
`streaming_receiver` shall not missed the notification, even if the
notification is sent before waiting
@dantengsky
add some comments about the notification receiving
587e4a7
@dantengsky dantengsky added ci-benchmark Benchmark: run all test and removed ci-benchmark Benchmark: run all test labels Dec 10, 2023
@github-actions GitHub Actions
Copy link
Contributor

Docker Image for PR

  • tag: pr-13956-324f605

note: this image tag is only available for internal use,
please check the internal doc for more details.

@dantengsky dantengsky changed the title fix: use current rt to create FlightExchange::Receiver fix: query flight rpc connection leaking Dec 10, 2023
@github-actions GitHub Actions
Copy link
Contributor

cloud cloud

ClickBench Report

@dantengsky dantengsky added ci-benchmark Benchmark: run all test and removed ci-benchmark Benchmark: run all test labels Dec 10, 2023
@github-actions GitHub Actions
Copy link
Contributor

Docker Image for PR

  • tag: pr-13956-324f605

note: this image tag is only available for internal use,
please check the internal doc for more details.

@github-actions GitHub Actions
Copy link
Contributor

cloud cloud

ClickBench Report

@dantengsky dantengsky marked this pull request as ready for review December 13, 2023 09:47
@github-actions GitHub Actions
Copy link
Contributor

Pull request description must contain CLA like the following:

I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/

## Summary

Summary about this PR

- Close #issue

@BohuTANG BohuTANG merged commit e76a65e into datafuselabs:main Dec 14, 2023
66 of 68 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lichuang lichuang lichuang approved these changes

@zhang2014 zhang2014 zhang2014 approved these changes

@SkyFan2002 SkyFan2002 SkyFan2002 approved these changes

@BohuTANG BohuTANG Awaiting requested review from BohuTANG

Assignees
No one assigned
Labels
ci-benchmark Benchmark: run all test pr-bugfix this PR patches a bug in codebase
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@dantengsky @lichuang @zhang2014 @SkyFan2002 @BohuTANG