polygon-io / client-js

The official JS client library for the Polygon REST and WebSocket API.
MIT License
184 stars 57 forks source link

Support for connection keepalives and connection pooling? #169

Open jdconley opened 1 year ago

jdconley commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 5 months ago

@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 commented 5 months ago

@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 commented 5 months ago

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 commented 4 months ago

@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