algolia / algolia/algoliasearch-client-javascript

[bug]: Memory leak in Node requester

Aperta
#1,570 0 commenti 3 reazioni 0 assegnatari Vedi su GitHub
bug
Lingua principale
TypeScript
Stelle
1.4k
Fork
226
Metriche di merge delle PR
Nessuna PR unita negli ultimi 30g

Descrizione

### Description

We got reports of memory leaks from users of React InstantSearch with Next.js.

It's easy to reproduce, with an `index.mjs` file :
```js
import process from 'process';

import { algoliasearch } from 'algoliasearch';

process.stdin.resume();

const client = algoliasearch('latency', '6be0576ff61c053d5f9a3225e2a90f76');

for (let i = 0; i < 1000; i++) {
client.search({ requests: [{ indexName: 'instant_search' }] });
}
```

Then run `node --inspect index.mjs`, in Chrome open `chrome://inspect` and select `index.mjs`.

In the memory tab, the VM instance heap size hovers around 90MB without ever going down.

image

All responses are retained by the error handler.

I tried patching it by adding `req.removeAllListeners()` as well as `response.removeAllListeners()` in this `end` handler : https://github.com/algolia/algoliasearch-client-javascript/blob/36ccbc9799823ccb4868460292f921d1929941e6/packages/requester-node-http/src/createHttpRequester.ts#L60-L69

It's not perfect but it now settles around 20MB. Some unnecessary values are still there but maybe this will properly get GC'd if needed.
image

I told them to use `requester-fetch` instead as it's now supported by recent versions of Node.js. Maybe we could make that the default ? From my own benchmarks it seems better than the Node requester currently but with my proposed fix it would make the Node requester more performant than the fetch one 😅 (with undici)

### Client

All

### Version

5.15.0

### Relevant log output

_No response_

Guida per i contributori

Nessuna guida per i contributori indicizzata per questo repository

Direzione di ricerca

Inizia da packages/requester-node-http/src/createHttpRequester.ts, in particolare dall'end handler collegato nel report, e riproduci il leak con lo script index.mjs fornito sotto Node's inspector. Traccia quali listener di request e response trattengono l'error handler, quindi verifica che le ricerche ripetute non mantengano più in vita tutte le responses e che l'utilizzo dell'heap possa stabilizzarsi dopo la garbage collection.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Valutazione

Stack tecnologico
node.js, typescript
Ambito
backend-api-design, performance
Tipo di issue
Bug
Difficoltà
4/5
Tempo stimato
3-5 giorni
Stato di attività
Ferma
Chiarezza
Abbastanza chiara
Idoneità per principianti
35/100

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.