Skip to content

1.9.79: one identity merge that fails no longer stops the attribution worker - #315

Merged
tracking202 merged 2 commits into
1.9.79from
1.9.79-attribution-merge-walks-past
Oct 28, 2026
Merged

tracking202 merged 2 commits into
1.9.79from
1.9.79-attribution-merge-walks-past

Conversation

@tracking202

Copy link
Copy Markdown
Owner

Backlog: K0NM

What was wrong (measured)

AttributionWorker::requeueMerges() took 202_identity_merges WHERE requeued_at IS NULL ORDER BY merge_id LIMIT 100 and ran each merge in transaction() with nothing around it. A merge whose transaction threw ended the loop and, because the merges are step 1 of run(), the whole run: the model fan-outs, the outbox and the rollup did not run either. The next run took the same merge first and threw again.

Measured on a live instance served from origin/1.9.79's tree: one merged person with 65,534 visitor keys (merge 1) ahead of two ordinary merges made by the real identity graph, and a conversion recorded after them.

  • attribution-worker.php, twice: QueryException: Failed to prepare MySQL statement: Prepared statement contains too many placeholders [errno 1390], exit 1.
  • index.php: logged Attribution worker failed: … [errno 1390], exit 0.
  • After all three runs, merges 2 and 3 still had requeued_at NULL, their conversions' journeys were still one touch, and the new conversion was still in 202_attribution_pending.

What can fail inside a merge, and whose failure it is

