Document additional bugs/perf issues found in second review pass
Added B6 to UMSETZUNGSPLAN.md: unbounded MarketSyncWorker re-sync of all closed markets, watchlisted traders not exempt from auto-cleanup deletion, full trade-history load in GetKnownPlatformTradeIdsAsync, a race condition in MarketRepository.AddOrUpdateAsync under concurrent access, and an early-break assumption in TradeHistoryWorker that isn't applied consistently. No functional code changed yet — these are earmarked for Phase 2 (B3-adjacent).
This commit is contained in:
@@ -91,6 +91,13 @@ Beim Review ist aufgefallen, dass `TraderAnalyticsWorker.CalculatePnL` **jeden T
|
|||||||
- [ ] N+1-Datenbankzugriffe in `ScoringService.RecalculateAllScoresAsync` durch Batch-Queries ersetzen
|
- [ ] N+1-Datenbankzugriffe in `ScoringService.RecalculateAllScoresAsync` durch Batch-Queries ersetzen
|
||||||
- [ ] Eigenes Intervall für volle Neuberechnung/Ranking (z.B. alle 15 Min) getrennt vom reinen Trade-Polling (60s)
|
- [ ] Eigenes Intervall für volle Neuberechnung/Ranking (z.B. alle 15 Min) getrennt vom reinen Trade-Polling (60s)
|
||||||
|
|
||||||
|
### B6. Weitere gefundene Bugs/Performance-Probleme (Review-Pass 2026-07-01)
|
||||||
|
- [ ] **`MarketSyncWorker` re-synct alle 30 Minuten ALLE Märkte, inkl. `includeClosed=true`, von Offset 0** ([MarketSyncWorker.cs](src/Predictalytics.Worker/Services/MarketSyncWorker.cs)) — holt damit bei jedem Zyklus jeden jemals geschlossenen Polymarket-Markt erneut komplett durch. Wächst unbegrenzt mit der Zeit, echtes Risiko für Rate-Limiting/Sperrung. Sollte auf: aktive Märkte häufig, geschlossene Märkte selten/inkrementell (z.B. nur kürzlich geschlossene, nicht der komplette Bestand) umgestellt werden
|
||||||
|
- [ ] **Watchlisted Trader nicht von Auto-Löschung ausgeschlossen** (`TraderRepository.GetTradersForCleanupAsync` / `TraderCleanupWorker`) — da `Trades`/`TraderScore`/`WatchlistEntries` per Cascade am Trader hängen, könnte ein manuell beobachteter Trader nach 1 Jahr Inaktivität (oder bei kurzzeitigem API-Fehler) unbemerkt komplett gelöscht werden. Watchlist-Einträge sollten von der Cleanup-Query ausgenommen werden
|
||||||
|
- [ ] **`TradeRepository.GetKnownPlatformTradeIdsAsync` lädt die komplette Trade-ID-Historie eines Traders ins RAM**, nur um eine kleine neu geholte Charge (~100-1000 Trades) zu deduplizieren — aufgerufen alle 60s (`PollingWorker`) bzw. alle 12h (`TradeHistoryWorker`) pro Trader. Wird mit wachsender Trade-Zahl immer teurer. Fix: nur `WHERE PlatformTradeId IN (<geholte Batch-IDs>)` abfragen statt der gesamten Historie
|
||||||
|
- [ ] **Race Condition in `MarketRepository.AddOrUpdateAsync`** (im Gegensatz zu `AddOrUpdateRangeAsync` ohne Locking) — `TradeHistoryWorker` verarbeitet bis zu 5 Trader parallel (`Parallel.ForEachAsync`); referenzieren zwei gleichzeitig denselben noch unbekannten Markt, prüfen beide unabhängig "existiert nicht" und einer crasht beim `Add` mit Unique-Constraint-Verletzung (wird geloggt, Trader-Sync für den Zyklus bricht ab, nächster Zyklus heilt es meist). Fix: gleiches Locking-Muster wie `AddOrUpdateRangeAsync` verwenden, oder Insert-Konflikt sauber abfangen/retry
|
||||||
|
- [ ] **`TradeHistoryWorker` bricht die Trade-Verarbeitung beim ersten bekannten Trade ab** (`if (!isInitial) break;`), verlässt sich also darauf, dass die API immer streng neueste-zuerst liefert. `PollingWorker`s äquivalente Schleife macht das NICHT. Sollte angeglichen werden — der Performance-Gewinn ist gering gegenüber dem Risiko einer stillen Datenlücke, falls die Annahme mal nicht stimmt
|
||||||
|
|
||||||
### B4. Testabdeckung für die kritische Logik
|
### B4. Testabdeckung für die kritische Logik
|
||||||
- [ ] Neues Testprojekt (z.B. `Predictalytics.Application.Tests`) anlegen — aktuell existiert **kein einziges** Testprojekt
|
- [ ] Neues Testprojekt (z.B. `Predictalytics.Application.Tests`) anlegen — aktuell existiert **kein einziges** Testprojekt
|
||||||
- [ ] Unit-Tests für die neue PnL-Engine (A1) — insbesondere Grenzfälle: nur offene Position, nur geschlossene Position, Split/Merge, Redeem-Verlust vs. -Gewinn
|
- [ ] Unit-Tests für die neue PnL-Engine (A1) — insbesondere Grenzfälle: nur offene Position, nur geschlossene Position, Split/Merge, Redeem-Verlust vs. -Gewinn
|
||||||
|
|||||||
Reference in New Issue
Block a user