Skip to content

Vie privée : mise à jour des dates pivot de la notification [2/2] - #6630

Merged
tonial merged 3 commits into
masterfrom
vp/GEN-2642_part2_jobapplication
Aug 1, 2025
Merged

Vie privée : mise à jour des dates pivot de la notification [2/2]#6630
tonial merged 3 commits into
masterfrom
vp/GEN-2642_part2_jobapplication

Conversation

@vincentporte

@vincentporte vincentporte commented Jul 29, 2025

Copy link
Copy Markdown

🤔 Pourquoi ?

Certaines dates utilisées pour déclencher la notification d'archivage ne sont pas appropriées.
Elles sont remplacées par des dates "métiers" plus précises (ou supprimées)

Périmètre :

  • JobApplication : created_at
  • JobApplicationTransitionLog : timestamp

⚠️ tests sensible à leur date d'execution mis à jour dans cette PR :

  • test_archive_jobseeker_with_approval
  • test_archive_not_eligible_jobapplications_of_inactive_jobseekers_after_grace_period

@notion-workspace

Copy link
Copy Markdown

@vincentporte vincentporte self-assigned this Jul 29, 2025
@vincentporte vincentporte added the modifié Modifié dans le changelog. label Jul 29, 2025
@vincentporte
vincentporte changed the base branch from master to vp/GEN-2642_gps_against_archivism July 29, 2025 05:57
Comment thread itou/archive/utils.py Outdated
)
recent_job_application = JobApplication.objects.filter(job_seeker_id=OuterRef("pk"), updated_at__gt=inactive_since)
recent_job_application = JobApplication.objects.filter(
Q(created_at__gt=inactive_since) | Q(processed_at__gt=inactive_since) | Q(logs__timestamp__gt=inactive_since),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

l'utilisation de logs__timestamp pourrait alourdir fortement la requête. Est-ce que tu as testé ?

@vincentporte vincentporte Jul 30, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@tonial, ces 2 tests selon que nous utilisons logs__timestamp ou une sous-requête directement sur JobApplicationTransitionLog, sortent des temps d'execution similaire.

question : cette comparaison est-elle signifiante pour toi ?

inactive_since = timezone.now() - INACTIVITY_PERIOD

test 1 : one subquery

recent_job_application = JobApplication.objects.filter(Q(created_at__gt=inactive_since) | Q(logs__timestamp__gt=inactive_since),job_seeker_id=OuterRef("pk"))
User.objects.filter(kind='job_seeker').filter(~Exists(recent_job_application)).count()
638778

connection.queries
[{'sql': 'SELECT COUNT(*) AS "__count" FROM "users_user" WHERE ("users_user"."kind" = \'job_seeker\' AND NOT EXISTS(SELECT 1 AS "a" FROM "job_applications_jobapplication" U0 LEFT OUTER JOIN "job_applications_jobapplicationtransitionlog" U1 ON (U0."id" = U1."job_application_id") WHERE ((U0."created_at" > \'2023-08-30 05:57:23.660350+00:00\'::timestamptz OR U1."timestamp" > \'2023-08-30 05:57:23.660350+00:00\'::timestamptz) AND U0."job_seeker_id" = ("users_user"."id")) LIMIT 1))',
  'time': '1.760'}]

test 2 : two subqueries

recent_job_application = JobApplication.objects.filter(job_seeker_id=OuterRef("pk"),created_at__gt=inactive_since)
recent_transitions = JobApplicationTransitionLog.objects.filter(job_application__job_seeker_id=OuterRef("pk"), timestamp__gt=inactive_since)
User.objects.filter(kind='job_seeker',upcoming_deletion_notified_at__isnull=True).filter(~Exists(recent_job_application), ~Exists(recent_transitions)).count()

connection.queries
[{'sql': 'SELECT COUNT(*) AS "__count" FROM "users_user" WHERE ("users_user"."kind" = \'job_seeker\' AND "users_user"."upcoming_deletion_notified_at" IS NULL AND NOT EXISTS(SELECT 1 AS "a" FROM "job_applications_jobapplication" U0 WHERE (U0."created_at" > \'2023-08-30 05:57:23.660350+00:00\'::timestamptz AND U0."job_seeker_id" = ("users_user"."id")) LIMIT 1) AND NOT EXISTS(SELECT 1 AS "a" FROM "job_applications_jobapplicationtransitionlog" U0 INNER JOIN "job_applications_jobapplication" U1 ON (U0."job_application_id" = U1."id") WHERE (U1."job_seeker_id" = ("users_user"."id") AND U0."timestamp" > \'2023-08-30 05:57:23.660350+00:00\'::timestamptz) LIMIT 1))',
  'time': '1.685'}]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

C'est plus une comparaison avec sans que j'avais en tête (par rapport aux gains de performance que tu as obtenus avant (23 -> 0.8s)

@vincentporte vincentporte Jul 30, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Temps d'exécution en milli secondes

time.time()

master one subquery with OR two subqueries
8.086442947387695 7.432460784912109 8.379220962524414
8.732318878173828 8.600711822509766 8.518457412719727
7.921695709228516 7.796764373779297 8.507251739501953
7.441520690917969 8.025884628295898 8.31747055053711

process_time()

master one subquery with OR two subqueries
8.876145000002111 8.003589000001199 9.772650000002159
7.457892000001465 7.081010000000276 7.616086999998828
6.589542000000392 6.787968000001143 7.803689999999364
6.524236000000627 6.829377999999053 8.220791000002947

Pas de variation incroyable dans un sens ou dans l'autre. La version à 2 sous-requetes distinctes est pénalisante.

Détail surprenant, les résultats sont inversés entre la version master et la version à une sous-requête, selon qu'on prend time() ou process_time()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

du coup c'est bon avec une seule subquery 👍

Base automatically changed from vp/GEN-2642_gps_against_archivism to master July 29, 2025 08:06
@vincentporte
vincentporte force-pushed the vp/GEN-2642_part2_jobapplication branch 2 times, most recently from e294409 to 7191beb Compare July 30, 2025 13:27
@vincentporte
vincentporte requested a review from tonial July 30, 2025 13:27
@vincentporte
vincentporte marked this pull request as ready for review July 30, 2025 13:27

@tonial tonial left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Il faudrait revoir les noms des variables de tests pour plus de clarté.
Le test est super au demeurant :)

Comment thread tests/archive/tests_management_command.py Outdated
Comment thread tests/archive/tests_management_command.py Outdated
Comment thread tests/archive/tests_management_command.py Outdated
Comment thread tests/archive/tests_management_command.py Outdated
Comment thread tests/archive/tests_management_command.py Outdated
…nt_related_objects

Replace updated_at by created_at
Adjust tests and make them insensitive to their execution date
@tonial
tonial force-pushed the vp/GEN-2642_part2_jobapplication branch from 1e762c6 to 77b14ba Compare August 1, 2025 09:02
vincent porte added 2 commits August 1, 2025 11:04
Use timestamp from JobApplicationTransitionLog in inactive_jobseekers_without_recent_related_objects
@tonial
tonial force-pushed the vp/GEN-2642_part2_jobapplication branch from 77b14ba to 71ee077 Compare August 1, 2025 09:04
@tonial
tonial enabled auto-merge August 1, 2025 09:04
@tonial
tonial added this pull request to the merge queue Aug 1, 2025
Merged via the queue into master with commit 656c914 Aug 1, 2025
14 checks passed
@tonial
tonial deleted the vp/GEN-2642_part2_jobapplication branch August 1, 2025 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

modifié Modifié dans le changelog.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants