Retour d’expérience : quand Doctrine Collections rencontre de grandes cardinalités

Dans le cadre d’un gros projet, sur lequel nous intervenons en support, une réécriture du mécanisme des codes promo a été mise en ligne.

Nous sommes souvent assez sereins sur le déploiement de nouveau code, car nous avons mis tous les atouts de notre côté :

• Tout est tracé et logué, ce qui nous donne une vision claire de ce qui se passe en prod.
• Nous sommes précautionneux ! Des dashboards fonctionnels de chaque partie du code sont en place.
• Le code a une très belle couverture de tests, ce qui permet de faire des modifications un peu à chaud sereinement.

Ce matin, grosse sortie d’un nouveau code promo qui fonctionne bien. Mais nous remarquons que les temps d’ajout du code promo au panier augmentent à vue d’œil.

Ce n’est pas lié à la charge à ce moment-là, car sur la partie droite du graphe, le pic de trafic est passé depuis longtemps et pourtant, cela continue de monter. C’est donc lié au contenu : le nombre d’utilisations du code promo. C’est exactement ce que nous ne voulons pas. Avec l’infrastructure en place, il n’y a pas de raison que ce soit le cas, c’est forcément le code applicatif.

Pour illustration, voici un macro schéma des données en question :

Cart
    - id

VoucherCode
    - id
    - code

User
    - id

VoucherCodeUser
    - cart_id
    - user_id
    - voucher_code_id

Il y a des règles sur l’application d’un code promo (VoucherCode) donc on doit récupérer le VoucherCodeUser pour effectuer des modifications et analyses dessus.

Le code est très (trop) simple et se base sur la « magie » de Doctrine :

// Action.php
$voucherCodeUser = $voucherCode->getVoucherCodeUser($cartUser);

// Entity/VoucherCode.php
public function getVoucherCodeUser(User $user = null): ?VoucherCodeUser
{

    $arrayCollection = $this->getVoucherCodeUsers()->matching(
   	 VoucherCodeRepository::createUserCriteria($user)
    );

    return $arrayCollection->isEmpty() ? null : $arrayCollection->first();

}

Ici, Doctrine, à l’appel getVoucherCodeUsers, va aller faire cette requête en cachette :

SELECT * FROM voucher_code_user t0 WHERE t0.voucher_code_id = ?

Du coup, Doctrine va récupérer, à chaque ajout d’un code au panier, TOUTES les utilisations de ce code par TOUS les autres clients.

Or un VoucherCode peut être utilisé par des millions d’User mais un User n’a en moyenne que quelques dizaines d’utilisations de VoucherCode. Oups.

En ayant idée de cette contrainte, un code tout aussi naïf mais bien plus “safe” niveau performance aurait pu être :

$arrayCollection = $user->getVoucherCodeUsers()->matching(
	VoucherCodeRepository::createVoucherCriteria($this)
;

Pour 98% des utilisateurs, cela remonterait quelques lignes, et pour les plus fidèles clients, cela en remonterait plusieurs centaines. Donc très loin des plusieurs dizaines de milliers, voire potentiellement des millions pour tout le monde.

Section intitulée premier-niveauPremier niveau

Tant qu’à être plusieurs sur le sujet et en n’étant pas excessivement dans l’urgence (la plateforme était loin d’être en danger), nous prenons le parti de faire un fix définitif : une requête SQL propre et qui, elle, sera instantanée pour tous (les index étant présents de base vu que ce sont des clés étrangères).

// Action.php
$voucherCodeUser = $this->voucherCodeUserRepository->getVoucherCodeUserByVoucherCodeAndUser($voucherCode, $cartUser);

Hop, ni vu ni connu, ça devrait être bon.

Mais pas si vite ! Le reste du code est le suivant :

// Action.php
if (!$voucherCodeUser instanceof VoucherCodeUser) {
	$voucherCodeUser = new VoucherCodeUser($voucherCode, $cartUser);
	$voucherCodeUser->setCurrencyCode($cart->getCurrencyCode());
	$this->entityManager->persist($voucherCodeUser);
}

$voucherCodeUser->addCart($cart);

$this->cartStateMachine->apply($cart, Transition::UPDATE);

// ici le apply() appelle différentes actions et guard, et on retrouve dans une de ces actions ce code là
// WorkflwAction.php
$voucherCodeUser = $this->voucherCodeUserRepository->getVoucherCodeUserByVoucherCodeAndUser($voucherCode, $cartUser);
$voucherCode->canAddAVoucherCode($voucherCodeUser);

// VoucherCode.php
public function canAddAVoucherCode(?VoucherCodeUser $currentVoucherCodeUser): bool
{
	/** @var ArrayCollection<VoucherCodeUser> $arrayCollection */
	$arrayCollection = $this->getVoucherCodeUsers()->filter(
    	function (VoucherCodeUser $voucherCodeUser) use ($currentVoucherCodeUser): bool {
        	return
            	$voucherCodeUser !== $currentVoucherCodeUser &&
            	$voucherCodeUser->getVoucherCode()->getVoucherCampaign()->isCombinable() === false;
    	}
	);

	return $arrayCollection->isEmpty();
}

Le persist est bien effectué mais PAS le flush. Donc le nouveau $voucherCodeUser est bien présent dans les collections Doctrine mais PAS disponible via requête SQL. La première comparaison n’est pas bonne (null !== $currentVoucherCodeUser)), donc on retourne le isCombinable… du voucher que l’on est en train d’ajouter.

