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

Tech: limite l'engorgement des DossierIndexSearchTermsJob #10448

Merged

Conversation

colinux
Copy link
Member

@colinux colinux commented May 23, 2024

  • augmente le délai du debounce quand l'update vient d'un brouillon
  • passe sur la file low_priority car c'est littéralement du low priority

Copy link

codecov bot commented May 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.85%. Comparing base (2c07f02) to head (3617368).
Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10448      +/-   ##
==========================================
- Coverage   80.90%   80.85%   -0.05%     
==========================================
  Files        1204     1205       +1     
  Lines       25548    25565      +17     
  Branches     4624     4624              
==========================================
+ Hits        20670    20671       +1     
- Misses       4878     4894      +16     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@colinux colinux changed the title Tech: limite l'engorgement des DossierIndexSearchTerrmsJob Tech: limite l'engorgement des DossierIndexSearchTermsJob May 23, 2024
@colinux colinux force-pushed the increase-search-debounce-delay branch from 1c8122e to 1e8fa56 Compare May 23, 2024 18:17
Copy link
Contributor

@mfo mfo left a comment

Choose a reason for hiding this comment

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

En suivant cette logique de debouncer l'indexation des search_terms quand un usager met à jour son dossier, il faudrait p-e faire pareil du côté de l'instructeur qui met à jour les annotations ?

Donc idée pour simplifier (et eviter des surprises ailleurs), pourquoi ne pas mettre à jour la constante SEARCH_TERMS_DEBOUNCE = 5.minutes ? Concrètement, quelque soit l'entité maj [Champ, Etablissement etc...], on index qu'après 5 min.

hope it helps.

app/controllers/users/dossiers_controller.rb Outdated Show resolved Hide resolved
@@ -30,11 +30,11 @@ def index_search_terms
self.class.connection.execute(sanitized_sql)
end

def index_search_terms_later
def index_search_terms_later(debounce_delay: SEARCH_TERMS_DEBOUNCE)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
def index_search_terms_later(debounce_delay: SEARCH_TERMS_DEBOUNCE)
def index_search_terms_later

app/models/concerns/dossier_searchable_concern.rb Outdated Show resolved Hide resolved
@LeSim
Copy link
Member

LeSim commented May 24, 2024

Alors moi j ai buggué sur le fonctionnement (scopé !) du flag, mais j'ai compris :) . D'acc avec @mfo mais envie que ca passe vite :)

@LeSim
Copy link
Member

LeSim commented May 24, 2024

D'ailleurs, j y pense, @colinux est ce que t'as prévenu les instances de cette utilisation du redis de cache ? J'ai l'impression que s'ils n'en ont pas, ils risquent d'engorger leurs jobs.

@colinux colinux force-pushed the increase-search-debounce-delay branch from 1e8fa56 to 3617368 Compare May 27, 2024 07:57
@colinux
Copy link
Member Author

colinux commented May 27, 2024

OK j'ai mis tout le monde à 5 minutes, merci

@colinux
Copy link
Member Author

colinux commented May 27, 2024

@LeSim je crois que j'avais fait une note il y a quelques mois où redis était devenu quasi obligatoire pour d'autres situations sans être formellement indispensable. Je vais en refaire une pour enfoncer le clou

@colinux colinux added this pull request to the merge queue May 27, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 27, 2024
@colinux colinux added this pull request to the merge queue May 27, 2024
Merged via the queue into demarches-simplifiees:main with commit 8431771 May 27, 2024
18 checks passed
@colinux colinux deleted the increase-search-debounce-delay branch May 27, 2024 08:26
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.

None yet

3 participants