iiuni / projektzapisy

System Zapisów na zajęcia w Instytucie Informatyki Uniwersytetu Wrocławskiego
https://zapisy.ii.uni.wroc.pl
30 stars 10 forks source link

Wprowadzenie RQ Scheduler zamiast time.sleep w zadaniach DjangoRQ #1621

Open mqt2019 opened 8 months ago

mqt2019 commented 8 months ago

W naszym projekcie Django używamy DjangoRQ do zarządzania zadaniami w tle, w szczególności dla zadania dispatch_notifications_task. Zauważyłem, że obecna implementacja wykorzystuje time.sleep(settings.EMAIL_THROTTLE_SECONDS) do zarządzania interwałem między wysyłkami powiadomień e-mail.

Proponuję rozważenie biblioteki RQ Scheduler (https://github.com/rq/rq-scheduler), która jest rozszerzeniem RQ i pozwala na planowanie zadań.

Należy na początku sprawdzić zgodność RQ Scheduler z DjangoRQ w naszym projekcie (defaultowo RQ Scheduler jest kompatybilne z RQ). Ewentualnie można rozpatrzeć inną bibliotekę do schedulowania zadań (zamiast korzystać z time.sleep na końcu funkcji)

Dawid-Sroka commented 6 months ago

Zgłoszenie tego issue nie zawiera motywacji, dlaczego taka zmiana byłaby korzystna. Przedstawiam moje wnioski po analizie obecnej funkcjonalności.

Wprowadzenie

Ponieważ ani na wiki, ani w żadnym pliku .md funkcjonalność django-rq nie jest opisana, opiszę tutaj to, co udało mi się zrozumieć analizując ten issue.

O czym mowa

W pliku zapisy/requirements.common.txt mamy wymienione takie zależności:

rq==1.14.1
django-rq==2.8.1

rq to biblioteka, która dostarcza abstrakcji na to, żeby móc tworzyć kolejki, na które można wrzucać zadania (typowo takie, które wykonują się asynchronicznie). Potem można zapuścić wykonywanie takiej kolejki w tle, za pomocą obiektu nazywanego worker. django-rq to pakiet, który integruje rq z Django, żeby można było go wygodnie używać i konfigurować w projekcie postawionym na Django.

Jak my tego używamy

Przeszukałem grepem cały nasz projekt, patrząc gdzie w ogóle jest mowa o worker, django-rq, django_rq i rq. Patrząc na to w logicznym porządku, idzie to tak:

W pliku zapisy/zapisy/settings.py są zdefiniowane dwie kolejki: default oraz dispatch-notifications.

W infra/playbooks/dev/ są dwa skrypty rqworker1.service oraz rqworker2.service. Każdy uruchamia swojego workera z jedną kolejką jako argument.

Zgodnie z opisem na stronie django-rq kolejki są wykorzystywane w naszym projekcie przy pomocy dekoratora @job:

projektzapisy[master-dev]$ grep -riT '@job'
zapisy/apps/notifications/tasks.py:     @job('dispatch-notifications')
zapisy/apps/enrollment/records/tasks.py:        @job
zapisy/apps/enrollment/records/tasks.py:        @job

Czyli kolejka dispatch-notifications jest używana tylko przez aplikację notifications, konkretnie w funkcji dispatch_notifications_task, czyli tam, gdzie wskazywał @mqt2019. Nie zastanawiając się, jak ta funkcja dokładnie działa, widać, że na końcu jest wywołanie time.sleep(settings.EMAIL_THROTTLE_SECONDS) i nawet w docstringu jest wyjaśnione, że jest to spowodowane jakimś ograniczeniem przepustowości ze strony serwera, który obsługuje nasze maile. Jest to jedyne miejsce w naszym projekcie, gdzie korzystamy z funkcji time.sleep (nie licząc plików w katalogu zapisy/mailer, ale oficjalna informacja jest taka, że mailer jest legacy i zostanie usunięty).

Propozycja w issue

Nie wiem jaka była motywacja, żeby zastąpić powyższe rozwiązanie wykorzystaniem niewielkiej biblioteki rq-scheduler. Wyobrażam sobie dwa powody:

  1. wydajność
  2. dodatkowa abstrakcja

Ad1. Funkcja time.sleep jak jestem głęboko przekonany, jest realizowana w systemie operacyjnym zapewne jako uśpienie procesu, nie jest aktywnym czekaniem. Nie wiem, jak w porównaniu do niej będzie wypadać zaproponowane rozwiązanie, nie zagłębiłem się w implementację. Tak intuicyjnie patrząc nie mam poczucia, że jest tu dużo do ugrania.

Ad2. Abstrakcje są fajne, ale pytanie, czy chcemy w nie inwestować w sytuacji, gdy ta funkcjonalność jest używana tylko w jednym miejscu? Czy spodziewamy się w przyszłości większego zapotrzebowania na tę funkcjonalność?

Co można jeszcze sprawdzić