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

Support for connection keepalives and connection pooling? #169

Open
jdconley opened this issue Jul 26, 2023 · 6 comments
Open

Support for connection keepalives and connection pooling? #169

jdconley opened this issue Jul 26, 2023 · 6 comments
Assignees

Comments

@jdconley
Copy link

Is there a way to turn on connection pooling and keep-alives for the REST API? Does the backend API support this? We are Launchpad users and looking at our logs it seems we are establishing a new tcp connection and TLS negotiation with every request adding around 100ms. This is a request for documentation....

@justinpolygon
Copy link
Contributor

Hi @jdconley,

Sorry for the delay. Here's an example that should work that passes the Connection: "keep-alive" header with the requests:

import('@polygon.io/client-js').then(({ restClient }) => {

	const rest = restClient("XXXXX", "https://api.polygon.io");
	const requestOptions = { headers: { Connection: "keep-alive" } };

        const timeRequest = async (request) => {
        const start = performance.now();
        const result = await request;
        const end = performance.now();

        const elapsedTime = end - start;
        console.log(`Request took ${elapsedTime.toFixed(2)} milliseconds.`);
        return result;
    };

    const overallStart = performance.now();

    Promise.all([
        timeRequest(rest.crypto.aggregates("X:BTCUSD", 1, "day", "2023-03-30", "2023-03-31", {}, requestOptions)),
        timeRequest(rest.crypto.aggregates("X:BTCUSD", 1, "day", "2023-03-31", "2023-04-01", {}, requestOptions)),
        timeRequest(rest.crypto.aggregates("X:BTCUSD", 1, "day", "2023-04-01", "2023-04-02", {}, requestOptions)),
        timeRequest(rest.crypto.aggregates("X:BTCUSD", 1, "day", "2023-04-02", "2023-04-03", {}, requestOptions))
    ]).then((data) => {
        const overallEnd = performance.now();
        const totalElapsedTime = overallEnd - overallStart;
        console.log(`Total time for all requests: ${totalElapsedTime.toFixed(2)} milliseconds.`);
    }).catch(e => {
        console.error('An error happened:', e.message);
    });
});

When I run this I see something like:

$ node example.js
Request took 274.06 milliseconds.
Request took 277.18 milliseconds.
Request took 284.45 milliseconds.
Request took 331.46 milliseconds.
Total time for all requests: 348.57 milliseconds.

I'm on the west coast so your times will likely look different but you can see that the sum of all the requests is less than the total time taken and the connection was re-used. This seems to be dependant how how you're making requests in that you'll likely need to work the added header into your workflow.

@justinpolygon justinpolygon self-assigned this Aug 14, 2023
@justinpolygon
Copy link
Contributor

Hey @jdconley, I'm going to close this out unless you need anything else. The short answer is you can pass the Connection: "keep-alive" header along with your requests. The script above should work as an example to test that but it really depends on your workflow and how you're making requests. Please let us know if you need anything else. Cheers.

@nitzan-blink
Copy link

@justinpolygon hi,
the header is not the same as what @jdconley meant.
specifically, I'll quote node js http agent docs:

keepAlive Keep sockets around even when there are no outstanding requests, so they can be used for future requests without having to reestablish a TCP connection. Not to be confused with the keep-alive value of the Connection header. The Connection: keep-alive header is always sent when using an agent except when the Connection header is explicitly specified or when the keepAlive and maxSockets options are respectively set to false and Infinity, in which case Connection: close will be used.

from what I gathered, your library uses cross-fetch, so this is relevant.
to really support it, you should pass a custom http(s) agent, otherwise this will not work.
I really hope you'll implement this soon, as we're considering using your services, and this is a crucial factor in our decision, even more so when your servers are only US based.

thanks

@jdconley
Copy link
Author

jdconley commented Apr 16, 2024

@justinpolygon hi, the header is not the same as what @jdconley meant. specifically, I'll quote node js http agent docs:

I really hope you'll implement this soon, as we're considering using your services, and this is a crucial factor in our decision, even more so when your servers are only US based.

Yeah, you're right. I didn't notice they closed this. Every request requires a new connection with this library at this time.

@justinpolygon justinpolygon reopened this Apr 16, 2024
@justinpolygon
Copy link
Contributor

Hey, thanks for the heads up @nitzan-blink. I'll explore this.

Sorry, @jdconley I thought this was actually closed as resolved with the Connection: "keep-alive" tweak but I'll explore the suggested option here. I've re-opened this.

@jdconley
Copy link
Author

@justinpolygon I looked through the code and was able to get around this by ignoring the typing on the global options for the rest client and provide my own agent option to fetch, since it looks like client-js just passes the unknown properties of the options object straight through to fetch.

Looking through performance traces I no longer see time spent in connection opening per request.

@nitzan-blink Snippet for the fix here... Looks like you can pass any fetch option in this way.

        const client = restClient(this.polygonApiKey, this.apiRootEndpoint, {
          agent: function (_parsedURL: URL) {
            if (_parsedURL.protocol == 'http:') {
              return httpAgent;
            } else {
              return httpsAgent;
            }
          },
        } as any); // casting to any to pass in our agent config

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

No branches or pull requests

3 participants