Got: Performance issue with concurrent requests to a new hostname

[ajmcgarry1608]

Describe the bug

• Node.js version: 12

• OS & version: Win 10

  • Blocked slow requests

Actual behavior

Simultaneous requests causes a blocking behaviour where all activity stops for a number of seconds

Expected behavior

Compared to 'request' this results in a performance of 10x slower.

Code to reproduce

// let request = require('request-promise') //runtime: 1555.632ms
let request = require('got') //runtime: 15837.386ms

let running = 0
function getGoogle () {
    running ++
    console.log('running', running)
    return request.get('https://www.google.com')
    .catch(
        err => {
            console.log('Error:', err)
        }
    )
    .then(
        () => {
            running --
            console.log('running', running)
        }
    )
}

console.time('runtime')
let p = [...Array(100)].map(() => getGoogle())

Promise.all(p)
.then(
    () => {
        console.timeEnd('runtime')
    }
)

[szmarczak]

On my side Got scored 974.973ms and request scored 23.229s

[szmarczak]

@sindresorhus can you confirm?

[ajmcgarry1608]

My times are in the the code as comments.
Memory may play a factor.
Try to raise the number of simultaneous requests to 1000 and repeat.

[ajmcgarry1608]

So far i have been testing locally and not in a production like environment. I intend to run this in AWS lambda and will run a full production simulation tomorrow there with similar memory sizes for both.
Bearing in mind cold start times etc with lambda i will run multiple tests for average and run timings in the code.

The latency happens when the execution appears to halt for a number of seconds with every ~40 completed requests in the pool. The count no longer counts down

[sindresorhus]

runtime: 667.319ms for me for 100. With 1000, I get runtime: 37107.218ms.

[ajmcgarry1608]

runtime: 667.319ms for me for 100. With 1000, I get runtime: 37107.218ms.

How does that compare to request?
Do you notice any pausing during execution of does it run through all the requests smoothly?

[adityapatadia]

I can confirm the issue on following system specs:

Node version: 12.16
OS: MacOS 10.15
GOT version: 11.0.2

I can also confirm that this does not happen with GOT version 10. I am getting around 900 ms for GOT v10. For GOT v11, I get around 77329.729ms.

[ajmcgarry1608]

got.log

request.log

I created a fresh EC2 instances and modified the script slightly to include a promise response time.
Logs for both Got and Request attached

[adityapatadia]

@ajmcgarry1608 Can you check with GOT v10 vs v11?

[szmarczak]

@adityapatadia Can you try disabling the DNS cache?

[ajmcgarry1608]

@ajmcgarry1608 Can you check with GOT v10 vs v11?

I can confirm the problem does not exist in v10

[adityapatadia]

@adityapatadia Can you try disabling the DNS cache?

Disabling DNS cache solved the issue. runtime: 887.433ms

[adityapatadia]

For the record, enabling DNS cache in GOT v10 also has same issue. 🙁

[ajmcgarry1608]

For the record, enabling DNS cache in GOT v10 also has same issue. 🙁

What is the option to disable DNS cache?
I could not see it in the documentation.

This is my first experience using Got. Brilliant library and we are refactoring out Request over 10 repositories and replacing it with Got. As a drop in replacement we have had no issues up until this.

At the moment it looks like DNS cache can have a negative effect. Will there be any action to resolve this?

[adityapatadia]

You can pass dnsCache: false as option.

[ajmcgarry1608]

You can pass dnsCache: false as option.

Given that the default is true for this option and there are obviously issues with this, would it be prudent to patch it back to default false until this is resolved?

[szmarczak]

I think I might know what the problem is. If you got clean cache and try sending 100 requests at once, it may make another 100 DNS queries instead of only one.

[szmarczak]

What is the option to disable DNS cache? I could not see it in the documentation.

Fixed the docs. It's quite hard to manually manage them.

This is my first experience using Got. Brilliant library and we are refactoring out Request over 10 repositories and replacing it with Got. As a drop in replacement we have had no issues up until this.

Awesome!

Given that the default is true for this option and there are obviously issues with this, would it be prudent to patch it back to default false until this is resolved?

We could, but I don't think it affects many people. If you don't spam the requests (your requests are delayed by ~30ms or more), you're good. Otherwise you need to disable the DNS cache.

I'll try to fix the issue now and make a cacheable-lookup release.

[szmarczak]

I can sometimes reproduce:

const CacheableRequest = require('.');
const {get} = require('https');

const cacheable = new CacheableRequest();

const requests = 100;
const options = {
    lookup: cacheable.lookup
};

// cacheable-lookup: 6.707s
// without:          2.494s

let responses = 0;

const callback = () => {
    responses++;

    if (responses === requests) {
        console.timeEnd('x');
    }
};

console.time('x');

for (let i = 0; i < requests; i++) {
    get('https://httpbin.org/anything', options).once('response', response => {
        response.resume();

        response.once('end', () => {
            callback();
        });
    })
}

[szmarczak]

https://github.com/szmarczak/cacheable-lookup/commit/a40009aa73075b2fa93f224bb654f6c37dd10904

[szmarczak]

Can you try again with [email protected]?

[szmarczak]

With 4.2.0 I get 17.408s per 1000 and 831.302ms per 100. So it seems to be solved.

[ajmcgarry1608]

Can you try again with [email protected]?

Looks to be fixed.
Performance now consistently exceeds request.

1000 requests
Got: runtime: 11097.388ms
Request: runtime: 17512.555ms

Great work

[Nisthar]

@ajmcgarry1608 does using [email protected] improve the speed?

[ajmcgarry1608]

@Nisthar it removed the lag that i had in this particular version of Got while it was using 4.1.0. of you check the current dependencies i think the version has moved on to version 5+ last time i looked