Skip to content

[TECH] Prévenir les incompréhensions due aux expirations de jobs. #12012

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

Merged
merged 1 commit into from
Apr 14, 2025

Conversation

VincentHardouin
Copy link
Member

@VincentHardouin VincentHardouin commented Apr 9, 2025

🌸 Problème

Pg-boss utilise Promise.race(), pour définir si un job a son délai de traitement qui a expiré.
Le problème est que Promise.race n'arrête pas les promesses en cours, il retourne juste la première promesse qui est terminée. Dans notre cas, lorsqu'un job expire, il est alors marqué comme expiré par pg-boss qui va le retenter plus tard, alors que le job continue en tâche de fond.

Des ressources pour comprendre le souhait et l'usage de expirein par pg-boss :

timgit/pg-boss#238
timgit/pg-boss#102

🌳 Proposition

Utiliser une valeur complétement arbitraire et exagéré pour ne pas se soucier de la config expireIn qui peut causer plus de mal que de bien.
Nous proposons de la mettre à 48h, car nous souhaitons déployer du code à un intervalle plus court que 24h, ce qui fait que la durée de vie d'un conteneur ne peut pas excéder le temps entre deux déploiements.

🐝 Remarques

🤧 Pour tester

Faire une action qui ajoute un job
Constater en base que le job a un expirein à 48:00:00

@VincentHardouin VincentHardouin added the cross-team Toutes les équipes de dev label Apr 9, 2025
@VincentHardouin VincentHardouin requested a review from a team as a code owner April 9, 2025 15:28
@pix-bot-github
Copy link

Une fois les applications déployées, elles seront accessibles via les liens suivants :

Les variables d'environnement seront accessibles via les liens suivants :

@VincentHardouin VincentHardouin force-pushed the expand-expirein-for-all-jobs branch from 02af489 to 93e51d0 Compare April 10, 2025 07:51
@HEYGUL HEYGUL added 👀 Tech Review Needed 👀 Func Review Needed Need PO validation for this functionally labels Apr 14, 2025
Copy link
Member

@yaf yaf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 « vers l'infini et au-delà »

Copy link
Contributor

@bpetetot bpetetot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bien vu 😉

Ça peut valoir le coup de créer une issue côté pgBoss pour indiquer le problème et avoir une explication de ce choix.

@pix-service-auto-merge pix-service-auto-merge force-pushed the expand-expirein-for-all-jobs branch from 93e51d0 to b5c82a8 Compare April 14, 2025 07:33
@pix-service-auto-merge pix-service-auto-merge merged commit 4d9ad67 into dev Apr 14, 2025
9 of 11 checks passed
@pix-service-auto-merge pix-service-auto-merge deleted the expand-expirein-for-all-jobs branch April 14, 2025 07:40
@HEYGUL
Copy link
Contributor

HEYGUL commented Apr 14, 2025

Bien vu 😉

Ça peut valoir le coup de créer une issue côté pgBoss pour indiquer le problème et avoir une explication de ce choix.

à suivre ici : timgit/pg-boss#555 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants