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

HTTP connection pooling #20

Closed
klaemo opened this issue Jun 1, 2023 · 2 comments · Fixed by #30
Closed

HTTP connection pooling #20

klaemo opened this issue Jun 1, 2023 · 2 comments · Fixed by #30
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@klaemo
Copy link

klaemo commented Jun 1, 2023

Hey there,
methods like batchCheck and listRelations can potentially issue a lot of individual requests. We noticed some rather severe performance issues which we narrowed down to each of those requests doing a new DNS lookup and TCP connection setup.

Once we enabled connection re-use/pooling/keep alive (whatever you want to call it) we saw big improvements.

http.globalAgent = new http.Agent({ keepAlive: true });
https.globalAgent = new https.Agent({ keepAlive: true });

I think this should either be the default for the SDK's axios instance, a parameter for the OpenFgaClient or the very least mentioned in the docs.

What do you think?

@rhamzeh rhamzeh added enhancement New feature or request good first issue Good for newcomers labels Jun 1, 2023
@rhamzeh
Copy link
Member

rhamzeh commented Jun 1, 2023

Thanks for the feedback @klaemo!

Please feel free to submit a PR, both in the documentation and setting it as default in the code would be accepted.

Here is the relevant part of the code that instantiates axios if not provided: https://github.com/openfga/js-sdk/blob/main/base.ts#LL43C67-L43C67

Side-note: At some point we'd like to replace axios and move to a framework that allows http2/3 (see: #18), would love your feedback on that then!

@rhamzeh
Copy link
Member

rhamzeh commented Aug 18, 2023

Thanks for the report & suggestion @klaemo . v0.2.8 is out with keep alive enabled by default

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants