Włącz FETCH_PEERS (Django 6.1) w adminie - #733
Merged
Merged
Conversation
…zamiast 7 wydziałów `AutorAdmin.list_filter` miał goły string "aktualna_jednostka__wydzial", z którego Django budowało `RelatedFieldListFilter`, a ten woła `field.get_choices()`. Po Fazie B (#438) denorm `Jednostka.wydzial` jest self-FK na `Jednostka`, więc `get_choices()` enumerowało CAŁĄ tabelę jednostek: na kopii bazy produkcyjnej 504 opcje w dropdownie zamiast 7 jednostek-korzeni ("wydziałów"), plus 504 zapytania na każdy request changelisty (każde `Jednostka.__str__` czyta `self.uczelnia`). Faza B naprawiła to `WydzialFilter`-em, ale tylko dla `JednostkaAdmin` — `AutorAdmin` został przeoczony. Dokładamy `WydzialAutoraFilter`: podklasę `WydzialFilter`, która dziedziczy bez zmian `lookups()` (tylko korzenie, zawężone do uczelni z requestu) i `has_output()` (bramka `uzywaj_wydzialow`), a nadpisuje jedynie `queryset()` — bo tu zawężamy `Autor`, więc do korzenia trzeba dojść przez `aktualna_jednostka__` (`Q(aktualna_jednostka__wydzial_id=v) | Q(aktualna_jednostka_id=v)`). Świadoma zmiana kontraktu URL: parametr filtra to teraz `?wydzial=<id>` zamiast `?aktualna_jednostka__wydzial__id__exact=<id>`. Filtry admina to ulotny stan UI, nie trwałe linki, więc to akceptujemy; stary querystring degraduje się do 400 (`DisallowedModelAdminLookup`, zmierzone testem), a nie do 500. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XWWemKMkQMQmHZZPSmCdid
Dwa niezalezne defekty w `bpp/admin/filters.py`, oba wykryte pomiarem na
kopii bazy produkcyjnej (504 jednostki, 68 tys. autorow):
1. `JednostkaFilter.lookups()` mial `select_related("wydzial")`, ale
`Jednostka.__str__` czyta DWA FK -- takze `self.uczelnia` (bramka
`uzywaj_wydzialow`). Kazda pozycja listy kosztowala osobny SELECT:
504 zapytania na KAZDE wejscie na changeliste autorow. Cacheops
zamienial je na trafienia w Redis (`bpp.uczelnia` jest w regulach),
wiec licznik zapytan SQL tego nie pokazywal -- ale 504 round-tripy
do Redisa nadal kosztowaly ~200 ms na request.
2. `LogEntryFilterBase.lookups()` zawezal queryset przez
`.only("pk", "username")`, a `BppUser.__str__` czyta jeszcze
`last_name` i `first_name`. Kazde pole odroczone to osobny
`refresh_from_db()` per uzytkownik, czyli DWA dodatkowe SELECT-y na
wiersz -- "optymalizacja", ktora kosztowala zamiast oszczedzac. Na
produkcji 156 zapytan na wejscie na changeliste wydawnictw ciaglych,
i w przeciwienstwie do slownikow `bpp.bppuser` NIE jest cache'owany
przez cacheops, wiec szly wprost do PostgreSQL.
Zmierzony efekt (kopia produkcji, produkcyjne reguly CACHEOPS):
changelist wyd. ciaglych 190 -> 34 zapytania, changelist autorow
620 -> 452 ms (przy tej samej liczbie zapytan SQL -- zniknely
round-tripy do Redisa).
Testy pilnuja sedna: obie listy powstaja DOKLADNIE jednym zapytaniem,
niezaleznie od liczby pozycji. Drugi test dodatkowo asertuje, ze imie
i nazwisko sa w etykiecie -- inaczej `only()` znow by je pominelo, a
`__str__` po cichu degradowalby do samego `username`.
Uporzadkowana tez kolejnosc importow w `test_filters.py` (plik nie
przechodzil `ruff check` juz przed ta zmiana; pre-commit sprawdza tylko
pliki zmieniane, wiec nikt tego nie zauwazyl).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XWWemKMkQMQmHZZPSmCdid
`browse/jednostki.html` wolal `item.aktualna_jednostka.count` -- relacja ODWROTNA (Autor -> Jednostka), wiec kazde uzycie to osobny `COUNT(*)`. Szablon uzywal go 3-4 razy na wiersz (warunek `> 0`, sama liczba, dwa warunki odmiany), a strona pokazuje domyslnie 150 jednostek. Na kopii bazy produkcyjnej dawalo to 514 zapytan na JEDNO wejscie na indeks jednostek -- najwiekszy pojedynczy koszt zapytan w calej czesci publicznej. Ani cacheops, ani fetch modes z Django 6.1 tego NIE lapaly: queryset dotyczy `bpp.autor`, ktorego nie ma w regulach CACHEOPS, a `FETCH_PEERS` obsluguje leniwe FK i pola odroczone -- nie `RelatedManager.count()`. Zmierzone: z globalnym FETCH_PEERS bylo 514 -> 514 zapytan, czyli zero zmiany. Lekarstwem jest adnotacja, nie tryb pobierania. Zmierzony efekt (kopia produkcji, produkcyjne reguly CACHEOPS): 514 -> 3 zapytania, 137 -> 20 ms. Adnotacja jest tu bezpieczna (nie zawyza liczb przez zdublowane wiersze), bo -- jak dokumentuje komentarz w `Browser.get_queryset` -- zadna sciezka filtrowania `JednostkiView` nie mnozy wierszy: filtr literki to `istartswith` na wlasnej kolumnie, fulltext dla `Jednostka` to predykat na jednokolumnowym tsvectorze bez JOIN-a, a `scope_jednostki_do_uczelni` porownuje skalarny FK. Sprawdzone tez wyczerpujaco: dla WSZYSTKICH 504 jednostek z bazy produkcyjnej adnotacja dala te same liczby, co `.count()` per wiersz (0 roznic, suma 60 711 autorow), a wyrenderowany HTML jest identyczny bajt w bajt. Dwa testy, bo zmiana ma dwa rozne ryzyka: * `test_JednostkiView_liczba_autorow_jednym_zapytaniem` -- sedno, czyli brak zapytania per wiersz, * `test_browse_jednostki_pokazuje_liczbe_autorow` -- literowka w nazwie adnotacji NIE wywalilaby wyjatku (Django renderuje nieistniejaca zmienna jako pusty lancuch), wiec strona po cichu przestalaby pokazywac liczby. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XWWemKMkQMQmHZZPSmCdid
Django 6.1 wprowadzilo *fetch modes*. `FETCH_PEERS` sprawia, ze PIERWSZE leniwe dotkniecie relacji (albo pola odroczonego) na obiekcie z querysetu dociaga ja HURTEM dla calego rodzenstwa z tego samego pobrania (`prefetch_related_objects` pod spodem) -- N+1 zamienia sie w 2 zapytania, bez zgadywania z gory, ktore FK dotknie szablon. Changelisty admina to najgestsze w BPP skupisko tego wzorca: `list_display` i `__str__` modeli siegaja po FK, ktorych nikt nie zadeklarowal w `list_select_related`. Ustawiamy tryb w `BaseBppAdminMixin.get_queryset`, czyli w jednym miejscu dla 31 adminow. Zmierzone na kopii bazy produkcyjnej (produkcyjne reguly CACHEOPS), zapytania i mediana czasu na request: changelist jednostek 66 -> 17 zapytan, 185 -> 120 ms changelist wyd. zwartych 220 -> 40 zapytan, 227 -> 184 ms changelist wyd. ciaglych 190 -> 38 zapytan, 221 -> 190 ms Dlaczego TU, a nie globalnie (podstawienie `DEFAULT_FETCH_MODE`): `track_peers` trzyma `weakref` do kazdej instancji z pobrania, wiec koszt ponosilby KAZDY queryset w aplikacji, a zysk jest skoncentrowany w adminie. Samo `get_queryset` wystarcza, bo `QuerySet._clone()` przenosi `_fetch_mode` -- tryb przezywa filtry, sortowanie i slicing dokladane przez dalsze mixiny i przez sam `ChangeList`. Test pilnuje obu tych wlasnosci osobno. Semantyka sie NIE zmienia: `fetch_one` i `fetch_many` ida ta sama sciezka managera (`_base_manager`) -- tryb nie zaczyna nagle odfiltrowywac rekordow, co jest istotne przy soft-delete. Sprawdzone tez empirycznie na danych produkcyjnych: 15 stron (publiczne, admin, API) zwrocilo wynik identyczny bajt w bajt w obu trybach. WYMAGA Django >= 6.1, dlatego ta gałąź celuje w `django-6.1`, a nie w `dev`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XWWemKMkQMQmHZZPSmCdid
mpasternak
force-pushed
the
django-6.1-optimizations
branch
from
August 7, 2026 11:38
8ed909b to
fdc1197
Compare
mpasternak
changed the title
mpasternak
added a commit
that referenced
this pull request
Aug 7, 2026
* fix(admin): brakujace JOIN-y na listach jednostek i wydawnictw zwartych Dwie deklaracje `list_select_related` byly martwe — kod je deklarowal, a Django ich do zapytania nie wstawialo. Objaw: N+1 na changelistach, maskowany od #733 przez FETCH_PEERS (2 zapytania zamiast 2N), ale nadal falszywa deklaracja. 1. JednostkaAdmin — pulapka warunkowego `apply_select_related`. `ChangeList.get_queryset` (django/contrib/admin/views/main.py) aplikuje deklaracje WARUNKOWO: if not qs.query.select_related: qs = self.apply_select_related(qs) czyli TYLKO gdy queryset bazowy admina nie ma jeszcze zadnego `select_related`. A `JednostkaManager.get_queryset()` dokłada `.select_related("wydzial")` (zdenormalizowany self-FK), wiec warunek jest falszywy i CALA deklaracja admina przepada — do zapytania szedl sam `wydzial`. Leniwie, per wiersz, leciały wiec: * `rodzaj` — kolumna `list_display`, * `uczelnia` — czytana przez `Jednostka.__str__` (sprawdza `uzywaj_wydzialow`), czyli w KAZDYM wierszu, * `parent` — kolumna `parent_nazwa` (`item.parent.nazwa`), ktorej w deklaracji w ogole nie bylo. Nie ma tu zadnego bledu ani ostrzezenia — z samego diffa/kodu admina tego nie widac, bo deklaracja wyglada poprawnie. Nadpisujemy wiec `get_queryset` i dokladamy `select_related` wprost (select_related sie scala, wiec `wydzial` z managera nie ginie) oraz dopisujemy `parent`. Przeglad calego `src/`: to JEDYNY manager w projekcie, ktory dokłada domyslny `select_related`, wiec pulapka wystepuje tylko tutaj. Pozostale `get_queryset` z `select_related` siedza na `ModelResource` (django-import-export) albo na adminach bez `list_select_related` — tam nie ma czego zgubic. 2. Wydawnictwo_ZwarteAdmin — deklaracja pod zla nazwa kolumny. BPP uzywa slownikowego dialektu `list_select_related` (django-dynamic-admin-columns): JOIN wchodzi tylko, gdy kolumna o danej nazwie jest widoczna. Wpis `"wydawca": ["wydawca"]` celowal w kolumne `wydawca`, ktorej NIE MA w `list_display_default`. Tymczasem po wydawce siega domyslnie widoczna kolumna `wydawnictwo` — property modelu `get_wydawnictwo()` sklada `self.wydawca.nazwa` z `wydawca_opis`. JOIN wchodzil wiec tylko przy recznie wlaczonej kolumnie `wydawca`, czyli praktycznie nigdy. Dokladamy `"wydawnictwo": ["wydawca"]`. Oba przypadki znalezione przez bramke FETCH_RAISE (kolejny commit). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F134BWb3YoYPQ6zjzqoqds * test(admin): bramka regresyjna na N+1 oparta o FETCH_RAISE (Django 6.1) Istniejacy `test_fetch_peers.py` sprawdza wylacznie, ze tryb FETCH_PEERS jest ustawiony i przezywa `_clone()`. To dowodzi, ze mechanizm jest podpiety — ale NIE dowodzi, ze N+1 faktycznie zniklo, i nie zatrzyma jego powrotu. Ktos dokłada kolumne do `list_display` albo relacje do `__str__`, liczba zapytan rosnie liniowo z liczba wierszy, a testy zostaja zielone. Nowy plik przybija KSZTALT ZAPYTAN czterech najgoretszych changelist (wydawnictwa ciagle i zwarte, autorzy, jednostki) trzema bramkami: 1. FETCH_RAISE. Renderujemy prawdziwa changeliste przez klienta HTTP, podmieniajac `bpp.admin.core.FETCH_PEERS` na FETCH_RAISE (monkeypatch na czas jednego testu — kod produkcyjny i DEFAULT_FETCH_MODE zostaja nietkniete). Wg kodu Django 6.1 tryb wedruje z `QuerySet._fetch_mode` przez `Model.from_db(fetch_mode=...)` do `instance._state.fetch_mode` (dziedzicza go tez obiekty z select_related i prefetch_related), a trzy deskryptory — DeferredAttribute, ForwardManyToOneDescriptor i ReverseOneToOneDescriptor — zamiast dociagac dane wołaja `fetch_mode.fetch(...)`. FetchRaise rzuca tam FieldFetchBlocked (podklasa FieldError) z komunikatem "Fetching of <Model>.<pole> blocked." Efekt: kazde leniwe dotkniecie relacji wywala test natychmiast, z nazwa pola — zamiast po cichu dolozyc N SELECT-ow. 2. Liczba zapytan niezalezna od liczby wierszy. Ta sama changelista z 2 i z 8 wierszami musi kosztowac DOKLADNIE tyle samo zapytan. To operacyjna definicja braku N+1 i — w odroznieniu od przybicia konkretnej liczby — nie wymaga aktualizacji przy niewinnych zmianach. Lapie tez to, czego FETCH_RAISE z zasady nie widzi: menedzery relacji odwrotnych (`obj.cos_set.all()`) i M2M tryb tylko DZIEDZICZA, nie sa przez niego blokowane. 3. Deklaracja `list_select_related` naprawde trafia do zapytania. Django aplikuje ja warunkowo (`if not qs.query.select_related`), wiec sama jej obecnosc w kodzie niczego nie gwarantuje — to wlasnie ta pulapka zjadla trzy JOIN-y na liscie jednostek (poprzedni commit). Test przechodzi po wszystkich adminach `bpp` i flaguje kazdego, ktorego queryset bazowy ma wlasny select_related, a zadeklarowanych sciezek w nim brakuje. Dane testowe maja WYPELNIONE FK widocznych kolumn — FETCH_RAISE odpala sie tylko dla relacji o niepustym kluczu (`has_value`), wiec rekordy z samymi NULL-ami przepuscilyby bramke na pusto. Bramka pilnuje tez samej siebie: `test_bramka_fetch_raise_faktycznie _gryzie` kasuje deklaracje `list_select_related` i wymaga, zeby FieldFetchBlocked FAKTYCZNIE poleciał. Bez tego caly plik moglby zrobic sie zielony na pusto, gdyby podmiana trybu przestala dzialac. Autouse fixture czysci `DynamicColumnsMixin._modeladmin_enabled` — to `cached_property` na singletonie admina, wiec przezywa rollback bazy i sprawia, ze kolejny test w module widzi UBOZSZY uklad kolumn, niz sadzi (czyli bramka cicho sie rozbraja). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F134BWb3YoYPQ6zjzqoqds --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Włącza tryb pobierania relacji z Django 6.1 (
FETCH_PEERS) w adminie.Zakres tego PR-a zawęził się po Waszej uwadze: trzy poprawki, które
działają też na Django 5.2, poszły wprost na
dev(
f509c7bdc,5ffc83f50,6416e2ac0— już wypchnięte). Tutaj zostałwyłącznie commit wymagający Django 6.1, plus merge
dev, który tepoprawki wciąga. Dzięki temu żadna zmiana nie istnieje w dwóch miejscach
z różnymi SHA.
Własny wkład tego PR-a:
FETCH_PEERSwBaseBppAdminMixinDjango 6.1 wprowadziło fetch modes.
FETCH_PEERSsprawia, że pierwszeleniwe dotknięcie relacji (albo pola odroczonego) na obiekcie z querysetu
dociąga ją hurtem dla całego rodzeństwa z tego samego pobrania
(
prefetch_related_objectspod spodem) — N+1 zamienia się w 2 zapytania,bez deklarowania
select_relatedz góry.Changelisty admina to najgęstsze w BPP skupisko tego wzorca:
list_displayi
__str__modeli sięgają po FK, których nikt nie zadeklarował wlist_select_related. Tryb ustawiamy w jednym miejscu, dla 31 adminów.Zmierzone na kopii bazy produkcyjnej (122 232 rekordy, 68 355 autorów,
504 jednostki), z produkcyjnymi regułami
CACHEOPS:Dlaczego tu, a nie globalnie
track_peerstrzymaweakrefdo każdej instancji z pobrania, więcpodstawienie
DEFAULT_FETCH_MODEobciążyłoby każdy querysetw aplikacji, a zysk jest skoncentrowany w adminie. Samo
get_querysetwystarcza, bo
QuerySet._clone()przenosi_fetch_mode— tryb przeżywafiltry, sortowanie i slicing dokładane przez dalsze mixiny i przez sam
ChangeList. Test pilnuje obu tych własności osobno.Bezpieczeństwo
FETCH_PEERSnie zmienia semantyki managerów:fetch_onei
fetch_manyidą tą samą ścieżką_base_manager(dla pól odroczonych_base_manager…in_bulk()), więc tryb nie zaczyna odfiltrowywać rekordów —istotne przy soft-delete. Sprawdzone też empirycznie na danych
produkcyjnych: 15 stron (publiczne, admin, API) zwróciło wynik identyczny
bajt w bajt w obu trybach.
Jedyna znaleziona różnica, warta wiedzy: dla pola odroczonego
fetch_manyrobi
value_by_pk[instance.pk]— goły lookup w dict — więc gdyby wierszzniknął między pobraniem listy a dotknięciem pola, poleci
KeyErrorzamiast
DoesNotExist. Wyścig egzotyczny, ale to inny typ wyjątku.O pomiarach — czytaj przed oceną liczb
Liczby zapytań są dokładne i powtórzyły się identycznie w pięciu
niezależnych przebiegach. Na nich opieram wnioski.
Czasów NIE podaję jako wartości. Host pomiarowy jest współdzielony
i był obłożony (load ~6,5, 26 kontenerów). Dowód, że to szum hosta, a nie
kod:
admin: źródło (changelist)ma te same 16 zapytań przed i po(żadna zmiana go nie dotyka), a zmierzony czas skakał 320 → 420 ms.
Czasy w wiadomości commita pochodzą z wcześniejszego, spokojniejszego
okna — rząd wielkości, nie pomiar. Warto powtórzyć na maszynie bez obcego
obciążenia.
Testy
Te same testy na
dev(Django 5.2, beztest_fetch_peers.py):1012 passed, 1 skipped— czyli poprawki przeniesione nadevfaktyczniedziałają na 5.2, a nie tylko „powinny".
Skąd te znaleziska
Nie z przeglądu kodu, a z pomiaru: nowy
FetchModez 6.1 użyty jakodetektor N+1 (szpieg liczący każde leniwe pobranie razem ze stosem
wywołań). To zresztą, moim zdaniem, cenniejsza część Django 6.1 niż samo
FETCH_PEERS—FETCH_RAISEda się użyć jako trwały gejt regresjiw testach.
Pełny raport z metodą, oboma wariantami konfiguracji cache i dowodem
równoważności wyniku:
AUDYT-DJANGO-6.1-WYDAJNOSC.mdna gałęzidjango-6.1. Wniosek o samym upgrade'zie: jest wydajnościowoneutralny — identyczne liczby zapytań na 15 scenariuszach, zero regresji.
🤖 Generated with Claude Code
https://claude.ai/code/session_01XWWemKMkQMQmHZZPSmCdid