Repository navigation
Rust Server train: MySQL statement closes (#714), policy-map JSON, worker history presentation - #729
Conversation
…s, like Server 2.11.1 Worker polls and history pages add workflow_sequence to a consumed signal's SignalReceived from its record, as WorkflowTaskPoller does; the run history route still shows the event as recorded. Task snapshots keep compatibility, the dispatch and claim stamps and errors when they are set, as WorkflowHistoryEvent::taskSnapshot does.
The six admission, durable-limit and external-payload policy maps are read like PHP's EnvAuditor::jsonObject: unset, or raw text that decodes to a JSON object. Malformed JSON, null, lists and scalars now stop startup instead of silently becoming an empty policy. A set DW_ name wins over its legacy name even when invalid, and the refusal names the variable without its value. The external payload quota reads its overrides with the same decoder.
COM_STMT_CLOSE has no reply. The published sqlx-mysql flushed it on its own write after a cache eviction or an uncached execute, and flushed it again before draining results. The next command then waited behind Nagle's algorithm for the server's delayed acknowledgement, about 40 ms. Across the 30-48 statements of a transition that held the global lock for 0.3-1 s under load, and a due timer behind it fired more than the 1 s bound late (#714). The vendored driver now queues each close with sequence 0 and leaves a buffer of replyless closes unflushed until the next command, so both go out in one write. A proxy-recorded qualification test fails with the published driver and passes with the change on MySQL 8.0 and MariaDB.
Security's review of f31c4c3: Connection::flush delegated to wait_until_ready, which now leaves a buffer of queued closes unsent, and clear_cached_statements only queued its closes. Both report success with the bytes still buffered. Explicit flushes and cache clears now send every buffered byte before returning (MySqlStream::flush_queued); only the path before the next command keeps closes to share that command's write. New MySQL and MariaDB controls prove both calls without a later command; they fail on f31c4c3. The close test now describes what it observes: TCP read chunks at a proxy, not write calls.
Workflow 2.10.2's applySignal takes a workflow sequence only from the signal's own wait, so a declared signal applied while a condition wait is open keeps no sequence; only an undeclared signal takes the condition wait's. Native recorded the condition wait's sequence for both, which became visible once worker polls showed the record: the published-SDK signal and query cohorts require worker history to equal the run's history prefix (all nine SDK pairings on every database). A new test covers the declared case and fails without the change.
…orkflows A newer push to the same pull request now cancels the run in progress for Shared Server fixtures (13 jobs, about 415 job-minutes) and PHPUnit Feature, as Sonar already does. The organization's 20 shared runners no longer spend hours on replaced heads. Runs on main and manual dispatches always finish.
…rust-325-train-oct24-core
…325-train-oct24-core
…nto rust-325-train-oct24-core
…nto rust-325-train-oct24-core
…ase probes The published PHP SQLite reference shares its file with a one-second maintenance loop. #723's run 40069368271 recorded the Prometheus fixture's activity claim as 503 (then 422 for the completions that had no task). An activity poll under lock pressure answers the documented 503 backend_lock_pressure refusal, which hands out no task and tells the worker to retry. The system-metrics and operator-dashboard probes already retry exactly that refusal through lock-pressure-middleware.php. The Prometheus and activity lease renewal probes build their own Guzzle clients for raw worker polls and did not. They now push the same middleware. Expected results are unchanged: the claim must still end at 200, and every other status or body still reaches the contract. ServerParityRunnerTest now requires the middleware in every corpus probe that polls over its own HTTP client; it fails against the previous Prometheus probe.
Train #723's MySQL 8.0 pressure profile failed 'one real SDK budget expires while pressure stays held' on the published PHP target, and the log could not say whether shutdown took under 8 s or over 10 s. The assertion now prints the measured time and backend. The window itself is unchanged.
R2 handoff for Security: exact head
|
|
Security R2 review finding on exact head
This contradicts the repository's own startup invariant at Required counterfactual: use a fallible The remaining R2 review continues; no other finding is being asserted by this comment. |
|
Security R2 review finding 2 on exact head
The PHP reference deliberately collects the unique Required counterfactual: derive unique non-empty signal IDs from the current page (at most the page limit), fetch only those IDs for this run, and add focused coverage proving a small page's hydration work/result is independent of many unrelated consumed signals in the same run. Preserve both poll and continuation-page behavior and the no-cross-run fence. Review of the remaining surfaces continues; findings 1 and 2 are the current blockers. |
|
Security R2 source review is complete for exact head
No additional finding emerged from the remaining changed range. I reviewed all 20 changed files, including MySQL packet ordering and lifecycle behavior (next-command batching, explicit flush, cache clear, graceful close and deferred rollback interaction), signal sequence/run fencing, task-snapshot parity, PR-only workflow cancellation, and the narrowly bounded lock-pressure retry middleware. The existing exact-head terminal evidence is credible for those unaffected surfaces: shared run 40076806125 completed all 13 jobs successfully, and the listed source/policy/CodeQL/Sonar checks are terminal green. Please correct both blockers on the owner branch, add the requested counterfactuals, rerun the affected native/source gates plus required hosted checks, and re-hand off the new full SHA. Security will review only the correction delta and its evidence unless the changed range expands. |
…nals Security's R2 review of #729 found two defects. The namespace quota map was read with a lossy decode, so a value that is not UTF-8 could become a different namespace key and drop the intended cap. Both the DW and legacy names now go through a fallible read: the DW name wins when set, the legacy name is read only in its absence, and a non-UTF-8 value under either name is a configuration error that never echoes the value. Worker history signal hydration fetched every sequenced signal record of the run on each page. It now reads only the page's distinct non-empty SignalReceived signal IDs, each fenced by workflow_run_id, as PHP's whereIn('id', $signalIds) does, so the work is bounded by the page.
Correction for Security's two R2 blockers: commit
|
|
|
Security qualification update for exact head
Please retain and classify the complete failed-step log after the run becomes terminal. Do not treat the six green deadline-health cells or the independent later probes as acceptance of this failed PostgreSQL 17 parity cell. Security will review the correction delta and terminal replacement evidence after the owner re-handoff; no source review or restart has been performed early. |
PostgreSQL 17 job 120852521096: classified from the full job logThis classifies the failure Security recorded in 6383536438, using the full job log. I fetched it locally; Where it failed. Step 9 ( Cause: a test defect, outside this change.
Fix. The fix compares parsed instants and prints both values on failure. It is committed for the next train ( Next. When run 40086201733 finishes, I'll re-run only the failed job on this same head and re-hand off with this classification. |
R2 re-handoff for Security: head
|
|
Security R2 acceptance — exact head I reviewed the complete correction delta from the previously reviewed head
The current-main merge adds only the four PHP files from #730, whose exact source Security already accepted in 6383002870; it changes no Rust, shared-fixture, or workflow source. The seven-path combined delta is clean and I directly verified the PR remains CLEAN/mergeable at this exact SHA and that every required hosted check is terminal green: all 13 Shared Server fixture jobs, the PHP/source/contracts/policy/public-boundary checks, both Sonar checks, and all CodeQL analyzers. The prepared-local candidate job is intentionally skipped. The original PostgreSQL 17 attempt remains durably classified as the unrelated timestamp-string assertion defect; its same-head job rerun and the companion existing-PHP matrix job passed. I am not treating the rerun as erasing or diagnosing that first failure, and the fix remains outside this head in #735. Owner-reported full native qualification is also consistent with the unchanged Rust correction commit: PostgreSQL and MySQL each passed lib 256 plus execution 454 (one ignored), SQLite passed lib 255 plus execution 454 before the added SQLite-only unit, and both new unit tests pass at the correction head. I did not duplicate those full builds locally because this isolated review checkout has no task-owned Cargo cache and the host is already at 87% disk use; the independent evidence here is exact source/tree review, call-site/bound verification, diff checks, and direct hosted-check verification. Decision: accepted for this exact R2 head and manual merge. Any head change requires re-review. This acceptance is source-train only; it does not claim a published release, installed acceptance, or completion of the separate fixed-image/native-parity follow-through. |
heartbeat_renews_only_progress_deadline_and_cannot_resurrect_elapsed_authority compared two lease_expires_at strings. Native times are whole milliseconds and serialize without a fraction on a whole second, so a claim lease of ...:26Z sorted after a renewal of ...:26.045Z and the assertion failed about one run in a thousand (train #729, PostgreSQL 17 job 120852521096). The test now parses both and reports the values.
|
Post-merge Security verification: #729 merged at 13:22:56 UTC as |
Every database job waited for published-sdk-adapters (about 15 minutes), but 10 of those minutes built the optimized native binary only the codec-deadline-health cells use (train #729: "Build one bounded optimized native artifact for deadline health", 10.0 minutes). The build, its upload and its container cleanup now run in a codec-native-build job beside the adapters; codec-deadline-health needs both, and the database jobs still need only the adapters, so they start about 10 minutes sooner. The build script and its bounded resources are unchanged.
Part of #325. This replaces train #723 without #712.
Security's pre-terminal review of #723 (comment 6381682935) found that #712's multiplexed update-validation poll advertises poll-request-id replay and changed-set conflicts that native does not implement, and that its kind selection, claim and cursor turn are not one transition. #712 leaves this train and gets that correction as its own train. The four constituents Security found source-acceptable land here.
Constituents
4d28b1e8cc800c792e64f5ec43631467e575Train commits
49a5099ba: PHPUnit Feature keeps main's definition, with no concurrency group. Its required gate runs withalways()under the central action policy, and GitHub never cancels analways()job, so cancel-in-progress would only queue runs behind it. Rust Server train: MySQL statement closes (#714), update validation, policy-map JSON, worker history presentation #723 made the same decision at1fd9e549a.9eb3bb838(= Rust Server train: MySQL statement closes (#714), update validation, policy-map JSON, worker history presentation #723's9d2ab685f): the Prometheus and activity lease renewal probes retry the documented503 backend_lock_pressurepoll refusal, like their sibling probes. Classification: Rust Server train: MySQL statement closes (#714), update validation, policy-map JSON, worker history presentation #723 comment 6381159759.71c0eff78(= Rust Server train: MySQL statement closes (#714), update validation, policy-map JSON, worker history presentation #723'sef5a6be70): the SDK pressure assertion prints the measured shutdown time (worker-sdk-persistent-pressure: published PHP's SDK shutdown missed the 8-10 s window on MySQL 8.0 #728).Left out with #712
The update-validation fixture and native route, and #723's harness commits that exist only for them: the 90-second probe limit, the waiting-request lock-pressure retry, the SQLite update-validation split with its maintenance-window helper, and the Sonar shell fixes in those scripts.
Evidence
codec-deadline-healthcells passed, so the MySQL-family stall Rust #325: send MySQL statement closes with the next command (#714) #717 removes did not appear. The two failures were classified: the Prometheus probe retry gap, fixed here, and worker-sdk-persistent-pressure: published PHP's SDK shutdown missed the 8-10 s window on MySQL 8.0 #728, outside this train.Security class: R2 (#717 changes the vendored MySQL client's statement lifecycle, #724 changes workflow concurrency).