API Diff : passer la réponse en streaming #462
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pauletienney
requested changes
Sep 26, 2024
Co-authored-by: Paul Etienney <[email protected]>
for more information, see https://pre-commit.ci
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Dans cette PR, j'utilise la fonction
StreamingHttpResponsefournie par Django qui permet de renvoyer une réponse HTTP dont le contenu est généré au fur et à mesure de l'envoi, par opposition àHttpResponsequi prend en argument un contenu qui doit déja être généré au moment où l'envoi commence.La fonction
copy_expertde psycopg2 permet également de streamer du contenu depuis la base de donnée vers le code Python.La difficultée rencontrée a été de mettre ces 2 streams "bout à bout", c'est à dire que le résultat de
copy_expertsoit passé au fur et à mesure àStreamingHttpResponse, sans que l’entièreté de la donnée de soit stockée à aucun moment par le code python.La seule manière de faire que j'ai trouvée jusqu'à présent a été de forker le processus, que le processus child fasse la requête Postgres et stream le résultat sur un pipe qui peut être lu au fur et à mesure par le processus parent qui en envoie le contenu à
StreamingHttpResponse.J'aurais préféré que tout soit fait par le même processus, pour ne pas créer de process au niveau de l'OS pour répondre à une requête GET sur le site, parce qu'on ne peut créer qu'un nombre limité de process sur le serveur avant d'avoir des problèmes.
On peut quand même merger la fonctionnalité, quitte à revenir et l'améliorer par la suite si on voit que créer un thread pour l'occasion pose problème.
J'ai fait un test en local sur un diff massif. Avant la PR, la requête HTTP restait en attente de réponse pendant 8minutes, puis se mettait à télécharger le diff à toute vitesse (toute la donnée est donc générée sur le serveur avant de commencer à être envoyée), risque de timeout important et de crash du serveur si toute la ram est utilisée. Avec le streaming, la réponse met seulement quelques secondes à démarrer, puis se télécharge à un rythme bien plus lent, qui est en fait la vitesse à laquelle la base de donnée arrive à sortir ses résultats. La durée totale de l'opération est sensiblement la même.
j'ai posé une question ici aussi, pas eu de réponse pour le moment.