mrhhsg opened a new pull request, #70474: URL: https://github.com/apache/doris/pull/70474
### Background This is a master follow-up to the review comments on #69635, not the branch-4.1 backport. Synchronizing these fixes to that backport is a separate task. Spill files can use local disks or a cloud object-storage vault. Their cleanup and capacity accounting are owned by SpillFileManager; uploads share the BE S3 thread pool with unrelated writers. ### What problem does this PR solve? Issue Number: None Related PR: #69635 (backport review), #68032 (original feature) PR Type: Bugfix Problem Summary: Synchronous spill deletion and unfinished-upload draining can delay query teardown, while a shared-pool queue length incorrectly throttles spill behind unrelated loads. Queue cleanup for GC, preserve its accounting and filesystem ownership, and measure only submitted spill uploads. Keep existing s3 configurations and rolling-upgrade column positions compatible. ### Details - **Change map:** BE source +541/-433, BE tests +446/-59, docs +86/-0, FE source +2/-1, FE tests +19/-12. The source totals include moving LocalSpillDataDir into its own files; most new behavior coverage is in focused unit tests. - **Frequency:** When spill is released under slow storage, an unrelated upload backlog, cancellation or shutdown; initialization failure also exposed an invalid metric-entity teardown assumption. - **Root cause:** Query teardown performed filesystem work, upload admission used the shared pool's queue as a spill-specific budget, and dropped queued tasks could leave a detached writer waiting forever. Metrics were registered only after fallible initialization. - **Why existing tests missed it:** Tests assumed synchronous cleanup and did not cover unrelated pool tasks, dropped upload callbacks, dynamic buffer-size accounting, or failed initialization followed by destruction. The expanded legacy S3 mock suite also needed isolated file-cache configuration. - **Fix:** Enqueue file/query cleanup with pinned filesystem, retained bytes and detached I/O writers; drain before deletion and release capacity only after success. Complete dropped upload tasks, account actual submitted buffer allocations, register metrics during construction, and fail startup if stale local roots cannot be quarantined. - **Other paths and branches:** Cover local and remote cleanup while preserving external-writer leases. Use opaque remote roots, shared storage-type constants with a remote/s3 alias, explicit reserve/release semantics and a remote part-size floor. No serialization or protocol field change. Preserve old FE column positions; branch-4.1 synchronization remains a separate task. ### Testing - ASAN: ./build.sh --be -j16, full BE build/package succeeded in this worktree. - ./run-be-ut.sh --run --filter='SpillFileS3Test.*:SpillFileTest.*:SpillFileManagerTest.*:SpillRepartitionerTest.*:SpillSortSourceOperatorTest.*:S3FileWriterTest.*' -j16: 132 passed. - ./run-fe-ut.sh --run 'org.apache.doris.common.proc.CurrentQueryStatisticsProcDirTest,org.apache.doris.httpv2.rest.manager.QueryProfileActionTest': 13 passed, including Checkstyle. - Repository clang-format v16, check-format, build hygiene and git diff --check passed. Changed-line clang-tidy rechecks passed for the modified behavior; the unchanged main() complexity warning remains outside this scope. - SQL regression, real object-storage E2E, branch-4.1 builds and mixed-version execution were not run. ### Notes for Reviewers Operator guide: [Remote spill configuration and operational boundaries](https://github.com/mrhhsg/doris/blob/d4e2118b84082d937c6df929582680c4a8b5c63b/docs/remote-spill.md). This repository guide has not yet been published on the documentation website. Keep upload submission/completion accounting and the dropped-task completion guard together when backporting. Two concurrency tests use narrowly justified cognitive-complexity suppressions for GoogleTest macro expansion. The append-only current-query columns are intentional because old FE aggregators concatenate rows by position. ### Release note Move spill cleanup out of query teardown and isolate remote spill upload backpressure from unrelated uploads. Remote spill accepts spill_storage_type=remote while retaining the s3 alias; remote object keys no longer contain BE addresses. ### Check List (For Author) - Behavior changed: Yes (see Details) - Does this need documentation: Yes; docs/remote-spill.md added, website publication pending. - Pre-submission status: Partially verified: real object-storage E2E and target-branch mixed-version verification remain outstanding. ### AI Disclosure - Tool: Codex - Generated: Code, tests, documentation and this description. - Human review: Pending author review. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
