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

refactor: add retry for flight service #16234

Open
wants to merge 22 commits into
base: main
Choose a base branch
from

Conversation

dqhl76
Copy link
Collaborator

@dqhl76 dqhl76 commented Aug 13, 2024

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
@dqhl76 dqhl76 added ci-benchmark Benchmark: run all test and removed ci-benchmark Benchmark: run all test labels Aug 19, 2024
@dqhl76
Copy link
Collaborator Author

dqhl76 commented Aug 25, 2024

Continue from work on PR #16168 by @zhang2014.

@dqhl76 dqhl76 marked this pull request as draft August 25, 2024 13:43
@datafuselabs datafuselabs deleted a comment from github-actions bot Aug 30, 2024
@datafuselabs datafuselabs deleted a comment from github-actions bot Aug 30, 2024
@datafuselabs datafuselabs deleted a comment from github-actions bot Aug 30, 2024
@datafuselabs datafuselabs deleted a comment from github-actions bot Aug 30, 2024
@dqhl76 dqhl76 added the ci-benchmark Benchmark: run all test label Aug 31, 2024
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.

@dqhl76 dqhl76 added ci-benchmark Benchmark: run all test and removed ci-benchmark Benchmark: run all test labels Sep 2, 2024
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.

@dqhl76 dqhl76 added ci-benchmark Benchmark: run all test and removed ci-benchmark Benchmark: run all test labels Sep 2, 2024
@dqhl76 dqhl76 added the ci-cloud Build docker image for cloud test label Sep 4, 2024
@zhang2014 zhang2014 added ci-benchmark Benchmark: run all test and removed ci-benchmark Benchmark: run all test labels Sep 18, 2024
Copy link
Contributor

Docker Image for PR

  • tag: pr-16234-65a2cf0-1726678279

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

@dqhl76 dqhl76 added ci-benchmark Benchmark: run all test and removed ci-benchmark Benchmark: run all test labels Sep 19, 2024
Copy link
Contributor

Docker Image for PR

  • tag: pr-16234-69ced53-1726717981

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

@zhang2014 zhang2014 added ci-benchmark Benchmark: run all test and removed ci-benchmark Benchmark: run all test labels Sep 19, 2024
Copy link
Contributor

Docker Image for PR

  • tag: pr-16234-f3b386c-1726721618

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

@dqhl76 dqhl76 added ci-benchmark Benchmark: run all test and removed ci-benchmark Benchmark: run all test labels Sep 19, 2024
Copy link
Contributor

Docker Image for PR

  • tag: pr-16234-a216c3e-1726734760

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

@dqhl76
Copy link
Collaborator Author

dqhl76 commented Sep 20, 2024

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
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-query Area: databend query ci-benchmark Benchmark: run all test pr-refactor this PR changes the code base without new features or bugfix size:XXL This PR changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants