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

Accélérer la récupération du statut "jobs are old" d'un cluster #184

Merged
merged 2 commits into from
Mar 26, 2024

Conversation

notoraptor
Copy link
Contributor

Salut, @soline-b ! Voici la PR contenant la petite optimisation dont on parlait concernant la récupération du statut d'un cluster !

…if cluster jobs are old. Much faster to use than get_jobs().
@gyom
Copy link
Collaborator

gyom commented Mar 20, 2024

Moi, ça me va comme PR.

Notons qu'on aurait intérêt à plus moyen terme à mettre en place une autre solution qui calcule périodiquement cela (comme un petit poisson qui lèche les parois de l'aquarium pour les nettoyer) et qui met la valeur dans une "cache" pour éviter de recalculer ça chaque fois que qqun ouvre une page. Ça me semble excessif de faire 4-5 requêtes à MongoDB à chaque chargement de page (comme fait cette PR), mais c'est vraiment mieux que la situation actuelle.

Vu qu'on est dans une situation où la version en "prod" est très affectée par les ralentissements causés par ces requêtes sur des listes complètes de jobs, et qu'on remédier à la situation au plus vite, je suis en faveur de passer cette PR et de demander à l'équipe d'Infra de la déployer.

La prochaine étape, où ces valeurs seront calculées une fois par jour, viendra plus tard et sera gérée séparément.

@gyom
Copy link
Collaborator

gyom commented Mar 20, 2024

Par ailleurs, je pense que le choix de 30 jours comme délais est trop long. Il faut penser à combien de temps on aimerait laisser s'écouler avant de signaler à quelqu'un qu'une grappe n'est plus vraiment mise à jour.

Je pense qu'on devrait mettre une valeur qui ressemble davantage à "2 jours" au lieu de "30 jours". L'inconvénient est qu'on pourrait accidentellement laisser croire à des gens qu'une grappe n'est plus accessible par Clockwork, alors qu'elle en bon état mais sans jobs récentes.

Tout cela renforce la perspective comme quoi cette mesure de "est-ce que telle grappe est bien 'scrapée' ?" devrait être calculée de manière externe, possiblement par le script qui fait le scraping tout simplement. Le script qui fait le scraping peut facilement voir si des jobs ont été lues, si sa commande a réussi. L'idée d'inférer le succès de manière indirecte en regardant la job la plus récente était peut-être une mauvaise approche.

Au minimum, donc, il faudrait mettre le chiffre plus bas que 30. Disons 2. Ensuite, il faudrait repenser à comment on gère cette feature visuelle qui signifie quelles grappes sont potentiellement mal renseignées.

@notoraptor
Copy link
Contributor Author

@gyom je viens de voir tes commentaires !

Du coup, est-ce que je fais un commit pour diminuer le délai de 30 jours à 2 jours dans cette PR-ci ?

@soline-b

@gyom
Copy link
Collaborator

gyom commented Mar 21, 2024

Oui, change le délais de 30 jours pour 2 jours, svp, et comme ça Soline pourra merger quand elle sera satisfaite.

@notoraptor
Copy link
Contributor Author

@gyom fait ! 32be075

@soline-b soline-b merged commit eabc734 into mila-iqia:master Mar 26, 2024
2 checks passed
@notoraptor notoraptor deleted the optimize-getting-cluster-status branch April 2, 2024 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants