refactor(agent-service): redesign sync-execution result and error model#7
Closed
bobbai00 wants to merge 1 commit into
Closed
Conversation
Restructure the per-operator summary the sync-execution backend returns and the
agent-service/frontend consume, for a leaner and consistent wire contract:
- Replace flat OperatorInfo with OperatorExecutionSummary: state, errorMessages,
resultSummary?, consoleLogsSummary? (orthogonal sub-summaries; no shape stats).
- Rename SyncExecutionResult -> WorkflowExecutionSummary; drop compilationErrors
(folded into errors). errors and per-op errorMessages are non-optional (empty
means none).
- OperatorResultSummary.sampleTuples is now List[SampleRow] ({rowIndex, tuple})
instead of a JSON array with an embedded __row_index__. Drop the table-shape
types (TableShape/InputPortTableShape): the agent derives input-port shapes
from the DAG + each upstream's output shape; output shape comes from the result
summary.
- Reuse the engine's WorkflowFatalError for per-operator errors (the same type
the compiling service returns for compilation errors), replacing the bespoke
OperatorError so compile and execution errors share one wire shape.
- Collapse console messages onto one type; derive warnings from WARNING-titled
messages rather than a separate field.
- Replace OperatorResultSummaryWs: the WS operatorResults payload now carries the
canonical OperatorExecutionSummary; the frontend maps it to its flat display
type (re-flattening sampleTuples to keep the display components unchanged).
Touches the Scala producer (SyncExecutionResource), the agent-service consumers
(result-formatting, workflow-execution-tools, workflow-result-state, server) and
the frontend WS mapping. Representation/type-level change; behavior preserved,
except input-port shape lines are now derived rather than explicitly rendered.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012qFkyrpTd5PrkNBPcBeo4Q
Automated Reviewer SuggestionsBased on the
|
Owner
Author
|
Superseded by apache#5927 (opened against apache:main). |
|
| config | throughput | MB/s | latency | max Δ latest / 7d | |
|---|---|---|---|---|---|
| 🔴 | bs=10 sw=10 sl=64 | 385 | 0.235 | 25,109/42,262/42,262 us | 🔴 +25.4% / ⚪ within ±5% |
| 🟢 | bs=100 sw=10 sl=64 | 835 | 0.51 | 118,572/140,438/140,438 us | 🟢 -7.7% / ⚪ within ±5% |
| ⚪ | bs=1000 sw=10 sl=64 | 949 | 0.58 | 1,057,487/1,096,278/1,096,278 us | ⚪ within ±5% / ⚪ within ±5% |
Baseline details
Latest main 2bae592 from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 385 tuples/sec | 416 tuples/sec | n/a | -7.5% | n/a |
| bs=10 sw=10 sl=64 | MB/s | 0.235 MB/s | 0.254 MB/s | n/a | -7.5% | n/a |
| bs=10 sw=10 sl=64 | p50 | 25,109 us | 23,847 us | n/a | +5.3% | n/a |
| bs=10 sw=10 sl=64 | p95 | 42,262 us | 33,714 us | n/a | +25.4% | n/a |
| bs=10 sw=10 sl=64 | p99 | 42,262 us | 33,714 us | n/a | +25.4% | n/a |
| bs=100 sw=10 sl=64 | throughput | 835 tuples/sec | 849 tuples/sec | n/a | -1.6% | n/a |
| bs=100 sw=10 sl=64 | MB/s | 0.51 MB/s | 0.518 MB/s | n/a | -1.5% | n/a |
| bs=100 sw=10 sl=64 | p50 | 118,572 us | 115,733 us | n/a | +2.5% | n/a |
| bs=100 sw=10 sl=64 | p95 | 140,438 us | 152,184 us | n/a | -7.7% | n/a |
| bs=100 sw=10 sl=64 | p99 | 140,438 us | 152,184 us | n/a | -7.7% | n/a |
| bs=1000 sw=10 sl=64 | throughput | 949 tuples/sec | 948 tuples/sec | n/a | +0.1% | n/a |
| bs=1000 sw=10 sl=64 | MB/s | 0.58 MB/s | 0.579 MB/s | n/a | +0.2% | n/a |
| bs=1000 sw=10 sl=64 | p50 | 1,057,487 us | 1,053,859 us | n/a | +0.3% | n/a |
| bs=1000 sw=10 sl=64 | p95 | 1,096,278 us | 1,088,189 us | n/a | +0.7% | n/a |
| bs=1000 sw=10 sl=64 | p99 | 1,096,278 us | 1,088,189 us | n/a | +0.7% | n/a |
Raw CSV
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,519.41,200,128000,385,0.235,25108.90,42261.57,42261.57
1,100,10,64,20,2395.00,2000,1280000,835,0.510,118572.45,140438.42,140438.42
2,1000,10,64,20,21064.07,20000,12800000,949,0.580,1057487.22,1096277.97,1096277.97
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this PR?
Stacked on apache#5751 (foundation). This is the second of the split: it carries the sync-execution result/error model redesign (no overlap with the foundation PR —
types/execution.tsis untouched there).OperatorInfowithOperatorExecutionSummary(orthogonal sub-summaries:state,errorMessages,resultSummary?,consoleLogsSummary?); renameSyncExecutionResult→WorkflowExecutionSummary.resultSummary.sampleTuplesis nowSampleRow[]({ rowIndex, tuple }) instead of JSON rows with an embedded__row_index__; drop the table-shape types (the agent derives input-port shapes from the DAG).WorkflowFatalErrorfor per-operator errors (the same type the compiling service returns), replacing the bespokeOperatorErrorso compile and execution errors share one wire shape.errorMessages/errorsare non-optional (empty = none); dropcompilationErrors; collapse the console-message types and derive warnings fromWARNING:-titled messages.operatorResultspayload carries the canonicalOperatorExecutionSummary; the frontend maps it to its flat display type.Touches the Scala producer (
SyncExecutionResource), the agent-service consumers (result-formatting,workflow-execution-tools,workflow-result-state,server), and the frontend WS mapping. Representation/type-level; behavior preserved (input-port shape lines are now derived rather than explicitly rendered).Any related issues, documentation, discussions?
Part of apache#5747. Depends on apache#5751 — review/merge that first. (This is a draft stacked on the foundation branch; it will be retargeted to
mainonce apache#5751 lands.)How was this PR tested?
bunx tsc --noEmit,bun test(121 pass / 0 fail),prettier --checkinagent-service;sbt WorkflowExecutionService/compilefor amber./operator-resultsreturned the new shape —resultSummary.sampleTuples: [{ rowIndex, tuple }],errorMessages: []— and the agent rendered the rows correctly.Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Opus 4.8 (1M context)