Et c’est ainsi que notre code promo n’est PAS applicable la première fois, applicable la 2e, puis PAS applicable toutes les fois d’après.

La suite de tests a évidemment détecté tout ça et nous avons pu fixer.

Avec ma manière préférée : en supprimant ce code. La vérification est déjà faite en amont de l’application au workflow, donc inutile, à ce moment-là de l’action, de faire appel à ce code. Inutile de corriger ou d’optimiser du code inutile.

Section intitulée second-niveauSecond niveau

Confiants, mais nous constatons quand même dans l’onglet profiler Doctrine que notre requête est toujours présente… Et cette fois, nous ne savons pas où. Heureusement, Doctrine nous permet d’obtenir la backtrace.

doctrine:
	dbal:
    	# ...
    	profiling_collect_backtrace: true

La coupable est donc :

public function addVoucherCodeUser(VoucherCodeUser $voucherCodeUser): void
{
	if (!$this->voucherCodeUsers->contains($voucherCodeUser)) {
    	$this->voucherCodeUsers->add($voucherCodeUser);
	}
}

Il est parfois utile de vérifier la présence dans la collection, mais ici, aucun intérêt car il y a un seul appelant et nous avons une contrainte d’unicité en BDD, donc même dans le pire cas, nous n’aurons pas de corruption de données.

public function addVoucherCodeUser(VoucherCodeUser $voucherCodeUser): void
{
    $this->voucherCodeUsers->add($voucherCodeUser);
}

Une mise en prod plus tard… Voilà qui est mieux.

Section intitulée conclusionConclusion

Une analyse comme on les aime. Nous sommes restés dans des bornes supportables pour nos clients et ça n’a pas duré longtemps. Un bon test de charge en production, rendu possible grâce à l’excellent monitoring en place. Sur combien de projets ce genre de souci est-il invisible avant qu’il ne devienne critique ?

Ce code utilisant des Doctrine Collection abstrayant totalement les requêtes SQL remplissait parfaitement la tâche d’un point de vue fonctionnel et a permis un développement et une mise en production très rapide.

Doctrine permet d’écrire du code “simple” en toute abstraction de pas mal de concepts, qu’il est néanmoins très intéressant de connaître.

Ici, une conscience basique des cardinalités en jeu aurait permis un code tout aussi fonctionnel et basique. Juste en partant de l’User plutôt que du VoucherCode, il n’y aurait jamais eu aucun souci de performance à notre échelle. Est-ce que ça aurait été optimal ? Non. Suffisant ? Oui.

Pour la beauté de l’art, nous sommes passés sur une requête SQL qui est à peine plus performante que la version naïve dans l’immense majorité des cas. Mais en faisant cela, on a introduit une complexité nouvelle qui n’apporte pas tant que ça tout en créant des risques un peu partout dans le code.

De plus, ce code fait potentiellement des requêtes supplémentaires pour récupérer des données de collection déjà hydratées par d’autres appels ailleurs, cassant une partie des bénéfices.

“The real problem is that programmers have spent far too much time worrying about efficiency in the wrong places and at the wrong times; premature optimization is the root of all evil (or at least most of it) in programming.”

– Donald Knuth, The Art of Computer Programming

Sur le principe, utiliser des Doctrines Collections apporte un gain fonctionnel évident, il faut juste avoir en tête certaines choses qu’il est toujours bon de comprendre : comment ça fonctionne sous le capot ET évaluer les cardinalités en jeu.

Rien ne sert de pré-optimiser, il vaut mieux livrer à point… Puis on optimise quand le besoin se fait sentir.