This is an automated email from the ASF dual-hosted git repository.
gavinchou pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 6fd63df2ccf [fix](cloud) Fail closed on unknown MetaService response
codes (#66364)
6fd63df2ccf is described below
commit 6fd63df2ccf97e5cd062853290854c47150c03f1
Author: Yixuan Wang <[email protected]>
AuthorDate: Thu Aug 13 13:32:25 2026 +0800
[fix](cloud) Fail closed on unknown MetaService response codes (#66364)
Related PR: https://github.com/apache/doris/pull/64148
Problem Summary:
Meta Service responses contain both an exact actual_code and a
legacy-compatible code. When a client recognizes the actual_code, it
should use that exact value. When actual_code is unknown, the legacy
code is safe only if it is explicitly present and non-OK. The previous
logic could fall back to an unset or OK code and incorrectly treat an
unknown Meta Service error as success.
This change makes BE and FE fail closed with UNDEFINED_ERR in that case,
while preserving compatible non-OK fallbacks. It also adds a message
explaining the MS_TOO_BUSY to KV_TXN_CONFLICT conversion for legacy
clients.
---
be/src/cloud/cloud_meta_mgr.cpp | 50 ++++-
be/src/cloud/cloud_meta_mgr.h | 3 +-
be/test/cloud/cloud_meta_mgr_test.cpp | 32 ++-
cloud/src/meta-service/meta_service_helper.h | 26 ++-
cloud/test/meta_service_helper_test.cpp | 220 +++++++++++----------
cloud/test/txn_lazy_commit_test.cpp | 2 +-
.../apache/doris/cloud/rpc/MetaServiceClient.java | 55 +++++-
.../doris/cloud/rpc/MetaServiceProxyTest.java | 48 +++++
gensrc/proto/cloud.proto | 7 +-
9 files changed, 320 insertions(+), 123 deletions(-)
diff --git a/be/src/cloud/cloud_meta_mgr.cpp b/be/src/cloud/cloud_meta_mgr.cpp
index 7a8b2ee8ff2..1a395852519 100644
--- a/be/src/cloud/cloud_meta_mgr.cpp
+++ b/be/src/cloud/cloud_meta_mgr.cpp
@@ -156,11 +156,55 @@ Status
bthread_fork_join(std::vector<std::function<Status()>>&& tasks, int concu
return Status::OK();
}
+// Resolve the status code returned by Meta Service (MS) for BE/FE clients of
different version.
+// Assuming MS is always the latest version, it sends both the meta-service
error code and a code that
+// older clients can decode:
+//
+// latest MS
+// +---------------------------------------+
+// | actual_code = meta-service error code |
+// | code = compatible code |
+// +----------------+----------------------+
+// |
+// +-------------+-------------+
+// | |
+// old BE/FE without old BE/FE with
+// the actual_code field the actual_code field
+// | |
+// ignores actual_code local enum recognizes
+// and reads code actual_code value?
+// yes no
+// | |
+// use actual code use code only when it
+// is explicit and non-OK
+// |
+// otherwise return
+// UNDEFINED_ERR
+//
+// After MS adds an error code, an older actual_code-aware client may not have
that enum value;
+// MetaServiceCode_IsValid detects this case. The non-OK fallback check is
essential:
+// if MS ignore or incorrectly converts the compatible code to OK, an unknown
error
+// must remain an error instead of becoming a false success.
MetaServiceCode get_response_code(const MetaServiceResponseStatus& status) {
- if (status.has_actual_code() &&
MetaServiceCode_IsValid(status.actual_code())) {
- return static_cast<MetaServiceCode>(status.actual_code());
+ if (status.has_actual_code()) {
+ // Check whether this client build contains the code in its
MetaServiceCode enum.
+ if (MetaServiceCode_IsValid(status.actual_code())) {
+ return static_cast<MetaServiceCode>(status.actual_code());
+ }
+ // An older client may use the compatible code, but unsupported cases
must return an explicit error.
+ // Return the non-OK compatible code prepared by MS for older clients.
+ if (status.has_code() && status.code() != MetaServiceCode::OK) {
+ return status.code();
+ }
+ // Never return OK when the compatible code is absent or invalid.
+ return MetaServiceCode::UNDEFINED_ERR;
+ }
+ // A legacy response has only code, so return its explicit value,
including a real OK.
+ if (status.has_code()) {
+ return status.code();
}
- return status.code();
+ // A response missing both fields is invalid and must be rejected.
+ return MetaServiceCode::UNDEFINED_ERR;
}
namespace {
diff --git a/be/src/cloud/cloud_meta_mgr.h b/be/src/cloud/cloud_meta_mgr.h
index 1bcfbe69a81..f99ed4ce95a 100644
--- a/be/src/cloud/cloud_meta_mgr.h
+++ b/be/src/cloud/cloud_meta_mgr.h
@@ -65,7 +65,8 @@ Status bthread_fork_join(const
std::vector<std::function<Status()>>& tasks, int
Status bthread_fork_join(std::vector<std::function<Status()>>&& tasks, int
concurrency,
std::future<Status>* fut);
-// Returns the exact actual_code when recognized, otherwise the
legacy-compatible code.
+// Returns the exact actual_code when recognized. An unknown actual_code uses
an explicit non-OK
+// legacy fallback and otherwise fails closed. Responses from a legacy Meta
Service use code.
// Exposed for unit tests.
MetaServiceCode get_response_code(const MetaServiceResponseStatus& status);
diff --git a/be/test/cloud/cloud_meta_mgr_test.cpp
b/be/test/cloud/cloud_meta_mgr_test.cpp
index ff87378348e..9b10f80a218 100644
--- a/be/test/cloud/cloud_meta_mgr_test.cpp
+++ b/be/test/cloud/cloud_meta_mgr_test.cpp
@@ -56,17 +56,47 @@ TEST_F(CloudMetaMgrTest,
response_status_uses_actual_code_when_valid) {
status.set_actual_code(static_cast<int32_t>(MetaServiceCode::KV_TXN_CONFLICT));
EXPECT_EQ(get_response_code(status), MetaServiceCode::KV_TXN_CONFLICT);
+ status.set_code(MetaServiceCode::KV_TXN_CONFLICT);
+ status.set_actual_code(static_cast<int32_t>(MetaServiceCode::OK));
+ EXPECT_EQ(get_response_code(status), MetaServiceCode::OK);
+
+ status.clear_code();
+ status.set_actual_code(static_cast<int32_t>(MetaServiceCode::MS_TOO_BUSY));
+ EXPECT_EQ(get_response_code(status), MetaServiceCode::MS_TOO_BUSY);
+
+ status.set_code(MetaServiceCode::KV_TXN_CONFLICT);
status.clear_actual_code();
EXPECT_EQ(get_response_code(status), MetaServiceCode::KV_TXN_CONFLICT);
}
-TEST_F(CloudMetaMgrTest, response_status_falls_back_for_invalid_actual_code) {
+TEST_F(CloudMetaMgrTest,
response_status_falls_back_to_non_ok_code_for_invalid_actual_code) {
MetaServiceResponseStatus status;
status.set_code(MetaServiceCode::KV_TXN_CONFLICT);
status.set_actual_code(std::numeric_limits<int32_t>::max());
EXPECT_EQ(get_response_code(status), MetaServiceCode::KV_TXN_CONFLICT);
}
+TEST_F(CloudMetaMgrTest,
response_status_returns_undefined_for_invalid_actual_code_with_ok) {
+ MetaServiceResponseStatus status;
+ status.set_code(MetaServiceCode::OK);
+ status.set_actual_code(std::numeric_limits<int32_t>::max());
+ EXPECT_EQ(get_response_code(status), MetaServiceCode::UNDEFINED_ERR);
+}
+
+TEST_F(CloudMetaMgrTest,
response_status_returns_undefined_for_invalid_actual_code_without_code) {
+ MetaServiceResponseStatus status;
+ status.set_actual_code(std::numeric_limits<int32_t>::max());
+ EXPECT_EQ(get_response_code(status), MetaServiceCode::UNDEFINED_ERR);
+}
+
+TEST_F(CloudMetaMgrTest, response_status_returns_undefined_without_any_code) {
+ MetaServiceResponseStatus status;
+ EXPECT_EQ(get_response_code(status), MetaServiceCode::UNDEFINED_ERR);
+
+ status.set_code(MetaServiceCode::OK);
+ EXPECT_EQ(get_response_code(status), MetaServiceCode::OK);
+}
+
static AbortTxnRequest get_abort_txn_request(CloudMetaMgr* meta_mgr, const
StreamLoadContext& ctx) {
auto* sp = SyncPoint::get_instance();
sp->clear_all_call_backs();
diff --git a/cloud/src/meta-service/meta_service_helper.h
b/cloud/src/meta-service/meta_service_helper.h
index 9b1fbed4648..ab8605c20a1 100644
--- a/cloud/src/meta-service/meta_service_helper.h
+++ b/cloud/src/meta-service/meta_service_helper.h
@@ -18,6 +18,7 @@
#pragma once
#include <brpc/controller.h>
+#include <fmt/core.h>
#include <gen_cpp/cloud.pb.h>
#include <openssl/md5.h>
@@ -42,22 +43,35 @@
#include "resource-manager/resource_manager.h"
namespace doris::cloud {
-inline MetaServiceCode get_legacy_code(MetaServiceCode code) {
+// Converts a response code and message to values that older clients can read.
+// set_response_code() stores the original code in actual_code
+// Call this function only from set_response_code() or from unit tests; do not
call it from other production code.
+// When adding an error code that may be returned to clients, must add its
conversion here.
+inline std::pair<MetaServiceCode, std::string>
resolve_response_code_and_msg(MetaServiceCode code,
+
std::string msg) {
switch (code) {
- // MS_TOO_BUSY is a overload signal. Map it to KV_TXN_CONFLICT so the BE's
existing
+ // MS_TOO_BUSY is an 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:
- return MetaServiceCode::KV_TXN_CONFLICT;
+ msg += std::string((msg.empty() ? "" : ", ")) +
+ "[MS_TOO_BUSY will be converted to code=KV_TXN_CONFLICT for old
version clients]";
+ return {MetaServiceCode::KV_TXN_CONFLICT, std::move(msg)};
+ case MetaServiceCode::TXN_ALREADY_COMMITED:
+ msg += std::string((msg.empty() ? "" : ", ")) +
+ "[TXN_ALREADY_COMMITED will be converted to code=UNDEFINED_ERR
for old version "
+ "clients]";
+ return {MetaServiceCode::UNDEFINED_ERR, std::move(msg)};
default:
- return code;
+ return {code, std::move(msg)};
}
}
inline void set_response_code(MetaServiceResponseStatus* status,
MetaServiceCode code,
std::string msg) {
+ auto [resolved_code, resolved_msg] = resolve_response_code_and_msg(code,
std::move(msg));
status->set_actual_code(static_cast<int32_t>(code));
- status->set_code(get_legacy_code(code));
- status->set_msg(std::move(msg));
+ status->set_code(resolved_code);
+ status->set_msg(std::move(resolved_msg));
}
inline std::string md5(const std::string& str) {
diff --git a/cloud/test/meta_service_helper_test.cpp
b/cloud/test/meta_service_helper_test.cpp
index 7b50792f88b..73ccf12f42d 100644
--- a/cloud/test/meta_service_helper_test.cpp
+++ b/cloud/test/meta_service_helper_test.cpp
@@ -41,6 +41,105 @@ int64_t calculate_cpu_usage_percent(double delta_cpu_ns,
double delta_wall_ns, d
} // namespace internal
namespace {
+// IMPORTANT: Never-Never-Never add new codes to this snapshot. New codes must
be mapped to a
+// LegacyFallbackCode and verified with
expect_legacy_fallback_response_status().
+const std::set<MetaServiceCode> identity_snapshot = {
+ MetaServiceCode::OK,
+ MetaServiceCode::INVALID_ARGUMENT,
+ MetaServiceCode::KV_TXN_CREATE_ERR,
+ MetaServiceCode::KV_TXN_GET_ERR,
+ MetaServiceCode::KV_TXN_COMMIT_ERR,
+ MetaServiceCode::KV_TXN_CONFLICT,
+ MetaServiceCode::PROTOBUF_PARSE_ERR,
+ MetaServiceCode::PROTOBUF_SERIALIZE_ERR,
+ MetaServiceCode::KV_TXN_STORE_GET_RETRYABLE,
+ MetaServiceCode::KV_TXN_STORE_COMMIT_RETRYABLE,
+ MetaServiceCode::KV_TXN_STORE_CREATE_RETRYABLE,
+ MetaServiceCode::KV_TXN_TOO_OLD,
+ MetaServiceCode::KV_TXN_MAYBE_COMMITTED,
+ MetaServiceCode::TXN_GEN_ID_ERR,
+ MetaServiceCode::TXN_DUPLICATED_REQ,
+ MetaServiceCode::TXN_LABEL_ALREADY_USED,
+ MetaServiceCode::TXN_INVALID_STATUS,
+ MetaServiceCode::TXN_LABEL_NOT_FOUND,
+ MetaServiceCode::TXN_ID_NOT_FOUND,
+ MetaServiceCode::TXN_ALREADY_ABORTED,
+ MetaServiceCode::TXN_ALREADY_VISIBLE,
+ MetaServiceCode::TXN_ALREADY_PRECOMMITED,
+ MetaServiceCode::VERSION_NOT_FOUND,
+ MetaServiceCode::TABLET_NOT_FOUND,
+ MetaServiceCode::STALE_TABLET_CACHE,
+ MetaServiceCode::STALE_PREPARE_ROWSET,
+ MetaServiceCode::CLUSTER_NOT_FOUND,
+ MetaServiceCode::ALREADY_EXISTED,
+ MetaServiceCode::CLUSTER_ENDPOINT_MISSING,
+ MetaServiceCode::STORAGE_VAULT_NOT_FOUND,
+ MetaServiceCode::STAGE_NOT_FOUND,
+ MetaServiceCode::STAGE_GET_ERR,
+ MetaServiceCode::STATE_ALREADY_EXISTED_FOR_USER,
+ MetaServiceCode::COPY_JOB_NOT_FOUND,
+ MetaServiceCode::JOB_EXPIRED,
+ MetaServiceCode::JOB_TABLET_BUSY,
+ MetaServiceCode::JOB_ALREADY_SUCCESS,
+ MetaServiceCode::ROUTINE_LOAD_DATA_INCONSISTENT,
+ MetaServiceCode::ROUTINE_LOAD_PROGRESS_NOT_FOUND,
+ MetaServiceCode::JOB_CHECK_ALTER_VERSION,
+ MetaServiceCode::STREAMING_JOB_PROGRESS_NOT_FOUND,
+ MetaServiceCode::MAX_QPS_LIMIT,
+ MetaServiceCode::ERR_ENCRYPT,
+ MetaServiceCode::ERR_DECPYPT,
+ MetaServiceCode::LOCK_EXPIRED,
+ MetaServiceCode::LOCK_CONFLICT,
+ MetaServiceCode::ROWSETS_EXPIRED,
+ MetaServiceCode::VERSION_NOT_MATCH,
+ MetaServiceCode::UPDATE_OVERRIDE_EXISTING_KV,
+ MetaServiceCode::ROWSET_META_NOT_FOUND,
+ MetaServiceCode::KV_TXN_CONFLICT_RETRY_EXCEEDED_MAX_TIMES,
+ MetaServiceCode::SCHEMA_DICT_NOT_FOUND,
+ MetaServiceCode::UNDEFINED_ERR,
+};
+
+// IMPORTANT: Never-Never-Never modify or extend this enum. New error codes
must be mapped to one of the
+// existing legacy fallback codes below.
+enum class LegacyFallbackCode : int32_t {
+ UNDEFINED_ERR = static_cast<int32_t>(MetaServiceCode::UNDEFINED_ERR),
+ KV_TXN_CONFLICT = static_cast<int32_t>(MetaServiceCode::KV_TXN_CONFLICT),
+};
+
+void verify_response_status_impl(std::set<MetaServiceCode>& covered_codes,
MetaServiceCode code,
+ int32_t expected_legacy_code) {
+ EXPECT_TRUE(covered_codes.insert(code).second)
+ << "Duplicate MetaServiceCode: " << MetaServiceCode_Name(code);
+
+ MetaServiceResponseStatus status;
+ set_response_code(&status, code, "");
+ EXPECT_EQ(static_cast<int32_t>(status.code()), expected_legacy_code)
+ << "MetaServiceCode: " << MetaServiceCode_Name(code);
+ EXPECT_EQ(status.actual_code(), static_cast<int32_t>(code))
+ << "MetaServiceCode: " << MetaServiceCode_Name(code);
+}
+
+void verify_response_status(std::set<MetaServiceCode>& covered_codes,
MetaServiceCode code,
+ int32_t expected_legacy_code) {
+ if (!identity_snapshot.contains(code)) {
+ EXPECT_TRUE(false)
+ << "MetaServiceCode " << MetaServiceCode_Name(code)
+ << " is not in identity_snapshot. New error codes must be
mapped to a "
+ "LegacyFallbackCode in resolve_response_code_and_msg() and
verified with "
+ "expect_legacy_fallback_response_status().";
+ }
+ verify_response_status_impl(covered_codes, code, expected_legacy_code);
+}
+
+// New error codes may only be converted to a value allowed by
LegacyFallbackCode.
+// Resolve the conversion in resolve_response_code_and_msg();
+// For example, MS_TOO_BUSY maps to KV_TXN_CONFLICT so that the BE can retry
it.
+void expect_legacy_fallback_response_status(std::set<MetaServiceCode>&
covered_codes,
+ MetaServiceCode code,
+ LegacyFallbackCode
expected_legacy_code) {
+ verify_response_status_impl(covered_codes, code,
static_cast<int32_t>(expected_legacy_code));
+}
+
struct MsRateLimitInjectionConfigGuard {
~MsRateLimitInjectionConfigGuard() {
config::enable_ms_rate_limit_injection = original_enable;
@@ -228,7 +327,9 @@ TEST_F(MetaServiceWireCompatibilityTest,
LegacyClientReadsFallbackAndIgnoresActu
ASSERT_TRUE(reflection->HasField(*legacy_status, legacy_code_field_));
EXPECT_EQ(reflection->GetEnumValue(*legacy_status, legacy_code_field_),
MetaServiceCode::KV_TXN_CONFLICT);
- EXPECT_EQ(reflection->GetString(*legacy_status, legacy_msg_field_),
"busy");
+ EXPECT_EQ(reflection->GetString(*legacy_status, legacy_msg_field_),
+ "busy, [MS_TOO_BUSY will be converted to code=KV_TXN_CONFLICT
for old version "
+ "clients]");
EXPECT_EQ(legacy_status_descriptor_->FindFieldByName("actual_code"),
nullptr);
const auto& unknown_fields = reflection->GetUnknownFields(*legacy_status);
@@ -287,115 +388,36 @@ TEST(MetaServiceHelperTest,
ResponseStatusUsesExactAndLegacyCodes) {
set_response_code(&status, MetaServiceCode::MS_TOO_BUSY, "busy");
EXPECT_EQ(status.code(), MetaServiceCode::KV_TXN_CONFLICT);
EXPECT_EQ(status.actual_code(), MetaServiceCode::MS_TOO_BUSY);
- EXPECT_EQ(status.msg(), "busy");
+ EXPECT_EQ(status.msg(),
+ "busy, [MS_TOO_BUSY will be converted to code=KV_TXN_CONFLICT
for old version "
+ "clients]");
set_response_code(&status, MetaServiceCode::KV_TXN_CONFLICT, "conflict");
EXPECT_EQ(status.code(), MetaServiceCode::KV_TXN_CONFLICT);
EXPECT_EQ(status.actual_code(), MetaServiceCode::KV_TXN_CONFLICT);
EXPECT_EQ(status.msg(), "conflict");
+
+ set_response_code(&status, MetaServiceCode::MS_TOO_BUSY, "");
+ EXPECT_EQ(status.msg(),
+ "[MS_TOO_BUSY will be converted to code=KV_TXN_CONFLICT for old
version clients]");
}
TEST(MetaServiceHelperTest, ResponseStatusCoversEveryMetaServiceCode) {
std::set<MetaServiceCode> covered_codes;
- auto expect_response_status = [&](MetaServiceCode code, MetaServiceCode
expected_legacy_code) {
- EXPECT_TRUE(covered_codes.insert(code).second)
- << "Duplicate MetaServiceCode: " << MetaServiceCode_Name(code);
-
- MetaServiceResponseStatus status;
- set_response_code(&status, code, "");
- EXPECT_EQ(status.code(), expected_legacy_code)
- << "MetaServiceCode: " << MetaServiceCode_Name(code);
- EXPECT_EQ(status.actual_code(), static_cast<int32_t>(code))
- << "MetaServiceCode: " << MetaServiceCode_Name(code);
- };
-
- expect_response_status(MetaServiceCode::OK, MetaServiceCode::OK);
- expect_response_status(MetaServiceCode::INVALID_ARGUMENT,
MetaServiceCode::INVALID_ARGUMENT);
- expect_response_status(MetaServiceCode::KV_TXN_CREATE_ERR,
MetaServiceCode::KV_TXN_CREATE_ERR);
- expect_response_status(MetaServiceCode::KV_TXN_GET_ERR,
MetaServiceCode::KV_TXN_GET_ERR);
- expect_response_status(MetaServiceCode::KV_TXN_COMMIT_ERR,
MetaServiceCode::KV_TXN_COMMIT_ERR);
- expect_response_status(MetaServiceCode::KV_TXN_CONFLICT,
MetaServiceCode::KV_TXN_CONFLICT);
- expect_response_status(MetaServiceCode::PROTOBUF_PARSE_ERR,
- MetaServiceCode::PROTOBUF_PARSE_ERR);
- expect_response_status(MetaServiceCode::PROTOBUF_SERIALIZE_ERR,
- MetaServiceCode::PROTOBUF_SERIALIZE_ERR);
- expect_response_status(MetaServiceCode::KV_TXN_STORE_GET_RETRYABLE,
- MetaServiceCode::KV_TXN_STORE_GET_RETRYABLE);
- expect_response_status(MetaServiceCode::KV_TXN_STORE_COMMIT_RETRYABLE,
- MetaServiceCode::KV_TXN_STORE_COMMIT_RETRYABLE);
- expect_response_status(MetaServiceCode::KV_TXN_STORE_CREATE_RETRYABLE,
- MetaServiceCode::KV_TXN_STORE_CREATE_RETRYABLE);
- expect_response_status(MetaServiceCode::KV_TXN_TOO_OLD,
MetaServiceCode::KV_TXN_TOO_OLD);
- expect_response_status(MetaServiceCode::KV_TXN_MAYBE_COMMITTED,
- MetaServiceCode::KV_TXN_MAYBE_COMMITTED);
- expect_response_status(MetaServiceCode::TXN_GEN_ID_ERR,
MetaServiceCode::TXN_GEN_ID_ERR);
- expect_response_status(MetaServiceCode::TXN_DUPLICATED_REQ,
- MetaServiceCode::TXN_DUPLICATED_REQ);
- expect_response_status(MetaServiceCode::TXN_LABEL_ALREADY_USED,
- MetaServiceCode::TXN_LABEL_ALREADY_USED);
- expect_response_status(MetaServiceCode::TXN_INVALID_STATUS,
- MetaServiceCode::TXN_INVALID_STATUS);
- expect_response_status(MetaServiceCode::TXN_LABEL_NOT_FOUND,
- MetaServiceCode::TXN_LABEL_NOT_FOUND);
- expect_response_status(MetaServiceCode::TXN_ID_NOT_FOUND,
MetaServiceCode::TXN_ID_NOT_FOUND);
- expect_response_status(MetaServiceCode::TXN_ALREADY_ABORTED,
- MetaServiceCode::TXN_ALREADY_ABORTED);
- expect_response_status(MetaServiceCode::TXN_ALREADY_VISIBLE,
- MetaServiceCode::TXN_ALREADY_VISIBLE);
- expect_response_status(MetaServiceCode::TXN_ALREADY_PRECOMMITED,
- MetaServiceCode::TXN_ALREADY_PRECOMMITED);
- expect_response_status(MetaServiceCode::VERSION_NOT_FOUND,
MetaServiceCode::VERSION_NOT_FOUND);
- expect_response_status(MetaServiceCode::TABLET_NOT_FOUND,
MetaServiceCode::TABLET_NOT_FOUND);
- expect_response_status(MetaServiceCode::STALE_TABLET_CACHE,
- MetaServiceCode::STALE_TABLET_CACHE);
- expect_response_status(MetaServiceCode::STALE_PREPARE_ROWSET,
- MetaServiceCode::STALE_PREPARE_ROWSET);
- expect_response_status(MetaServiceCode::TXN_ALREADY_COMMITED,
- MetaServiceCode::TXN_ALREADY_COMMITED);
- expect_response_status(MetaServiceCode::CLUSTER_NOT_FOUND,
MetaServiceCode::CLUSTER_NOT_FOUND);
- expect_response_status(MetaServiceCode::ALREADY_EXISTED,
MetaServiceCode::ALREADY_EXISTED);
- expect_response_status(MetaServiceCode::CLUSTER_ENDPOINT_MISSING,
- MetaServiceCode::CLUSTER_ENDPOINT_MISSING);
- expect_response_status(MetaServiceCode::STORAGE_VAULT_NOT_FOUND,
- MetaServiceCode::STORAGE_VAULT_NOT_FOUND);
- expect_response_status(MetaServiceCode::STAGE_NOT_FOUND,
MetaServiceCode::STAGE_NOT_FOUND);
- expect_response_status(MetaServiceCode::STAGE_GET_ERR,
MetaServiceCode::STAGE_GET_ERR);
- expect_response_status(MetaServiceCode::STATE_ALREADY_EXISTED_FOR_USER,
- MetaServiceCode::STATE_ALREADY_EXISTED_FOR_USER);
- expect_response_status(MetaServiceCode::COPY_JOB_NOT_FOUND,
- MetaServiceCode::COPY_JOB_NOT_FOUND);
- expect_response_status(MetaServiceCode::JOB_EXPIRED,
MetaServiceCode::JOB_EXPIRED);
- expect_response_status(MetaServiceCode::JOB_TABLET_BUSY,
MetaServiceCode::JOB_TABLET_BUSY);
- expect_response_status(MetaServiceCode::JOB_ALREADY_SUCCESS,
- MetaServiceCode::JOB_ALREADY_SUCCESS);
- expect_response_status(MetaServiceCode::ROUTINE_LOAD_DATA_INCONSISTENT,
- MetaServiceCode::ROUTINE_LOAD_DATA_INCONSISTENT);
- expect_response_status(MetaServiceCode::ROUTINE_LOAD_PROGRESS_NOT_FOUND,
- MetaServiceCode::ROUTINE_LOAD_PROGRESS_NOT_FOUND);
- expect_response_status(MetaServiceCode::JOB_CHECK_ALTER_VERSION,
- MetaServiceCode::JOB_CHECK_ALTER_VERSION);
- expect_response_status(MetaServiceCode::STREAMING_JOB_PROGRESS_NOT_FOUND,
- MetaServiceCode::STREAMING_JOB_PROGRESS_NOT_FOUND);
- expect_response_status(MetaServiceCode::MAX_QPS_LIMIT,
MetaServiceCode::MAX_QPS_LIMIT);
- expect_response_status(MetaServiceCode::MS_TOO_BUSY,
MetaServiceCode::KV_TXN_CONFLICT);
- expect_response_status(MetaServiceCode::ERR_ENCRYPT,
MetaServiceCode::ERR_ENCRYPT);
- expect_response_status(MetaServiceCode::ERR_DECPYPT,
MetaServiceCode::ERR_DECPYPT);
- expect_response_status(MetaServiceCode::LOCK_EXPIRED,
MetaServiceCode::LOCK_EXPIRED);
- expect_response_status(MetaServiceCode::LOCK_CONFLICT,
MetaServiceCode::LOCK_CONFLICT);
- expect_response_status(MetaServiceCode::ROWSETS_EXPIRED,
MetaServiceCode::ROWSETS_EXPIRED);
- expect_response_status(MetaServiceCode::VERSION_NOT_MATCH,
MetaServiceCode::VERSION_NOT_MATCH);
- expect_response_status(MetaServiceCode::UPDATE_OVERRIDE_EXISTING_KV,
- MetaServiceCode::UPDATE_OVERRIDE_EXISTING_KV);
- expect_response_status(MetaServiceCode::ROWSET_META_NOT_FOUND,
- MetaServiceCode::ROWSET_META_NOT_FOUND);
-
expect_response_status(MetaServiceCode::KV_TXN_CONFLICT_RETRY_EXCEEDED_MAX_TIMES,
-
MetaServiceCode::KV_TXN_CONFLICT_RETRY_EXCEEDED_MAX_TIMES);
- expect_response_status(MetaServiceCode::SCHEMA_DICT_NOT_FOUND,
- MetaServiceCode::SCHEMA_DICT_NOT_FOUND);
- expect_response_status(MetaServiceCode::UNDEFINED_ERR,
MetaServiceCode::UNDEFINED_ERR);
+ for (auto code : identity_snapshot) {
+ verify_response_status(covered_codes, code,
static_cast<int32_t>(code));
+ }
+
+ expect_legacy_fallback_response_status(covered_codes,
MetaServiceCode::MS_TOO_BUSY,
+
LegacyFallbackCode::KV_TXN_CONFLICT);
+ expect_legacy_fallback_response_status(covered_codes,
MetaServiceCode::TXN_ALREADY_COMMITED,
+ LegacyFallbackCode::UNDEFINED_ERR);
EXPECT_EQ(covered_codes.size(),
-
static_cast<size_t>(MetaServiceCode_descriptor()->value_count()));
+ static_cast<size_t>(MetaServiceCode_descriptor()->value_count()))
+ << "A new MetaServiceCode was added. Map it to a
LegacyFallbackCode in "
+ "resolve_response_code_and_msg() and verify it with "
+ "expect_legacy_fallback_response_status().";
}
} // namespace doris::cloud
diff --git a/cloud/test/txn_lazy_commit_test.cpp
b/cloud/test/txn_lazy_commit_test.cpp
index 60a33067143..41374e67a75 100644
--- a/cloud/test/txn_lazy_commit_test.cpp
+++ b/cloud/test/txn_lazy_commit_test.cpp
@@ -3448,7 +3448,7 @@ TEST(TxnLazyCommitTest,
CommitTxnEventuallyWithAbortAfterCommitTest) {
req.set_cloud_unique_id("test_cloud_unique_id");
meta_service->abort_txn(reinterpret_cast<::google::protobuf::RpcController*>(&cntl),
&req,
&res, nullptr);
- ASSERT_EQ(res.status().code(), MetaServiceCode::TXN_ALREADY_COMMITED);
+ ASSERT_EQ(res.status().actual_code(),
MetaServiceCode::TXN_ALREADY_COMMITED);
});
// mock rowset and tablet
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/cloud/rpc/MetaServiceClient.java
b/fe/fe-core/src/main/java/org/apache/doris/cloud/rpc/MetaServiceClient.java
index a14f30299b9..ebefdb44aa9 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/cloud/rpc/MetaServiceClient.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/cloud/rpc/MetaServiceClient.java
@@ -110,18 +110,44 @@ public class MetaServiceClient {
}
}
+ // Resolve the status code returned by Meta Service (MS) for FE clients of
different versions.
+ // Assuming MS is always the latest version, it sends both the
meta-service error code and a code that
+ // older clients can decode:
+ //
+ // latest MS
+ // +---------------------------------------+
+ // | actual_code = meta-service error code |
+ // | code = compatible code |
+ // +----------------+----------------------+
+ // |
+ // +-------------+-------------+
+ // | |
+ // old FE without the old FE with the
+ // actual_code field actual_code field
+ // | |
+ // ignores actual_code local enum recognizes
+ // and reads code actual_code value?
+ // / \
+ // yes no
+ // | |
+ // use actual code use code only
when it
+ // is explicit and
non-OK
+ // |
+ // otherwise
return
+ // UNDEFINED_ERR
+ //
+ // After MS adds an error code, an older actual_code-aware client may not
have that enum value;
+ // Cloud.MetaServiceCode.forNumber returns null in this case. The non-OK
fallback check is essential:
+ // if MS ignores or incorrectly converts the compatible code to OK, an
unknown error must remain an
+ // error instead of becoming a false success.
@SuppressWarnings("unchecked")
- // Restore the exact status code from actual_code when this FE recognizes
it.
- // Otherwise, keep
- // the legacy-compatible value in code so responses from a newer Meta
Service
- // remain readable.
private static <Response> Response restoreActualCode(Response response) {
if (!(response instanceof Message)) {
return response;
}
Message message = (Message) response;
Descriptors.FieldDescriptor statusField =
message.getDescriptorForType().findFieldByName("status");
- if (statusField == null || !message.hasField(statusField)) {
+ if (statusField == null) {
return response;
}
Object statusObject = message.getField(statusField);
@@ -130,11 +156,22 @@ public class MetaServiceClient {
}
Cloud.MetaServiceResponseStatus status =
(Cloud.MetaServiceResponseStatus) statusObject;
- if (!status.hasActualCode()) {
- return response;
+ Cloud.MetaServiceCode code;
+ if (status.hasActualCode()) {
+ code = Cloud.MetaServiceCode.forNumber(status.getActualCode());
+ if (code == null) {
+ if (status.hasCode() && status.getCode() !=
Cloud.MetaServiceCode.OK) {
+ return response;
+ }
+ code = Cloud.MetaServiceCode.UNDEFINED_ERR;
+ }
+ } else {
+ if (status.hasCode()) {
+ return response;
+ }
+ code = Cloud.MetaServiceCode.UNDEFINED_ERR;
}
- Cloud.MetaServiceCode code =
Cloud.MetaServiceCode.forNumber(status.getActualCode());
- if (code == null || code == status.getCode()) {
+ if (status.hasCode() && code == status.getCode()) {
return response;
}
Cloud.MetaServiceResponseStatus restoredStatus =
status.toBuilder().setCode(code).build();
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/cloud/rpc/MetaServiceProxyTest.java
b/fe/fe-core/src/test/java/org/apache/doris/cloud/rpc/MetaServiceProxyTest.java
index 31e0887f02e..a9f25586bee 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/cloud/rpc/MetaServiceProxyTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/cloud/rpc/MetaServiceProxyTest.java
@@ -229,6 +229,16 @@ public class MetaServiceProxyTest {
Assert.assertEquals(Cloud.MetaServiceCode.MS_TOO_BUSY.getNumber(),
status.getActualCode());
}
+ @Test
+ public void testGetInstanceUsesKnownActualCodeWithoutFallback() throws
RpcException {
+ Cloud.MetaServiceResponseStatus status = callGetInstanceWithStatus(
+ Cloud.MetaServiceResponseStatus.newBuilder()
+
.setActualCode(Cloud.MetaServiceCode.MS_TOO_BUSY.getNumber())
+ .build());
+
+ Assert.assertEquals(Cloud.MetaServiceCode.MS_TOO_BUSY,
status.getCode());
+ }
+
@Test
public void testGetInstanceKeepsLegacyCodeForUnknownActualCode() throws
RpcException {
Cloud.MetaServiceResponseStatus status = callGetInstanceWithStatus(
@@ -241,6 +251,44 @@ public class MetaServiceProxyTest {
Assert.assertEquals(Integer.MAX_VALUE, status.getActualCode());
}
+ @Test
+ public void
testGetInstanceFailsClosedForUnknownActualCodeWithoutErrorFallback() throws
RpcException {
+ Cloud.MetaServiceResponseStatus status = callGetInstanceWithStatus(
+ Cloud.MetaServiceResponseStatus.newBuilder()
+ .setCode(Cloud.MetaServiceCode.OK)
+ .setActualCode(Integer.MAX_VALUE)
+ .build());
+
+ Assert.assertEquals(Cloud.MetaServiceCode.UNDEFINED_ERR,
status.getCode());
+ Assert.assertEquals(Integer.MAX_VALUE, status.getActualCode());
+
+ status =
callGetInstanceWithStatus(Cloud.MetaServiceResponseStatus.newBuilder()
+ .setActualCode(Integer.MAX_VALUE)
+ .build());
+
+ Assert.assertEquals(Cloud.MetaServiceCode.UNDEFINED_ERR,
status.getCode());
+ Assert.assertEquals(Integer.MAX_VALUE, status.getActualCode());
+ }
+
+ @Test
+ public void testGetInstanceFailsClosedWithoutAnyCode() throws RpcException
{
+ Cloud.MetaServiceResponseStatus status = callGetInstanceWithStatus(
+ Cloud.MetaServiceResponseStatus.getDefaultInstance());
+
+ Assert.assertEquals(Cloud.MetaServiceCode.UNDEFINED_ERR,
status.getCode());
+ }
+
+ @Test
+ public void testResponseFailsClosedWithoutStatus() {
+ Cloud.GetInstanceResponse response = Deencapsulation.invoke(
+ MetaServiceClient.class,
+ "restoreActualCode",
+ Cloud.GetInstanceResponse.getDefaultInstance());
+
+ Assert.assertTrue(response.hasStatus());
+ Assert.assertEquals(Cloud.MetaServiceCode.UNDEFINED_ERR,
response.getStatus().getCode());
+ }
+
@Test
public void testGetInstanceKeepsLegacyCodeWithoutActualCode() throws
RpcException {
Cloud.MetaServiceResponseStatus status = callGetInstanceWithStatus(
diff --git a/gensrc/proto/cloud.proto b/gensrc/proto/cloud.proto
index 94e4b923d00..4addf9385b6 100644
--- a/gensrc/proto/cloud.proto
+++ b/gensrc/proto/cloud.proto
@@ -1567,8 +1567,9 @@ message MetaServiceResponseStatus {
optional string msg = 2;
// Exact client-visible status code encoded as int32, so proto2 clients do
not drop unknown
// enum values. Internal retry signals must be converted before the
response is sent.
- // New clients should use this field when the local enum descriptor
recognizes the value,
- // otherwise fall back to `code`.
+ // New clients should use this field when the local enum descriptor
recognizes the value.
+ // Otherwise, use `code` only when it is explicitly present and non-OK,
and fail closed with
+ // UNDEFINED_ERR when no recognizable error code is available.
optional int32 actual_code = 3;
}
@@ -1912,7 +1913,7 @@ enum MetaServiceCode {
// MetaService must write the exact client-visible code to
// `MetaServiceResponseStatus.actual_code` and write only a legacy
fallback code to
// `MetaServiceResponseStatus.code`. Any newly added error code that may
be returned to
- // clients must be mapped in get_legacy_code().
+ // clients must be mapped in resolve_response_code_and_msg().
OK = 0;
//Meta service internal error
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
