feat(cn): fail the result instances the frontend polls when an intermediate fragment fails - #1715
Draft
aocsa wants to merge 1 commit into
Draft
Conversation
…ediate fragment fails Today a failure in an intermediate fragment never reaches the result instance the FE polls with fetch_data, so the FE spins until its 600 s timeout with no cause. The store now records failures per query: reserve(id, query) ties a result instance to its query, fail_query marks every reserved instance with the origin's error, and a reservation that arrives after the failure fails on the spot. wait_ready blocks on a Condvar until rows or a failure land, so a long-poll never consumes an FE packet sequence with an empty reply. Additive: dev's insert/take_next keep their shape and compute_node_service.rs is untouched. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Draft
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Layer 2 of the cn stack; base
stacked/cn-cluster-bringup.The failure this removes. On the source branch (
feat/pin-table-cn) a multi-fragment query runs its result fragment on one CN and its intermediate fragments elsewhere. When an intermediate fragment fails (a relay guard fires, a sender never delivers), nothing tells the result instance the FE is polling. TheResultStoreonly knowsPendingandDrained, sofetch_datakeeps answering not-ready and the FE waits out its 600 s query timeout, then reports a timeout with no cause.What changed, all in
experimental/starrocks/src/result_store.rs:StoreStatebehind the store's single mutex. It holds the fragment states plus two per-query maps, the result instances reserved for each query and the first failure recorded for each query.reserve(id, query_id)marks a result fragmentWaitingand ties it to its query. If that query has already failed, the reservation lands asFailedat once, so the very first poll reports the cause.fail(id, cause)marks one fragment failed.fail_query(query_id, failed_id, cause)also marks every reserved result instance of that query withfragment instance <failed_id> failed: <cause>and records the failure at query level. First failure wins; later ones are usually downstream echoes.cancel(id, reason)turns a stillWaitingentry into a failure so a long-poll returns. Delivered, drained, failed and unknown entries are left alone.wait_ready(id, timeout)blocks on a Condvar until the entry leavesWaiting, then polls. A timeout is a loudFailed, not an empty reply. The FE's ResultReceiver counts every packet, so a not-ready reply burns a sequence number and the rows that follow arrive stale ("expect=1, receive=0").poll(id)is the non-blocking state machine and returnsFetchOutcome::{Rows, Failed}.insertnow refuses to overwrite aFailedentry, so late rows cannot mask a recorded failure.Why failure is attributed per query. The FE dispatches the result fragment and the intermediate fragments independently and in no fixed order. Keying failures by fragment instance alone loses the case where the intermediate fails before the result fragment has reserved. Keying by query lets a late reservation find the failure waiting for it.
What is additive. Dev's
insertkeeps its signature andcompute_node_service.rsis untouched.take_nextkeeps its call shape (the handler never names the return type) but now returns the rows-onlyFetchProgressstruct, the renamed version of dev's oldFetchOutcome, as a wrapper overpoll. AFailedentry reads asNonethrough it, so dev's handler still answers its "no buffered result" error rather than hanging. Nothing on this base records a failure, so that path is reachable only from tests until the dispatch layer movesfetch_dataontowait_ready. The source branch changedFetchOutcomefrom a struct to an enum in place, which would have broken dev's handler; the wrapper is the one deliberate departure from the source file. The methods only the dispatch layer calls carry a targeted#[allow(dead_code)]naming that PR.How I tested it. On a GB200 box (aarch64) I ran the CI trio: cargo fmt, clippy with warnings as errors, and the workspace test suite without the engine feature. All 214 tests pass.
Not handled here: the dispatch worker that calls
reserve,fail_queryandwait_readyand removes thetake_nextwrapper (stacked/cn-exchange-dispatch); cancellation beyondcancel()marking a waiting entry; eviction of drained entries and the per-query maps (the existing TODO);FragmentInstanceId::as_halvesand itspubvisibility, which belong to the nixl agent tier layer.Checklist
References
aocsa/feat/pin-table-cnstacked/cn-cluster-bringup(layer 1 of the cn stack)stacked/cn-exchange-dispatchwires these methods into the dispatch worker andfetch_data