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

Remplacer la session MemoryStore (fuite mémoire) pour connect-mongo #183

Closed
ggrossetie opened this issue May 26, 2020 · 7 comments · Fixed by #275
Closed

Remplacer la session MemoryStore (fuite mémoire) pour connect-mongo #183

ggrossetie opened this issue May 26, 2020 · 7 comments · Fixed by #275
Labels

Comments

@ggrossetie
Copy link
Collaborator

Si pour une raison ou une autre le token JWT n'est pas envoyé, le backend gère aussi une session côté serveur avec un cookie.
Je pense qu'il faudrait supprimer ce mode d'authentification et se baser uniquement sur le token JWT.

@ggrossetie
Copy link
Collaborator Author

Comme on ne configure pas le "store", il y a de forte chance que notre consommation mémoire ne cesse d'augmenter avec le temps:

Warning: connect.session() MemoryStore is not
designed for a production environment, as it will leak
memory, and will not scale past a single process.

@thom4parisot
Copy link
Member

thom4parisot commented Jun 23, 2020

J'ai l'impression qu'on n'a pas le choix, qu'on va devoir garder le middleware session, à cause de la stratégie openid.

Par contre oui, utiliser un autre store de session qui ne grignote pas la mémoire au fur et à mesure 👍

@ggrossetie
Copy link
Collaborator Author

J'ai l'impression qu'on n'a pas le choix, qu'on va devoir garder le middleware session, à cause de la stratégie openid.

Oui c'est bien ce qui me semblait...

Par contre oui, utiliser un autre store de session qui ne grignote pas la mémoire au fur et à mesure +1

Oui, pour référence la liste des "stores" disponibles.

Comme on utilise déjà MongoDB pourquoi pas mongodb-js/connect-mongodb-session ou jdesboeufs/connect-mongo.
A noter que la librairie éditée par MongoDB est nettement moins populaire mais n'ayant utilisé ni l'une ni l'autre je ne sais pas si il y a une raison... peut être l'intégration "native" avec mongoose 🤔

https://github.com/jdesboeufs/connect-mongo#re-use-an-existing-mongoose-connection

Sinon sur le disque mais dans un conteneur Docker c'est peut être s’embêter pour rien.

@thom4parisot thom4parisot changed the title Désactiver/supprimer la session côté serveur Remplacer la session MemoryStore (fuite mémoire) pour connect-mongo Sep 13, 2020
@thom4parisot
Copy link
Member

J'ai ajouté cette issue au travail en cours, suite à une nouvelle interruption de service reportée par @antoinentl.

@ggrossetie
Copy link
Collaborator Author

Cela ne fera pas de mal mais même en m'authentifiant plusieurs milliers de fois je n'arrive pas à faire monter la mémoire au dessus de 150Mb 🤔

Avec Node.js 12.x, il est possible de générer des heap dumps à la demande :

Heap Dumps
If you ever needed to generate heap dumps in order to investigate memory issues but were slowed down by having to install a new module into production, the good news is that Node.js 12 brings integrated heap dump capability out of the box. You can check out the documentation in nodejs/node#27133 and nodejs/node#26501 to learn more.

Exemple d'utilisation :

$ node --heapsnapshot-signal=SIGUSR2 index.js &
$ ps aux
USER       PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
node         1  5.5  6.1 787252 247004 ?       Ssl  16:43   0:02 node --heapsnapshot-signal=SIGUSR2 index.js
$ kill -USR2 1
$ ls
Heap.20190718.133405.15554.0.001.heapsnapshot

Cela peut être pratique quand on constate que la mémoire utilisée est importante.
Le soucis c'est que la mémoire semble relativement stable quand on regarde ponctuellement avec docker stats.

L'autre solution serait de vérifier periodiquement dans l'application Node si la mémoire consommée est supérieure à 80% de la mémoire maximum (avec v8.getHeapStatistics() ou process.memoryUsage()) et, si tel est le cas, demander un heap dump en utilisant v8.writeHeapSnapshot([filename]).

Qu'est-ce que tu en penses ?

@thom4parisot
Copy link
Member

Effectivement, ça sera marginal.

J'ai envoyé un message au support technique pour voir ce qui peut être mis à disposition, et je regarde à faire une PR qui met à jour 2-3 trucs.

@ggrossetie
Copy link
Collaborator Author

La mémoire augmente petit à petit sur stylo.huma-num.fr

CONTAINER ID        NAME                CPU %               MEM USAGE / LIMIT     MEM %               NET I/O             BLOCK I/O           PIDS
b1ba187e4ae8        graphql-stylo       0.08%               189.3MiB / 7.789GiB   2.37%               30.8GB / 1.52GB     0B / 0B             23

ggrossetie added a commit to ggrossetie/stylo that referenced this issue Dec 2, 2020
thom4parisot pushed a commit that referenced this issue Dec 2, 2020
…ect-store

resolves #183 met en place connect-mongo pour enregistrer les sessions dans MongoDB
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants