fix(uitvraag): begrens de Redis-commando's per ophaalronde - #296
Open
ericwout-overheid wants to merge 9 commits into
Open
fix(uitvraag): begrens de Redis-commando's per ophaalronde#296ericwout-overheid wants to merge 9 commits into
ericwout-overheid wants to merge 9 commits into
Conversation
Beschrijft de sequentiele batcher in fbs-berichtensessiecache en het verbreden van twee reactieve foutfilters naar Throwable, met per taak een mutatietabel en de verificatie op de demo-stack. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016oiwkSpLFuoQSt6QnozM5Y
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016oiwkSpLFuoQSt6QnozM5Y
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016oiwkSpLFuoQSt6QnozM5Y
…ch raken De test asserteerde op berichten.last() in de aanname dat dat het laatst verwerkte bericht was, maar store() sorteert vóór het batchen op publicatietijdstip aflopend — het bericht met de hoogste timestamp landt dus als eerste in die gesorteerde lijst, in de eerste batch. De test slaagde daardoor vacuous en bewees niets over gedrag voorbij de eerste batch. Assert nu op berichten.first() (laagste timestamp, komt als laatste uit de sortering) en benoem in de comment de echte reden. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016oiwkSpLFuoQSt6QnozM5Y
renewBerichtTtls en pruneListEnDelHash boden hun EXPIRE- resp. LREM-commando's nog met Uni.join().all() in één keer aan de connection-wachtrij aan. Beide zijn vandaag begrensd (renewBerichtTtls door pageSize, pruneListEnDelHash door 0-of-1 match), maar het onbegrensde fan-out-patroon stond nog in het bestand en kon terugkeren zodra een van beide lijsten later meegroeit. Beide gaan nu via dezelfde RedisBatching.inBatches-helper als store, binnen dezelfde MULTI/EXEC-transactie. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016oiwkSpLFuoQSt6QnozM5Y
…iken Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016oiwkSpLFuoQSt6QnozM5Y
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016oiwkSpLFuoQSt6QnozM5Y
Contributor
JaCoCo coverage
Files
|
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016oiwkSpLFuoQSt6QnozM5Y
…l de documentatie Voegt een fail-fast guard toe die redisBatchgrootte tegen quarkus.redis.max-waiting-handlers valideert bij het opstarten, en geeft die laatste sleutel dezelfde env-override-vorm als de batchgrootte — een operator kan nu geen configuratie draaien die de invariant breekt zonder dat de service dat meldt. Corrigeert daarnaast drie factual bugs in comments/docs die de eindreview van fix/redis-store-batchgrootte vond: de in-flight piek per batch is de batchgrootte zelf (HSET en EXPIRE zijn per bericht met .chain geregen, nooit beide tegelijk in de wachtrij), niet het dubbele; een ronde van 2700 berichten kost 11 batches à twee round-trips (22 totaal), niet 11 round-trips; en de connectiepool speelt geen rol in het store-pad omdat withTransaction één connection claimt voor de hele MULTI/EXEC. Voegt ook een test toe die pint dat een mislukte TTL-verlenging een geslaagde read nooit laat falen, en breidt de KDoc van RedisBatching.inBatches uit met het contract bij een gefaalde batch buiten een transactie. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016oiwkSpLFuoQSt6QnozM5Y
ericwout-overheid
left a comment
Contributor
Author
There was a problem hiding this comment.
Ready for review
ericwout-overheid
marked this pull request as ready for review
September 7, 2026 17:35
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
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.
Wat er aan de hand was
Een ondernemer die bij honderd organisaties is aangesloten, zag alle honderd organisaties netjes
langskomen — en kreeg dan geen lijst. Het ophalen eindigde met "Stream afgebroken: TypeError:
network error". Alle organisaties waren op dat moment wél bevraagd en hadden geantwoord; het ging
mis in de allerlaatste stap, het bewaren.
Juist de ondernemer met de meeste aansluitingen kreeg dus niets te zien, én geen uitleg: de
verbinding viel gewoon weg, wat aan zijn kant op een netwerkstoring lijkt.
Het bleken twee oorzaken die elkaar maskeerden.
Oorzaak 1 — het bewaren bood alle commando's tegelijk aan
RedisBerichtenCache.storeschreef per bericht twee commando's (HSET+EXPIRE) en bood zeallemaal tegelijk aan met
Uni.join().all(...). Die subscribet op alles tegelijk, dus alle2×Ncommando's gingen in één keer de connection-wachtrij in. De Vert.x-client begrenst die op
quarkus.redis.max-waiting-handlers(default 2048).Bij honderd organisaties × 27 berichten zijn dat ruim 4300 commando's. De configuratie staat
bovendien 500 berichten per organisatie toe, dus het theoretische plafond is 100 × 500 × 2 =
100.000. Een hogere
max-waiting-handlersverplaatst die grens dus alleen — het issue vraagtexpliciet om een grens die niet met het aantal organisaties meegroeit.
Wat er nu gebeurt. Een nieuwe herbruikbare helper
RedisBatching.inBatches(...)biedt decommando's in batches aan: een batch wordt pas aangeboden als de vorige beantwoord is. Het aantal
in-flight commando's is daarmee losgekoppeld van de fan-out.
De transactie blijft daarbij volledig intact. Het issue vermoedde dat batchen de
transactie-semantiek zou raken; dat geldt alleen voor de variant met een transactie per batch.
Redis antwoordt op elk commando binnen een
MULTImet+QUEUEDen voert pas bijEXECuit, dussequentieel aanbieden begrenst enkel het aanbieden. De hele berichtenlijst blijft één transactie.
Dezelfde helper vervangt het
Uni.join()-patroon ook inrenewBerichtTtlsenpruneListEnDelHash. Die twee zijn vandaag níet stuk (begrensd doorpageSize, respectievelijk0 of 1
LREM); ze gaan mee zodat het onbegrensde patroon nergens meer in het bestand staat en nietterugkeert zodra zo'n lijst later wél meegroeit.
Oorzaak 2 — de foutmelding bereikte de gebruiker nooit
Het issue merkte op dat de
OPHALEN_FOUT-route al bestond maar niet geraakt werd. De reden:NoStackTraceThrowable— het type waarmee Vert.x "Redis waiting queue is full" meldt — erft vanThrowable, niet vanException(geverifieerd metjavapop vertx-core-4.5.30). Tweereactieve recover-punten in
BerichtensessiecacheServicefilterden opExceptionen lieten dattype dus door:
aggregeerEnSlaOpOPHALEN_FOUT-route werd overgeslagen, de fout bereikte de SSE-emitter, RESTEasy kapte de response af zonder afsluitende chunk →curlexit 18, browserTypeError: network errorThrowableviel door naar het vangnet en werd daar alsOVERBELAST("niet bevraagd") geclassificeerd, terwijl het magazijn wél bevraagd was en faaldeBeide staan nu op het ongetypeerde
.onFailure().De blocking-paden zijn bewust níet aangepast.
await().atMost(...)verpakt eenniet-
RuntimeExceptionin eenCompletionException, die wél eenExceptionis — duscatch (e: Exception)is daar correct. Dat is met bytecode-inspectie van Mutiny'sUniBlockingAwait.awaitbevestigd, en er staat nu een test die die asymmetrie vastpint, zodat eenlatere onderhouder ze niet "voor de consistentie" meeverbreedt.
Knoppen
berichtensessiecache.redis-batchgrootte> 0gevalideerd bij bootquarkus.redis.max-waiting-handlersBeide sleutels hebben een env-override, zodat een operator niet één kant van de invariant kan
verzetten zonder de andere. De invariant
redis-batchgrootte < max-waiting-handlerswordtbij het opstarten afgedwongen: bij een conflict start de dienst niet, met een melding die beide
sleutels én hun waarden noemt.
max-waiting-handlers=2048is de bestaande Vert.x-default die nuexpliciet is opgeschreven — geen gedragswijziging.
De piek is
batchgrootte, niet2 × batchgrootte: deHSETen deEXPIREvan één bericht zijnaan elkaar ge-
chained, dus ze staan nooit samen in de wachtrij.Wat de tests bewijzen
De integratietest reproduceert de productiestoring op testschaal: het TestProfile zet
max-waiting-handlersop 64, waardoor 200 berichten al ruim over de grens gaan. Die test isaantoonbaar rood op de ongewijzigde
store, metRedis waiting queue is full— dat is explicietgemeten vóór de fix, niet achteraf beredeneerd.
Daarbij een waarneming die het issue nog niet noemt: een overflow laat de gepoolde connection
midden in een
MULTIachter, waarna vervolgoperaties op diezelfde connection stuklopen metERR MULTI calls can not be nested. De storing is dus besmettelijker dan alleen de ronde die hemveroorzaakt.
Mutatietesten
Er zit geen pitest in deze repo, dus elke nieuwe test is handmatig gemuteerd: mutant toepassen,
test draaien, rood bevestigen, terugdraaien, groen bevestigen. Dat geldt ook voor de bestaande
tests waarvan de helper is aangeraakt.
RedisBatchingchunked+foldover een lege lijst is een no-op)store+ boot-guardrenewBerichtTtls/pruneListEnDelHashcatch Exception→RuntimeException, via bytecode vanUniBlockingAwait)Twee mutanten zijn dus niet geforceerd rood gemaakt met een kunstmatige assertie, maar als
equivalent onderbouwd — ze veranderen het waarneembare gedrag niet.
Eén mutatie legde een echte zwakke test bloot: de eerste TTL-test asserteerde op
berichten.last(),maar
storesorteert aflopend op publicatietijdstip, dus dat bericht zat juist in de eerstebatch. De test slaagde vacuüm voor precies het scenario dat hij claimde te controleren. Gecorrigeerd
naar het bericht dat werkelijk in de laatste batch valt.
Wat het nalopen van de foutfilters opleverde
De rest van de productiecode is nagelopen op dezelfde bugklasse: 26 filter- en catch-plekken
beoordeeld in
fbs-common,fbs-magazijnregister,fbs-berichtensessiecache,berichtenuitvraagen
berichtenmagazijn. Geen enkele bleek te lekken, dus geen wijzigingen. De inventarisatiestaat in het plandocument.
Eén nevenbevinding, buiten scope en niet verslechterd door deze PR: het ongefilterde
getPage-pad roept alleenrenewSessionTtlaan, nooitrenewBerichtTtls— ongefilterde readsverlengen de per-bericht-hash-TTL's dus niet. Bestaand gedrag, maar het lijkt de moeite waard om
apart te bekijken.
Verificatie
Suites (alle drie modules,
clean verify):fbs-berichtensessiecache411/411,berichtenuitvraag246/246,
berichtenmagazijn439/439. JaCoCo 90%-gate gehaald, detekt 0 bevindingen. Dealways-run-filter-deprecatiewaarschuwing in het magazijn is pre-existing en niet door deze branchveroorzaakt.
Demo-meting, tegen de lokale stack met de uitvraag herbouwd uit deze branch (geen dev-mode
surrogaat), persona met honderd aangesloten organisaties:
curlexitDaarmee is acceptatiecriterium 1 gehaald. Het uitblijven van
curlexit 18 is explicietgecontroleerd: dat was het symptoom uit het issue.
Foutpad. Met het bewaren opzettelijk kapot krijgt de ondernemer:
De stream sluit daarbij netjes af (
curlexit 0, HTTP 200). Acceptatiecriterium 2 gehaald.Een eerlijke kanttekening bij die tweede meting. De overflow terugbrengen door alleen de
batchgrootte absurd hoog te zetten bleek op demo-schaal niet genoeg: de
+QUEUED-acks komenvrijwel direct terug op een onbelaste lokale Redis, waardoor de wachtrij zich sneller leegt dan hij
zich vult. Reproductie vereiste daarnaast
max-pool-size=1en 50 ms kunstmatige latency. Eencontrole-ronde onder exact dezelfde condities maar mét de standaard batchgrootte slaagde wel — dat
isoleert de batcher als de werkzame factor.
Dat versterkt het beeld uit het issue in plaats van het te ondergraven: dat concludeerde zelf al
"geen harde drempel maar een race — of het misgaat hangt af van hoe snel Redis meekomt". Een
onbelaste lokale Redis is nu eenmaal sneller dan productie.
Basisbranch
Deze PR staat op
feature/magazijn-bulkhead-wachtrij(#283), niet opmain— het probleem werdpas zichtbaar door de wachtrij uit die PR plus de doorpaginering, en hoort er in dezelfde volgorde
achteraan.
Closes MinBZK/MijnOverheidZakelijk#1077
🤖 Generated with Claude Code
https://claude.ai/code/session_016oiwkSpLFuoQSt6QnozM5Y