I read the three calls and decided from the code:

  • keysOfPerson(): two plain reads of 202_identity_visitors. It answers [] for a key with no row, never a throw of its own. Its failures are the server's (a lost connection, a missing table).
  • conversionsOnKeys(): one IN (…) with count($keys) + 2 placeholders. A prepared statement carries at most 65,535 values: 65,536 or more is refused at prepare, measured on MariaDB 10.11. That is the one failure the row determines. A person's keys only grow (IdentityGraph::merge() only adds aliases; only the account purge deletes them, and it deletes the merges too), so every run would meet it again. Everything else here is the server's.
  • enqueue(): an upsert into 202_attribution_pending, which the ledger writes inside every conversion's transaction. A lock wait or deadlock there is the server's (CLAUDE.md Fix undefined mysql array keys #34). Its values (conv_id int unsigned from 202_conversion_logs, a 14-character reason) cannot be refused by a row.
  • The UPDATE of requeued_at: the server's.

The row-determined case is checked before the statement (count($keys) > MAX_PERSON_KEYS), not read from the error. On one connection, a 65,536-placeholder prepare right after a 65,535 one came back false with errno 0, so an errno test would have read it as the server's.

What changed

  • Turns. 202_identity_merges gains requeue_failures, requeue_retry_at (0 until a failure), requeue_error, requeue_retired_at and KEY requeue_due (requeued_at, requeue_retired_at, requeue_retry_at).
    • A run takes requeued_at IS NULL AND requeue_retired_at IS NULL AND requeue_retry_at <= now ORDER BY requeue_retry_at, merge_id. Merges never tried come first, then the one whose turn came first.
    • Read from requeue_due with no sort (ANALYZE on MariaDB 10.11, 50,300 merges: range on requeue_due, 100 rows read for LIMIT 100).
  • Walk past. The catch sits around transaction(), never inside its callback. transaction() has already rolled the merge back whole.
  • Retire only what the row determines. A person over MAX_PERSON_KEYS (65,533) is retired inside the merge's own transaction: requeue_retired_at and the reason in requeue_error. It is logged, not tried again, and nothing of it is queued.
    • Every other failure (lock wait, deadlock, the trigger's SIGNAL in the test, a lost connection, a missing table) is retried for as long as it lasts and never retires.
    • 202-cronjobs/attribution-worker.php --retry-now now also makes failed and retired merges due again. It is the existing recovery switch, and the way back once a later version can read such a person.
  • Shown where attribution health is shown:
    • GET /attribution/queue (p202 attribution queue, attribution:queue) adds merges_failing, merges_retired and merge_rows (merge_id, merged_at, failures, last_error, retry_at, retired_at; retired first, up to limit).
    • merges_awaiting_requeue no longer counts a retired merge, since nothing will re-queue it.
    • The Attribution page shows a "Failing" strip ("N merges failed to credit their conversions again; each is tried again… / N merge cannot… and is not tried again").
    • The worker's summary line reads merges re-queued N (F failed, R retired) when F or R is non-zero. index.php now prints the line when a merge failed or was retired, even if nothing was processed.
    • GET /system/info and p202 system carry no attribution health today, so nothing was added there.
  • Schema. The columns and the key are in IdentityTables::identityMerges(). The 1.9.78 → 1.9.79 rung adds that one definition to its existing _upgrade_measurement_tables([...]) list, with a one-line comment, as 1.9.79: the pending-click settler takes installs in turns, so installs that cannot settle no longer stop the rest #309 does for AppTables::appInstalls(). Whichever of the two merges second has a one-line conflict there.
  • Docs: openapi (AttributionQueue, the path's description), documentation/api/13-attribution.md, features/advanced-attribution-engine.md (failure handling), the troubleshooting guide (a row and the summary line), docs/cli-agent.md. Changelog lines in both files.
  • No open 1.9.79 branch touches AttributionWorker.php (checked every origin/1.9.79-* against its merge base), so this is not stacked.

Tests

tests/Attribution/MergeRequeueFailuresIntegrationTest.php (new, @group integration, on the real schema, the real identity graph and the real ledger):

  • A merge its own data refuses: a 65,534-key person merged first, ahead of two graph-made merges and a conversion recorded after them. One run:
    • re-queues both merges (their journeys now span both visitors);
    • processes the new conversion;
    • retires the wide merge with the reason.
      The next run does not take it, and the queue counts 0 awaiting, 0 failing, 1 retired and lists it.
  • The limit is the server's: a 65,533-key person is re-queued (65,535 placeholders), not retired.
  • A server failure (a trigger SIGNALs on the first merge's own requeued_at write, after its conversions were enqueued), ahead of a good merge:
    • The good merge is re-queued.
    • The failing one is rolled back whole (nothing of it in the outbox, its journeys one touch), recorded as 1 failure, and due on the next run.
    • It is then due 60 s after its second failure, not taken before its turn, and at an hour after any number of failures (401). It is never retired.
    • Once the trigger is gone, it is re-queued.
  • A lock wait (another connection holds an uncommitted outbox row of the first merge's conversion; innodb_lock_wait_timeout = 1): the merge after it runs, and the failure is recorded with [errno 1205], not retired. The next run re-queues it.
  • Turn order: with limit 1, a never-tried merge goes before a failed one whose retry is due.

Plants: each was backed up to a fresh mktemp, compared with cmp before planting, run against this suite, and restored and compared again. Each counts only when a named test fails, not on a lint error.

Plant Tests that failed
the old loop (origin's selection and loop, no catch) the retire test, the server-failure test, the lock-wait test, the turn-order test (each erroring with the real 1390 / 1644 / 1205 QueryException)
the old selection (WHERE requeued_at IS NULL ORDER BY merge_id) retire, server-failure, turn-order
record the failure, then stop the loop server-failure, lock-wait, turn-order
retire every failure server-failure, lock-wait, turn-order
catch inside the transaction callback server-failure, lock-wait, turn-order
no width check retire
MAX_PERSON_KEYS one below (65,532) the at-the-limit test
MAX_PERSON_KEYS one above (65,534) retire
no backoff server-failure
uncapped backoff server-failure
failure not written server-failure, lock-wait, turn-order
queue counts a retired merge as awaiting retire
queue's merge_rows misses retired merges retire

Live pass

The instance was served from this worktree with php -S on the router. It was installed with install.php's own steps but not its licence check, because the stub is down, with every proxy variable at 127.0.0.1:9.

  • Seed. The conversions went through the real ledger writer (MysqlConversionRepository::record()) and the merges through the real identity graph (IdentityGraph::attachClick() on clicks carrying both visitors' cookies). The click rows and the 65,534-key person were written by SQL.
  • Before (the tree at origin/1.9.79): attribution-worker.php twice, then index.php. The results are in "What was wrong".
  • Upgrade. 202_version was wound back to 1.9.78, and the upgrade ran through the upgrade page (GET the form, POST its token). It said Success!, logged no Prosper202 upgrade line, and added the four columns and requeue_due. The upgraded SHOW CREATE TABLE 202_identity_merges is identical to a fresh install's.
  • After (this branch):
    • attribution-worker.php printed processed 5 (credited=5); merges re-queued 2 (0 failed, 1 retired); …, exit 0.
      • Merges 2 and 3 were re-queued, and their conversions now have 2-touch journeys.
      • The new conversion was credited with a 3-touch journey.
      • Merge 1 is retired with the reason in requeue_error, and logged once.
      • The next run did nothing (merges re-queued 0).
    • A lock wait. A second mysql session held an uncommitted outbox row of a conversion of the first of two new merges, for 70 s, under the default 50-second lock wait.
      • The run took 50 s and printed merges re-queued 0 (1 failed, 0 retired). The merge holds requeue_failures 1 and …Lock wait timeout exceeded… [errno 1205], due on the next run. The wait used up the run's 50-second budget, so the second merge waited for the next run.
      • The next run took the never-tried merge first (requeued_at 9 s earlier), then the retried one once the lock was released: merges re-queued 2, both journeys rebuilt.
    • GET /api/v3/attribution/queue answered merges_failing, merges_retired and merge_rows with the reason. Signed in, Account › Attribution showed the strip: "1 merge failed to credit its conversions again; each is tried again, less often the longer it fails; 1 merge cannot credit its conversions again and is not tried again (p202 attribution queue shows why)." The strip is absent when none are failing. The page was read again on the final commit.
    • attribution-worker.php --retry-now printed 0 failed row(s) and 3 failed identity merge(s) made due again. The run re-queued the two and retired the wide one again.
    • index.php printed Attribution: processed 0 (none); merges re-queued 0 (0 failed, 1 retired); ….

Verification

.claude/skills/p202-verify/scripts/verify.sh --since origin/1.9.79 --changed (schema tier on a scratch MariaDB 10.11 database), on the committed branch:

Scope report
------------
  openapi    PASS
  patterns   PASS
  phpcs      PASS
  phpstan    PASS
  schema     PASS
  syntax     PASS
  unit       PASS

Not covered by this script:
  live      stand up an instance, drive the path a user would take
  agent     add and run a case under tests/fixtures/agent-eval/cases/
  ci        a workflow still in_progress has not passed; read the run

The unit tier ran 6104 tests with 4 skipped (the documented "User class requires DB singleton" skips). Beyond the script:

  • Every @group integration suite (tests/run-integration-suites.sh, each on a fresh scratch database): 146 suites and 1370 tests passed, and 1 suite failed. The failure was tests/Repository/MysqlLookupRepositoriesIntegrationTest.php::testTheFactoryCachesThroughTheBootstrapHelpers, with Class "Memcached" not found, because this sandbox's PHP has no memcached extension. The run includes the new suite and every Attribution, Identity, Ltv, Upgrade and Schema suite.
  • The plants above, on the final code.
  • tests/live/upgrade-equals-install.sh (1.9.55 on PHP 7.4, upgraded through the upgrade page, compared with a fresh install): 25 passed, 0 failed. Its notable lines:
    • "the upgrade logged no error or warning";
    • "compared 162 tables after the upgrade with 162 in a fresh install: identical up to index order, table comment, partition boundaries and AUTO_INCREMENT";
    • "the rows of 155 tables are identical".
      The 1.9.55 upgrade creates 202_identity_merges from today's definition in an earlier rung, so this run does not exercise the new rung entry. The live pass above does: a table in 1.9.78's shape, upgraded through the page, then compared.
  • ANALYZE of the due-merge selection (above).

What could not run, and what ran without the licence

  • install-instance.sh and the Go CLI could not run against the live instance: the licence stub is down, and an instance without the licence answers the CLI 402. The live pass drove the API with curl and the cron jobs on the CLI. The queue fields render through render() in the Go CLI and $this->render() in the PHP CLI, generic JSON, and neither CLI was run here.
  • tests/live/upgrade-equals-install.sh ran through a copy that differs in two ways:
    • its fresh install (step 2) used install.php's steps without the licence check, the same scratch installer as the live pass, instead of install-instance.sh;
    • it used one port for both phases.
      Steps 1 (1.9.55 installed on PHP 7.4), 3 (the upgrade page) and 4–5 (schema and rows) are the script's own.
  • Not exercised by any test or by the live pass:
    • the WorkerHalted path for a rollback that failed (no way found to make rollback() fail on a live handle);
    • recording a failure that itself fails (only logged).
  • MySQL 8.0 was not available here: every database run and the plan measurement were on MariaDB 10.11. CI's integration job runs MySQL 8.0. The new test uses only what the existing worker tests already use there (a trigger with SIGNAL, innodb_lock_wait_timeout).
  • No agent-eval case was added. The change adds read-only fields to GET /attribution/queue and has no new agent task.

For the reviewer

  • Merges now differ from the outbox on purpose. In processPending(), a QueryException still ends the run with every row untouched. For merges, the brief's rule applies: record, back off, walk past. The reason is that a row-determined failure can be a QueryException (this one is), and the outbox's rule turns that into a head-of-line block. The same is true of the outbox itself (see "Found outside this item").
  • Retiring a merge loses little. Its conversions keep the journeys they had until anything else re-queues them (a new conversion, a model change). --retry-now brings it back.
  • requeue_failures is kept after a merge finally succeeds, as history. It is counted as failing only while requeued_at is NULL.
  • The 20-second budget of index.php (50 s for attribution-worker.php) is no longer than InnoDB's default 50-second lock wait. In the live pass, one lock wait used the whole run. The next run then took the fresh merge before the retry, which is the turn order working. The budget itself is unchanged.
  • CLAUDE.md is not edited. 1.9.79: company linking walks past names it cannot link #304's Remove Intercom integration #37 is this shape. A paragraph there could add "a row-determined failure can arrive as a database error: classify it before the statement, not by its errno".

Found outside this item

  • AttributionWorker::processPending(): JourneyBuilder::build() names every key of the person in one statement (count + 6 placeholders). A conversion of a person with more than 65,529 keys fails at prepare. The outbox reads that as the engine's failure (WorkerHalted), and claim() takes that conversion first on every run, so the whole outbox stops for good. This is the same head-of-line shape, on the outbox.
  • AttributionWorker::fanOutModelRecomputes(): the same loop as the old requeueMerges(). A model whose fan-out transaction throws ends the run before the outbox, and is first again on the next run (ORDER BY recompute_requested_at, model_id). I found nothing row-determined in it, so it is the server-failure half only.

Generated by Claude Code

… worker

AttributionWorker::requeueMerges() took the merges with requeued_at NULL in
merge_id order and ran each in transaction() with nothing around it, so a
merge whose transaction threw ended the loop and, the merges being step 1
of run(), the whole run: the model fan-outs, the outbox and the rollup did
not run either, and the next run took the same merge first again. Measured
on an instance from origin/1.9.79: a merged person of 65,534 visitor keys
ahead of two ordinary merges failed every run with "too many placeholders"
(errno 1390), and neither the merges after it nor a new conversion were
ever processed.

Each merge now takes its turn (202_identity_merges.requeue_retry_at, the
merges never tried first). A failure is caught outside the transaction,
which transaction() has rolled back whole (CLAUDE.md #34), recorded on the
row (requeue_failures, requeue_error) and passed; it is retried on the next
run, then after a minute, doubling to an hour. Only a failure the merge's
own data determines ends its retries: a person with more visitor keys than
one prepared statement can name (65,533, the statement naming the account
twice besides; 65,536 values are refused). That is checked before the
statement rather than read from the error, which once came back with no
errno at all, and the merge is retired (requeue_retired_at, with why) in its
own transaction. A lock wait, deadlock, lost connection or missing table is
retried for as long as it lasts and never retires a merge. A rollback that
failed still ends the run.

GET /attribution/queue (p202 attribution queue) adds merges_failing,
merges_retired and merge_rows; merges_awaiting_requeue no longer counts a
retired merge. The Attribution page warns when merges are failing or
retired, the worker's summary line counts them, and attribution-worker.php
--retry-now makes failed and retired merges due again. The 1.9.78 -> 1.9.79
rung reconciles 202_identity_merges for the four columns and requeue_due.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
Copilot AI balanced review requested due to automatic review settings October 28, 2026 11:35
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ⚠️ Failed 2026-10-28T11:35:04.994461Z
We couldn’t complete this review. Comment @codex review to try again.
40c4770 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

@github-actions

Copy link
Copy Markdown
Contributor

Code review: CLEAN

For 40c4770: 0 issue(s). The review is encrypted for the maintainers in the review-findings artifact of this run.

@github-actions

github-actions Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Adversarial review: NON-BLOCKING

For b255070: 4 finding(s). The report is encrypted for the maintainers in the review-findings artifact of this run.

Rewritten on every push; the check fails on BLOCKING.

Brings in 1.9.79's new commits. The conflicts were in 202-config/changelogs.json, 202-config/functions-upgrade.php, changelogs.txt, where both sides added lines: the incoming lines are kept as they are and this branch's follow.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
@tracking202
tracking202 merged commit a7b6e48 into 1.9.79 Oct 28, 2026
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants