Skip to content

refactor: add retry for flight service #16234

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
Closed
wants to merge 26 commits into from
Closed

refactor: add retry for flight service #16234

wants to merge 26 commits into from

Conversation

dqhl76
Copy link
Collaborator

@dqhl76 dqhl76 commented Aug 13, 2024 •
edited by drmingdrmer
Loading

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

Summary

Support Retry for Cluster Connection: If a network issue is detected, the system will automatically establish a new connection and attempt to continue operations.

Changes Details:

  • The existing implementation of the FlightService lacks support for encoding data with Arc. This leads to inefficient and resource-intensive cloning operations. Vendor the FlightService trait to develop and implement a custom codec that supports Arc encoding.
  • Change the do_get method to do_exchange to enable a bidirectional streaming connection, allowing data to be sent from the client side to the server side.

Tests

  • Unit Test
  • Logic Test
  • Benchmark Test
  • No Test - Explain why

Type of change

  • Bug Fix (non-breaking change which fixes an issue)
  • New Feature (non-breaking change which adds functionality)
  • Breaking Change (fix or feature that could cause existing functionality not to work as expected)
  • Documentation Update
  • Refactoring
  • Performance Improvement
  • Other (please describe):

This change is Reviewable

@github-actions github-actions bot added the pr-refactor this PR changes the code base without new features or bugfix label Aug 13, 2024
@dqhl76 dqhl76 added ci-benchmark Benchmark: run all test and removed ci-benchmark Benchmark: run all test labels Aug 13, 2024
@dqhl76 dqhl76 marked this pull request as ready for review August 16, 2024 02:34
@dqhl76 dqhl76 requested a review from zhang2014 August 16, 2024 02:34
@dqhl76 dqhl76 removed the ci-benchmark Benchmark: run all test label Aug 16, 2024
zhang2014
zhang2014 reviewed Aug 16, 2024
View reviewed changes
@dqhl76 dqhl76 added ci-benchmark Benchmark: run all test and removed ci-benchmark Benchmark: run all test labels Aug 19, 2024
@dqhl76 dqhl76 force-pushed the test_remove_flight branch from 97a3c48 to 4914933 Compare August 25, 2024 13:21
@dqhl76
Copy link
Collaborator Author

dqhl76 commented Aug 25, 2024 •
edited
Loading

Continue from work on PR #16168 by @zhang2014.

@dqhl76 dqhl76 marked this pull request as draft August 25, 2024 13:43
@databendlabs databendlabs deleted a comment from github-actions bot Aug 30, 2024
@databendlabs databendlabs deleted a comment from github-actions bot Aug 30, 2024
@databendlabs databendlabs deleted a comment from github-actions bot Aug 30, 2024
@databendlabs databendlabs deleted a comment from github-actions bot Aug 30, 2024
@dqhl76 dqhl76 added the ci-benchmark Benchmark: run all test label Aug 31, 2024
@github-actions GitHub Actions
Copy link
Contributor

Docker Image for PR

  • tag: pr-16234-b378d7a-1725072756

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

ClickBench Report

@dqhl76 dqhl76 added ci-benchmark Benchmark: run all test and removed ci-benchmark Benchmark: run all test labels Sep 2, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Sep 2, 2024

Docker Image for PR

  • tag: pr-16234-b6df933-1725266272

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

github-actions bot commented Sep 2, 2024

ClickBench Report

@dqhl76 dqhl76 added ci-benchmark Benchmark: run all test and removed ci-benchmark Benchmark: run all test labels Sep 2, 2024
@dqhl76 dqhl76 force-pushed the test_remove_flight branch from f8189d7 to a6cd2a2 Compare September 4, 2024 08:39
@dqhl76 dqhl76 added the ci-cloud Build docker image for cloud test label Sep 4, 2024
@dqhl76 dqhl76 force-pushed the test_remove_flight branch from a695a7d to 9b6658c Compare September 5, 2024 08:12
@github-actions GitHub Actions
Copy link
Contributor

ClickBench Report

@dqhl76
Copy link
Collaborator Author

dqhl76 commented Sep 20, 2024 •
edited
Loading

add a switch setting for retry behaviour:

root@localhost:8000/default> SET GLOBAL max_flight_connection_retry_times=0;

0 row read in 0.085 sec. Processed 0 row, 0 B (0 row/s, 0 B/s)

kill the network connection during query process:

error: APIError: ResponseError with 1038: h2 protocol error: error reading a body from connection: connection reset, source: Some(hyper::Error(Body, Error { kind: Io(Kind(ConnectionReset)) }))

@dqhl76 dqhl76 removed the ci-benchmark Benchmark: run all test label Sep 20, 2024
@dqhl76 dqhl76 added the ci-benchmark Benchmark: run all test label Sep 20, 2024
@github-actions GitHub Actions
Copy link
Contributor

Docker Image for PR

  • tag: pr-16234-957829b-1726821614

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

ClickBench Report

@zhang2014 zhang2014 added ci-benchmark Benchmark: run all test and removed ci-benchmark Benchmark: run all test labels Sep 22, 2024
@github-actions GitHub Actions
Copy link
Contributor

Docker Image for PR

  • tag: pr-16234-daeebed-1726985112

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

ClickBench Report

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Sep 23, 2024
@github-actions GitHub Actions
Copy link
Contributor

Docker Image for PR

  • tag: pr-16234-0fa2f00-1727085123

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

@dqhl76 dqhl76 removed the ci-benchmark Benchmark: run all test label Sep 23, 2024
@github-actions GitHub Actions
Copy link
Contributor

ClickBench Report

@dqhl76 dqhl76 added this pull request to the merge queue Sep 23, 2024
@zhang2014 zhang2014 removed this pull request from the merge queue due to a manual request Sep 23, 2024
@dqhl76 dqhl76 closed this Nov 4, 2024
@dqhl76 dqhl76 deleted the test_remove_flight branch November 4, 2024 05:41
@dqhl76 dqhl76 restored the test_remove_flight branch December 4, 2024 10:29
@b41sh b41sh mentioned this pull request Dec 5, 2024
Merged
11 tasks
@dqhl76 dqhl76 deleted the test_remove_flight branch March 28, 2025 01:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Xuanwo Xuanwo Xuanwo left review comments

@zhang2014 zhang2014 zhang2014 approved these changes

Assignees
No one assigned
Labels
lgtm This PR has been approved by a maintainer pr-refactor this PR changes the code base without new features or bugfix
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dqhl76 @Xuanwo @zhang2014