You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@texera.apache.org by "aglinxinyuan (via GitHub)" <gi...@apache.org> on 2026/08/27 05:17:43 UTC
[PR] test(amber): cover WorkflowExecutionService state events and teardown [texera]
aglinxinyuan opened a new pull request, #8035:
URL: https://github.com/apache/texera/pull/8035
### What changes were proposed in this PR?
`WorkflowExecutionServiceSpec` goes from 3 tests to 11. The three existing tests stop at construction; the new ones drive the metadata-store handler and the teardown path.
| Metric | Before | After |
|---|---|---|
| **Codecov (fully-covered lines)** | 26/72 = 36.1% | **36/72 = 50.0%** |
| JaCoCo line-hit | 30/72 | 39/72 |
| Branch arms | 5/18 | 10/18 |
**+10 fully-covered lines and +5 branch arms.** The two metrics differ by one because line 179 was already line-hit and flips only by completing its second arm — they are not interchangeable, so both are given.
**Of the +10, six are logic and four are not.** Lines 95, 179, 181–184 are the recovery-banner arm and the four `unsubscribeAll` calls. Lines 101, 107, 108 and 109 are scalac-generated public accessor pairs (confirmed with `javap -p -l`) that move because the teardown test assigns those vars from outside. Those four are a real consequence of driving teardown rather than coverage farming, but they are not logic and I would rather split them out than present all ten as equivalent.
### What the reviewers found
Two independent adversarial reviewers ran against the first draft. **Six of eight fresh mutants survived it.** The three worth naming:
- **`workflowContext.workflowSettings = request.workflowSettings` (line 90) could be deleted outright and all 8 tests still passed.** Every test executes that line, so it was fully covered and entirely unconstrained. It is not cosmetic: `outputPortsNeedingStorage` is what `CostBasedScheduleGenerator` reads to decide which output ports get materialized, and `dataTransferBatchSize` / `executionMode` feed the resource allocator. Dropping it silently reverts every execution to the default settings while the request's are ignored.
- **The recovering-state guard was pinned one-sidedly.** The test only ever drove `isRecovering` false → true, so weakening `newState.isRecovering != oldState.isRecovering` to a bare `newState.isRecovering` survived. That mutant is the mirror of the failure the test's own comment claimed to guard: an update that *clears* the flag emits no event, so the frontend's Recovering banner never comes down.
- **Both the state guard and the `fatalErrors` guard could be deleted wholesale** and nothing failed.
All are now killed by a named test. The published mutation table was also re-run from scratch, one mutant at a time, because one row's failure message had been copy-pasted from another row.
### Verification
Measured with an identical suite-name filter on both sides, one fresh sbt JVM per measurement, `rm -rf` of the jacoco dir between runs, counters read per-line out of `jacoco.xml`.
An independent measurer re-derived the figure a second way — over the **whole amber unit module**, the actual CI scope, rather than the six-suite filter — and got byte-identical per-line data on both sides. That independently confirms the six-suite list is complete and that 26 → 36 is what Codecov will show.
Two corrections that measurement forced, both worth stating:
- The repair round added 3 tests **after** the original measurement, and the expectation was that the figure would rise above 36. It did not. Those 3 tests bought **zero** additional fully-covered lines — they add branch arms on line 94 and mutation-kill strength only. 36 is the number.
- Line 185 already counted as a Codecov hit before this PR despite `mi=4, ci=1`, because the method's `return` instruction is attributed to it. It is genuinely executed only now.
Full amber unit scope, both sides: **190 suites, the same 7 pre-existing Windows-only failures by name, zero new.** The only per-suite change anywhere is this spec going 3 → 11 tests, so the new `beforeAll` inserts do not leak — `MockTexeraDB` gives each suite its own database, and ids 9207–9210 are unique across the repo because the Iceberg statistics URI is machine-global.
### Deliberately not included
`executeWorkflow`'s live-runtime half — the 44-line hole — has no seam. Line 124 calls `ComputingUnitMaster.createAmberRuntime`, which builds an `AmberClient` over `AmberRuntime.actorSystem`, a JVM-global `private var` that nothing in unit scope initialises. Injecting a seam would be a production change.
Lines 113/114/115 are refused for a sharper reason: JaCoCo probes the `try` block only at its exit, so they flip only if compile *and* `Workflow.fromCompilationResult` both succeed — which falls straight into `createAmberRuntime`. Whether that is survivable depends on `ClientEventSpec` having restored the global to `null` in its `afterAll`. `WorkflowServiceSpec`'s own header documents this cross-suite hazard as its reason for deliberately steering into a compile failure instead. Cementing an accident is worse than leaving three lines.
Three mutants are reported as live rather than dropped: reordering `client.shutdown()` against the four `unsubscribeAll` calls (order is not a stated contract), and two whose only kill would be to assert current behaviour that is arguably wrong — adding `FAILED` to the stuck-banner guard, and pinning which field the teardown guard reads, where the only discriminating state currently NPEs.
No production file is touched.
### Any related issues, documentation, discussions?
Closes #8033
### How was this PR tested?
```
sbt "WorkflowExecutionService/testOnly org.apache.texera.web.service.WorkflowExecutionServiceSpec"
```
```
[info] Tests: succeeded 11, failed 0, canceled 0, ignored 0, pending 0
[info] All tests passed.
```
`WorkflowExecutionService/Test/scalafmtCheck` and `WorkflowExecutionService/Test/scalafix --check` both pass.
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 5)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(amber): cover WorkflowExecutionService state events and teardown [texera]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #8035:
URL: https://github.com/apache/texera/pull/8035#discussion_r3869472282
##########
amber/src/test/scala/org/apache/texera/web/service/WorkflowExecutionServiceSpec.scala:
##########
@@ -54,19 +83,155 @@ import scala.collection.mutable
* purpose: construction must stay side-effect-free (all throwing work is in
* `executeWorkflow`), so a future change that dereferences them during
* construction would fail here.
+ *
+ * The suite also owns two paths that need more scaffolding than that:
+ *
+ * - `executeWorkflow`'s compilation-failure arm, driven for real (not simulated
+ * through `errorHandler`) with a CSV scan that has no file selected -- the
+ * recipe `SyncExecutionResourceSpec` establishes and `WorkflowServiceSpec`
+ * reuses. Compilation rejects it inside `LogicalPlan.resolveScanSourceOpFileName`
+ * before `FileResolver` is reached, so no storage is involved, and the early
+ * `return` stops the method before `ComputingUnitMaster.createAmberRuntime`,
+ * which would build an `AmberClient` on `AmberRuntime.actorSystem` -- null
+ * outside a started coordinator, and whatever another suite left behind inside
+ * the shared serial JVM.
+ *
+ * - `unsubscribeAll`, the session-teardown contract. This is what makes the
+ * suite carry a TestKit `ActorSystem` and `MockTexeraDB`: the method walks the
+ * four runtime services, and `ExecutionStatsService`'s constructor creates its
+ * iceberg runtime-statistics table and stamps the URI onto a `workflow_executions`
+ * row, so a non-null value for that field cannot be had more cheaply. There is
+ * no second home for the test -- one spec file per source class -- and stubbing
+ * the services out would leave the four teardown calls unobserved, which is the
+ * entire content of the method.
+ *
+ * The runtime half of `executeWorkflow` (everything from `createAmberRuntime` on) is
+ * out of reach here for the reason above and is exercised by the integration suites.
+ *
+ * Known, pre-existing: `ExecutionStatsService` never shuts down its private
+ * `metricsPersistThread` (it does not override `unsubscribeAll`), so the teardown
+ * test below leaks one single-thread executor into the shared JVM, as every
+ * construction of that service already does.
+ *
+ * Three further gaps are deliberately described and NOT pinned, because a test that
+ * froze the current behaviour would cement it:
+ *
+ * - Partial construction. `executeWorkflow` assigns `client` first and the four
+ * services after it, and `WorkflowService` catches whatever they throw while
+ * leaving the half-built service published. `unsubscribeAll` then passes its
+ * `client != null` guard and NPEs on the first null service, aborting teardown.
+ *
+ * - A recovery that ends in failure. `createStateEvent` carves out COMPLETED only,
+ * while the flag itself is lowered only by a `WorkflowRecoveryStatus(false)` from
+ * the engine, which an execution that dies mid-recovery need not ever send. Such an
+ * execution reaches FAILED with the flag still raised and is announced as
+ * "Recovering" from then on.
+ *
+ * - `ExecutionReconfigurationService.registerWorkerCompletionCallback` discards the
+ * `Disposable` its `registerCallback` returns instead of handing it to
+ * `addSubscription`, so that one callback outlives the teardown below. Not this
+ * class's defect, and not this suite's to pin.
*/
-class WorkflowExecutionServiceSpec extends AnyFlatSpec with Matchers {
+class WorkflowExecutionServiceSpec
+ extends TestKit(ActorSystem("WorkflowExecutionServiceSpec"))
+ with AnyFlatSpecLike
+ with Matchers
+ with BeforeAndAfterAll
+ with MockTexeraDB {
+
+ // Distinct from every other suite's ids: the statistics URI is derived from wid/eid and
+ // `createDocument` truncates whatever table already sits at it (ExecutionStatsServiceSpec
+ // owns 9107/9108).
+ private val testUid: Integer = 9207
+ private val testWid: Integer = 9207
+ private val testEid: Integer = 9208
+
+ /**
+ * The computing unit the fixture execution ran on. A different literal from `testWid` on
+ * purpose: `getLatestExecutionID(wid, cuid)` binds two bare `Integer`s into one predicate, so a
+ * fixture that reused one number for both would make a transposition of the two arguments
+ * produce byte-identical SQL.
+ */
+ private val testCuid: Integer = 9209
+
+ /** A computing unit with no executions, so the cuid leg of the predicate is not vacuous. */
+ private val otherCuid: Integer = 9210
+
+ override protected def beforeAll(): Unit = {
+ initializeDBAndReplaceDSLContext()
+
+ val user = new User
+ user.setUid(testUid)
Review Comment:
`beforeAll` overrides ScalaTest lifecycle but doesn't call `super.beforeAll()`. If another mixed-in trait later adds its own `beforeAll`, this override will prevent it from running (trait-stacking contract). Call `super.beforeAll()` as the first statement to keep lifecycle hooks composable.
##########
amber/src/test/scala/org/apache/texera/web/service/WorkflowExecutionServiceSpec.scala:
##########
@@ -54,19 +83,155 @@ import scala.collection.mutable
* purpose: construction must stay side-effect-free (all throwing work is in
* `executeWorkflow`), so a future change that dereferences them during
* construction would fail here.
+ *
+ * The suite also owns two paths that need more scaffolding than that:
+ *
+ * - `executeWorkflow`'s compilation-failure arm, driven for real (not simulated
+ * through `errorHandler`) with a CSV scan that has no file selected -- the
+ * recipe `SyncExecutionResourceSpec` establishes and `WorkflowServiceSpec`
+ * reuses. Compilation rejects it inside `LogicalPlan.resolveScanSourceOpFileName`
+ * before `FileResolver` is reached, so no storage is involved, and the early
+ * `return` stops the method before `ComputingUnitMaster.createAmberRuntime`,
+ * which would build an `AmberClient` on `AmberRuntime.actorSystem` -- null
+ * outside a started coordinator, and whatever another suite left behind inside
+ * the shared serial JVM.
+ *
+ * - `unsubscribeAll`, the session-teardown contract. This is what makes the
+ * suite carry a TestKit `ActorSystem` and `MockTexeraDB`: the method walks the
+ * four runtime services, and `ExecutionStatsService`'s constructor creates its
+ * iceberg runtime-statistics table and stamps the URI onto a `workflow_executions`
+ * row, so a non-null value for that field cannot be had more cheaply. There is
+ * no second home for the test -- one spec file per source class -- and stubbing
+ * the services out would leave the four teardown calls unobserved, which is the
+ * entire content of the method.
+ *
+ * The runtime half of `executeWorkflow` (everything from `createAmberRuntime` on) is
+ * out of reach here for the reason above and is exercised by the integration suites.
+ *
+ * Known, pre-existing: `ExecutionStatsService` never shuts down its private
+ * `metricsPersistThread` (it does not override `unsubscribeAll`), so the teardown
+ * test below leaks one single-thread executor into the shared JVM, as every
+ * construction of that service already does.
+ *
+ * Three further gaps are deliberately described and NOT pinned, because a test that
+ * froze the current behaviour would cement it:
+ *
+ * - Partial construction. `executeWorkflow` assigns `client` first and the four
+ * services after it, and `WorkflowService` catches whatever they throw while
+ * leaving the half-built service published. `unsubscribeAll` then passes its
+ * `client != null` guard and NPEs on the first null service, aborting teardown.
+ *
+ * - A recovery that ends in failure. `createStateEvent` carves out COMPLETED only,
+ * while the flag itself is lowered only by a `WorkflowRecoveryStatus(false)` from
+ * the engine, which an execution that dies mid-recovery need not ever send. Such an
+ * execution reaches FAILED with the flag still raised and is announced as
+ * "Recovering" from then on.
+ *
+ * - `ExecutionReconfigurationService.registerWorkerCompletionCallback` discards the
+ * `Disposable` its `registerCallback` returns instead of handing it to
+ * `addSubscription`, so that one callback outlives the teardown below. Not this
+ * class's defect, and not this suite's to pin.
*/
-class WorkflowExecutionServiceSpec extends AnyFlatSpec with Matchers {
+class WorkflowExecutionServiceSpec
+ extends TestKit(ActorSystem("WorkflowExecutionServiceSpec"))
+ with AnyFlatSpecLike
+ with Matchers
+ with BeforeAndAfterAll
+ with MockTexeraDB {
+
+ // Distinct from every other suite's ids: the statistics URI is derived from wid/eid and
+ // `createDocument` truncates whatever table already sits at it (ExecutionStatsServiceSpec
+ // owns 9107/9108).
+ private val testUid: Integer = 9207
+ private val testWid: Integer = 9207
+ private val testEid: Integer = 9208
+
+ /**
+ * The computing unit the fixture execution ran on. A different literal from `testWid` on
+ * purpose: `getLatestExecutionID(wid, cuid)` binds two bare `Integer`s into one predicate, so a
+ * fixture that reused one number for both would make a transposition of the two arguments
+ * produce byte-identical SQL.
+ */
+ private val testCuid: Integer = 9209
+
+ /** A computing unit with no executions, so the cuid leg of the predicate is not vacuous. */
+ private val otherCuid: Integer = 9210
+
+ override protected def beforeAll(): Unit = {
+ initializeDBAndReplaceDSLContext()
+
+ val user = new User
+ user.setUid(testUid)
+ user.setName("workflow-execution-test-user")
+ user.setEmail(s"u$testUid@example.com")
+ new UserDao(getDSLContext.configuration()).insert(user)
+
+ val workflow = new Workflow
+ workflow.setWid(testWid)
+ workflow.setName(s"workflow-execution-test-$testWid")
+ workflow.setContent("{}")
+ workflow.setDescription("")
+ workflow.setCreationTime(new SqlTimestamp(System.currentTimeMillis()))
+ workflow.setLastModifiedTime(new SqlTimestamp(System.currentTimeMillis()))
+ new WorkflowDao(getDSLContext.configuration()).insert(workflow)
+
+ val version = new WorkflowVersion
+ version.setWid(testWid)
+ version.setContent("{}")
+ version.setCreationTime(new SqlTimestamp(System.currentTimeMillis()))
+ new WorkflowVersionDao(getDSLContext.configuration()).insert(version)
+
+ val computingUnitDao = new WorkflowComputingUnitDao(getDSLContext.configuration())
+ List(testCuid -> "workflow-execution-test-unit", otherCuid -> "workflow-execution-idle-unit")
+ .foreach {
+ case (cuid, name) =>
+ val unit = new WorkflowComputingUnit
+ unit.setCuid(cuid)
+ unit.setUid(testUid)
+ unit.setName(name)
+ unit.setCreationTime(new SqlTimestamp(System.currentTimeMillis()))
+ unit.setType(WorkflowComputingUnitTypeEnum.local)
+ unit.setUri("local://test")
+ unit.setResource("{}")
+ computingUnitDao.insert(unit)
+ }
+
+ // `ExecutionStatsService`'s constructor stamps the runtime-statistics URI onto this row,
+ // reaching it from the workflow through workflow_version. `cuid` is set because
+ // `getLatestExecutionID` matches on it and a NULL cuid matches no value at all.
+ val execution = new WorkflowExecutions
+ execution.setEid(testEid)
+ execution.setVid(version.getVid)
+ execution.setUid(testUid)
+ execution.setCuid(testCuid)
+ execution.setStatus(0.toByte)
+ execution.setStartingTime(new SqlTimestamp(System.currentTimeMillis()))
+ execution.setBookmarked(false)
+ execution.setName("workflow-execution-test-execution")
+ execution.setEnvironmentVersion("test-env")
+ new WorkflowExecutionsDao(getDSLContext.configuration()).insert(execution)
+ }
+
+ override protected def afterAll(): Unit = {
+ try {
+ TestKit.shutdownActorSystem(system)
+ closeConnectionPool()
+ } finally super.afterAll()
+ }
Review Comment:
`afterAll` calls `TestKit.shutdownActorSystem(system)` and then `closeConnectionPool()`. If shutting down the actor system throws (e.g., timeout), the DB connection pool will never be closed, leaving resources pinned for the remainder of the sbt JVM. Wrap the DB cleanup in a `finally` so it always runs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(amber): cover WorkflowExecutionService state events and teardown [texera]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #8035:
URL: https://github.com/apache/texera/pull/8035#issuecomment-5434794811
## [Codecov](https://app.codecov.io/gh/apache/texera/pull/8035?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 93.35%. Comparing base ([`57a4230`](https://app.codecov.io/gh/apache/texera/commit/57a4230b6f744ba009aa0099893da825999f3077?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`02c19c9`](https://app.codecov.io/gh/apache/texera/commit/02c19c9aeca823fee02cbdd20fb17f9404c15aca?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on main.
:white_check_mark: All tests successful. No failed tests found.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #8035 +/- ##
============================================
+ Coverage 93.33% 93.35% +0.01%
- Complexity 4692 4698 +6
============================================
Files 1179 1179
Lines 47713 47713
Branches 5314 5314
============================================
+ Hits 44534 44543 +9
+ Misses 1730 1721 -9
Partials 1449 1449
```
| [Flag](https://app.codecov.io/gh/apache/texera/pull/8035/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | *Carryforward flag |
|---|---|---|---|
| [access-control-service](https://app.codecov.io/gh/apache/texera/pull/8035/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `81.00% <ø> (ø)` | | Carriedforward from [57a4230](https://app.codecov.io/gh/apache/texera/commit/57a4230b6f744ba009aa0099893da825999f3077?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [agent-service](https://app.codecov.io/gh/apache/texera/pull/8035/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `99.32% <ø> (ø)` | | Carriedforward from [57a4230](https://app.codecov.io/gh/apache/texera/commit/57a4230b6f744ba009aa0099893da825999f3077?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [amber](https://app.codecov.io/gh/apache/texera/pull/8035/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `89.52% <ø> (+0.05%)` | :arrow_up: | |
| [computing-unit-managing-service](https://app.codecov.io/gh/apache/texera/pull/8035/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `73.67% <ø> (ø)` | | Carriedforward from [57a4230](https://app.codecov.io/gh/apache/texera/commit/57a4230b6f744ba009aa0099893da825999f3077?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [config-service](https://app.codecov.io/gh/apache/texera/pull/8035/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `86.73% <ø> (ø)` | | Carriedforward from [57a4230](https://app.codecov.io/gh/apache/texera/commit/57a4230b6f744ba009aa0099893da825999f3077?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [file-service](https://app.codecov.io/gh/apache/texera/pull/8035/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `86.70% <ø> (ø)` | | Carriedforward from [57a4230](https://app.codecov.io/gh/apache/texera/commit/57a4230b6f744ba009aa0099893da825999f3077?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [frontend](https://app.codecov.io/gh/apache/texera/pull/8035/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `95.82% <ø> (ø)` | | Carriedforward from [57a4230](https://app.codecov.io/gh/apache/texera/commit/57a4230b6f744ba009aa0099893da825999f3077?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [notebook-migration-service](https://app.codecov.io/gh/apache/texera/pull/8035/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `79.31% <ø> (ø)` | | Carriedforward from [57a4230](https://app.codecov.io/gh/apache/texera/commit/57a4230b6f744ba009aa0099893da825999f3077?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [pyamber](https://app.codecov.io/gh/apache/texera/pull/8035/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `97.74% <ø> (ø)` | | Carriedforward from [57a4230](https://app.codecov.io/gh/apache/texera/commit/57a4230b6f744ba009aa0099893da825999f3077?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [workflow-compiling-service](https://app.codecov.io/gh/apache/texera/pull/8035/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `77.19% <ø> (ø)` | | Carriedforward from [57a4230](https://app.codecov.io/gh/apache/texera/commit/57a4230b6f744ba009aa0099893da825999f3077?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
*This pull request uses carry forward flags. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/texera/pull/8035?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(amber): cover WorkflowExecutionService state events and teardown [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8035:
URL: https://github.com/apache/texera/pull/8035#issuecomment-5434700819
<!-- texera-reviewer-suggestion -->
### Automated Reviewer Suggestions
Based on the `git blame` history of the changed files, we recommend the following reviewers:
- **Contributors with relevant context:** `@mengw15`
You can notify them by mentioning `@mengw15` in a comment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(amber): cover WorkflowExecutionService state events and teardown [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8035:
URL: https://github.com/apache/texera/pull/8035#issuecomment-5434734971
<!-- texera-benchmarks-comment -->
## ✅ No material benchmark regressions detected
🟢 4 better · 🔴 0 worse · ⚪ 11 noise (<±5%) · 0 without baseline
> Compared against main `97e3585` benchmarked on **this same runner**, so the delta is largely free of cross-runner hardware noise. The "7d avg" column still reflects the gh-pages dashboard. Treat <±5% as noise unless repeated.
[Dashboard](https://apache.github.io/texera/dev/bench/) · [Run](https://github.com/apache/texera/actions/runs/33042012256)
| | config | throughput | MB/s | latency | max Δ latest / 7d |
|---|---|---:|---:|---:|---:|
| 🟢 | bs=10 sw=10 sl=64 | 596 | 0.364 | 16,353/20,842/20,842 us | 🟢 -25.2% / 🔴 +50.6% |
| 🟢 | bs=100 sw=10 sl=64 | 1,168 | 0.713 | 82,501/114,693/114,693 us | 🟢 -6.7% / 🔴 +22.4% |
| ⚪ | bs=1000 sw=10 sl=64 | 1,428 | 0.872 | 698,561/739,776/739,776 us | ⚪ within ±5% / 🟢 -21.1% |
<details><summary>Baseline details</summary>
Latest main `97e3585` from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---:|---:|---:|---:|---:|
| bs=10 sw=10 sl=64 | throughput | 596 tuples/sec | 572 tuples/sec | 894.86 tuples/sec | +4.2% | -33.4% |
| bs=10 sw=10 sl=64 | MB/s | 0.364 MB/s | 0.349 MB/s | 0.546 MB/s | +4.3% | -33.4% |
| bs=10 sw=10 sl=64 | p50 | 16,353 us | 16,349 us | 11,139 us | +0.0% | +46.8% |
| bs=10 sw=10 sl=64 | p95 | 20,842 us | 27,879 us | 13,843 us | -25.2% | +50.6% |
| bs=10 sw=10 sl=64 | p99 | 20,842 us | 27,879 us | 16,839 us | -25.2% | +23.8% |
| bs=100 sw=10 sl=64 | throughput | 1,168 tuples/sec | 1,123 tuples/sec | 1,166 tuples/sec | +4.0% | +0.2% |
| bs=100 sw=10 sl=64 | MB/s | 0.713 MB/s | 0.686 MB/s | 0.711 MB/s | +3.9% | +0.2% |
| bs=100 sw=10 sl=64 | p50 | 82,501 us | 83,988 us | 87,509 us | -1.8% | -5.7% |
| bs=100 sw=10 sl=64 | p95 | 114,693 us | 122,984 us | 93,675 us | -6.7% | +22.4% |
| bs=100 sw=10 sl=64 | p99 | 114,693 us | 122,984 us | 102,153 us | -6.7% | +12.3% |
| bs=1000 sw=10 sl=64 | throughput | 1,428 tuples/sec | 1,404 tuples/sec | 1,198 tuples/sec | +1.7% | +19.2% |
| bs=1000 sw=10 sl=64 | MB/s | 0.872 MB/s | 0.857 MB/s | 0.731 MB/s | +1.8% | +19.2% |
| bs=1000 sw=10 sl=64 | p50 | 698,561 us | 716,187 us | 859,766 us | -2.5% | -18.7% |
| bs=1000 sw=10 sl=64 | p95 | 739,776 us | 775,973 us | 902,897 us | -4.7% | -18.1% |
| bs=1000 sw=10 sl=64 | p99 | 739,776 us | 775,973 us | 937,957 us | -4.7% | -21.1% |
</details>
<details><summary>Raw CSV</summary>
```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,335.65,200,128000,596,0.364,16353.27,20841.91,20841.91
1,100,10,64,20,1712.61,2000,1280000,1168,0.713,82501.04,114692.88,114692.88
2,1000,10,64,20,14004.26,20000,12800000,1428,0.872,698561.07,739775.69,739775.69
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(amber): cover WorkflowExecutionService state events and teardown [texera]
Posted by "aglinxinyuan (via GitHub)" <gi...@apache.org>.
aglinxinyuan merged PR #8035:
URL: https://github.com/apache/texera/pull/8035
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org