Skip to content

Commit 5ffdef9

Browse files
committed
[TRTLLM-17105][fix] Report a bounce failure only after the receiver has answered every sent chunk
Only the receiver's scatter of a received DATA writes the destination, and the receiver answers (ACK or NACK) after that scatter's last write. A failed request used to resolve kFAILURE at once, even while chunks whose DATA was already sent were queued or running on the receiver, so a caller that reused the destination could have it overwritten. Contract: BounceResult gains `quiesced`. A failed request whose sent chunks are not all answered now waits for those answers: it posts, gathers and consumes credits no more, releases its staging and sends its cancel as before, and resolves FAILURE with quiesced=true once the last answer arrives. Answers match by (peer, rid, chunk, region), and failRequest is idempotent, so a later forgetPeer keeps the first reason and the answer deadline. The receiver drains the exec stream before NACKing a failed launch, so a NACK never precedes a write. In-doubt: with a sent chunk still unanswered, a receiver silent for requestTimeoutMs after the failure or its last answer (the dead-peer assumption the lease and quarantine already make), a kNoProgressTimeout, or shutdown resolves the request with quiesced=false and a WARNING. BounceTransferStatus moves to a header-only file; isCompleted() is true on SUCCESS and on a quiesced FAILURE, and the status string marks an in-doubt one. The bounce reactor gtests get a 300 s per-test ctest TIMEOUT, so a hang fails fast under its own name instead of consuming the 2700 s stage budget. The success path is unchanged: no new messages, locks, clock reads or allocations. No wire or Python change. Signed-off-by: Chuang Zhu <111838961+chuangz0@users.noreply.github.com>
1 parent 9f8ec26 commit 5ffdef9

9 files changed

Lines changed: 1415 additions & 143 deletions

File tree

‎cpp/include/tensorrt_llm/executor/transferAgent.h‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -374,9 +374,11 @@ class TransferStatus
374374
{
375375
public:
376376
virtual ~TransferStatus() = default;
377-
/// NIXL and bounce statuses return true only on success; backends that cannot distinguish
378-
/// failure from completion (mooncake) return true on any terminal state — use
379-
/// wait()/getLastStatus for the outcome.
377+
/// NIXL statuses return true only on success. Bounce statuses return true on success and on a
378+
/// failure proven never to write the destination again (a source read by an orphaned local
379+
/// gather may still be in flight); a failure without that proof stays false. Backends that
380+
/// cannot distinguish failure from completion (mooncake) return true on any terminal state —
381+
/// use wait() for the outcome.
380382
[[nodiscard]] virtual bool isCompleted() const = 0;
381383
virtual TransferState wait(int64_t timeout_ms = -1) const = 0;
382384

‎cpp/tensorrt_llm/executor/cache_transmission/nixl_utils/bounce/BounceConfig.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,9 @@ struct BounceConfig
166166
int requestTimeoutMs{30000}; // request_timeout_ms; must be > 0 — the whole failure model
167167
// (abandoned-flow resolution, receiver lease, quarantine) hangs off
168168
// this timer, so applyParam rejects 0 (negatives already fail the
169-
// strict unsigned parse) and keeps the current value.
169+
// strict unsigned parse) and keeps the current value. It also bounds
170+
// how long a failed request waits for the receiver's answers (after
171+
// the failure, and again after each answer) before resolving in-doubt.
170172
// Receiver-side lease on granted regions — DERIVED in deriveDependentTimeouts() as
171173
// 2 x requestTimeoutMs, not an independent knob (tests may still set the field directly). A
172174
// dead sender emits neither DATA nor a cancel, which is unobservable through the protocol
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
/*
2+
* SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
3+
* SPDX-License-Identifier: Apache-2.0
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
#pragma once
19+
20+
#include "tensorrt_llm/executor/cache_transmission/nixl_utils/bounce/BounceTransport.h"
21+
#include "tensorrt_llm/executor/transferAgent.h"
22+
23+
#include <chrono>
24+
#include <cstdint>
25+
#include <future>
26+
#include <string>
27+
#include <utility>
28+
29+
namespace tensorrt_llm::executor::kv_cache::bounce
30+
{
31+
32+
/// TransferStatus over the bounce transport's completion future (the value Python's wait() reads).
33+
/// Header-only so its contract can be unit-tested over hand-built futures.
34+
class BounceTransferStatus final : public TransferStatus
35+
{
36+
public:
37+
explicit BounceTransferStatus(std::shared_future<BounceResult> fut)
38+
: mFut(std::move(fut))
39+
{
40+
}
41+
42+
[[nodiscard]] bool isCompleted() const override
43+
{
44+
// Terminal AND the destination is quiesced (BounceResult::quiesced): kSUCCESS, or a kFAILURE the
45+
// receiver has fully answered; an in-doubt kFAILURE stays false. Callers read the outcome from
46+
// wait()/getLastStatusStr. get() cannot throw: every erase/shutdown path resolves the promise
47+
// with set_value first, so broken_promise is unreachable.
48+
if (!isReady())
49+
{
50+
return false;
51+
}
52+
auto const& result = mFut.get();
53+
return result.state == TransferState::kSUCCESS || result.quiesced;
54+
}
55+
56+
[[nodiscard]] TransferState wait(int64_t timeoutMs) const override
57+
{
58+
if (!mFut.valid())
59+
{
60+
return TransferState::kFAILURE;
61+
}
62+
if (timeoutMs < 0)
63+
{
64+
return mFut.get().state;
65+
}
66+
if (mFut.wait_for(std::chrono::milliseconds(timeoutMs)) == std::future_status::ready)
67+
{
68+
return mFut.get().state;
69+
}
70+
return TransferState::kIN_PROGRESS;
71+
}
72+
73+
/// Failure cause recorded by the bounce transport (empty while in flight / on success). An
74+
/// in-doubt failure says so, since only then may the destination still be written.
75+
[[nodiscard]] std::string getLastStatusStr() const override
76+
{
77+
if (!isReady())
78+
{
79+
return {};
80+
}
81+
auto const& result = mFut.get();
82+
if (result.reason == BounceFailReason::kNone)
83+
{
84+
return {};
85+
}
86+
std::string detail = toString(result.reason);
87+
if (!result.quiesced)
88+
{
89+
detail += "; destination not proven quiescent";
90+
}
91+
return detail;
92+
}
93+
94+
private:
95+
[[nodiscard]] bool isReady() const
96+
{
97+
return mFut.valid() && mFut.wait_for(std::chrono::seconds(0)) == std::future_status::ready;
98+
}
99+
100+
std::shared_future<BounceResult> mFut;
101+
};
102+
103+
} // namespace tensorrt_llm::executor::kv_cache::bounce

0 commit comments

Comments
 (0)