github-actions[bot] commented on code in PR #64148:
URL: https://github.com/apache/doris/pull/64148#discussion_r3688307625
##########
cloud/src/meta-service/meta_service.h:
##########
@@ -1035,6 +1037,11 @@ class MetaServiceProxy final : public MetaService {
using namespace std::chrono;
brpc::ClosureGuard done_guard(done);
+ DORIS_CLOUD_DEFER {
+ auto* status = resp->mutable_status();
+ set_response_status(status, get_response_code(*status),
status->msg());
Review Comment:
[P1] Restore the HTTP/RPC parity test before merging
This finalizer now adds `aux_code: 0` to the `MetaServiceProxy` response
used by `get_tablet_stats(...)`, while the HTTP forwarding path calls
`MetaServiceImpl` directly and still returns a status without `aux_code`. On
this exact head (`a92ea12e`), Cloud UT build 1011278 has one deterministic
failure: `MetaServiceHttpTest.GetTabletStatsTest` at line 1440 compares those
payloads and sees the extra field only in `res.DebugString()`. Please make the
test/parity boundary account for the dual-channel status (or align the two
response paths if parity is intended) and rerun Cloud UT; the required suite is
currently red.
##########
cloud/src/meta-service/meta_service_helper.h:
##########
@@ -41,6 +43,24 @@
#include "resource-manager/resource_manager.h"
namespace doris::cloud {
+inline MetaServiceCode get_legacy_code(MetaServiceCode code) {
+ switch (code) {
+ // MS_TOO_BUSY is a overload signal. Map it to KV_TXN_CONFLICT so the BE's
existing
+ // conflict-retry path can retry the request.
+ case MetaServiceCode::MS_TOO_BUSY:
Review Comment:
[P1] Choose a fallback every rate-limited legacy path can retry
`KV_TXN_CONFLICT` is retryable in the released BE's shared RPC helper, but
not on every path that this runtime whitelist can select. A released FE's
generic wrapper retries only `MS_TOO_BUSY`, so during a server-first upgrade,
dynamically including `get_stage` (or using the empty/all whitelist) makes it
return this fallback after one attempt; `getStageRpc` then surfaces an error.
The released BE's direct `get_rowset` loop has the same mismatch: it retries
transport failures and exact busy, then turns conflict into `InternalError`.
Thus the new aux field repairs upgraded clients but transient backpressure
still fails during mixed-version operation outside the default whitelist. This
is distinct from the earlier legacy-rejection thread: legacy callers now
receive a safe non-OK code, but the selected fallback is not retryable by all
of them. Please restrict rejection to methods whose released clients retry
conflict, or capability-gate/select the fallback per client or method, and cov
er both old-FE and direct-old-BE fallback-only cases.
--
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]
