fix(cohort): stop target retry storm and ledger duplicate-key failures#369
Merged
Conversation
Add red tests for terminal missing cohort targets, no-create materialization, and filling an unbound existing mapping.
Make missing cohort targets terminal, keep stale materialize jobs no-create, and allow existing unbound cohort mappings to be linked.
Add red ordering tests for claim-before-stamp ledger transitions and a MariaDB integration test for the reconcile duplicate-key and soft-delete precision cases.
Collapse matching stranger rows before desired rows receive external ids, use microsecond soft-delete timestamps, and recompute reconciliation mappings inside the write transaction.
Add a red remediation contract and unit test for re-enqueueing unsynced desired ADDs after an operator binds an external target.
Expose an admin repair action that requires a bound cohort and re-enqueues ADD jobs for desired rows stranded by no-op rule evaluation.
Remove stale lazy-create wording now that materialize-target is no-create compatibility behavior and target creation is explicit.
jorisjonkers-dev-agents
Bot
force-pushed
the
platform/cohort-incident-fix
branch
from
8d3a286 to
c67fc0f
Compare
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.
Summary
Two cohort-sync failure modes produced the recent wave of failed job executions; both are fixed here, reproduced by tests first.
Missing external target (retry storm + wrong auto-create). A membership
ADDfor a cohort with no external target enqueuedcohort.materialize-targetand threw a retryable exception, so the job exhausted its four attempts within seconds, whilematerialize()created a brand-new Brevo list instead of binding the list the members were already in.add()now fails terminally when the target is missing — no materialize enqueue, no provider call — while a missing user external id stays retryable.materialize()returns an existing id or fails terminally and never creates a target.linkExistingnow fills an existing unbound cohort row, so an operator can bind the real list.Reconcile duplicate key. The verify path stamped a desired row's
external_user_idand then queried for the matching stranger; that SELECT auto-flushed the desired update ahead of the stranger soft-delete, colliding onuk_cohort_member_external. The ledger now claims and soft-deletes a matching stranger with a single flush before stamping the desired row, and refuses to stamp an external id already owned by another desired row. Cohort-member soft-delete usesNOW(6)so a rapid re-add cannot collide on a second-truncated sentinel. Reconcile recomputes external-id mappings inside the write transaction.An admin endpoint
POST /management/cohorts/{id}/repair-missing-addsre-enqueues missing ADDs for a bound cohort so members that previously failed converge once the target is linked.No external-system deletion paths are added or widened; all removals here are local soft-deletes.
Tests
./gradlew :services:api:compileKotlin :services:api:compileTestKotlin :services:api:test --tests "*cohort*"— 111 passed./gradlew :services:api:compileIntegrationTestKotlin— passedCohortLedgerAutoflushITreproduces the duplicate-key autoflush and the rapid soft-delete/re-add precision case; it runs against MariaDB in CI.