Skip to content

Avoid rewriting unchanged retained waits - #941

Merged
rmcdaniel merged 2 commits into
mainfrom
perf/899-retained-wait-writes-20261026
Oct 26, 2026
Merged

rmcdaniel merged 2 commits into
mainfrom
perf/899-retained-wait-writes-20261026

Conversation

@rmcdaniel

@rmcdaniel rmcdaniel commented Oct 26, 2026 •

Copy link
Copy Markdown
Member

Retained wait rows are rewritten as a workflow progresses because status-ranked positions shift, MySQL reorders JSON object keys, and PostgreSQL can trim timestamp fractions. This makes a long workflow update rows whose logical state has not changed.

Keep stored positions chronological and apply canonical status priority when reading waits. Compare standard JSON payloads and UTC timestamps by their typed values while preserving list order, scalar/null distinctions, model hooks and configured casts. The shared unique-key recovery helper is unchanged; there is no schema or protocol change.

The same real 65-signal workload completes on published Workflow 2.14.0 and this candidate on SQLite, MySQL and PostgreSQL. At 64 retained waits, wait-table UPDATEs fall from 128 (SQLite) / 256 (MySQL and PostgreSQL) to 2, and timestamp-only changes from 63 to 0. Both versions issue 9 SELECTs and 1 INSERT at that checkpoint. Bounded measurements and limits.

Validation at exact head c608431d4250b731b6f261fca540d59028006712:

  • Whole-repository ECS passes. Static analysis passed before the last pure cast/null test; exact-head hosted quality now passes both checks.
  • Focused storage/cast/rollback/competing-insert regressions passed 21 tests / 247 assertions on each database before that final pure cast/null test was added.
  • Committed real 17-signal regression asserts constant UPDATEs, unchanged resolved rows, canonical ordering and completed output. Legacy positions, timestamp ties, configured storage, missing rows and hooks are covered.
  • Full exact-head qualification run passed: 22 successful jobs and 3 trigger-inapplicable skips. This includes every database feature shard, all five Laravel upgrade lanes, complete unit/coverage and the coverage ratchet. Hosted unit totals: 4,997 tests / 71,383 assertions, 15 notices and 43 skips. The protected PR run also passed. A separate slower local disk-backed unit run is still active with errors under diagnosis; it is not counted as passing. The suspected tracing controls pass in isolation (3 tests / 323 assertions).

The consumer now normally installs published 2.14.1 at af9dbadb; the three affected source files are byte-identical to the 2.14.0 baseline. Native open-wait reads remain compatible. Native currently ranks only open waits; resolved native-written rows can retain stale ordinals. Server #783 under #325 will align all retained ordinals and avoid unchanged position writes before mixed native-to-PHP retained-order handoff. The native owner confirmed no PHP source or wire/fixture change is needed in #941.

Refs #899. Keep this issue open through source review, compatible publication and ordinary installed-package acceptance. The next compatible patch is coordinated with the package owner's HTTP/protocol correction; no tag has been selected here.

@sonarqubecloud

Copy link
Copy Markdown

@codecov

codecov Bot commented Oct 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.82%. Comparing base (af9dbad) to head (c608431).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #941   +/-   ##
=======================================
  Coverage   93.82%   93.82%           
=======================================
  Files         578      578           
  Lines       79683    79725   +42     
=======================================
+ Hits        74760    74803   +43     
+ Misses       4923     4922    -1     
Flag Coverage Δ
v2 93.82% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@rmcdaniel
rmcdaniel marked this pull request as ready for review October 26, 2026 13:39

@rmcdaniel rmcdaniel left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

General ordinary source review at exact head c608431 / tree 44112954d677f6cbde7a4d3cb88492f22a4118b8: no blocking source or compatibility concern found. Reviewed the three product files plus the focused regression surface. Stored positions are now status-independent chronological ordinals using the same millisecond/tie identity as the canonical wait views; preserves the prior open/resolved/cancelled presentation for both legacy and reprojected rows. Payload equivalence remains strict for list/scalar/null/type changes while tolerating JSON object-key normalization, and timestamp equivalence compares actual instants only under the configured cast. The existing idempotent upsert, model-hook, configured-connection, rollback, missing-row, and independent-insert behavior remains covered. Native parity reports no wire/fixture change. This review is source-only and exact-head-bound; merge/release still waits for the owner's terminal full qualification and any head change requires re-review.

@rmcdaniel rmcdaniel left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

General ordinary source review at exact head c608431 / tree 44112954d677f6cbde7a4d3cb88492f22a4118b8: no blocking source or compatibility concern found.

Reviewed the three product files plus the focused regression surface. Stored positions are now status-independent chronological ordinals using the same millisecond/tie identity as the canonical wait views; inDisplayOrder() preserves the prior open/resolved/cancelled presentation for both legacy and reprojected rows. Payload equivalence remains strict for list/scalar/null/type changes while tolerating JSON object-key normalization, and timestamp equivalence compares actual instants only under the configured cast. The existing idempotent upsert, model-hook, configured-connection, rollback, missing-row, and independent-insert behavior remains covered. Native parity reports no wire/fixture change.

This review is source-only and exact-head-bound; merge/release still waits for the owner's terminal full qualification, and any head change requires re-review.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Exact source qualification is terminal on c608431d4250b731b6f261fca540d59028006712, tree 44112954d677f6cbde7a4d3cb88492f22a4118b8:

  • Full build 40329949900 succeeded at 13:37:53 UTC: 22 successful jobs, 3 trigger-inapplicable skips, no failures. It covers every MySQL/PostgreSQL feature shard, MariaDB, all five supported Laravel upgrade lanes, ECS/PHPStan, full unit coverage and the coverage ratchet.
  • The unit lane completed 4,997 tests / 71,383 assertions with 15 notices and 43 skips. Unit log.
  • Protected PR build 40329807872 succeeded at the same exact head. Public Boundary, Sonar and CodeQL also pass.
  • The product-source blobs remain those measured in the three-database 65-signal workload; the successor adds regression coverage and formatting only. Write counts and limits.

A separate local disk-backed full unit run is still active and has errors under diagnosis. It is not counted as passing; suspected tracing controls pass separately in a RAM-backed fixture (3 tests / 323 assertions). The complete hosted suite above passed independently.

PR #941 is ready for source review. The package owner has acknowledged one compatible patch release with the separate #942 HTTP/protocol correction. Publication and ordinary installed-package acceptance remain required before #899 closes. Published Waterline 2.13.0 is installed in the consumer for its bounded wait-resource verification. Native compatibility and its separate write-churn follow-up are recorded in the parity receipt.

@rmcdaniel

Copy link
Copy Markdown
Member Author

The source candidate now also passes the ordinary client resource check with unchanged, published Waterline 2.13.0 / 30ec59eac270dafd3025afd65175670c6a0f7ab6, Laravel 13.38.0 and PHP 8.3.36:

  • Each database completes the real 65-signal workflow with 1 test / 96 assertions.
  • At signals 1, 16, 64 and completion, Waterline’s bounded V2RunObservationResource preserves canonical open-wait IDs/order and returned count, and performs zero INSERT/UPDATE/DELETE statements while reading.
  • The published bounded contract remains explicit: a running view is partial with an unknown total; the completed view is available with total 0. The test does not relabel bounded evidence as complete.
  • The 64-wait write count remains 9 SELECT / 2 UPDATE / 1 INSERT, 2 net changed rows including the new wait, and 0 timestamp-only retained-row changes, on SQLite, MySQL and PostgreSQL.

These runs deliberately override Workflow’s autoload path to the unreleased c608431d candidate and verify the source fingerprints. They are source qualification, not published-package acceptance. The final consumer will install the released Workflow package normally and repeat this check without that override.

Sonar finding AaF4T9VFVHLR8MI8gtFh is a style rule on WorkflowRunWait::originalIsEquivalent: 5 returns versus its preferred maximum of 3. I reviewed it and retained the explicit guards for Eloquent’s base equivalence, missing originals, the standard payload cast, UTC timestamp casts, and the default refusal to coerce custom casts. No rule, threshold or required check was changed; the quality gate passes. The source reviewer can reassess this readability decision.

Source qualification and publication remain separate. #899 stays open through the coordinated compatible patch and its ordinary installed consumer verification.

@rmcdaniel

Copy link
Copy Markdown
Member Author

The three tracing groups corresponding to the local error positions now pass together with RAM-backed fixture storage and PCOV enabled: 16 tests / 2,161 assertions, PHP 8.3.36, exact unchanged c608431d source. They are AdmissionExecutionTracingTest, ChildRetryExecutionTracingTest and DispatchExecutionTracingTest.

The original slower disk-backed full run remains active so its final error diagnostics can be retained; it is still not counted as passing. Complete hosted qualification remains 22 successful jobs, including 4,997 unit tests / 71,383 assertions and the coverage ratchet. No source change or gate relaxation was needed for these controls.

@rmcdaniel
rmcdaniel merged commit 847b416 into main Oct 26, 2026
50 checks passed
@rmcdaniel
rmcdaniel deleted the perf/899-retained-wait-writes-20261026 branch October 26, 2026 14:03
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.

1 participant