perf: fix performance audit findings and production-log bottlenecks - #47
Merged
Merged
Conversation
…counter
Planning keys become PLANNING:{uid}:v{n}:...; any plan, empire, junction or
cx change bumps the user's counter once on commit (add, then incr). This
drops the full-keyspace SCANs, the scan per cascaded junction row, the
'*1:empire:retrieve*' match hitting users 11 and 101, and the stale cx list
after plan or empire renames.
sync_state no longer invalidates: update_state writes through a queryset
update, since empire_state is in no cached payload.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
get_or_set_response takes a short rebuild lock with cache.add on a miss. Concurrent misses poll the key for up to 3s instead of all rebuilding the payload, and build themselves if the builder is slow or died. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The multiple endpoint builds its cache key from the sorted, de-duplicated planet ids, so the same set of planets hits the cache regardless of request order or repeated ids. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
empire_state is in no serialized payload, so empires loaded for the empire, plan, cx and shared views defer it. The cx views prefetch empires with their plans, and the public shared retrieve prefetches the plan's empires with cx instead of querying cx per empire. Shared retrieve no longer reloads the row after bumping view_count. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The API-key lookup joins the user instead of loading it lazily, and last_used is written at most every 5 minutes instead of on every request. The user pre_save signal reads the previous row once, with only the fields it compares. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The snapshot task clears the flag with a queryset update conditioned on the modified_at it read. It fires no post_save, so planning caches stay put, and a state sync landing mid-run keeps the empire dirty. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t lookups - CacheManager.get_or_set_response takes private=True; planning endpoints and FIO storage send Cache-Control: private instead of public. - The exchange CSV is rendered once on a miss and cached as bytes (new csv-v2 key so old JSON entries are never served as CSV); hits skip JSON parsing and CSV rendering. - latest_popr and the analytics plan aggregate retrieve check the cache before touching the DB. - import_planet invalidates the planet list, import_all_buildings the building list. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
total_calls was a read-modify-write on the row loaded at request start and lost concurrent increments. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
FIOService used a new httpx.Client per task, a fresh TLS handshake for each of the ~2,200 CXPC requests per trigger. The client is now created lazily once per process (after the prefork fork, never at import), reused across tasks and closed on worker_process_shutdown. get_fio_service() keeps its interface. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
gamedata_refresh_user_fiodata takes only user_id and loads the credentials itself. The dispatcher, user signals, user tasks and the admin action pass the id only; the dispatcher reads just the user ids instead of full player data rows. The old (prun_username, fio_apikey) positional args are accepted and ignored for one release so already queued tasks still run. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
_get_recipe_distribution rebound `ticker` inside the recipe loop, so each building's top recipes landed under the last recipe's prefix instead of the building. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The unique (ticker, exchange_code, date_epoch) constraint's index already serves lookups by its leading columns; the extra index only cost writes on every CXPC upsert. Migration generated with makemigrations. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… rows Non-full CXPC refreshes bulk-inserted the whole history with ignore_conflicts on every run, ~2,200 times per trigger. History is now inserted only on full refreshes or for a (ticker, exchange_code) pair with no rows yet (one exists() check); the last 3 days keep their upsert. Chord and callback are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
user_handle_post_refresh runs on every token refresh (~78k/week). It no longer queues gamedata_refresh_user_fiodata while the user's FIO refresh lock is held (a cache read; the task still takes the lock itself), and no longer queues gamedata_clean_user_fiodata for users without credentials: user signals already clean up when credentials are removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ority apply_async(..., priority=10) overrode the priority 1 from CELERY_TASK_ANNOTATIONS. On the Redis broker 0 is the highest priority, so the code emails were queued at the lowest one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
GamePlanetViewSet.queryset was built at import, freezing now_ms in the active COGC subquery (gunicorn preloads the app), so active programs were computed against the process start time. get_queryset() now builds it per request; the generated OpenAPI schema is unchanged without the class attribute. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
import_planet records its own refresh result, so the task's second update_refresh_result() was an extra save and cache invalidation every ~9s. It also overwrote an error import_planet had recorded (it returns False instead of raising) with status 'ok'. The pending mark now saves only automation_refresh_status. Error handling is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Multi-MB list payloads (planets, exchanges) were sent uncompressed. The middleware sits after WhiteNoise (which serves its own precompressed files) and before everything else that touches the body. Streaming responses, including the SSE stream, are compressed with a flush per chunk. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The owner kept the current behaviour for these, so their pinned tests go: - plan list keeps nesting full cx_data per empire (frontend may rely on it) - the public shared view keeps exposing the plan's empires and cx - BasicAuthentication stays in the DRF defaults Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 13 |
| Duplication | 0 |
🟢 Coverage 97.79% diff coverage · +0.31% coverage variation
Metric Results Coverage variation ✅ +0.31% coverage variation (-1.00%) Diff coverage ✅ 97.79% diff coverage Coverage variation details
Coverable lines Covered lines Coverage Common ancestor commit (bee9950) 3278 2958 90.24% Head commit (37abeaa) 3322 (+44) 3008 (+50) 90.55% (+0.31%) Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch:
<coverage of head commit> - <coverage of common ancestor commit>Diff coverage details
Coverable lines Covered lines Diff coverage Pull request (#47) 136 133 97.79% Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified:
<covered lines added or modified>/<coverable lines added or modified> * 100%
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
update_last_login() saves the user, and the post_save signal already queued gamedata_refresh_user_fiodata on every save, so user_handle_post_refresh queued it a second time. The task now only updates last_login and leaves queueing to the signal. The signal skips it while the refresh lock is held (a cache read; a credential change clears the lock first, so it still refreshes). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…port Invalidating the planet list on every import_planet meant rebuilding the full list about every 9s, the planet refresh rate. The list is no longer invalidated per import and expires after 15 min instead of 1 day, so it is at most 15 min stale. Co-Authored-By: Claude Opus 5.5 <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.
Fixes the performance audit findings pinned by #46, plus the items from a week of production logs (Axiom,
prunplanner_prod). One commit per fix. Every commit passesruff check,ruff format --check,ty checkandpytest(with the CI placeholder broker env).pytest --runxfailreports no remaining audit failures: 291 passed, 0 xfailed.Audit fixes
PLANNING:{uid}:v{n}:.... Any plan, empire, junction or CX change bumps the user's counter once on commit (add, thenincr). No moredelete_patternSCANs, no scan per cascaded junction, no*1:empire:retrieve*hitting users 11/101, and no stale CX list after plan or empire renames.sync_stateno longer invalidates:update_statewrites via a queryset update.get_or_set_responsetakes acache.addlock (30s TTL). Waiters poll for up to 3s, then build themselves.multiplecache key is built from the sorted, de-duplicated ids. See the decisions below: no throttle, no id cap.defer('empire_state')in the empire, plan, CX and shared views. CX views prefetch empires with their plans. Shared retrieve prefetches the plan's empires with CX and drops therefresh_from_db.UserAPIKeyManager.get_usable_keys, soget_from_keystays the entry point).last_usedis written at most every 5 min. The userpre_savereads the previous row once with.only(...).filter(pk, modified_at=<read>).update(...): no signals, no lost update.private=TrueonCacheManager, used by the planning endpoints and FIO storage. The exchange CSV is cached as rendered bytes.latest_poprand the analytics aggregateretrievecheck the cache first.import_all_buildingsinvalidates the building list. The planet list expires after 15 min (was 1 day) instead of being invalidated per import, since planets refresh about every 9s.total_callsuses anF()update.httpx.Clientper process, created lazily after fork and closed onworker_process_shutdown.get_fio_service()keeps its interface. Also covers log item 2.gamedata_refresh_user_fiodata(user_id)loads the credentials itself; the FIO key no longer goes through the broker. All callers are updated: dispatcher, user signals, user tasks, and the admin action. Old positional args are accepted and ignored for one release.tickerin_get_recipe_distribution.idx_ticker_exchange(migration generated bymakemigrations).GZipMiddleware, placed after WhiteNoise.Production-log items
tests/gamedata/test_tasks.py::TestRefreshCXPCHistorytests/gamedata/fio/test_services.py::TestFIOServiceConnectionReusegamedata_clean_user_fiodatabranch for users without credentials is removed.update_last_login()already triggered a queue through the userpost_savesignal, so the task no longer queues its own; the lock check lives in the signal. A credential change clears the lock first, so it still refreshes.tests/user/test_user_tasks.py(new),tests/user/test_user_signals.py::TestTriggerFioRefreshpriority=10on the verification and reset code emails, so the annotation's priority 1 applies.test_verification_service.py::test_codes_are_sent_with_the_annotated_priorityGamePlanetViewSet.get_queryset()builds the queryset per request. The class attribute is removed; the generated OpenAPI schema is byte-identical without it.test_gamedata_viewsets.py::TestGamePlanetActiveCOGCgamedata_refresh_planetdrops the redundantupdate_refresh_result(), and the pending mark saves onlyautomation_refresh_status. This also fixes a bug:import_planetreturnsFalseafter recording an error, and the extra call overwrote that error withok.tests/gamedata/test_tasks.py::TestRefreshPlanetResultUnchanged as required: the CXPC refresh (3h) and planet refresh (~9s) schedules, and worker
--concurrency 1.Decisions
cx_dataper empire: declined. No API change; test deleted.BasicAuthenticationremoval: declined. It stays in the DRF defaults; test deleted.GZipMiddleware: accepted.--concurrency 1, single queue. No change.planet_searchthrottle and 100-id limit onmultiplewere dropped with their tests; only the cache-key change remains.Notes for review
needs_state_syncin cached empire payloads.PlanningEmpireDetailSerializerserializes it, so cached empire payloads can show a stale value now that state syncs and snapshot runs don't invalidate. It is an internal flag....:exchange:list:csv-v2, so JSON bytes cached by the old code are never served as CSV after deploy.Out of scope
DEBUG = Truedefault in base settings.🤖 Generated with Claude Code