Projet

Général

Profil

Bug #65388

Non réutilisation des sessions PostgreSQL dans tenant_command

Ajouté par Pierre Ducroquet il y a presque 2 ans. Mis à jour il y a presque 2 ans.

Statut:
Fermé
Priorité:
Normal
Assigné à:
Catégorie:
-
Version cible:
-
Début:
18 mai 2022
Echéance:
% réalisé:

0%

Temps estimé:
Patch proposed:
Oui
Planning:
Non

Description

Suite à #64627, j'ai trouvé dans le fichier ./hobo/multitenant/management/commands/tenant_command.py

def run_command_from_argv(command, argv):
    # blabla initialization
    try:
        command.execute(*args, **cmd_options)
    except Exception as e:
        # blabla pas cool
    finally:
        try:
            connections.close_all()
        except ImproperlyConfigured:
            # Ignore if connections aren't setup at this point (e.g. no
            # configured settings).
            pass

Alors que dans le code principal (Command::run_from_argv):

        if args_namespace.all_tenants:
            errors = []
            for tenant in TenantMiddleware.get_tenants():
                connection.set_tenant(tenant)
                # blabla logs
                error = run_command_from_argv(klass, args)
                if error:
                    errors.append(error)
            if errors:
                self.stderr.write('Command failed on multiple tenants')
                sys.exit(1)

Du coup, à chaque fois qu'un cron est lancé sur tous les tenants, on va avoir près de 200 sessions de quelques millisecondes.
Dans le cas de passerelle, c'est 2 crons toutes les 5 minutes, 2 crons toutes les heures. Donc environ 5200 sessions par heure.
On retrouve à peu près ce chiffre côté PG, sur une heure on a pour passerelle environ 4770 sessions de moins d'une minute.
Étant donné le coût d'une session TLS, et le coût d'ouverture d'une session côté serveur PG, il vaudrait mieux changer ce comportement.


Fichiers

Révisions associées

Révision 71fd12f2 (diff)
Ajouté par Pierre Ducroquet il y a presque 2 ans

tenant_command: don't close sql sessions after each tenant (#65388)

Historique

#2

Mis à jour par Thomas Noël il y a presque 2 ans

  • Description mis à jour (diff)
#3

Mis à jour par Pierre Ducroquet il y a presque 2 ans

Le patch (oubli de ma part, désolé)

#4

Mis à jour par Thomas Noël il y a presque 2 ans

  • Assigné à mis à Pierre Ducroquet

Juste par cohérence/propreté, j'ajouterais le même « try: connections.close_all(); except ImproperlyConfigured: pass » également dans le else: en dessous qui gère le cas d'un tenant unique. Non ?

#5

Mis à jour par Pierre Ducroquet il y a presque 2 ans

  • Assigné à Pierre Ducroquet supprimé

Thomas Noël a écrit :

Juste par cohérence/propreté, j'ajouterais le même « try: connections.close_all(); except ImproperlyConfigured: pass » également dans le else: en dessous qui gère le cas d'un tenant unique. Non ?

Comme ce else n'appelait pas la fonction, et donc ne passait pas par le close_all, j'ai considéré préférable de ne pas changer ce chemin de code.

#6

Mis à jour par Thomas Noël il y a presque 2 ans

  • Statut changé de Solution proposée à Solution validée

Pierre Ducroquet a écrit :

Thomas Noël a écrit :

Juste par cohérence/propreté, j'ajouterais le même « try: connections.close_all(); except ImproperlyConfigured: pass » également dans le else: en dessous qui gère le cas d'un tenant unique. Non ?

Comme ce else n'appelait pas la fonction, et donc ne passait pas par le close_all, j'ai considéré préférable de ne pas changer ce chemin de code.

Oui je suis bête, notre close_all venait d'une copie du run_from_argv de Django -- et ce dernier le contient toujours, donc inutile de le répéter après klass.run_from_argv(args)

Go !

#7

Mis à jour par Pierre Ducroquet il y a presque 2 ans

  • Statut changé de Solution validée à Résolu (à déployer)
  • Assigné à mis à Pierre Ducroquet

=> Mergé

commit 71fd12f2606b01bff56087664a45c5dbf4890a29 (HEAD -> main, origin/main, origin/HEAD, wip/65388-command-tenants-connection)
Author: Pierre Ducroquet <pducroquet@entrouvert.com>
Date:   Wed May 18 12:26:10 2022 +0200

    tenant_command: don't close sql sessions after each tenant (#65388)

#8

Mis à jour par Transition automatique il y a presque 2 ans

  • Statut changé de Résolu (à déployer) à Solution déployée
#9

Mis à jour par Transition automatique il y a presque 2 ans

Automatic expiration

Formats disponibles : Atom PDF