Skip to content

Pin the previous run's storage-registry clean-up #7711

Description

@aglinxinyuan

Task Summary

amber/src/main/scala/org/apache/texera/web/service/WorkflowService.scala sits at 79.2% of 96 lines, and clearExecutionResources -- the method that wipes the previous run's storage registry before a new execution starts -- is never entered by any test. It is unreachable today only by accident: SyncExecutionResourceSpec:145-151 documents parking its fixtures under a second computing unit specifically to avoid this path, so the recipe is proven and merely needs inverting.

The structural fact that should shape any plan here: JaCoCo's SyntheticFilter drops methods that are ACC_SYNTHETIC and not lambda$-prefixed, and Scala 2.13 compiles closures to synthetic $anonfun$ methods. Every lambda body in this file is therefore absent from the 96 tracked lines -- the lifecycle cleanup callback, the constructor's completion diff-handler, the errorHandler, both cleanup loop bodies, and the replay foreach. That is roughly 30 lines where a test moves the coverage number by exactly zero. Anyone targeting them is wasting effort.

Traps worth knowing, each of which produces a green test that pins nothing:

  1. Reusing one number for every id. If wid == uid == cuid, a transposed WID.eq(cuid).and(CUID.eq(wid)) produces byte-identical SQL. Every id domain needs its own literal.
  2. A single previous execution cannot pin "latest". With one row under the unit, Some(executions.max) -> .min is undetectable. Seed an older one and assert it survives.
  3. A single workflow cannot pin the WID leg. workflow_computing_unit carries uid and no wid, so one unit legitimately runs many workflows -- dropping the WID leg needs a second workflow on the same unit to be caught.
  4. Relying on SERIAL ids hides an off-by-one. With no execution below the one under test, .eq -> .le on the delete survives. Set explicit eids and place one below.
  5. Line 406 makes a real network call. LargeBinaryManager.deleteByExecution reaches S3StorageClient, whose client is a private lazy val in a Scala object with no seam (the injectable overload is private[util]). With SERIAL ids the first execution gets eid 1, which is LargeBinaryManager's DEFAULT_EXECUTION_ID sentinel -- so a dev box running bin/local-dev.sh up really does recursively delete objects/1/ in MinIO. Explicit non-1 eids avoid that.
  6. Do not encode a non-terminal previous execution as clearable. Guarding the clean-up on the previous execution being terminal is a defensible production fix; a fixture that persists a running execution and asserts it is wiped would block it.

The fault-tolerance block is out of reach: it is gated on ApplicationConfig.faultToleranceLogRootFolder, a val in a Scala object read at object-init time, with no parameter or override -- and ApplicationConfigSpec:90 asserts it is None.

Task Type

  • Refactor / Cleanup
  • DevOps / Deployment / CI
  • Testing / QA
  • Documentation
  • Performance
  • Other

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions