The GitHub Actions job "Required Checks" on texera.git/gh-readonly-queue/main/pr-5643-1aae0059166a4557a0ccc82c2128d48fa7192f36 has failed. Run started by GitHub user Yicong-Huang (triggered by Yicong-Huang).
Head commit for run: 810d6da834afb00ff230efd06631a98a32772d1c / Eugene Gu <[email protected]> feat(file-service): cleanup stale uncommitted dataset uploads (#5643) ### What changes were proposed in this PR? Adds a configurable scheduled cleanup job to file-service that reclaims the storage held by uploads that were started (or staged) but never committed. **Problem.** Files uploaded to a dataset sit in LakeFS staging until the user commits a version. LakeFS GC only manages committed data, and Texera has no background cleanup, so abandoned uploads accumulate forever in MinIO/S3, along with their `DATASET_UPLOAD_SESSION` rows. The only existing mitigations are the 24h presigned-URL expiry and the lazy re-init cleanup in `initMultipartUpload`, neither of which removes staged data for datasets nobody touches again. **Design.** ``` FileService.run() ──(if enabled)──> Managed StagedFileCleanupJob every interval-minutes, on one daemon thread: ├─ path 1: DATASET_UPLOAD_SESSION rows with created_at < now − retention │ → abort LakeFS multipart, then delete row (parts cascade) ├─ path 2: per dataset repo, staged objects with mtime < now − retention │ → branch-reset via existing LakeFS API └─ audit log: sessions deleted / objects reset / errors ``` Safety properties (each independently enforced): | Guard | Mechanism | |---|---| | Committed data is untouchable | the job only calls staging-scoped APIs (`diffBranch`, branch reset, multipart abort) — committed objects never appear in its inputs | | No race with an in-flight upload | staged objects whose path belongs to a non-expired session are excluded (exact complement of the deletion cutoff); default retention 72h is far above the 24h `PHYSICAL_ADDRESS_EXPIRATION_TIME_HRS`, so deleted sessions are already non-resumable | | Crash-safe ordering | multipart abort happens before the row delete; a crash in between leaves the row, which is retried next round (abort 404 = already aborted) | | Idempotent & isolated | every action is safely repeatable; per-item failures are logged, counted in the report, and never abort the batch; staged deletions and orphaned repos (dataset row without a live LakeFS repo) are skipped instead of erroring every round | | Operator control | `storage.cleanup.enabled` / `retention-hours` / `interval-minutes`, each overridable via `STORAGE_CLEANUP_*` env vars; the whole feature can be disabled | Also in this PR: file-service test suites now fork one JVM per suite (`build.sbt`). Each testcontainers-based suite boots its own LakeFS/MinIO/Postgres stack and mutates JVM-wide singletons (`StorageConfig` endpoints), so sharing one JVM broke whichever container suite ran second; forking isolates the Docker client and singleton state. `LakeFSStorageClient` gains one helper (`getStagedObjectMtime`) because the diff API carries no timestamps. Feedback welcome on the default retention (72h ≈ covers a weekend; it is a config default, not a hard-coded value). > **Note:** since this change automatically deletes user-uploaded data, it touches a fairly high-risk area. I have been deliberately conservative (staging-scoped APIs only, generous retention, kill switch, audit logging), but I'm happy to discuss any part of the design — including making the feature opt-in (`enabled = false` by default) if that feels safer for a first release. ### Any related issues, documentation, discussions? Closes #3681. ### How was this PR tested? New `StagedFileCleanupJobSpec` (11 tests, real LakeFS/MinIO/Postgres via testcontainers + embedded Postgres for the DAO): expired session deleted with part-row cascade; fresh session survives; expired staged object reset while a committed object survives; fresh staged object survives; idempotent second run reports zeros; active upload and its staged file untouched; mixed-batch exact counts; ±1min boundary around the cutoff; out-of-band-aborted multipart treated as already-aborted (404 rule); per-item failure counted without aborting the batch. ``` sbt "FileService/test" ``` 111 tests, 4 suites, all passed (includes the pre-existing `DatasetResourceSpec` 93 tests — no regression). Also ran `sbt scalafixAll` and `sbt scalafmtAll` (clean). Requires Docker for the testcontainers suites. ### Was this PR authored or co-authored using generative AI tooling? Co-authored by: Claude Code (Claude Fable 5) --------- Signed-off-by: Eugene Gu <[email protected]> Co-authored-by: Copilot Autofix powered by AI <[email protected]> Co-authored-by: Claude Opus 4.8 <[email protected]> Report URL: https://github.com/apache/texera/actions/runs/28332192939 With regards, GitHub Actions via GitBox
