[feature](cloud) Spill to object storage in cloud mode and report the traffic in SHOW DATA by mrhhsg · Pull Request #68032 · apache/doris

mrhhsg · 2026-09-15T16:24:39Z

What problem does this PR solve?

Issue Number: None

Problem Summary:

Cloud-mode BEs could only spill to local disks. This PR lets query spill go to an S3 storage vault, selected in be.conf (spill_storage_type = local | s3), while keeping the existing spill operators and part-file format. Each BE reports the bytes of spill objects it currently holds in its disk report, and SHOW DATA lists the sum. This is a best-effort, BE-reported value, not a durable source for exact billing; see Known limitations.

BE: where spill goes

spill_storage_type = s3 is accepted only in cloud mode; SpillFileManager::init() validates it and builds the spill stores itself (the remote store for query spill plus the local roots that external writers such as Paimon JNI still need). The store is split into SpillDataDir (base), LocalSpillDataDir and RemoteSpillDataDir.
Vault selection (RemoteSpillDataDir::select_vault_fs), without touching storage vault code:
1. spill_s3_storage_vault_id (a vault ID, not a name) when configured;
2. otherwise the instance's latest object storage (latest_fs(), set only when storage vaults are disabled);
3. otherwise the S3 vault with the largest numeric ID, i.e. the newest one: changing the default vault does not move spill, a newly added S3 vault receives new spill files.
Every spill file pins the file system it was created on; writing, reading, deletion and retried deletion all use it, so files already spilled stay on their vault when the selection changes.
Objects live under <vault prefix>/spill/{ip}_{port}/{query_id}/{spill file}/{part} (ip/port: the BE address and heartbeat port). A spill file deletes its prefix when the query releases it; failed deletions are retried by the GC thread within its time slice (spill_gc_work_time_ms), at least one per pass and rotating so that one slow failing prefix does not starve the others. stop() and the manager destructor (reached only when the BE exits through ExecEnv::destroy(), e.g. with enable_graceful_exit_check) retry local directories only; remote prefixes still pending at exit are left to the lifecycle rule. What a crashed BE leaves behind is left to a bucket lifecycle rule on <vault prefix>/spill/ (plus AbortIncompleteMultipartUpload). No meta-service RPC or recycler change.

BE: writing and reading

