merge: compose T748 bounded execution with T743 public callers
This commit is contained in:
@@ -0,0 +1,166 @@
|
||||
# T-748: default-parallel suite failure diagnosis
|
||||
|
||||
## Sources and preserved observations
|
||||
|
||||
- Original local candidate: `61533fadb6b39465af60cb55aed2b4d0650319cd`.
|
||||
- Comparison source: `56795fb92cb9def9bcd701bd51d92e99526f8eff` (develop/T742).
|
||||
- Same implementation branch: `work/T-748-bounded-db-execution`.
|
||||
- The original `target/t748-server-tests.log` remains untouched. Its 97 FAILED
|
||||
lines and 600-second timeout were symptoms, not proof of one cause.
|
||||
- Initial diagnosis used `cargo test -p yoi-workspace-server -- --nocapture`,
|
||||
default libtest parallelism, without changing queue limits, features, or startup
|
||||
settings. `target/t748-diagnostic-environment.txt` records the environment;
|
||||
this host reports 24 available CPUs. Diagnostic captures were bounded to
|
||||
90/150 seconds to obtain errors before the original timeout, not counted as
|
||||
successful full-suite runs.
|
||||
|
||||
Logs under `target/` are local diagnostic artifacts, not tracked review inputs.
|
||||
This report records their meaning and the permanent regressions that reproduce it.
|
||||
|
||||
## 1. Different fixtures consumed one global Host admission budget
|
||||
|
||||
The first non-intentional errors in `target/t748-server-nocapture.log` were
|
||||
`DbCapacityExceeded` in manual Worker setup (`server.rs` at the then-current
|
||||
line 50614), followed by assignment fixture construction (52990/53045) and
|
||||
HTTP 503 responses. The deliberate before/after-commit executor panic tests
|
||||
are different: those passed and verified unknown-outcome diagnostics.
|
||||
|
||||
Temporary submit/start/cleanup instrumentation recorded submitting test names,
|
||||
closure types, actual job IDs and the execution-owner identity. It showed a
|
||||
single Host lane with `max=18`, with accepted jobs from unrelated fixtures.
|
||||
A rejection snapshot contained `{32,45,46,...,61}`. The complete diagnostic
|
||||
capture later contained cleanup for every submitted Host job on that owner;
|
||||
it did not show permanently abandoned accepted Host jobs. Instrumentation was
|
||||
removed before the source was committed/published.
|
||||
|
||||
`target/t748-live-process-18.txt` sampled the test process and threads at the
|
||||
actual rejection. That sample had 126 threads; the Host workers were runnable,
|
||||
and the process was using approximately 17 CPU cores. There was no captured
|
||||
thread-creation EAGAIN/unavailable error. This evidence does not establish an
|
||||
OS thread/CPU hard limit as the cause. No test processes remained after the
|
||||
bounded diagnostic run or the completed suite.
|
||||
|
||||
A representative failure passed alone (0.56 seconds); the eight logging cases
|
||||
passed with four test threads (2.46 seconds). The isolated comparison source's
|
||||
full default-parallel suite passed: 931 library tests, one pre-existing ignored
|
||||
migration fixture, 11 binary tests; library duration 168.74 seconds. This did
|
||||
not prove that all new failures were capacity-related, but did distinguish the
|
||||
new shared-admission rejection from an observed baseline failure.
|
||||
|
||||
### Ownership fix (`f3d4aba5`)
|
||||
|
||||
Production budgets remain shared and finite: DB 1 worker/32 queued, Host 2/16,
|
||||
cleanup 2/16. Ordinary fixtures now explicitly own a finite DB executor and lazy
|
||||
Host/cleanup owners at those same limits. API clones, child Workspace APIs, and
|
||||
weak retention adapters use their injected owner. No global fixture lane cache,
|
||||
parallelism reduction, unlimited queue, or capacity-wait retry was added.
|
||||
|
||||
The permanent saturated-Host regression fills both running slots and all 16
|
||||
queued slots, cancels waiters, advances paused Tokio time, and still observes
|
||||
rejection until actual completion. Another fixture and the separate cleanup lane
|
||||
can progress; draining restores the original budget and completes exactly 18
|
||||
accepted jobs. Existing real-SQLite cancellation/lease/drain tests remain active.
|
||||
|
||||
## 2. SQL offload invalidated fixture assumptions about first poll and publication
|
||||
|
||||
After Host isolation the default-parallel suite finished in 171.57 seconds:
|
||||
954 passed, three failed, one existing ignored. Actual errors were:
|
||||
|
||||
- removal subscriber saw a legitimate queued Runtime stop Upsert before Removed;
|
||||
- external session-open expiry exceeded its one-second bound while its DB lane
|
||||
was still shared across other fixtures;
|
||||
- grant replacement returned success because one initial poll had only submitted
|
||||
the offloaded authority read, not reached the held original-grant lock.
|
||||
|
||||
The ordinary DB fixtures were isolated too. The one-second expiry assertion was
|
||||
not increased. Grant testing now drains its actual authority read then polls the
|
||||
saved response to reach the held lock before replacing the grant. Removal testing
|
||||
observes the committed stopped projection before taking the removal baseline.
|
||||
The Removed payload assertion is unchanged. These are observable synchronization,
|
||||
not sleeps or retries. Focused expiry, grant, and removal tests all passed.
|
||||
|
||||
A captured cleanup job also attempted to poll a Tokio timer after its fixture
|
||||
runtime had shut down. The terminal checker fixture now retains that original
|
||||
runtime through accepted Host and cleanup jobs, then stops projection producers
|
||||
and drains actual DB work before ending. Terminal SQL state is not treated as
|
||||
actual cleanup completion. The subsequent full run contained only the three
|
||||
intentional executor panic diagnostics, not the shutdown-context panic.
|
||||
|
||||
The synchronization/teardown changes are in `df9d056d`.
|
||||
|
||||
## 3. Restore intent's DEFERRED read-to-write upgrade returned SQLite BUSY
|
||||
|
||||
The next default-parallel run finished in 171.48 seconds with 956 passed, two
|
||||
failed, one existing ignored. Both failures were SQLite `DatabaseBusy`, extended
|
||||
code 5, not capacity rejection:
|
||||
|
||||
- production-profile zero-input Orchestrator restore;
|
||||
- pending manual Worker claim restore reconciliation (500 instead of 409).
|
||||
|
||||
Both failed alone too. `target/t748-busy-origin.log` captured the error and a
|
||||
backtrace through `pin_internal_worker_restore_intent` -> the transaction helper
|
||||
-> internal restore -> manual claim. The journal read authority first in a
|
||||
DEFERRED transaction while the offloaded projection's other SQLite connection
|
||||
could write. Such an upgrade can return BUSY without invoking SQLite's busy
|
||||
handler; treating this as a retryable whole mutation would be incorrect.
|
||||
|
||||
A new real-SQLite regression holds an IMMEDIATE writer on another connection,
|
||||
registers an owned per-connection busy observer, and attempts one journal pin.
|
||||
The old helper fails deterministically with BUSY/5 before waiting for ownership
|
||||
(`target/t748-deferred-upgrade-red.log`, 0.23 seconds). The fixed helper waits
|
||||
before its first authority read, while a current-thread timer and control task
|
||||
progress. After releasing the writer it persists exactly one requested tuple;
|
||||
there is no application-level mutation retry.
|
||||
|
||||
`f597f1b5` makes this existing mutation helper explicitly
|
||||
`with_immediate_transaction`. Restore journal and deletion authority reads,
|
||||
mutation and commit remain in one transaction. Focused DB/HTTP tests (8), the
|
||||
original restore and claim cases, and deletion tests (13) passed.
|
||||
|
||||
## 4. Why a pre-dispatch failure could become the 600-second suite timeout
|
||||
|
||||
The original long-running logging case joined an HTTP request with a fake provider
|
||||
waiting for command dispatch. On pre-dispatch rejection the HTTP future can finish
|
||||
with 503 while the receiver remains Pending: its channel is still owned by the
|
||||
fixture, but no command will arrive. An unbounded `join!` waits forever in that
|
||||
state and hides the actual error behind the outer suite timeout.
|
||||
|
||||
The permanent `saturated_host_before_command_start_returns_availability_without_a_provider_operation`
|
||||
regression uses the real signed HTTP route with a controlled full Host budget. It
|
||||
observes the final 503/`db_capacity_exceeded` response and the still-live Pending
|
||||
provider receiver, with no process or command dispatch. The original logging
|
||||
fixture now asserts an early HTTP response before waiting for both branches, so
|
||||
an unexpected rejection reports its body immediately rather than hanging. This
|
||||
reproduces the causal timeout mechanism; the old log itself has no stack trace
|
||||
from the hung provider future.
|
||||
|
||||
The nine logging cases pass together at default parallelism (`49bfbfb2`).
|
||||
|
||||
## Artifact/source isolation during comparison
|
||||
|
||||
Initially the archived baseline used the existing Cargo target directory. That
|
||||
replaced a same-package artifact/fingerprint; one later post-fix representative
|
||||
command ran the baseline's 932-test binary (and `db_executor --list` had no tests).
|
||||
That result was rejected as candidate evidence. All later candidate checks/tests
|
||||
use `--target-dir target/t748-candidate-build`, separate from the baseline cache.
|
||||
This changes artifact location, not test parallelism, features or admission policy.
|
||||
The fresh candidate build listed the expected executor tests and total test count.
|
||||
|
||||
## Validation and remaining composition boundary
|
||||
|
||||
Final code source `49bfbfb2` passed the full default-parallel Server suite,
|
||||
including the command-overload case: 960 passed, zero failures, one existing
|
||||
ignored migration fixture; 11 binary tests, library duration 175.29 seconds.
|
||||
The only captured panics were the three deliberate unknown-outcome tests.
|
||||
Root `cargo check` compile closure passed (including TUI/Server), as did Workdir
|
||||
default (153 tests) and `http-client` (156 tests) full suites, format/diff checks
|
||||
and state-contract names (558 matches, zero unapproved, zero stale exceptions).
|
||||
These commands used the isolated candidate artifact directory; libtest default
|
||||
parallelism and the production budgets were unchanged. The MR records the exact
|
||||
published source, which also includes this report and the boundary document.
|
||||
|
||||
No latency benchmark, production incident measurement, target integration,
|
||||
Ticket conclusion, or running-environment update is claimed. T743 has now
|
||||
published its caller inventory/source; its owner will compose the T748 facade and
|
||||
new callers, then obtain review of that exact combined source. T748 does not copy
|
||||
those domain APIs or wait on the former unpublished branch.
|
||||
@@ -2,6 +2,12 @@
|
||||
|
||||
## Source identity and composition
|
||||
|
||||
This is the retained pre-composition caller inventory published with T-743
|
||||
`9b0d42fdbc2920736c3418bb894f26140e962bbc`. Its descriptions of direct callers
|
||||
below identify the work to move, not the current execution placement. The later
|
||||
normal merge, actual DB/Host placement, ten new regressions, full-suite evidence
|
||||
and unresolved upstream review findings are in `T-743-T748-composition.md`.
|
||||
|
||||
This inventory belongs to the T-743 commit carrying this file on `work/T-743-ticket-public-surfaces` (T-742 base `56795fb92cb9def9bcd701bd51d92e99526f8eff`). Its exact published SHA and MR ID are recorded in the Ticket publication handoff. It is not a copy of the unpublished T-748 adapter and is not evidence that async DB execution has been integrated or reviewed.
|
||||
|
||||
Normal-merge the validated public T-748 source into this existing work branch. Keep the MR's source/target selectors. Recheck the concrete new call graph below against the merged implementation; no placeholder executor, second connection authority, transaction split, fresh integration branch or force push is needed.
|
||||
|
||||
@@ -0,0 +1,130 @@
|
||||
# T-743 / T-748 composition: execution-boundary evidence
|
||||
|
||||
## Source and status
|
||||
|
||||
T-743 continues on `work/T-743-ticket-public-surfaces`, MR
|
||||
`01a12d19-b61b-79a2-baca-89241fd3c3d3` → `develop`. The published T-748
|
||||
source `66bb9d0b825d72e20923003b273b787007f3aed8` was normally fetched and
|
||||
merged into that same source branch, without conflicts, replacement selectors,
|
||||
force push, or a copied execution facade. The composition commit carrying this
|
||||
report also connects the added callers below. Its exact published hash is in the
|
||||
Ticket handoff and provider-resolved MR source.
|
||||
|
||||
**This is a validated intermediate composition, not final independent approval.**
|
||||
T-748 MR `01a12d3f-e90c-7b02-9ed9-cf3bd2e822a1` seq3 requests changes on this
|
||||
upstream source. Its two major findings are not fixed by the new assignment
|
||||
caller work: `T748-DB-NETWORK-LANE` (Subjektiv mixed Runtime/DB placement) and
|
||||
`T748-ASYNC-PROTOCOL-SIGNING` (cold protocol observation source signing).
|
||||
That authoritative verdict was checked again after the combined full suite;
|
||||
no newer published upstream source or verdict was present. The T-748 owner was
|
||||
informed through its Ticket of the composition and requested to provide the
|
||||
normally advanced validated fix source. Do not interpret a green suite as proof
|
||||
that those untested boundaries are safe. Normal-merge that fix into this same
|
||||
T-743 source, revalidate, and independently review the exact combined source.
|
||||
|
||||
## Caller placement
|
||||
|
||||
- Canonical and Browser assignment pages and sanitized diagnostics share one
|
||||
implementation. Workspace scope, Ticket reference resolution and each whole
|
||||
synchronous Store read run on the shared bounded DB owner. None is left as a
|
||||
synchronous pre-call on a Tokio request thread.
|
||||
- Typed Ticket query, including principal/handling/current-closure filtering
|
||||
before pagination, retains T-748's whole authority/read/enrichment DB job.
|
||||
The regression holds the **authority's actual candidate connection**, not the
|
||||
separate API Store connection used by only some enrichment paths. This also
|
||||
proves the empty-result path and wrong-principal filters offload correctly.
|
||||
- Account assignment answers derive attribution from Backend-authenticated
|
||||
context inside the owned DB closure. Runtime answers preserve the live current
|
||||
Orchestrator discovery check on the finite Host lane, because Runtime worker
|
||||
lookup can perform remote HTTP. The DB closure derives Runtime attribution
|
||||
without I/O and the unchanged answer helper validates current singleton and
|
||||
live binding **again under its IMMEDIATE write transaction**, including replay.
|
||||
Target principal and request-body prose never supply author authority.
|
||||
- The same answer helper keeps source authority, actionable-assignment gate,
|
||||
assignment/event CAS, full operation fingerprint/replay, body, audit, receipt
|
||||
and commit together. There is no await within that transaction and no automatic
|
||||
mutation retry.
|
||||
- The accepted SQL job admits the existing finite Host attention continuation
|
||||
**after commit and before returning its result**. It inherits the request
|
||||
lifecycle source, owns lookup/Runtime dispatch independently of the waiter and
|
||||
does not wait on Runtime/network on the sole DB worker. A live HTTP waiter
|
||||
observes best-effort notification completion; losing the waiter does not
|
||||
cancel accepted SQL or accepted publication. Rejected Host publication retains
|
||||
the existing saved-input scan/recovery and diagnostic behavior, not rollback
|
||||
or a synthetic successful delivery.
|
||||
- Normal Tools/WIP still converge on the canonical HTTP contracts. No second
|
||||
ledger, query adapter, source authority, read pool or delivery implementation
|
||||
was added. Existing T-748 set/clear/start/delivery boundaries remain intact.
|
||||
|
||||
## New regression proof
|
||||
|
||||
`crates/workspace-server/src/server_assignment_db_execution_tests.rs` contains
|
||||
10 tests on fixture-owned bounded DB/Host owners, temporary databases and explicit
|
||||
observable gates. Parent inspected the child's committed tool output, the actual
|
||||
new test code and then the combined default-parallel suite, rather than treating
|
||||
an end notification as proof. The child's delegated file scope was released.
|
||||
|
||||
The regressions prove:
|
||||
|
||||
1. Assignment/diagnostic read and Ticket reference resolution wait off the
|
||||
current-thread executor on their actual Store connection mutex.
|
||||
2. Typed principal/handling/closure query matrices, including empty results,
|
||||
hold the candidate connection and filter before the page limit.
|
||||
3. A real second SQLite connection's IMMEDIATE writer reaches the answer's owned
|
||||
busy observer while the same Tokio runtime's timer and independent control
|
||||
progress; one transaction preserves the answer/event/audit/receipt.
|
||||
4. Saturated DB admission returns HTTP 503 and does not accept an answer.
|
||||
5. Cancelling an accepted answer waiter retains its actual job slot/lifecycle
|
||||
through SQL and Host publication; body/audit/receipt persist, dispatch is once,
|
||||
and actual cleanup releases all observed leases.
|
||||
6. Successful HTTP answer waits for its admitted publication without occupying
|
||||
the DB worker; an independent DB callback can progress during publication.
|
||||
7. Account and Browser receipt replay, stale CAS, operation mismatch and bad
|
||||
cursor/error mapping retain their domain meanings.
|
||||
8. Real signed non-Orchestrator source and signed proof replay are denied with
|
||||
no answer/receipt; signed current Orchestrator canonical/Browser replay shares
|
||||
one source-bound receipt, and another source cannot replay it.
|
||||
9. Saturated **2-worker/16-queued Host** live-discovery rejects canonical/Browser
|
||||
Runtime answers before SQL admission, without receipt/audit/publication or
|
||||
retained lifecycle, while independent DB callbacks progress. This last test
|
||||
injects authenticated context at generated handlers to isolate discovery
|
||||
admission; the preceding tests exercise actual signed-proof middleware.
|
||||
|
||||
Read/query contention intentionally uses the actual connection mutex; answer
|
||||
contention uses a separate real SQLite writer. Neither is described as a full
|
||||
remote Runtime latency benchmark. No fixed completion sleeps, mutation retry,
|
||||
parallelism reduction, process-global fixture budgets or new ignored tests were
|
||||
introduced. Fixtures drain accepted Host/cleanup jobs, shut down projection
|
||||
producers, then drain DB jobs before ending their original Tokio driver. Owned
|
||||
busy-handler userdata and SQLite locks unregister/release through RAII.
|
||||
|
||||
## Validation on this composition
|
||||
|
||||
- Root `cargo check`: passed, normal default-member compile closure.
|
||||
- `cargo test -p yoi-workspace-server --lib assignment_db_execution_tests`:
|
||||
**10 passed, 0 failed**, 2.63s (child committed output). Parent reran the first
|
||||
nine after separating Runtime discovery and included all ten in the full suite.
|
||||
- `cargo test -p yoi-workspace-server`: original default parallelism,
|
||||
**985 library passed, 0 failed, 1 pre-existing ignored**, 176.60s;
|
||||
**11 binary passed**, doc tests passed. Log:
|
||||
`target/t743-combined-server-suite.log`.
|
||||
- `cargo test -p workdir`: **153 passed** across library/integration targets;
|
||||
`cargo test -p workdir --features http-client`: **156 passed**, doc tests passed.
|
||||
Logs: `target/t743-combined-workdir.log`, `target/t743-combined-workdir-http.log`.
|
||||
- Combined Web check: **0 errors, 0 warnings**; static production build passed
|
||||
with the existing chunk-size advisory. Web tests **613 passed**, components
|
||||
**318 passed**, Chromium Ticket/principal browser tests **7 passed**.
|
||||
Logs: `target/t743-combined-{web-check,web-build,web-tests,components,browser}.log`.
|
||||
The last change is Server execution placement/test code, not a UI change.
|
||||
- All six server-api TypeScript generator `--check` commands passed: ticket,
|
||||
repository OpenAPI, runtime, workdir, Worker launch and companion.
|
||||
- `cargo fmt --all -- --check`, `git diff --check HEAD` and
|
||||
`env -u LD_LIBRARY_PATH deno run --allow-read --allow-run=git tools/check-state-contract-names.ts`
|
||||
passed; naming: **558 matching lines, 0 unapproved, 0 stale exceptions**.
|
||||
|
||||
Earlier T-743 public-surface, five-crate, Prompt/Tool, visual and real-Worker
|
||||
fixture evidence is retained in `T-743-public-surface-validation.md`. Scripted
|
||||
model/loopback browser fixtures do not attest real model judgment or all T-744
|
||||
live paths. Target integration, Ticket/MR conclusion, live update and retained
|
||||
Worker/Workdir cleanup were not performed. Fresh independent combined-source
|
||||
review remains pending the upstream fixes, not silently waived.
|
||||
@@ -26,7 +26,7 @@ Canonical `/workspaces/{workspace_id}/tickets/{ticket_id}` and `/api/w/{workspac
|
||||
|
||||
## Validation
|
||||
|
||||
Passed on the combined working snapshot:
|
||||
Passed on the initial T-743 public-surface snapshot (before T-748 composition); later combined execution-boundary evidence is in `T-743-T748-composition.md`:
|
||||
|
||||
- Root `cargo check` (normal TUI/Server compile closure).
|
||||
- `cargo test --no-fail-fast -p ticket -p worker-runtime -p worker -p yoi-workspace-server -p server-api`: all selected crate targets passed. Library counts: server-api 98; ticket 99; worker 994; worker-runtime 417; workspace-server 946 with one existing ignored test. Included integration/binary targets also passed. Log: `target/t743-changed-crates-final.log`.
|
||||
@@ -51,7 +51,7 @@ List/Orchestrator final evidence: `t743-list-final-reviewed-light` and `t743-lis
|
||||
|
||||
## Explicit remaining work and limits
|
||||
|
||||
T-748's DB execution facade is **not** copied or integrated. Its latest notified candidate is local-only and its full Server suite has unresolved failures. When T-748 publishes a validated source, normal-merge it, place each new whole synchronous domain helper/authority query into the common bounded DB execution boundary, keep the answer transaction intact, and leave Runtime/network waiting off the DB worker. Recheck cancellation/overload and commit-before-publication at the combined boundary. The exact combined published source then needs independent trusted review.
|
||||
The published T-748 source `66bb9d0b825d72e20923003b273b787007f3aed8` has now been normally merged into the same T-743 source and the added callers connected to its existing bounded owner. The answer transaction remains intact; Runtime live discovery and post-commit dispatch use the finite Host lane. Combined full Server/Workdir and ten new boundary regressions pass; see `T-743-T748-composition.md` for exact proof and distinctions. However T-748 MR seq3 has two unresolved major findings on other Subjektiv/network and cold protocol signing paths. A validated upstream fix source, normal composition/revalidation and fresh independent exact combined-source review remain necessary. This is not final approval.
|
||||
|
||||
Current evidence does not attest actual model judgment quality. Prompt/Tool fixtures are scripted inclusion/schema/transport tests; Chromium uses a loopback API fixture. The real-Worker Server fixture uses a scripted model with actual Controller/session/Runtime acceptance and canonical processing synchronization. None is a live migration/rollout, proof of actual processing for every production delivery, or completion of T-744's full real-path validation.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user