Skip to content

subscriber: do not mask a failed BEGIN with 'cannot rollback' (fleet #46912) - #40

Open
echoomegaprime wants to merge 1 commit into
mainfrom
fix/subscriber-migration-rollback-46912
Open

echoomegaprime wants to merge 1 commit into
mainfrom
fix/subscriber-migration-rollback-46912

Conversation

@echoomegaprime

Copy link
Copy Markdown
Owner

Summary

Fixes Echo fleet item #46912: the echo-certforge-dispatcher.service restart storm on 2026-10-19 (incident #49782, auto-resolved).

Root cause. In SubscriberGovernance._migrate_legacy_platform_tables, the except BaseException handler always ran ROLLBACK. When BEGIN IMMEDIATE itself fails, there is no transaction open, so ROLLBACK raises sqlite3.OperationalError: cannot rollback - no transaction is active. A failed BEGIN IMMEDIATE happens, for example, when another process holds the write lock past the 10 s busy timeout and SQLite reports database is locked. That second error replaced the real one and crashed the dispatcher at start-up. This is the traceback in the incident.

Fix. Roll back only when connection.in_transaction is set, so the original error propagates and is the one logged.

Tests

New tests/test_subscriber_migration_rollback.py, written to fail first:

  • A failed BEGIN IMMEDIATE surfaces database is locked, not cannot rollback. Before the fix this test reproduces the production error exactly.
  • An error inside the transaction still rolls back and leaves the database unchanged.
  • A needed legacy rename is still committed.

With tests/test_p7_subscriber_governance.py as well: 61 passed. The 2 test_deploy_environment_boots_and_authenticated_smoke_is_green[...] cases fail identically on unpatched main (58 passed + the same 2), because they need the deployment environment.

Deployment

Production currently runs release d8ad9fd (2026-09-26), which has the same code, so the defect is latent there. It only triggers when start-up loses the write-lock race. I did not hot-patch the certification authority. This change should ship through deploy/deploy_forge.sh once reviewed.

🤖 Generated with Claude Code

https://claude.ai/code/session_01C4YSVhYLwZidNF5snQ4eec

…46912)

echo-certforge-dispatcher flapped at start-up on 2026-10-19 with
sqlite3.OperationalError: cannot rollback - no transaction is active.
When BEGIN IMMEDIATE itself fails in _migrate_legacy_platform_tables
(for example 'database is locked' once the busy timeout expires while
another process holds the write lock), the except branch ran ROLLBACK
with no transaction open. That raised a second error, hid the real
cause, and crashed the worker.

The handler now rolls back only when connection.in_transaction is set,
so the original error propagates.

Tests (red first): tests/test_subscriber_migration_rollback.py
- a failed BEGIN surfaces 'database is locked', not 'cannot rollback'
  (reproduces the production error before the fix);
- an error inside the transaction still rolls back and leaves the DB
  unchanged;
- a needed legacy rename is still committed.
Subscriber suites: 61 passed. The 2 test_deploy_environment_boots...
tests fail identically on unpatched main (they need the deployment
environment).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C4YSVhYLwZidNF5snQ4eec
echo-fleet-builder Bot pushed a commit that referenced this pull request Nov 3, 2026
…ompensation

Port PR #40 and PR #41 onto deployed d8ad9fd without unrelated main changes.
Retain historical crash credits by tenant/run identity and use a separate
run-compensation namespace for new credits. Add atomicity, concurrency,
legacy-credit, original-period, collision-order, and migration rollback tests.

Verified: 705 passed, 1 Docker-dependent skip; P1/P6/P7 acceptance green;
87 main integration tests passed; 7/7 isolated mutation checks killed.
No deployment, production state repair, or live compensation authorized.

Copy link
Copy Markdown
Owner Author

Integration acceptance update, November 10, 2026: independently applied PR #40 then PR #41 to isolated worktree at main 6515544d0083edde1eefba8bd89ab3d5c95ccf4b, without touching the active production checkout. Both patches applied cleanly. 7 focused tests passed, and all 60 tests in tests/test_p7_subscriber_governance.py passed in four independently bounded 15-test batches (15/15, 15/15, 15/15, 15/15). See the complete evidence on PR #41: #41 (comment). Earlier exact individual PR-head GitHub workflow runs completed successfully, but neither PR is merged. Do not promote yet: the latest live CertForge signed verdict remains NOT_READY for production_e2e_attestation_missing, and there is no certified exact combined release candidate. Next: reviewed combined candidate with current-source hosted checks, end-to-end attestation, signed READY and rollback proof.

Copy link
Copy Markdown
Owner Author

Integration coordination (Nov 10, 2026): The exact-target combined draft is now PR #49: #49, commit 0ab617ab6ed97e1a40e5cb3d160215dc93dc86d5, parent/main 4f7d99e2dd4422f3d81cf5b709581be963f5818a. It includes precisely the changes from #40 and #41; independent exact-canonical clone tests passed 67/67 (7 focused + 60 subscriber-governance). Local Git tree and GitHub tree both 38542565dc2725132d44571ca4883afaa958ada0. Use #49 for combined CI and integration review rather than independently merging #40 and #41 without reconciling the overlap. No merge or production deploy has occurred. Exact-head CI is in progress; signed CertForge PRODUCTION_READY and production_e2e attestation remain required.

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.

2 participants