fix(amber, v1.2): store execution sizes as BIGINT to stop silent >2GiB truncation - #7050
Conversation
…cation (#6980) ### What changes were proposed in this PR? `updateResultSize` / `updateRuntimeStatsSize` / `updateConsoleMessageSize` (`WorkflowExecutionsResource`) stored `Long` byte counts into `INT` columns via `Integer.valueOf(size.toInt)`. Scala's `Long.toInt` keeps only the low 32 bits without raising, so a size ≥ 2 GiB wrapped silently — values in [2 GiB, 4 GiB) became **negative** — and `UserQuotaResource`, which sums `result_size` / `runtime_stats_size` / `console_messages_size` into a user's storage quota, reported corrupted totals. With BigObject (#4067) supporting >2 GB results, such sizes are reachable in practice. - **Widen the three columns to `BIGINT`** in `sql/texera_ddl.sql`, with migration `sql/updates/29.sql` (registered as changelog changeSet 29) for existing deployments — a lossless in-place `ALTER COLUMN ... TYPE BIGINT` for each. - **Store the `Long` directly** at the three write sites, dropping the `.toInt` narrowing (`java.lang.Long.valueOf(size)`; the jOOQ-generated fields become `Long` from the widened schema). - **Adapt the quota reads** in `UserQuotaResource`: the `getOrElse(0).asInstanceOf[Integer]` pattern would throw `ClassCastException` on the now-`Long` fields; simplified to `Option(...).map(_.toLong).getOrElse(0L)`. - **Split the size write out of `updateRuntimeStatsSize` / `updateConsoleMessageSize`** into `(eid, size)` overloads, mirroring the existing `updateResultSize` shape. The outer signatures are unchanged (callers untouched), but the DB write is now reachable without an Iceberg/LakeFS-backed document — so all three writes are directly testable. ### Any related issues, documentation, discussions? Closes #6978. Size columns introduced with the execution result/stats storage; >2 GB results enabled by #4067 (BigObject). ### How was this PR tested? - Added regression cases to `WorkflowExecutionsResourceSpec` (unit spec on embedded Postgres, no external infra): each of the three size writes stores a 3 GiB value and is asserted to round-trip untruncated, plus the two no-URI no-op branches. **Verified the truncation case fails before the fix** — `-1073741824 did not equal 3221225472` (the low-32-bit wrap) — and passes after. - Full spec run locally: 27/27 passed (jOOQ regenerated against the widened schema; the embedded test DB loads the updated `texera_ddl.sql`). - `sql/updates/29.sql` applied cleanly to a local Postgres 15 `texera_db` (three `ALTER TABLE`s in one transaction); columns verified `bigint` afterwards. - `WorkflowExecutionService/scalafmtCheck` (main + Test) passes. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (claude-opus-4-8) --------- (backported from commit 791d7df) Co-authored-by: Yicong Huang <17627829+Yicong-Huang@users.noreply.github.com> Co-authored-by: Xinyuan Lin <xinyual3@uci.edu>
|
The cherry-pick conflicted and was committed with conflict markers. Resolve the conflicts on this branch, then mark this PR ready for review. Conflicting files:
|
Ids 23-28 are main-only changes brought in as cherry-pick context; their sql/updates files do not exist on release/v1.2. ChangeSet 29 keeps its main id so Liquibase treats it as the same applied change across branches.
Automated Reviewer SuggestionsBased on the
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## release/v1.2 #7050 +/- ##
==================================================
- Coverage 54.08% 52.56% -1.53%
- Complexity 1447 2498 +1051
==================================================
Files 809 1077 +268
Lines 34170 42301 +8131
Branches 3450 4551 +1101
==================================================
+ Hits 18481 22235 +3754
- Misses 14777 18755 +3978
- Partials 912 1311 +399
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The main-branch spec seeds the row via a local insertOperatorPortResult helper that wraps ExecutionResultService.persistOperatorPortResultUri; neither exists on release/v1.2. Use the branch's production insertOperatorPortResultUri directly, matching the sibling RESULT_SIZE test above.
There was a problem hiding this comment.
Pull request overview
Backport to release/v1.2 of the Amber fix that prevents silent truncation/overflow of execution byte-size fields by widening the backing DB columns to BIGINT and ensuring the application reads/writes these values as Long.
Changes:
- Widen
runtime_stats_size,console_messages_size, andresult_sizecolumns fromINTtoBIGINT(DDL + migration). - Store
Longsizes directly inWorkflowExecutionsResourceand adjust quota aggregation reads to handleLongvalues. - Add regression tests to ensure >2GiB sizes round-trip correctly and cover the URI-present/no-URI branches.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
sql/updates/29.sql |
Adds migration to widen execution size columns to BIGINT. |
sql/texera_ddl.sql |
Updates baseline schema so the three size columns are BIGINT. |
sql/changelog.xml |
Registers the new migration as changeSet 29. |
amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowExecutionsResource.scala |
Stops narrowing Long sizes to Int; adds overloads to store measured sizes directly. |
amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/quota/UserQuotaResource.scala |
Updates quota reads to safely handle Long-typed size fields. |
amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowExecutionsResourceSpec.scala |
Adds regression coverage for >2GiB size persistence and related branches. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Liquibase tracks an applied changeSet by (id, author, changelog path), not id alone; all three match main's changeSet 29 here, which is what makes the upgrade-without-re-execution guarantee hold.
|
@xuang7 PTAL |
What changes were proposed in this PR?
Automated backport of #6980 to
release/v1.2.Source: 791d7df · automation run
Any related issues, documentation, discussions?
Backport of #6980. Originally linked #6978.
How was this PR tested?
Release-branch CI runs on this branch once the conflicts are resolved and this PR is marked ready for review.
Was this PR authored or co-authored using generative AI tooling?
No.