Repository navigation
test(harness): bound the LazySavepoint deadlock partner via PartnerProcess (Closes #76, Ref #37) - #77
Conversation
…Process and clean up fail-safe (Ref #76, #37) Replaces the hand-rolled proc_open/fgets partner handling (nominal 30 s stream_set_timeout that does not bound a blocking read, teardown that terminated and closed without checking) with the bounded PartnerProcess from #75. tearDown now attempts every cleanup (stop partners, roll back connections, drop schema) and rethrows the first failure afterwards. Adds a frozen-LazySavepoint-partner negative to PartnerProcessIntegrationTest. Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
Technical Lead — Independent Fresh Full Re-Review: MERGE NO-GO (one bounded fail-closed cleanup finding)Identity/scope: PR #77 OPEN, not merged, mergeable, single commit, Base Accepted code: The LazySavepoint tests reuse existing CI is GREEN, not the blocker: exact HEAD run #39975565444, 10/10 SUCCESS. Actual logs independently read: Lowest job F77-01 — BLOCKER: rollback failure silently discarded in claimed fail-closed cleanupLocation: try {
if ($connection->inTransaction()) {
$connection->rollBack();
}
} catch (Throwable) {
}A thrown Required smallest remediation within the SAME PR: capture/propagate the first rollback or transaction-state failure using the same first-failure accumulator without skipping remaining connections or schema drop; handle Decision: MERGE NO-GO on exact HEAD |
…ailure cleanup honest (Ref #76, #37, F77-01) tearDown no longer swallows inTransaction()/rollBack() failures (a false rollBack() counts too). All failures are collected after every cleanup step was attempted: one is rethrown as is, several are reported with the first as primary (a failed PartnerProcess::stop() stays first). Docblock now promises attempts, not guaranteed release. Adds two regression tests: a real rollback failure (killed server session) and a failed partner stop with the database cleanup still performed. Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
|
F77-01 addressed in a new commit (no amend/force-push).
|
Technical Lead — FINAL Fresh Full Acceptance Re-Review #2: MERGE GO — exact cumulative TreeIndependently verified identity & scope
Prior finding F77-01 — CLOSED / PASS
Exact-head GitHub CI — 10/10 COMPLETED SUCCESSRun #39978250208, attempt 1, exact HEAD
Decision & remaining governanceFINAL MERGE GO for the exact HEAD Only Owner may authorize Squash Merge into main; Lead has not merged, enabled auto-merge or tagged anything. After Owner merges the exact reviewed HEAD, reconcile squash commit and merged Git Tree with |
Closes #76. Ref #37 (the legacy release remains NO-GO; no tag/release authorized by this PR).
Exact Git identity / scope
main@9b06ec5d0847fb84287b14d000192b78e2db12d8, Treed3446e86fd60fa62dbe407bd7db84445d39551a0.7ea5499325bba8396634d93318ee9a4a1a7b4f84, Treec1e25e7653a3173c3339a269b1ad23b26f335b9d.43f5450(reuse bounded partner) →7ea5499(F77-01 fail-closed rollback cleanup); no amend/force-push.tests/Integration/Pdo/Ordering/ScopedOrderingManagerLazySavepointTest.php,tests/Integration/Support/PartnerProcessIntegrationTest.php. No changes tosrc/, published PDO/retry/transaction/savepoint APIs, existingPartnerProcess, Composer, CI, standards or security policy.Full cumulative change
PartnerProcess::start/readLine(30.0)/writeLine('go')/stop()to replace the original privateproc_open+ blockingfgetsthat disregarded the declared stream timeout. Preserves the 15-second blocked-locking-read wait, genuine MySQL InnoDB deadlocks, C10/C15 SQLSTATE 40001 / driver 1213 assertions, and original transaction/savepoint counters.testFrozenLazySavepointPartnerFailsWithinTheDeadlineAndIsReapedfreezes before actual partnerexec, verifies a bounded read timeout and process cleanup.tearDown()attempts all partner stops, everyinTransaction()/rollback, and schema drop even when an earlier step fails. It records thrown exceptions and a false rollback return. Exactly one cleanup failure is rethrown unchanged; multiple failures are reported with the earliest reason first and original Throwable atgetPrevious(). This is not a guarantee that an OS-level child surviving SIGKILL is released.finally.RED/GREEN evidence (executor reports; Lead review evaluates independently)
stream_set_timeout(30)+fgetsdid not return under a real pre-exec frozen partner for 45 seconds; external watchdog freed it at 47.0 seconds (macOS; MySQL 8.4.10 in Docker).tearDownswallowed rollback exception and returned success; GREEN new path produces originalPDOException.ScopedOrderingManagerLazySavepointTest: original 23 tests unchanged, now 25 tests including the two F77-01 regression tests, three repeated full class runs GREEN. All local gates on PHP 8.4.4 with real MySQL 8.4.10:composer validate --strict, static analysis, syntax, whitespace, unit 383, regression 97, consumer, integration 231 tests twice, DB residue check and php-cs-fixer dry run all passed.Exact HEAD GitHub CI
7ea5499325bba8396634d93318ee9a4a1a7b4f84, 10/10 completed SUCCESS: Workflow lint, Quality PHP 8.2, Unit/Regression PHP 8.2–8.5, Lowest Dependencies PHP 8.2/MySQL 8.4.10, Integration PHP 8.2/8.5 on real MySQL 8.4.10, final CI Gate. Clean dependency advisory audit and PHPStan; Lowest 383 Unit + 97 Regression + 231 Integration twice, residue clean; both Integration jobs Consumer Verification Harness twice + 231 Integration twice, residue clean.Limits and governance
2 vs 4retry flake from test(reliability): investigate intermittent four-deadlock exhaustion under high host load #68 remains UNVERIFIED as to cause; this test harness fix does not modify production retries.maatify/php-persistenceextraction, Composer identity migration, tag or release in this PR.🤖 Generated with Claude Code