SpillFileWriter/SpillFileReader use the existing S3FileWriter/S3FileReader; all spill-specific logic is in the caller. This PR does not modify S3FileWriter or add an abort interface.
spill_storage_limit_bytes (new, mutable, 0 = no byte limit) caps every spill store separately: for each local path the limit is the smaller of it and spill_storage_limit (which still applies), and it is the only limit of the S3 store.
Upload back-pressure: S3FileWriter splits appended data into upload buffers and queues full ones without waiting, so before appending each serialized block the writer waits (checking query cancellation) while the BE's S3 upload queue holds at least spill_s3_max_inflight_upload_bytes of buffers (get_queue_size() * s3_write_buffer_size). It is a back-pressure threshold, not a hard memory limit; it is not based on the S3 buffer tracker because that counts the partially filled buffers of every open writer and partitioned spill would deadlock on it.
Parts are closed synchronously. A failed write makes the writer fail permanently: close() drops the current part instead of finishing the file. Failed or discarded parts drain in-flight uploads through the existing writer destructor before prefix cleanup; they are not closed just to publish a partial file. Doris does not actively abort their multipart uploads; these are reclaimed by the bucket lifecycle rule.
Reads coalesce adjacent blocks into one GET of at most spill_s3_read_coalesce_bytes (further capped by the query's spill buffer size).
Statistics: the spill layer records successful upload bytes (from its maintained part size including the footer), upload time and upload-queue wait time, without inspecting S3 writer upload IDs or completed upload parts. Write-request estimates and their profile/IOContext/global counters are not collected. Read GET counts remain. bvar adders spill_remote_{read,write}_bytes / spill_remote_get_requests remain (rates are left to Prometheus/Grafana); TQueryStatistics gains spill_write_bytes_to_remote_storage / spill_read_bytes_from_remote_storage.
Lifecycle logs are emitted per logical SpillFile, not per block or S3 UploadPart: successful create/close/cleanup at INFO and failures at WARNING, with query ID, local/remote storage, pinned filesystem ID, local path or full s3://bucket/prefix/... URI, bytes and completed spill-part count. Logical creation does not imply close or physical publication. Failed deletion retains each file identity across manager queue merging; retry warnings are rate-limited, and eventual success is logged for each logical file.
Per-store, path-labelled counters spill_file_created_total / spill_file_deleted_total count logical files created / successfully cleaned up. Multiple parts still count as one logical file; an unwritten file has a successful no-op cleanup. Failed delete attempts are not successful deletions, and explicit GC plus destruction cannot count twice. These process-local totals exclude native external-writer files and pre-restart residue. Remote deletion means completed-object prefix cleanup, not incomplete MPU reclamation.
RuntimeState releases the operator local states before its object pool, which owns the profiles whose counters those states hold (an unclosed spill writer used a freed timer).

FE: SHOW DATA

The cloud disk report carries TReportRequest.remote_spill_bytes. ReportHandler keeps it in the master's memory on Backend (no edit log, like disk capacity); it is reset when a heartbeat shows the BE restarted.
In cloud mode the two warehouse-level listings (entire_warehouse without db_names, and SHOW DATA with no database selected) are forwarded to the master (FORWARD_NO_SYNC, as SHOW DATA SKEW), which sums the value over alive BEs, and get a __remote_spill__ row (counted in the total). Database and table views, the db_names form and non-cloud deployments are served locally as before.

show data properties("entire_warehouse"="true"), before:

+------------------+-------------+-------------+------------+
| DBName           | DataSize    | RecycleSize | BinlogSize |
+------------------+-------------+-------------+------------+
| db1              | 10737418240 | 0           | 0          |
| db2              | 5368709120  | 1048576     | 0          |
| total            | 16106127360 | 1048576     | 0          |
+------------------+-------------+-------------+------------+

after (cloud mode, no db_names):

+------------------+-------------+-------------+------------+
| DBName           | DataSize    | RecycleSize | BinlogSize |
+------------------+-------------+-------------+------------+
| db1              | 10737418240 | 0           | 0          |
| db2              | 5368709120  | 1048576     | 0          |
| __remote_spill__ | 2147483648  | 0           | 0          |
| total            | 18253611008 | 1048576     | 0          |
+------------------+-------------+-------------+------------+

show data with no database selected, before:

+-------+------------------+-------------+------------+------------+-------------+-------------------+
| DbId  | DbName           | Size        | RemoteSize | BinlogSize | RecycleSize | RecycleRemoteSize |
+-------+------------------+-------------+------------+------------+-------------+-------------------+
| 10001 | db1              | 10737418240 | 0          | 0          | 0           | 0                 |
| 10002 | db2              | 5368709120  | 0          | 0          | 1048576     | 0                 |
| Total | NULL             | 16106127360 | 0          | 0          | 1048576     | 0                 |
+-------+------------------+-------------+------------+------------+-------------+-------------------+

after (cloud mode):

+-------+------------------+-------------+------------+------------+-------------+-------------------+
| DbId  | DbName           | Size        | RemoteSize | BinlogSize | RecycleSize | RecycleRemoteSize |
+-------+------------------+-------------+------------+------------+-------------+-------------------+
| 10001 | db1              | 10737418240 | 0          | 0          | 0           | 0                 |
| 10002 | db2              | 5368709120  | 0          | 0          | 1048576     | 0                 |
| NULL  | __remote_spill__ | 2147483648  | 0          | 0          | 0           | 0                 |
| Total | NULL             | 18253611008 | 0          | 0          | 1048576     | 0                 |
+-------+------------------+-------------+------------+------------+-------------+-------------------+

FE: audit

The audit log (fe.audit.log and __internal_schema.audit_log) gains spill_write_bytes_to_remote_storage / spill_read_bytes_from_remote_storage. The FE keeps the dispatched BE IDs across post-dispatch errors and forwarded cancellation and waits for their final TQueryStatistics reports before logging, with the existing bounded fallback.
During a follower-first upgrade the audit table may still lack the two columns; the stream load then projects them out of the batch at send time. Failure handling of audit loads is unchanged (a failed batch is discarded, as before).

Known limitations / follow-up items

Incomplete multipart uploads: enabling S3 spill requires a bucket lifecycle rule with AbortIncompleteMultipartUpload covering <vault prefix>/spill/, in addition to object expiration for crash residue. Ordinary object expiration and prefix deletion do not reclaim incomplete MPU parts. These parts remain billed until lifecycle cleanup and are not included in the best-effort completed-object bytes reported by SHOW DATA.
Crash-restart accounting: the reported bytes are process-local. Objects left by an abrupt BE exit are not rediscovered after restart, so SHOW DATA can report a low value until the lifecycle rule removes them. Review thread
After a master switch, a BE's value is missing until its next disk report.

Release note

Cloud mode: query spill can be written to an S3 storage vault (spill_storage_type = s3, spill_s3_storage_vault_id in be.conf); spill_storage_limit_bytes caps every spill store; SHOW DATA lists the remote spill object bytes in a __remote_spill__ row included in the total; the audit log records spill bytes written to and read from object storage per query.

Spill files have query-correlated lifecycle logs and per-store spill_file_created_total / spill_file_deleted_total logical-file counters. Spill upload byte/time statistics remain; inferred S3 write-request counts are not collected.

Check List (For Author)

Test
- Regression test
- Unit Test
  - Pre-rebase statistics/lifecycle follow-up: ASAN ./run-be-ut.sh --run --filter='SpillFileS3Test.*:SpillFileTest.*:S3FileWriterTest.*' -j32 passed (96 tests). Covers local/remote lifecycles, logical-vs-part counts, create/close failures, idempotent cleanup, both directory-merge directions, rate-limited retry warnings and eventual success, plus the existing read/write/cancellation/accounting cases. Real provider lifecycle timing and SQL regression were not exercised by this follow-up.
  - After rebasing on master 85adcce350b: ./run-fe-ut.sh --run org.apache.doris.nereids.trees.plans.commands.ShowDataCommandTest passed (12/12); ./build.sh --fe passed with Checkstyle; BE ASAN compilation reached the final link, but ./build.sh --be failed there because liblance_c.a and libpaimon_c.a both define rust_eh_personality. BE UT and SQL regression have not been rerun on this rebased head.
- Manual test (add detailed scripts or steps below)
- No need to test or manual test. Explain why:
  - This is a refactor/code format and no logic has been changed.
  - Previous test can cover this change.
  - No code files have been changed.
  - Other reason
Behavior changed:
- No.
- Yes. New be.conf options (spill_storage_type, spill_s3_storage_vault_id, spill_storage_limit_bytes, spill_s3_max_inflight_upload_bytes, spill_s3_read_coalesce_bytes); the two warehouse-level SHOW DATA listings are forwarded to the master in cloud mode and list a __remote_spill__ row; two new audit log columns.
Does this need documentation?
- No.
- Yes.

Check List (For Reviewer who merge this PR)

Confirm the release note
Confirm test cases
Confirm document
Add branch pick label

https://claude.ai/code/session_01FvMyoceJUUqSBddwautKSM

hello-stephen · 2026-09-15T16:24:44Z

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

What problem was fixed (it's best to include specific error reporting information). How it was fixed.
Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
What features were added. Why was this function added?
Which code was refactored and why was this part of the code refactored?
Which functions were optimized and what is the difference before and after the optimization?

mrhhsg · 2026-09-15T16:26:02Z

/review

github-actions

Static verdict: REQUEST_CHANGES. The S3-backed spill path and warehouse-level SHOW DATA plumbing are present, but 16 blocking correctness and operability issues remain. The highest risks are cross-BE deletion caused by a shared identity, non-durable and non-idempotent lifetime statistics, cleanup paths that can delete live data or leak billable storage, request counters that do not measure physical provider traffic, and unbounded recovery/metadata scans.

Critical checkpoints (Part 1.3):

Goal and proof: The goal is remote S3 spill with capacity control, request/byte accounting, persisted warehouse totals, cleanup, and SHOW DATA exposure. Happy-path BE/cloud/FE/regression tests were added, but the goal is not correctly achieved because identity, durability, cleanup, retry, and accounting failures remain.
Scope and focus: The 47-file change is cohesive around that feature, and the cross-layer breadth is necessary, but the accepted defects mean it is not yet review-ready.
Concurrency: I traced the spill-GC thread, writer/upload pools, non-blocking close, upload-budget mutex/condition variable, atomic counters, heartbeat mutation, and FDB transactions. Reachable upload callbacks are balanced and I found no separate deadlock, but release builds mask an underflow invariant (MMF-10), and transaction serialization does not fence stale generations (MMF-9).
Lifecycle and SIOF: No cross-TU static-initialization dependency was found. Process boot, query/file teardown, startup cleanup, graceful/abrupt shutdown, vault binding, and multipart ownership were traced; MMF-4, MMF-6, MMF-7, MMF-8, MMF-11, MMF-12, and MMF-13 show lifecycle failures.
Configuration: New mutable storage, upload-budget, and recycler controls are observed online, but negative storage limits disable the cap (MMF-3), non-positive TTLs can select live objects (MMF-7), and runtime accepts/persists an upload budget that startup rejects (MMF-15).
Compatibility: The protobuf additions are additive, SHOW DATA row width is consistent, non-cloud mode avoids the RPC, and the repository's supported latest-MS/older-FE direction remains compatible. The FE-first/older-MS hypothesis was dismissed under that explicit contract.
Parallel paths: Local and S3 writer paths, PutObject/UploadPart, data/footer reads, synchronous/non-blocking close, and hooks with no stats sink were checked. Local behavior remains balanced, but both read and write request accounting miss physical retries/failures (MMF-5, MMF-16).
Special conditions: Remote-only selection, default-vault lookup, positive-limit gates, age cutoffs, and release underflow handling were checked. The non-positive and defensive-continue conditions are not safe (MMF-3, MMF-7, MMF-10, MMF-15).
Test coverage: Ordinary round trips, rotation, cleanup retries, capacity concurrency, budget blocking/cancellation, several upload failures, meta-service aggregation, and SHOW DATA shape are covered. Missing negative coverage includes two live BEs sharing an identity, reordered/stale reports, crash before report, live identity migration, real expiry/liveness, abort failure/restart residue, paginated cleanup, churn cardinality, invalid dynamic updates, and SDK/GET retries.
Test results: The changed expected SHOW DATA output is internally consistent with the five-column metadata. Per the review-only contract, I did not run builds or tests; author/CI results were not independently verified here.
Observability: Profiles, workload counters, BE metrics, logs, and meta-service bvars were added, but the report RPC omits its FDB read metrics (MMF-1), physical S3 attempts are undercounted (MMF-5/MMF-16), and the persisted total itself is not reliable across crashes or identity transitions.
Transactions and persistence: The FDB read-modify-write is atomic per attempt, but unequal boot IDs are not an ordering fence (MMF-9), process counters lose unreported crash suffixes (MMF-6), and retired identity records are never compacted (MMF-14). No FE EditLog change is involved.
Data writes and crash behavior: S3 multipart creation/upload/abort, visible-object deletion, capacity reservation, and stats writes were traced. Crashes can lose totals and multipart ownership, deletion failure releases live capacity, and cleanup can delete another live BE or a long-running query's data (MMF-4/MMF-6/MMF-7/MMF-8/MMF-11).
FE/BE variable passing: No new scattered FE-to-BE session variable path was introduced; the new MS/FE protobuf and five-column mapping are consistent. The existing heartbeat cloud identity is nevertheless shared and mutable in ways incompatible with its new use as storage/stats identity (MMF-4/MMF-13).
Performance: Startup recursively materializes the entire spill namespace outside its work budget (MMF-12), and SHOW DATA range-reads every historical backend identity without pagination or compaction (MMF-14).
Other issues: Three capped normal/risk convergence rounds found no additional distinct issue after the 16 inline findings. Two hypotheses were dismissed with code evidence: dynamic default-vault rebinding is not an explicit contract, and FE-first upgrade is outside the documented MS-first rollout direction.

Focus response: no additional user-provided review focus was supplied; full-scope review was performed.

mrhhsg · 2026-09-16T03:46:08Z

Thanks for the review. Pushed a follow-up commit addressing it; per finding:

Fixed

meta_service.cpp RPC_PREPROCESS: now (report_spill_stats, get, put) with the get counter/byte bvars.
MetaServiceProxy.getSpillStats: goes through executeWithMetrics (standard retry / reconnect / MS_TOO_BUSY handling).
spill_s3_storage_limit_bytes: validator rejects negative values; spill_s3_max_inflight_upload_bytes: validator rejects <= 0, and the startup check for < 2 * s3_write_buffer_size now matches the runtime path (warning + degraded one-buffer-at-a-time mode) instead of being fatal.
Shared identity (spill_file_manager.cpp / cloud_meta_mgr.cpp): you are right that cloud_unique_id is shared by every BE added in one statement. Both the object root and the stats key now use the FE-assigned backend_id (spill/{backend_id}/data/{boot_id}/..., stats/{instance}/spill/{backend_id}); SpillStatsPB.backend_id added, cloud_unique_id kept as an informational field. A live BE whose id changes (DROP + ADD) keeps the id it was bound with and logs a warning; the new id takes effect at restart.
Stale reports (meta_service.cpp fold): a report whose boot_id is older than the recorded one is rejected (INVALID_ARGUMENT) instead of being folded again; a same-boot report never rolls totals back (max). Test covers old-after-new delivery.
Startup cleanup (spill_file_manager.cpp list): each boot now writes a small marker spill/{backend_id}/boots/{boot_id} (refreshed daily); cleanup lists only that directory and deletes one old generation per GC round, so the listing is bounded by the number of boot generations, not by the number of spill objects.
spill_remote_upload_budget.cpp: DORIS_CHECK_GE, no clamp.
recycler.cpp: spill_objects_expire_time_second <= 0 skips the task with a warning (test added); config comment documents the "must exceed the longest query" contract.
Proto comment: reporting is documented as best-effort (a crash loses at most one reporting interval; graceful shutdown reports after all tasks are done).

Kept as designed (happy to discuss)

Request counters (spill_file_reader.cpp, s3_file_writer.cpp): they count logical SDK calls, not physical retries. Attempt-level accounting needs a per-request hook inside the SDK retry strategy (S3CustomRetryStrategy is process-global with no request context); the byte counters, which are the primary billing input, are exact. Documented as the counting unit; can be a follow-up.
Durable per-upload checkpoint (spill_file_manager.cpp counters): writing to meta-service on every upload is not worth the cost; the loss bound is one reporting interval and only on crash.
Capacity release on failed delete (spill_file.cpp): same behaviour as the local spill path; retained objects are reclaimed by the query-directory retry and the recycler.
Incomplete multipart uploads (spill_file_writer.cpp abort): failed uploads are aborted; residue of a crashed process is invisible to any prefix listing, and neither the BE object client nor the recycler has ListMultipartUploads. The bucket-level AbortIncompleteMultipartUpload rule is the intended safety net (now stated in the config comments).
Stats cardinality (get_spill_stats): with backend_id the family grows with "backends ever created" rather than with restarts. Folding retired backends on DROP BACKEND is a reasonable follow-up.
Recycler liveness fence: the TTL is documented as "larger than the longest query the cluster allows"; a query holding spill for more than 7 days is outside what this PR targets.

mrhhsg · 2026-09-17T07:55:10Z

Local pipeline review — ✅ PASS

schema: doris-repo-review/v1
status: PASS
pr: apache/doris#68032
commit: 5d93bb00ab769d313f5868a764aafc3aa77c9865
base: 3c489e297d95191c26e7259a9939c2fabb3510cb
reviewed_at: 2026-09-17T15:55+08:00
reviewer: mrhhsg
model: gpt-6-astra
effort: xhigh
findings: {blocker: 0, major: 0, minor: 3, nit: 1}
rounds: 1
converged: true

Notes for maintainers

本次为只读静态评审，覆盖 59/59 个变更文件；未运行构建、UT、回归或真实对象存储验证。
既有 inline threads 已读取并去重；本收据不表示这些历史问题已解决，也不替代人工 Apache approval。

_{Reviewed locally with the doris-repo-review pipeline. Repository policy may accept this receipt for the matching commit; it is not a human Apache approval.}

mrhhsg · 2026-09-24T08:04:02Z

/review-light

github-actions

Request changes. Static review of exact head 71a21e2af8850b8467c997d9f0adef13cdf8d010 found nine distinct issues: five P1 correctness/scalability issues and four P2 observability/freshness gaps. Existing inline threads were treated as hard duplicate fences.

Critical-checkpoint conclusions:

Goal and tests: the PR adds cloud S3 spill storage, lifecycle cleanup, accounting/billing, and remote-spill observability. It has substantial BE, cloud, FE, and regression coverage, but the goal is not safely complete because shared-vault ownership, deleted-instance cleanup, terminal write-error propagation, error-unwind accounting, and recycler scalability are not covered.
Scope: the 62-file change is broad but cohesive. The new storage path itself is understandable; parallel consumer and producer paths were missed (MAIN-3, MAIN-5, MAIN-7, MAIN-9).
Concurrency and lifecycle: I traced GC, recycler, scheduled FE polling, async S3 upload callbacks, upload-budget admission/release, and RuntimeState/QueryContext teardown. The ordinary gate/callback branches and lock ordering appear balanced with no new deadlock found, but MAIN-1, MAIN-2, MAIN-4, and MAIN-9 expose cross-instance, teardown, or terminal-close lifecycle failures.
Configuration and compatibility: mutable limits, upload budget, heartbeat/TTL, poll interval, and max age are read on live paths; startup/dynamic parity already has an existing review thread. Appended optional Thrift/protobuf fields and name-mapped audit columns preserve rolling compatibility. MAIN-8 violates the configured freshness guarantee after a wall-clock rollback.
Conditions and parallel paths: TTL filtering is the direct trigger for MAIN-2. The local-spill analogues exposed missing live-stat aggregation, Iceberg counter registration, HTTP-plan audit initialization, and Iceberg terminal-close handling.
Tests/results: added tests cover core S3 writer/reader/quota behavior, cloud expiry, FE polling/audit aggregation, and SHOW DATA, and the checked-in audit schema output is consistent. Missing negative coverage is called out inline: shared instances/endpoints, default-TTL instance deletion, repartition error unwinding, multi-page shared-vault scans, Iceberg S3 close failures/counters, live consumers, HTTP-plan audit output, and clock rollback. No build or test was run for this review, as required by the task.
Observability: the PR adds useful counters, metrics, audit fields, and logs, but MAIN-3, MAIN-5, and MAIN-7 leave supported paths invisible or report sentinel values.
Persistence and data writes: no new FE EditLog path is involved. Remote multipart writes and cloud recycler state were traced through failure/cleanup boundaries; MAIN-2 loses the durable cleanup owner, MAIN-4 loses the accounting owner, and MAIN-9 suppresses a terminal write failure after intermediate inputs may already be deleted. Crash-before-report, failed delete, multipart residue, retry-attempt counting, and heartbeat deletion races are already covered by existing threads and were not duplicated.
FE/BE contracts and performance: the new optional wire fields are compatible, but live consumers are incomplete (MAIN-3). MAIN-6 adds repeated unbounded N * M object listings on shared vaults. The fixed 64-KiB footer probe is bounded and was not considered a separate high-value issue.
Other checkpoints: no new nullable-column, visible-version, MoW, static-initialization, or schema-format issue applies here. Error handling and ownership were rechecked; MAIN-4 and MAIN-9 are the remaining ownership/error-propagation defects.

The focus file contains only -light and names no subsystem; I kept the presentation concise but completed the full review. The final BE pass found MAIN-9 at the three-round cap, so this review is capped/incomplete rather than converged. Validation is static only.

mrhhsg · 2026-09-28T03:55:23Z

/review-light

github-actions · 2026-09-28T03:57:10Z

Codex automated review failed and did not complete.

Error: {"type":"error","status":400,"error":{"type":"invalid_request_error","message":"The 'gpt-6-sol' model is not supported when using Codex with a ChatGPT account."}}
Workflow run: https://github.com/apache/doris/actions/runs/36375600242

Please inspect the workflow logs and rerun the review after the underlying issue is resolved.

…ceberg spill writers explicitly ### What problem does this PR solve? Issue Number: None Related PR: apache#68032 Problem Summary: Follow-ups of the review of remote (object storage) spill. - A SpillFileWriter can outlive its SpillFile: the repartitioner keeps its output writers while the output files live in the caller, so an error after setup_output() destroys the files first. Closing the writer later force-charged a footer to no owner (leaking spill capacity) and, on object storage, could publish a part after the file's prefix was deleted. SpillFile::gc() now discards its active writer first: the unfinished part is dropped without footer or close(), in-flight uploads are drained, the budget is reconciled and the multipart upload is aborted. A writer destroyed after a failed close() unregisters itself so the file never reaches a dangling writer. - The Iceberg sort writer relied on the writer destructor to close its spill files, which only logs a failure. On object storage close() uploads the tail of the file, so a failure could silently lose rows, and the intermediate merge deleted its inputs before its output was complete. Both paths now close the writer and return its status. - The Iceberg spill sink registers the remote spill counters, so its profile shows remote requests, bytes and times like the other spilling operators. - The remote spill bytes are aggregated by ProfileManager and shown by the query profile REST API and show proc '/current_queries'; the HTTP query plan audit event sets them to 0 instead of the -1 default. - RemoteSpillStatsPoller measures the age of the polled value on the monotonic clock, so a wall clock moved backward cannot keep a stale value fresh. - Document that the remote request counters count logical requests: retries inside the object storage client are not included. ### Release note None ### Check List (For Author) - Test: Unit Test - BE: SpillFileS3Test (new SpillFileDestroyedBeforeWriterDiscardsPart, FailedCloseThenSpillFileDestroyed), SpillFileTest, SpillRepartitionerTest, partitioned and spill sort operators, SpillIcebergTableSinkOperatorTest, VIcebergPartitionWriterTest (213 passed) - FE: RemoteSpillStatsPollerTest, CurrentQueryStatisticsProcDirTest, AuditLoaderTest - Behavior changed: Yes. show proc '/current_queries' has two more columns, SpillWriteBytesToRemoteStorage and SpillReadBytesFromRemoteStorage. - Does this need documentation: No Claude-Session: https://claude.ai/code/session_01B5f3dfnSMXhasgsShyagfR

mrhhsg · 2026-09-28T07:00:23Z

/review-light

github-actions

Static review complete on head bdd4fad. I request changes for six distinct issues: default-vault rotation (P1), billing of buffered bytes, footer read cap, mixed-version current-query rows, completed-query audit stats, and a fresh total that omits a BRPC-reachable BE (P2 each). Inline comments give the reproductions and suggested fixes. Existing threads were treated as duplicate fences. The requested focus was light; the complete changed-file review found no other focus-specific concern. No build or test was run in this review environment.

Critical checkpoint conclusions:

Goal and proof: Cloud S3 spill and reporting are wired end to end and have BE unit, FE unit, and regression coverage. Those tests do not prove the six state transitions above, so the goal is not fully met.
Scope: The BE/FE/protocol/test changes are connected to this feature; no unrelated source change was identified.
Concurrency: Async upload callbacks, the spill GC thread, and FE polling introduce concurrency. Traced budget and pending-delete locks; heavy S3/RPC work is outside those locks. No separate lock-order deadlock was substantiated.
Lifecycle: Writer close, abort, query teardown, runtime-state destruction, and GC were traced. No further lifecycle issue survived, but file-system selection must be pinned per file across default-vault rotation (M1). No cross-TU static initialization dependency was found.
Configuration: New settings are validated or refreshed where intended. M1 ignores a changed default vault, and M3 exceeds the configured effective read cap.
Compatibility: Optional BE resource/Thrift fields support older peers; an older BE cannot create this S3 spill. The all-FE HTTP response is incompatible during a rolling FE upgrade (M4).
Parallel paths: Local and Iceberg spill paths were checked. Live statistics, completion audit, and warehouse totals need consistent remote data; M5 affects the final audit path.
Conditions: Capacity, cancellation, readiness, and polling branches were traced. Skipping heartbeat-dead but BRPC-reachable BEs causes M6.
Test coverage: Existing round-trip, failure, cleanup, limit, query-stat, schema, and SHOW DATA cases cover normal paths; add the six focused reproductions from the inline comments. Complex BE logic has unit tests, but these transitions lack them.
Test results: The changed audit expected-output rows match the added schema columns on static inspection. No test result was independently generated or executed here.
Observability: New remote byte/request counters, metrics, logs, and audit fields cover major paths. M5 leaves persisted audit fields misleading when the final report is late.
Persistence: No new EditLog transaction path or meta-service spill-stat record is in this diff. The documented crash-residue limitation is already covered by prior review context; M5 affects persisted audit rows.
Writes and crashes: S3 close/abort, GC retry, and budget reconciliation were traced without another substantiated issue. M2 reports in-memory reservations as stored object bytes.
FE/BE values: New Thrift and resource response fields are propagated for live query stats and billing; final PQueryStatistics lacks the remote pair needed for M5.
Performance: Upload admission and read coalescing bound normal work; M3 defeats the footer read bound under a small configured cap. No additional concrete performance issue survived.
Other concerns: All 63 changed paths and six candidate findings were reswept. The second normal and focused review round found no new valuable findings; this review is complete.

ignored

### What problem does this PR solve? Issue Number: None Related PR: apache#68032 Problem Summary: When reading a remote spill part, the footer is fetched by one probe of the part's tail. The probe was always min(part size, 64 KiB), even when spill_s3_read_coalesce_bytes (or the query's spill buffer size) was smaller. So a 1 KiB setting still issued a 40-64 KiB GET and allocated a buffer of that size, which breaks the documented per-GET bound. The probe is now bounded by the effective coalesced read size as well, but it is never smaller than the 16-byte tail. If the offsets array does not fit, it is fetched separately, as before. Also document that an empty spill_s3_storage_vault is resolved to the default vault when the BE spills for the first time. A later SET DEFAULT STORAGE VAULT applies to spill after the BE restarts. Also document that the rest of an offset array that does not fit in the probe is read whole, in the same way as a single block that is larger than the read size. ### Release note None ### Check List (For Author) - Test: Unit Test - SpillFileS3Test.ReadCoalescesAdjacentBlocks now expects the probe to be capped by a 1 KiB and a 40 KiB window; new FooterLargerThanProbe covers a window smaller than the block offset array (probe, then one GET for the rest of the footer); SpillFile* (63 tests) passed - Behavior changed: No - Does this need documentation: No Claude-Session: https://claude.ai/code/session_01B5f3dfnSMXhasgsShyagfR

mrhhsg · 2026-09-30T00:50:31Z

run buildall

…schema projection ### What problem does this PR solve? Issue Number: None Related PR: apache#68032 Problem Summary: An earlier commit of this PR made the audit loader keep a failed batch and retry it until it loaded, sent it with max_filter_ratio 0, and appended a 32-character UUID to the stream load label. Three regressions followed. A single row filtered by BE (for example an over-long field) made every retry fail, so the FE stopped loading audit rows for good. After an ambiguous transport failure the batch kept its label, and a later "Label Already Exists"/FINISHED reply reset the buffer without checking the row count, dropping rows assembled after the first attempt. With a long FE identity (FQDN plus port) the label exceeded the 128-character limit and every load was rejected. Restore the master behavior: max_filter_ratio 1.0, the original label, and discarding the batch on failure. Keep only the send-time projection that drops the two remote spill columns while the audit table still has its old schema, which is what a follower-first upgrade needs. ### Release note None ### Check List (For Author) - Test: Unit Test (AuditLoaderTest, AuditStreamLoaderTest: 9 tests); FE checkstyle - Behavior changed: No (audit loading failure handling matches master again) - Does this need documentation: No Claude-Session: https://claude.ai/code/session_01FvMyoceJUUqSBddwautKSM

… correct stale spill comments ### What problem does this PR solve? Issue Number: None Related PR: apache#68032 Problem Summary: Follow-ups from the local review of the spill-to-S3 changes. The audit projection test only checked the field count, the stmt position and that the query ids appeared, so dropping the wrong two columns would still pass; it now gives the neighbouring columns distinct values and checks every kept column against the full row by name. Three upload-queue tests declared their teardown Defer before the ScopedThreads, so the threads were joined before the Defer released the blocked uploads and a failing assertion hung the test binary; the Defer now follows the threads. Several comments described earlier revisions or were wrong: the lifecycle rule prefix (keys are under <vault prefix>/spill/), spill_s3_max_inflight_upload_bytes described as a memory bound instead of a back-pressure threshold, the SpillDataDir/writer/reader contract that all IO goes through fs() (each file uses the fs pinned in its SpillFile), the thread a synchronous close runs on, the persisted-bytes and store-view comments, spill_storage_limit not applying to the S3 store, and the destroy-order reason in ExecEnv. ### Release note None ### Check List (For Author) - Test: Unit Test (SpillFileS3Test + SpillFileTest: 73 tests; AuditLoaderTest + AuditStreamLoaderTest: 9 tests); clang-format 16, check-build-hygiene, FE checkstyle - Behavior changed: No - Does this need documentation: No Claude-Session: https://claude.ai/code/session_01FvMyoceJUUqSBddwautKSM

### What problem does this PR solve? Issue Number: None Related PR: apache#68032 Problem Summary: SpillFileWriter::_append_to_part sliced every serialized block into s3_write_buffer_size pieces and waited for the S3 upload queue before each piece. S3FileWriter already splits appended data into upload buffers, so the slicing only made the back-pressure check run once per buffer. The block is already in memory when it is appended, and what the back-pressure has to bound is the rate of later appends, so the queue is now checked once per append and the whole block is handed to S3FileWriter. A writer may queue the buffers of one block after the check; the config and class comments say so, and the upload-queue test allows the at most two buffers a test block can fill. ### Release note None ### Check List (For Author) - Test: Unit Test (run-be-ut.sh --filter='SpillFile*:SpillDataDir*', 73 tests passed) - Behavior changed: No - Does this need documentation: No Claude-Session: https://claude.ai/code/session_01FvMyoceJUUqSBddwautKSM

### What problem does this PR solve? Issue Number: None Related PR: apache#68032 Problem Summary: Without spill_s3_storage_vault_id, an instance with storage vaults spilled to the S3 vault with the smallest ID, so spill stayed on the oldest vault forever. It now uses the S3 vault with the largest ID, the newest one: changing the default vault still does not move spill, and an added S3 vault receives new spill files. Files already spilled keep the file system they were created on and are read and deleted there. ### Release note None ### Check List (For Author) - Test: Unit Test (run-be-ut.sh --filter='SpillFile*:SpillDataDir*', 73 tests passed, SelectVaultFileSystem covers the numeric comparison and a newly added vault) - Behavior changed: Yes (automatic vault selection picks the largest S3 vault ID) - Does this need documentation: No Claude-Session: https://claude.ai/code/session_01FvMyoceJUUqSBddwautKSM

… time ### What problem does this PR solve? Issue Number: None Related PR: apache#68032 Problem Summary: A spill directory whose deletion failed is queued and retried by the spill GC. Every round retried the whole queue before spill_gc_work_time_ms was checked, and stop() and the destructor retried it again. With an unavailable object store each remote deletion waits for a request timeout, so one GC round, and BE shutdown, could take one timeout per queued directory. Retries now stop once the time slice is used up (at least one directory is tried per round); the directories not tried are queued again ahead of the ones that just failed, so one slow failing directory cannot starve the others. stop() and the destructor are bounded by spill_gc_work_time_ms the same way. Remote objects left at shutdown are removed by the lifecycle rule of the bucket, local directories are moved to the GC root after restart. ### Release note None ### Check List (For Author) - Test: Unit Test (run-be-ut.sh --filter='SpillFile*:SpillDataDir*:*Spill*Manager*', 74 tests passed; new PendingRemoteDeletesAreBoundedByGcWorkTime uses a slow mock object store where one directory keeps failing, and fails when the queue order is not rotated) - Behavior changed: No - Does this need documentation: No Claude-Session: https://claude.ai/code/session_01FvMyoceJUUqSBddwautKSM

### What problem does this PR solve? Issue Number: None Related PR: apache#68032 Problem Summary: stop() and the SpillFileManager destructor retried the queued failed spill deletions within spill_gc_work_time_ms, and their comments promised that bound. It did not hold for remote directories: at least one directory is always tried, and a failed remote deletion waits for the object store twice (the delete and the reconcile LIST), each with the client's retries, so an unavailable object store could still delay a graceful shutdown by minutes. Shutdown now retries only directories of local stores; remote ones stay queued for a later gc() and are otherwise removed by the lifecycle rule of the bucket, as before. The GC round keeps its time slice; its comment now says that a remote deletion can exceed it. The bounded-retry test also checks that stop() sends no delete request, and releases its spill files in order instead of relying on the element destruction order of vector::clear(). ### Release note None ### Check List (For Author) - Test: Unit Test (run-be-ut.sh --filter='SpillFile*:SpillDataDir*:*Spill*Manager*', 74 tests passed) - Behavior changed: No - Does this need documentation: No Claude-Session: https://claude.ai/code/session_01FvMyoceJUUqSBddwautKSM

…ctation ### What problem does this PR solve? Issue Number: None Related PR: apache#68032 Problem Summary: BE UT CI failed on two tests that are unrelated to each other but both broke when run together with the rest of the suite. SpillFileS3Test.SelectVaultFileSystem enumerates every registered S3 storage resource to pick the one with the largest ID; TabletCooldownTest registers a mock S3 storage resource (id 10000) and a storage policy in SetUpTestSuite but never removes them in TearDownTestSuite, so they leak into the process-global registry for the rest of the gtest binary and get picked up as a spurious "largest ID" vault by any later test, including this one. TabletCooldownTest's teardown now also deletes its storage resource and policy. PartitionedAggregationSinkOperatorTest.SinkWithSpilError asserted that close() still returns OK after a fault-injected spill write failure; this PR's SpillFileWriter intentionally made close() sticky after a failed write_block() (see its header comment), so close() now correctly surfaces that earlier failure. The test still encoded the old swallow-the-error behavior; it is updated to expect close() to fail. ### Release note None ### Check List (For Author) - Test: Unit Test (run-be-ut.sh --filter reproduced both failures standalone and together with TabletCooldownTest before the fix, and verified green after; a broader run across all spill/partitioned-aggregation/hash-join/sort BE UT suites, 175 tests, also passed) - Behavior changed: No - Does this need documentation: No

…SHOW DATA only for the cloud warehouse listings ### What problem does this PR solve? Issue Number: close #xxx Related PR: #xxx Problem Summary: Two review findings on the S3 spill path. 1. `SpillFileManager::_retry_pending_spill_directories` put every directory it did not get to (and every one that failed) back through `_add_pending_directory`, which scans the queue twice per call. Remote spill queues one prefix per spill file, so after an object store outage during a partitioned spill a time-sliced GC pass that tries one slow directory re-adds thousands of entries one by one, under the pending-directory mutex, on every pass. The kept entries were already merged with each other when they were queued; now they go back as one batch (untried ones first, then the failed ones, as before) and only the directories queued while the pass ran are merged through `_add_pending_directory`. 2. `ShowDataCommand.toRedirectStatus()` returned `FORWARD_NO_SYNC` for every form of SHOW DATA, although only the two cloud listings that show the `__remote_spill__` row (`entire_warehouse` without `db_names`, and `SHOW DATA` with no database selected) read the remote spill size that the BEs report to the master FE. `SHOW DATA FROM db.table`, the per-database listing, the `db_names` form and every non-cloud deployment are served locally again, as on master. ### Release note None ### Check List (For Author) - Test: Unit Test - BE: `SpillFileS3Test.PendingRemoteDeleteBacklogIsRequeuedInBulk` (new) with `SpillFileS3Test.*` and `SpillFileTest.*` (75 tests) - FE: `ShowDataCommandTest` (12 tests) - Behavior changed: No - Does this need documentation: No

…er::abort() ### What problem does this PR solve? Issue Number: None Related PR: apache#68032 Problem Summary: A spill part that fails or is discarded before it is published leaves the parts of its multipart upload in object storage. Deleting the spill prefix does not remove them (they are not listed as objects), so they stay billed until the bucket's lifecycle rule aborts the upload. S3FileWriter never aborts on its own: other writers' uploads may be completed elsewhere (the S3 committer completes them on FE), so destroying a writer must not change the provider state. The spill writer therefore aborted the upload itself, rebuilding the bucket, key and upload id from S3FileWriter's getters and calling the object storage client of the spill file system directly, and relied on destroying the writer first to make sure no part was uploaded after the abort. Add S3FileWriter::abort(), an explicit call for an owner that knows no one will complete the upload: it marks the writer failed so that queued buffers skip their upload, waits for the running uploads, closes the writer and aborts the multipart upload if one was started. It is allowed on an open writer or one whose close() failed, never runs on destruction, and does not change any existing caller. The spill writer calls it for a failed or discarded remote part, and its own MultipartUploadId and abort helper are removed. ### Release note None ### Check List (For Author) - Test: Unit Test - S3FileWriterTest.abort: abort after multipart uploads started (one abort, nothing completed, close() fails afterwards); abort while 10 uploads are blocked in the mock and 2 are queued (the abort request waits for the running uploads, the queued ones are skipped); abort below one upload buffer (no request); abort after a failed close() - Existing spill S3 tests that check the aborted uploads of failed and discarded parts - Behavior changed: No - Does this need documentation: No Claude-Session: https://claude.ai/code/session_01FvMyoceJUUqSBddwautKSM

### What problem does this PR solve? Issue Number: None Related PR: apache#68032 Problem Summary: Failed or discarded remote spill parts do not need a new S3FileWriter abort interface just to reclaim incomplete multipart uploads sooner. Remove that interface and the spill-side abort logic. Keep waiting for in-flight uploads through writer destruction before prefix cleanup, and retain write failures instead of publishing partial spill files. Incomplete multipart uploads are reclaimed by the operator-configured bucket lifecycle rule. Update the tests and document this requirement. ### Release note Incomplete multipart uploads from failed or cancelled S3 spills are left to the bucket's AbortIncompleteMultipartUpload lifecycle rule rather than being actively aborted by Doris. ### Check List (For Author) - Test: ASAN unit tests via ./run-be-ut.sh --run --filter='SpillFileS3Test.*:S3FileWriterTest.*' -j24 (47 passed); scoped clang-format/check-format, build hygiene and git diff --check passed - Behavior changed: Yes. Incomplete spill MPU storage is reclaimed by lifecycle cleanup instead of an explicit abort RPC. - Does this need documentation: Yes. Configuration comments and the existing PR description explain the lifecycle prerequisite.

### What problem does this PR solve? Issue Number: None Related PR: apache#68032 Problem Summary: Spill upload statistics inferred request counts by inspecting S3FileWriter upload IDs and completed parts. Remove those write-request estimates while keeping successful upload bytes, upload time and queue wait. Add spill-layer lifecycle logs for logical file creation, close and cleanup, with query identity, local/remote storage, pinned filesystem/path, bytes, part count and status. Preserve per-file context when failed deletions merge in the manager, rate-limit retry warnings, and log eventual success. Add per-store logical-file creation and successful-cleanup counters; failed attempts and repeated gc/destruction must not count as another deletion. The generic S3 writer and incomplete-MPU lifecycle policy are unchanged. ### Release note Spill files gain query-correlated lifecycle logs and per-store spill_file_created_total / spill_file_deleted_total counters. Remote spill write-request estimates are no longer collected; byte/time statistics and read request counters remain. ### Check List (For Author) - Test: ASAN Unit Test via ./run-be-ut.sh --run --filter='SpillFileS3Test.*:SpillFileTest.*:S3FileWriterTest.*' -j32 (96 passed, including 7 new tests); clang-format/check-format v16, build hygiene, changed-line clang-tidy and git diff --check passed - Behavior changed: Yes. Add per-logical-file logs/metrics and remove inferred write-request counters; no query-result or spill-format change. - Does this need documentation: Yes. Code comments and the existing PR description document log fields, logical-file counter scope and lifecycle cleanup boundaries.

### What problem does this PR solve? Issue Number: None Related PR: apache#68032 Problem Summary: The upstream SHOW DATA column is now named ChangelogSize, while two spill regression-test comments still described the former BinlogSize column. Align those comments with the rebased command metadata without changing test behavior. ### Release note None ### Check List (For Author) - Test: Unit Test: ShowDataCommandTest 12/12 passed; FE build and Checkstyle passed; BE ASAN compilation reached final link but failed on duplicate rust_eh_personality in liblance_c.a and libpaimon_c.a - Behavior changed: No - Does this need documentation: No

mrhhsg · 2026-10-21T15:57:57Z

run buildall

hello-stephen · 2026-10-21T20:58:47Z

Cloud UT Coverage Report

Increment line coverage 🎉

Increment coverage report
Complete coverage report

Category	Coverage
Function Coverage	77.99% (2080/2667)
Line Coverage	66.27% (38350/57865)
Region Coverage	53.66% (35965/67022)
Branch Coverage	57.14% (11579/20264)

hello-stephen · 2026-10-21T21:33:46Z

TPC-H: Total hot run time: 28373 ms

machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 2811649fa760e18cbcdd6e61e86e30ad607bdbba, data reload: false

------ Round 1 ----------------------------------
============================================
q1	17384	5255	5047	5047
q2	2141	316	265	265
q3	10177	1388	829	829
q4	4685	461	346	346
q5	7433	817	536	536
q6	184	183	151	151
q7	745	800	605	605
q8	9271	1388	999	999
q9	5511	4235	4229	4229
q10	6841	1316	1023	1023
q11	424	245	228	228
q12	645	396	275	275
q13	18101	3591	3013	3013
q14	270	272	246	246
q15	q16	739	728	653	653
q17	1186	758	576	576
q18	6995	6198	6159	6159
q19	1130	966	594	594
q20	374	339	239	239
q21	4903	2103	2268	2103
q22	356	301	257	257
Total cold run time: 99495 ms
Total hot run time: 28373 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	5623	5571	5629	5571
q2	703	592	523	523
q3	5222	5399	4880	4880
q4	2001	2150	1552	1552
q5	5279	5158	5542	5158
q6	262	204	157	157
q7	2310	2086	1897	1897
q8	3019	2707	2712	2707
q9	8084	7895	7923	7895
q10	3721	3668	3198	3198
q11	617	455	549	455
q12	720	693	536	536
q13	3673	3972	3262	3262
q14	334	335	293	293
q15	q16	709	705	660	660
q17	8222	7524	6900	6900
q18	13045	12233	13007	12233
q19	895	779	797	779
q20	2164	2154	1918	1918
q21	5709	4794	4710	4710
q22	494	458	414	414
Total cold run time: 72806 ms
Total hot run time: 65698 ms

hello-stephen · 2026-10-21T21:43:35Z

TPC-DS: Total hot run time: 150686 ms

machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 2811649fa760e18cbcdd6e61e86e30ad607bdbba, data reload: false

query5	4326	583	446	446
query6	442	214	193	193
query7	4811	453	227	227
query8	333	163	156	156
query9	8776	3105	3065	3065
query10	452	321	277	277
query11	6029	3910	3583	3583
query12	152	94	88	88
query13	1255	441	315	315
query14	6159	3977	3637	3637
query14_1	3399	3385	3431	3385
query15	214	204	183	183
query16	984	463	443	443
query17	879	669	536	536
query18	2420	429	316	316
query19	192	163	134	134
query20	83	84	84	84
query21	204	125	107	107
query22	12950	13074	12792	12792
query23	13540	13009	12367	12367
query23_1	12573	12617	12493	12493
query24	7376	837	464	464
query24_1	463	483	471	471
query25	551	454	360	360
query26	1266	264	144	144
query27	2755	437	277	277
query28	4639	1930	1910	1910
query29	1547	609	451	451
query30	299	215	186	186
query31	1020	933	812	812
query32	146	97	97	97
query33	539	302	249	249
query34	924	848	493	493
query35	743	745	656	656
query36	833	821	739	739
query37	116	99	94	94
query38	1790	1718	1665	1665
query39	795	783	755	755
query39_1	737	737	741	737
query40	231	127	97	97
query41	72	69	70	69
query42	89	84	82	82
query43	347	367	309	309
query44	1296	680	677	677
query45	180	174	162	162
query46	857	967	560	560
query47	2956	2662	2707	2662
query48	344	308	209	209
query49	568	399	290	290
query50	686	270	202	202
query51	10635	10627	10442	10442
query52	84	77	78	77
query53	183	205	147	147
query54	229	185	174	174
query55	67	69	64	64
query56	235	210	211	210
query57	1641	1604	1593	1593
query58	286	261	255	255
query59	2074	2113	1963	1963
query60	290	245	236	236
query61	148	147	148	147
query62	389	342	277	277
query63	187	152	156	152
query64	2775	946	792	792
query65	3311	3301	3244	3244
query66	1765	410	305	305
query67	20397	20159	19946	19946
query68	3002	994	611	611
query69	378	300	254	254
query70	855	761	754	754
query71	298	236	248	236
query72	2843	2601	2326	2326
query73	536	524	288	288
query74	4661	4505	4322	4322
query75	2332	2318	1900	1900
query76	2287	1013	590	590
query77	333	378	270	270
query78	9946	9723	9228	9228
query79	989	828	518	518
query80	1084	414	348	348
query81	575	310	285	285
query82	567	132	111	111
query83	297	192	186	186
query84	304	121	107	107
query85	866	428	371	371
query86	362	238	237	237
query87	2033	1959	1801	1801
query88	3651	2655	2631	2631
query89	321	283	240	240
query90	1844	164	163	163
query91	161	150	127	127
query92	91	86	88	86
query93	992	988	573	573
query94	582	318	266	266
query95	600	387	319	319
query96	665	485	241	241
query97	2428	2425	2353	2353
query98	160	155	151	151
query99	774	762	654	654
Total cold run time: 235669 ms
Total hot run time: 150686 ms

hello-stephen · 2026-10-21T21:48:30Z

ClickBench: Total hot run time: 25.34 s

machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 2811649fa760e18cbcdd6e61e86e30ad607bdbba, data reload: false

query1	0.01	0.01	0.01
query2	0.13	0.08	0.09
query3	0.36	0.14	0.13
query4	1.61	0.14	0.13
query5	0.27	0.25	0.25
query6	1.15	0.66	0.65
query7	0.04	0.01	0.01
query8	0.09	0.08	0.07
query9	0.44	0.37	0.35
query10	0.57	0.58	0.59
query11	0.32	0.19	0.19
query12	0.33	0.19	0.19
query13	0.51	0.54	0.53
query14	0.93	0.95	0.93
query15	0.66	0.58	0.57
query16	0.36	0.37	0.36
query17	1.05	1.01	1.02
query18	0.30	0.28	0.28
query19	1.99	1.91	1.89
query20	0.02	0.01	0.01
query21	15.40	0.35	0.30
query22	4.90	0.13	0.14
query23	15.89	0.49	0.29
query24	2.23	0.54	0.38
query25	0.15	0.11	0.10
query26	0.77	0.27	0.21
query27	0.11	0.10	0.10
query28	3.37	0.77	0.41
query29	12.58	4.27	3.40
query30	0.36	0.17	0.14
query31	2.76	0.56	0.33
query32	3.23	0.60	0.48
query33	2.91	3.11	3.00
query34	15.97	4.11	3.48
query35	3.38	3.39	3.40
query36	0.58	0.52	0.47
query37	0.12	0.09	0.09
query38	0.07	0.07	0.06
query39	0.06	0.06	0.05
query40	0.19	0.16	0.15
query41	0.11	0.05	0.05
query42	0.05	0.07	0.05
query43	0.07	0.07	0.06
Total cold run time: 96.4 s
Total hot run time: 25.34 s

hello-stephen · 2026-10-21T21:51:41Z

FE UT Coverage Report

Increment line coverage 86.71% (137/158) 🎉
Increment coverage report
Complete coverage report

hello-stephen · 2026-10-22T00:15:45Z

FE Regression Coverage Report

Increment line coverage 75.32% (119/158) 🎉
Increment coverage report
Complete coverage report

hello-stephen · 2026-10-22T00:26:42Z

BE Regression && UT Coverage Report

Increment line coverage 95.74% (45/47) 🎉

Increment coverage report
Complete coverage report

Category	Coverage
Function Coverage	77.22% (35929/46529)
Line Coverage	62.49% (405606/649037)
Region Coverage	59.02% (343243/581586)
Branch Coverage	59.94% (158587/264580)

mrhhsg · 2026-10-22T01:20:15Z

Local pipeline review — ✅ PASS

schema: doris-repo-review/v1
status: PASS
pr: apache/doris#68032
commit: 2811649fa760e18cbcdd6e61e86e30ad607bdbba
base: 85adcce350bf4ece48955daa7ac426bef53a892f
reviewed_at: 2026-10-22T09:20+08:00
reviewer: mrhhsg
model: gpt-6-astra
effort: xhigh
findings: {blocker: 0, major: 0, minor: 5, nit: 13}
rounds: 1
converged: true

Notes for maintainers

F-07 (observability, Regression: yes) fe/fe-core/src/main/java/org/apache/doris/resource/workloadschedpolicy/WorkloadRuntimeStatusMgr.java:183-188 — ordinary SELECT now waits for final BE reports without retaining already received contributions; TTL expiry can permanently understate even existing scan/CPU/local-spill audit counters. Minor after independent consequence challenge: these are best-effort diagnostic logs/table/plugin values, not a demonstrated business-result, transaction or quota control path. Retain/snapshot contributions or declare this limitation; not merely delayed display.
F-01/F-03/F-05 (Minor, test coverage) — production vault resolution and S3 manager initialization remain ungated, and capacity-failure tests do not assert close/unreadability before destruction. Existing mock coverage is extensive but not a real-provider or mixed-FE E2E validation.
F-04 (Minor, non-regression performance) be/src/exec/spill/spill_file_writer.cpp:203-210 — append-only failure performs an avoidable LIST even though close was never attempted; uncertain publication after an attempted close still requires reconciliation.
All 18 findings are inherited and revalidated (5 Minor, 13 Nit), with no new Blocker/Major. Known concurrent forwarded-cancel, old-serving REST and best-effort report/shutdown limitations remain deduplicated, not declared fixed. Incomplete MPU cleanup requires explicitly configured bucket lifecycle. Static review only: no build/tests/SQL were run; this receipt does not override GitHub reviews or pending CI.

_{Reviewed locally with the doris-repo-review pipeline. Repository policy may accept this receipt for the matching commit; it is not a human Apache approval.}

924060929

Reviewed the FE changes at 2811649. The cloud disk-report -> Backend -> warehouse SHOW DATA flow and the remote query-statistics -> audit/proc/REST flow are consistent with the documented best-effort contract. Checked scoped SHOW DATA forwarding, dispatched-backend retention through errors/retries/forwarded cancellation, old audit-table projection, mixed-FE row normalization, related tests, and optional Thrift field IDs against branch-3.1/4.0/4.1/4.2. No new blocking FE issue found. FE-only static review; no local build or tests run. BE implementation is outside this approval's scope.

github-actions · 2026-10-22T04:29:41Z

PR approved by at least one committer and no changes requested.

github-actions Bot requested changes Sep 15, 2026

View reviewed changes

mrhhsg force-pushed the feat/cloud-spill-s3 branch from add6dfd to 02ba809 Compare September 17, 2026 02:02

mrhhsg force-pushed the feat/cloud-spill-s3 branch 3 times, most recently from 94124f3 to 71a21e2 Compare September 24, 2026 07:55

github-actions Bot requested changes Sep 24, 2026

View reviewed changes

github-actions Bot previously requested changes Sep 28, 2026

View reviewed changes

HappenLee previously approved these changes Sep 28, 2026

View reviewed changes

mrhhsg marked this pull request as ready for review September 29, 2026 06:32

mrhhsg requested review from 924060929, Gabriel39, englefly, gavinchou, liaoxin01, morningman, morrySnow and starocean999 as code owners September 29, 2026 06:32

mrhhsg dismissed HappenLee’s stale review via fcc529e September 29, 2026 06:36

mrhhsg added 12 commits October 21, 2026 18:13

mrhhsg force-pushed the feat/cloud-spill-s3 branch from 6d43a17 to 2811649 Compare October 21, 2026 10:49

gavinchou approved these changes Oct 22, 2026

View reviewed changes

starocean999 approved these changes Oct 22, 2026

View reviewed changes

924060929 approved these changes Oct 22, 2026

View reviewed changes

This was referenced Oct 26, 2026

[branch-4.2][feature](cloud) Spill to object storage in cloud mode and report the traffic in SHOW DATA (#68032) #69610

Closed

[feature](cloud) Backport S3-backed query spill to branch-4.1 (#68032) #69635

Open

xylaaaaa mentioned this pull request Oct 27, 2026

[Bug] Spill to an Azure vault with hierarchical namespace leaves empty directories after cleanup #69664

Open

2 of 3 tasks

mrhhsg mentioned this pull request Nov 11, 2026

[fix](be) Move spill deletion off query teardown threads #70474

Draft

Conversation

mrhhsg commented Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

Known limitations / follow-up items

Release note

Check List (For Author)

Check List (For Reviewer who merge this PR)

Uh oh!

hello-stephen commented Sep 15, 2026

Uh oh!

mrhhsg commented Sep 15, 2026

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mrhhsg commented Sep 16, 2026

Uh oh!

mrhhsg commented Sep 17, 2026

Local pipeline review — ✅ PASS

Uh oh!

mrhhsg commented Sep 24, 2026

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mrhhsg commented Sep 28, 2026

Uh oh!

github-actions Bot commented Sep 28, 2026

Uh oh!

mrhhsg commented Sep 28, 2026

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mrhhsg commented Sep 30, 2026

Uh oh!

mrhhsg commented Oct 21, 2026

Uh oh!

hello-stephen commented Oct 21, 2026

Cloud UT Coverage Report

Uh oh!

hello-stephen commented Oct 21, 2026

Uh oh!

hello-stephen commented Oct 21, 2026

Uh oh!

hello-stephen commented Oct 21, 2026

Uh oh!

hello-stephen commented Oct 21, 2026

FE UT Coverage Report

Uh oh!

hello-stephen commented Oct 22, 2026

FE Regression Coverage Report

Uh oh!

hello-stephen commented Oct 22, 2026

mrhhsg commented Sep 15, 2026 •

edited

Loading