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

Updated http client from net/http to fasthttp #96

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

Conversation

code-xD
Copy link

@code-xD code-xD commented Sep 22, 2024

Description

Modified Aptos Http client from net/http to fasthttp. There has been issues with connection issues with newer Aptos blockchain like Movement.

Test Plan

Related Links

@code-xD code-xD requested review from gregnazario and a team as code owners September 22, 2024 00:44
Copy link
Contributor

@gregnazario gregnazario left a comment

Choose a reason for hiding this comment

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

This looks generally good, but my main concern is that it doesn't support HTTP/2?

@code-xD
Copy link
Author

code-xD commented Oct 8, 2024

This looks generally good, but my main concern is that it doesn't support HTTP/2?

Hey @gregnazario,
I will have to check the compatibility of the new http package with HTTP/2. Do you have any sample RPC Endpoints which I can use to test this behaviour?

@gregnazario
Copy link
Contributor

This looks generally good, but my main concern is that it doesn't support HTTP/2?

Hey @gregnazario,

I will have to check the compatibility of the new http package with HTTP/2. Do you have any sample RPC Endpoints which I can use to test this behaviour?

All of the Aptos labs endpoints built in by default support HTTP/2 as far as I know, which by itself should give more performance boost than the handling on the client side afaik.

I found when I set the client pool configuration correctly, I got good concurrency and performance out of it

@code-xD
Copy link
Author

code-xD commented Oct 8, 2024

This looks generally good, but my main concern is that it doesn't support HTTP/2?

Hey @gregnazario,

I will have to check the compatibility of the new http package with HTTP/2. Do you have any sample RPC Endpoints which I can use to test this behaviour?

All of the Aptos labs endpoints built in by default support HTTP/2 as far as I know, which by itself should give more performance boost than the handling on the client side afaik.

I found when I set the client pool configuration correctly, I got good concurrency and performance out of it

Yeah, the connection pool management for this library is far better and it's architected in a manner wherein user have the control over when they want to release the request and response body thereby mitigating memory leaks.

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

Successfully merging this pull request may close these issues.

2 participants