You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by yi...@apache.org on 2026/04/22 07:02:23 UTC
(doris) branch branch-4.1 updated: branch-4.1: [fix](cloud) Hide KV_TXN_MAYBE_COMMITTED from clients #62244 (#62644)
This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch branch-4.1
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-4.1 by this push:
new 42440617e1d branch-4.1: [fix](cloud) Hide KV_TXN_MAYBE_COMMITTED from clients #62244 (#62644)
42440617e1d is described below
commit 42440617e1da7acd650959a95e182d584dc1b000
Author: github-actions[bot] <41...@users.noreply.github.com>
AuthorDate: Wed Apr 22 15:02:15 2026 +0800
branch-4.1: [fix](cloud) Hide KV_TXN_MAYBE_COMMITTED from clients #62244 (#62644)
Cherry-picked from #62244
Co-authored-by: Luwei <81...@qq.com>
---
cloud/src/meta-service/meta_service.h | 10 ++-
cloud/test/meta_service_test.cpp | 147 +++++++++++++++++++++++++++++++++-
gensrc/proto/cloud.proto | 9 +++
3 files changed, 162 insertions(+), 4 deletions(-)
diff --git a/cloud/src/meta-service/meta_service.h b/cloud/src/meta-service/meta_service.h
index b789aab8b21..95dbfc959ef 100644
--- a/cloud/src/meta-service/meta_service.h
+++ b/cloud/src/meta-service/meta_service.h
@@ -1041,6 +1041,11 @@ private:
if (!config::enable_txn_store_retry) {
(impl_.get()->*method)(ctrl, req, resp, brpc::DoNothing());
+ if (resp->status().code() == MetaServiceCode::KV_TXN_MAYBE_COMMITTED) {
+ // Keep maybe-committed as an internal retry signal only. Older proto2
+ // clients may treat unknown enum values as unset and fall back to OK.
+ resp->mutable_status()->set_code(MetaServiceCode::KV_TXN_COMMIT_ERR);
+ }
if (DCHECK_IS_ON()) {
MetaServiceCode code = resp->status().code();
DCHECK_NE(code, MetaServiceCode::KV_TXN_STORE_GET_RETRYABLE)
@@ -1049,6 +1054,8 @@ private:
<< "KV_TXN_STORE_COMMIT_RETRYABLE should not be sent back to client";
DCHECK_NE(code, MetaServiceCode::KV_TXN_STORE_CREATE_RETRYABLE)
<< "KV_TXN_STORE_CREATE_RETRYABLE should not be sent back to client";
+ DCHECK_NE(code, MetaServiceCode::KV_TXN_MAYBE_COMMITTED)
+ << "KV_TXN_MAYBE_COMMITTED should not be sent back to client";
}
return;
}
@@ -1088,8 +1095,7 @@ private:
code == MetaServiceCode::KV_TXN_STORE_COMMIT_RETRYABLE ? KV_TXN_COMMIT_ERR
: code == MetaServiceCode::KV_TXN_STORE_GET_RETRYABLE ? KV_TXN_GET_ERR
: code == MetaServiceCode::KV_TXN_STORE_CREATE_RETRYABLE ? KV_TXN_CREATE_ERR
- : code == MetaServiceCode::KV_TXN_MAYBE_COMMITTED
- ? MetaServiceCode::KV_TXN_MAYBE_COMMITTED
+ : code == MetaServiceCode::KV_TXN_MAYBE_COMMITTED ? KV_TXN_COMMIT_ERR
: code == MetaServiceCode::KV_TXN_CONFLICT
? KV_TXN_CONFLICT_RETRY_EXCEEDED_MAX_TIMES
: MetaServiceCode::KV_TXN_TOO_OLD);
diff --git a/cloud/test/meta_service_test.cpp b/cloud/test/meta_service_test.cpp
index 8bba8912eaf..a158629efd1 100644
--- a/cloud/test/meta_service_test.cpp
+++ b/cloud/test/meta_service_test.cpp
@@ -8458,7 +8458,111 @@ TEST(MetaServiceTxnStoreRetryableTest, DoNotReturnRetryableCode) {
config::txn_store_retry_times = retry_times;
}
-TEST(MetaServiceTxnStoreRetryableTest, RetryMaybeCommittedCode) {
+TEST(MetaServiceTxnStoreRetryableTest, CastAsPreservesMaybeCommittedForProxyRetry) {
+ bool enable_retry = config::enable_txn_store_retry;
+ DORIS_CLOUD_DEFER {
+ config::enable_txn_store_retry = enable_retry;
+ };
+
+ config::enable_txn_store_retry = false;
+ ASSERT_EQ(cast_as<ErrCategory::COMMIT>(TxnErrorCode::TXN_MAYBE_COMMITTED),
+ MetaServiceCode::KV_TXN_MAYBE_COMMITTED);
+ ASSERT_EQ(cast_as<ErrCategory::READ>(TxnErrorCode::TXN_RETRYABLE_NOT_COMMITTED),
+ MetaServiceCode::KV_TXN_MAYBE_COMMITTED);
+ ASSERT_EQ(cast_as<ErrCategory::CREATE>(TxnErrorCode::TXN_RETRYABLE_NOT_COMMITTED),
+ MetaServiceCode::KV_TXN_MAYBE_COMMITTED);
+
+ config::enable_txn_store_retry = true;
+ ASSERT_EQ(cast_as<ErrCategory::READ>(TxnErrorCode::TXN_RETRYABLE_NOT_COMMITTED),
+ MetaServiceCode::KV_TXN_STORE_GET_RETRYABLE);
+ ASSERT_EQ(cast_as<ErrCategory::COMMIT>(TxnErrorCode::TXN_RETRYABLE_NOT_COMMITTED),
+ MetaServiceCode::KV_TXN_STORE_COMMIT_RETRYABLE);
+ ASSERT_EQ(cast_as<ErrCategory::CREATE>(TxnErrorCode::TXN_RETRYABLE_NOT_COMMITTED),
+ MetaServiceCode::KV_TXN_STORE_COMMIT_RETRYABLE);
+ ASSERT_EQ(cast_as<ErrCategory::READ>(TxnErrorCode::TXN_TIMEOUT),
+ MetaServiceCode::KV_TXN_GET_ERR);
+ ASSERT_EQ(cast_as<ErrCategory::CREATE>(TxnErrorCode::TXN_TIMEOUT),
+ MetaServiceCode::KV_TXN_CREATE_ERR);
+}
+
+TEST(MetaServiceTxnStoreRetryableTest, MaybeCommittedCodeWithoutRetryReturnsCommitErr) {
+ size_t index = 0;
+ SyncPoint::get_instance()->set_call_back("update_delete_bitmap:commit:err", [&](auto&& args) {
+ ++index;
+ *doris::try_any_cast<TxnErrorCode*>(args[2]) = TxnErrorCode::TXN_MAYBE_COMMITTED;
+ });
+ SyncPoint::get_instance()->enable_processing();
+ bool enable_retry = config::enable_txn_store_retry;
+ int64_t max_txn_commit_byte = config::max_txn_commit_byte;
+ config::enable_txn_store_retry = false;
+ config::max_txn_commit_byte = 1;
+
+ auto service = get_meta_service();
+
+ brpc::Controller cntl;
+ UpdateDeleteBitmapRequest req;
+ UpdateDeleteBitmapResponse resp;
+ req.set_cloud_unique_id("test_cloud_unique_id");
+ req.set_table_id(100);
+ req.set_partition_id(123);
+ req.set_lock_id(-3);
+ req.set_without_lock(true);
+ req.set_initiator(-1);
+ req.set_tablet_id(333);
+ req.add_rowset_ids("r1");
+ req.add_segment_ids(0);
+ req.add_versions(2);
+ req.add_segment_delete_bitmaps("abc");
+
+ service->update_delete_bitmap(&cntl, &req, &resp, nullptr);
+
+ ASSERT_EQ(resp.status().code(), MetaServiceCode::KV_TXN_COMMIT_ERR)
+ << " status is " << resp.status().msg() << ", code=" << resp.status().code();
+ EXPECT_EQ(index, 1);
+
+ SyncPoint::get_instance()->disable_processing();
+ SyncPoint::get_instance()->clear_all_call_backs();
+ config::enable_txn_store_retry = enable_retry;
+ config::max_txn_commit_byte = max_txn_commit_byte;
+}
+
+TEST(MetaServiceTxnStoreRetryableTest, ReadMaybeCommittedCodeWithoutRetryReturnsCommitErr) {
+ size_t index = 0;
+ auto* sync_point = SyncPoint::get_instance();
+ sync_point->set_call_back("get_version_code", [&](auto&& args) {
+ ++index;
+ *doris::try_any_cast<MetaServiceCode*>(args[0]) = MetaServiceCode::KV_TXN_MAYBE_COMMITTED;
+ });
+ sync_point->enable_processing();
+ bool enable_retry = config::enable_txn_store_retry;
+ DORIS_CLOUD_DEFER {
+ config::enable_txn_store_retry = enable_retry;
+ sync_point->disable_processing();
+ sync_point->clear_all_call_backs();
+ };
+ config::enable_txn_store_retry = false;
+
+ auto service = get_meta_service();
+ create_tablet(service.get(), 1, 1, 1, 1);
+ insert_rowset(service.get(), 1, "read_maybe_committed_without_retry", 1, 1, 1);
+
+ brpc::Controller ctrl;
+ GetVersionRequest req;
+ req.set_cloud_unique_id("test_cloud_unique_id");
+ req.set_db_id(1);
+ req.set_table_id(1);
+ req.set_partition_id(1);
+
+ GetVersionResponse resp;
+ service->get_version(&ctrl, &req, &resp, nullptr);
+
+ ASSERT_EQ(resp.status().code(), MetaServiceCode::KV_TXN_COMMIT_ERR)
+ << " status is " << resp.status().msg() << ", code=" << resp.status().code();
+ EXPECT_EQ(resp.version(), 2);
+ EXPECT_EQ(index, 1);
+}
+
+TEST(MetaServiceTxnStoreRetryableTest, RetryMaybeCommittedCodeReturnsCommitErr) {
size_t index = 0;
SyncPoint::get_instance()->set_call_back("update_delete_bitmap:commit:err", [&](auto&& args) {
++index;
@@ -8491,7 +8595,7 @@ TEST(MetaServiceTxnStoreRetryableTest, RetryMaybeCommittedCode) {
service->update_delete_bitmap(&cntl, &req, &resp, nullptr);
- ASSERT_EQ(resp.status().code(), MetaServiceCode::KV_TXN_MAYBE_COMMITTED)
+ ASSERT_EQ(resp.status().code(), MetaServiceCode::KV_TXN_COMMIT_ERR)
<< " status is " << resp.status().msg() << ", code=" << resp.status().code();
EXPECT_GE(index, static_cast<size_t>(config::txn_store_retry_times + 1));
@@ -8502,6 +8606,45 @@ TEST(MetaServiceTxnStoreRetryableTest, RetryMaybeCommittedCode) {
config::max_txn_commit_byte = max_txn_commit_byte;
}
+TEST(MetaServiceTxnStoreRetryableTest, RetryReadMaybeCommittedCodeReturnsCommitErr) {
+ size_t index = 0;
+ auto* sync_point = SyncPoint::get_instance();
+ sync_point->set_call_back("get_version_code", [&](auto&& args) {
+ ++index;
+ *doris::try_any_cast<MetaServiceCode*>(args[0]) = MetaServiceCode::KV_TXN_MAYBE_COMMITTED;
+ });
+ sync_point->enable_processing();
+ int32_t retry_times = config::txn_store_retry_times;
+ bool enable_retry = config::enable_txn_store_retry;
+ DORIS_CLOUD_DEFER {
+ config::txn_store_retry_times = retry_times;
+ config::enable_txn_store_retry = enable_retry;
+ sync_point->disable_processing();
+ sync_point->clear_all_call_backs();
+ };
+ config::txn_store_retry_times = 2;
+ config::enable_txn_store_retry = true;
+
+ auto service = get_meta_service();
+ create_tablet(service.get(), 1, 1, 1, 1);
+ insert_rowset(service.get(), 1, "retry_read_maybe_committed", 1, 1, 1);
+
+ brpc::Controller ctrl;
+ GetVersionRequest req;
+ req.set_cloud_unique_id("test_cloud_unique_id");
+ req.set_db_id(1);
+ req.set_table_id(1);
+ req.set_partition_id(1);
+
+ GetVersionResponse resp;
+ service->get_version(&ctrl, &req, &resp, nullptr);
+
+ ASSERT_EQ(resp.status().code(), MetaServiceCode::KV_TXN_COMMIT_ERR)
+ << " status is " << resp.status().msg() << ", code=" << resp.status().code();
+ EXPECT_EQ(resp.version(), 2);
+ EXPECT_GE(index, static_cast<size_t>(config::txn_store_retry_times + 1));
+}
+
TEST(MetaServiceTest, GetClusterStatusTest) {
auto meta_service = get_meta_service();
diff --git a/gensrc/proto/cloud.proto b/gensrc/proto/cloud.proto
index 6e0d268f14a..569bcf6e023 100644
--- a/gensrc/proto/cloud.proto
+++ b/gensrc/proto/cloud.proto
@@ -1804,6 +1804,10 @@ enum MetaServiceCode {
KV_TXN_STORE_COMMIT_RETRYABLE = 1009;
KV_TXN_STORE_CREATE_RETRYABLE = 1010;
KV_TXN_TOO_OLD = 1011;
+ // WARNING: KV_TXN_MAYBE_COMMITTED is NOT returned to clients. It is kept as an
+ // internal retry signal inside MetaServiceProxy::call_impl(), then downgraded to
+ // KV_TXN_COMMIT_ERR before the response is sent back. Older BE/FE versions do not
+ // recognize this enum value, and proto2 would otherwise fall back to OK (= 0).
KV_TXN_MAYBE_COMMITTED = 1012;
//Doris error
@@ -1867,6 +1871,11 @@ enum MetaServiceCode {
SCHEMA_DICT_NOT_FOUND = 11001;
+ // WARNING: Before adding a new MetaServiceCode, consider backward compatibility.
+ // This is a proto2 optional enum field. If an older client receives an unrecognized
+ // enum value, the field is treated as unset and defaults to OK (= 0), silently
+ // turning errors into success. Any new error code that may be sent to older clients
+ // MUST be downgraded to a legacy code before the response is sent to the client.
UNDEFINED_ERR = 1000000;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org