This is an automated email from the ASF dual-hosted git repository.
dataroaring 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 c4732ca7088 [improve](cloud) unify snapshot properties to enum name
(#56337)
c4732ca7088 is described below
commit c4732ca70887607c118623761feac53e7d1f6589
Author: walter <[email protected]>
AuthorDate: Fri Sep 26 15:27:34 2025 +0800
[improve](cloud) unify snapshot properties to enum name (#56337)
---
cloud/src/meta-service/meta_service_http.cpp | 14 +++
cloud/src/meta-service/meta_service_resource.cpp | 127 +++++++++------------
cloud/test/meta_service_test.cpp | 100 +++++++++++-----
.../AdminSetAutoClusterSnapshotCommand.java | 10 +-
...dminSetClusterSnapshotFeatureSwitchCommand.java | 3 +-
gensrc/proto/cloud.proto | 10 +-
6 files changed, 162 insertions(+), 102 deletions(-)
diff --git a/cloud/src/meta-service/meta_service_http.cpp
b/cloud/src/meta-service/meta_service_http.cpp
index 54588ff4980..7eaa7dd6fd3 100644
--- a/cloud/src/meta-service/meta_service_http.cpp
+++ b/cloud/src/meta-service/meta_service_http.cpp
@@ -678,6 +678,20 @@ static HttpResponse
process_set_snapshot_property(MetaServiceImpl* service,
brpc::Controller* ctrl) {
AlterInstanceRequest req;
PARSE_MESSAGE_OR_RETURN(ctrl, req);
+ auto* properties = req.mutable_properties();
+ if (properties->contains("status")) {
+ std::string status = properties->at("status");
+ if (status != "ENABLED" && status != "DISABLED") {
+ return http_json_reply(MetaServiceCode::INVALID_ARGUMENT,
+ "Invalid value for status property: " +
status +
+ ", expected 'ENABLED' or 'DISABLED'
(case insensitive)");
+ }
+ std::string_view is_enable = (status == "ENABLED") ? "true" : "false";
+ const std::string& property_name =
+
AlterInstanceRequest::SnapshotProperty_Name(AlterInstanceRequest::ENABLE_SNAPSHOT);
+ (*properties)[property_name] = is_enable;
+ properties->erase("status");
+ }
req.set_op(AlterInstanceRequest::SET_SNAPSHOT_PROPERTY);
AlterInstanceResponse resp;
service->alter_instance(ctrl, &req, &resp, nullptr);
diff --git a/cloud/src/meta-service/meta_service_resource.cpp
b/cloud/src/meta-service/meta_service_resource.cpp
index 0701cc1db76..4bb5a99c5ad 100644
--- a/cloud/src/meta-service/meta_service_resource.cpp
+++ b/cloud/src/meta-service/meta_service_resource.cpp
@@ -48,10 +48,6 @@ using namespace std::chrono;
namespace {
constexpr char pattern_str[] = "^[a-zA-Z][0-9a-zA-Z_]*$";
-constexpr char SNAPSHOT_STATUS_KEY[] = "status";
-constexpr char SNAPSHOT_MAX_RESERVED_KEY[] = "max_reserved_snapshots";
-constexpr char SNAPSHOT_INTERVAL_SECONDS_KEY[] = "snapshot_interval_seconds";
-
bool is_valid_storage_vault_name(const std::string& str) {
const std::regex pattern(pattern_str);
return std::regex_match(str, pattern);
@@ -1818,17 +1814,14 @@ void
MetaServiceImpl::create_instance(google::protobuf::RpcController* controlle
}
std::pair<MetaServiceCode, std::string> handle_snapshot_switch(const
std::string& instance_id,
- const
std::string& key,
const
std::string& value,
InstanceInfoPB*
instance) {
- // Only allow "ENABLED" and "DISABLED" values (case insensitive)
- std::string value_upper = value;
- std::ranges::transform(value_upper, value_upper.begin(), ::toupper);
-
- if (value_upper != "ENABLED" && value_upper != "DISABLED") {
+ const std::string& property_name =
+
AlterInstanceRequest::SnapshotProperty_Name(AlterInstanceRequest::ENABLE_SNAPSHOT);
+ if (value != "true" && value != "false") {
return std::make_pair(MetaServiceCode::INVALID_ARGUMENT,
- "Invalid value for enabled property: " + value +
- ", expected 'ENABLED' or 'DISABLED'
(case insensitive)" +
+ "Invalid value for " + property_name + "
property: " + value +
+ ", expected 'true' or 'false'" +
", instance_id: " + instance_id);
}
@@ -1836,56 +1829,48 @@ std::pair<MetaServiceCode, std::string>
handle_snapshot_switch(const std::string
if (instance->snapshot_switch_status() == SNAPSHOT_SWITCH_DISABLED) {
return std::make_pair(MetaServiceCode::INVALID_ARGUMENT,
"Snapshot not ready, instance_id: " +
instance_id);
- }
-
- // Determine target status
- SnapshotSwitchStatus target_status =
- (value_upper == "ENABLED") ? SNAPSHOT_SWITCH_ON :
SNAPSHOT_SWITCH_OFF;
-
- // Check if the status is already set to the target value
- if (instance->snapshot_switch_status() == target_status) {
- std::string status_name = (target_status == SNAPSHOT_SWITCH_ON) ?
"ENABLED" : "DISABLED";
+ } else if (value == "false" && instance->snapshot_switch_status() ==
SNAPSHOT_SWITCH_OFF) {
return std::make_pair(
MetaServiceCode::INVALID_ARGUMENT,
- "Snapshot is already set to " + status_name + ", instance_id:
" + instance_id);
- }
-
- // Set the new status
- instance->set_snapshot_switch_status(target_status);
+ "Snapshot is already set to SNAPSHOT_SWITCH_OFF, instance_id:
" + instance_id);
+ } else if (value == "true") {
+ instance->set_snapshot_switch_status(SNAPSHOT_SWITCH_ON);
- // Set default values when first enabling snapshot
- if (target_status == SNAPSHOT_SWITCH_ON) {
+ // Set default values when first enabling snapshot
if (!instance->has_snapshot_interval_seconds() ||
instance->snapshot_interval_seconds() == 0) {
- instance->set_snapshot_interval_seconds(3600);
- LOG(INFO) << "Set default snapshot_interval_seconds to 3600 for
instance "
- << instance_id;
+
instance->set_snapshot_interval_seconds(config::snapshot_min_interval_seconds);
+ LOG(INFO) << "Set default snapshot_interval_seconds to "
+ << config::snapshot_min_interval_seconds << " for
instance " << instance_id;
}
if (!instance->has_max_reserved_snapshot() ||
instance->max_reserved_snapshot() == 0) {
instance->set_max_reserved_snapshot(1);
LOG(INFO) << "Set default max_reserved_snapshots to 1 for instance
" << instance_id;
}
+ } else {
+ instance->set_snapshot_switch_status(SNAPSHOT_SWITCH_OFF);
}
- std::string msg = "Set snapshot enabled to " + value + " for instance " +
instance_id;
- LOG(INFO) << msg;
+ LOG(INFO) << "Set snapshot " + property_name + " to " + value + " for
instance " + instance_id;
return std::make_pair(MetaServiceCode::OK, "");
}
std::pair<MetaServiceCode, std::string> handle_max_reserved_snapshots(
- const std::string& instance_id, const std::string& key, const
std::string& value,
- InstanceInfoPB* instance) {
+ const std::string& instance_id, const std::string& value,
InstanceInfoPB* instance) {
+ const std::string& property_name =
AlterInstanceRequest::SnapshotProperty_Name(
+ AlterInstanceRequest::MAX_RESERVED_SNAPSHOTS);
+
int max_snapshots;
try {
max_snapshots = std::stoi(value);
if (max_snapshots < 0) {
return std::make_pair(MetaServiceCode::INVALID_ARGUMENT,
- "max_reserved_snapshots must be
non-negative, got: " + value);
+ property_name + " must be non-negative, got:
" + value);
}
if (max_snapshots > config::snapshot_max_reserved_num) {
return std::make_pair(MetaServiceCode::INVALID_ARGUMENT,
- "max_reserved_snapshots too large, maximum
is " +
+ property_name + " too large, maximum is " +
std::to_string(config::snapshot_max_reserved_num) +
", got: " + value);
}
@@ -1896,34 +1881,34 @@ std::pair<MetaServiceCode, std::string>
handle_max_reserved_snapshots(
instance->set_max_reserved_snapshot(max_snapshots);
- std::string msg = "Set max_reserved_snapshots to " + value + " for
instance " + instance_id;
- LOG(INFO) << msg;
+ LOG(INFO) << "Set " + property_name + " to " + value + " for instance " +
instance_id;
return std::make_pair(MetaServiceCode::OK, "");
}
std::pair<MetaServiceCode, std::string> handle_snapshot_intervals(const
std::string& instance_id,
- const
std::string& key,
const
std::string& value,
InstanceInfoPB* instance) {
+ const std::string& property_name =
AlterInstanceRequest::SnapshotProperty_Name(
+ AlterInstanceRequest::SNAPSHOT_INTERVAL_SECONDS);
+
int intervals;
try {
intervals = std::stoi(value);
if (intervals < config::snapshot_min_interval_seconds) {
return std::make_pair(MetaServiceCode::INVALID_ARGUMENT,
- "snapshot_intervals too small, minimum is " +
+ property_name + " too small, minimum is " +
std::to_string(config::snapshot_min_interval_seconds) +
" seconds, got: " + value);
}
} catch (const std::exception& e) {
return std::make_pair(MetaServiceCode::INVALID_ARGUMENT,
- "Invalid numeric value for snapshot_intervals: "
+ value);
+ "Invalid numeric value for " + property_name +
": " + value);
}
instance->set_snapshot_interval_seconds(intervals);
- std::string msg = "Set snapshot_intervals to " + value + " seconds for
instance " + instance_id;
- LOG(INFO) << msg;
+ LOG(INFO) << "Set " + property_name + " to " + value + " seconds for
instance " + instance_id;
return std::make_pair(MetaServiceCode::OK, "");
}
@@ -2119,56 +2104,58 @@ void
MetaServiceImpl::alter_instance(google::protobuf::RpcController* controller
} break;
/**
* Handle SET_SNAPSHOT_PROPERTY operation - configures snapshot-related
properties for an instance.
- *
+ *
* Supported property keys and their expected values:
- * - "status": "UNSUPPORTED" | "ENABLED" | "DISABLED"
- * Controls the snapshot functionality status for the instance
- *
- * - "max_reserved_snapshots": numeric string (0-35)
+ * - "ENABLE_SNAPSHOT": "true" | "false"
+ * Controls whether snapshot functionality is enabled for the instance
+ *
+ * - "MAX_RESERVED_SNAPSHOTS": numeric string
(0-config::snapshot_max_reserved_num)
* Sets the maximum number of snapshots to retain for the instance
- *
- * - "snapshot_intervals": numeric string (60-max)
- * Sets the snapshot creation interval in seconds (minimum 60s)
- *
+ *
+ * - "SNAPSHOT_INTERVAL_SECONDS": numeric string
(config::snapshot_min_interval_seconds-max)
+ * Sets the snapshot creation interval in seconds (minimum controlled by
config)
+ *
* Each property is validated by its respective handler function which
ensures
* the provided values conform to the expected format and constraints.
*/
case AlterInstanceRequest::SET_SNAPSHOT_PROPERTY: {
- ret = alter_instance(request, [&request](InstanceInfoPB* instance) {
+ ret = alter_instance(request, [&request, &instance_id](InstanceInfoPB*
instance) {
std::string msg;
auto properties = request->properties();
if (properties.empty()) {
- msg = "propertiy is empty, instance_id = " +
request->instance_id();
+ msg = "snapshot properties is empty, instance_id = " +
request->instance_id();
LOG(WARNING) << msg;
return std::make_pair(MetaServiceCode::INVALID_ARGUMENT, msg);
}
for (const auto& property : properties) {
std::string key = property.first;
std::string value = property.second;
+ AlterInstanceRequest::SnapshotProperty snapshot_property =
+ AlterInstanceRequest::UNKNOWN_PROPERTY;
+ if (!AlterInstanceRequest_SnapshotProperty_Parse(key,
&snapshot_property) ||
+ snapshot_property ==
AlterInstanceRequest::UNKNOWN_PROPERTY) {
+ msg = "unknown snapshot property: " + key;
+ LOG(WARNING) << "alter instance failed: " << msg
+ << ", instance_id = " << instance_id;
+ return std::make_pair(MetaServiceCode::INVALID_ARGUMENT,
msg);
+ }
std::pair<MetaServiceCode, std::string> result;
- if (key == SNAPSHOT_STATUS_KEY) {
- result = handle_snapshot_switch(request->instance_id(),
key, value, instance);
- } else if (key == SNAPSHOT_MAX_RESERVED_KEY) {
- result =
handle_max_reserved_snapshots(request->instance_id(), key, value,
- instance);
- } else if (key == SNAPSHOT_INTERVAL_SECONDS_KEY) {
- result =
- handle_snapshot_intervals(request->instance_id(),
key, value, instance);
+ if (snapshot_property ==
AlterInstanceRequest::ENABLE_SNAPSHOT) {
+ result = handle_snapshot_switch(request->instance_id(),
value, instance);
+ } else if (snapshot_property ==
AlterInstanceRequest::MAX_RESERVED_SNAPSHOTS) {
+ result =
handle_max_reserved_snapshots(request->instance_id(), value, instance);
+ } else if (snapshot_property ==
AlterInstanceRequest::SNAPSHOT_INTERVAL_SECONDS) {
+ result = handle_snapshot_intervals(request->instance_id(),
value, instance);
} else {
msg = "unsupported property: " + key;
- LOG(WARNING) << msg;
+ LOG(WARNING) << "alter instance failed: " << msg
+ << ", instance_id = " << instance_id;
return std::make_pair(MetaServiceCode::INVALID_ARGUMENT,
msg);
}
- LOG(INFO) << "Property handling result for key=" << key
- << ", result_code=" << static_cast<int>(result.first)
- << ", result_msg=" << result.second;
-
if (result.first != MetaServiceCode::OK) {
- msg = result.second;
- LOG(WARNING) << msg;
return result;
}
}
diff --git a/cloud/test/meta_service_test.cpp b/cloud/test/meta_service_test.cpp
index d5185913c04..ecb9c2e5fad 100644
--- a/cloud/test/meta_service_test.cpp
+++ b/cloud/test/meta_service_test.cpp
@@ -11563,7 +11563,8 @@ TEST(MetaServiceTest, SetSnapshotPropertyTest) {
AlterInstanceResponse res;
req.set_op(AlterInstanceRequest::SET_SNAPSHOT_PROPERTY);
req.set_instance_id("test_snapshot_instance");
- (*req.mutable_properties())["status"] = "ENABLED";
+ (*req.mutable_properties())[AlterInstanceRequest_SnapshotProperty_Name(
+ AlterInstanceRequest::ENABLE_SNAPSHOT)] = "true";
meta_service->alter_instance(reinterpret_cast<::google::protobuf::RpcController*>(&cntl),
&req, &res, nullptr);
@@ -11595,7 +11596,8 @@ TEST(MetaServiceTest, SetSnapshotPropertyTest) {
AlterInstanceResponse res;
req.set_op(AlterInstanceRequest::SET_SNAPSHOT_PROPERTY);
req.set_instance_id("test_snapshot_instance");
- (*req.mutable_properties())["status"] = "ENABLED";
+ (*req.mutable_properties())[AlterInstanceRequest_SnapshotProperty_Name(
+ AlterInstanceRequest::ENABLE_SNAPSHOT)] = "true";
meta_service->alter_instance(reinterpret_cast<::google::protobuf::RpcController*>(&cntl),
&req, &res, nullptr);
@@ -11609,7 +11611,8 @@ TEST(MetaServiceTest, SetSnapshotPropertyTest) {
AlterInstanceResponse res;
req.set_op(AlterInstanceRequest::SET_SNAPSHOT_PROPERTY);
req.set_instance_id("test_snapshot_instance");
- (*req.mutable_properties())["status"] = "DISABLED";
+ (*req.mutable_properties())[AlterInstanceRequest_SnapshotProperty_Name(
+ AlterInstanceRequest::ENABLE_SNAPSHOT)] = "false";
meta_service->alter_instance(reinterpret_cast<::google::protobuf::RpcController*>(&cntl),
&req, &res, nullptr);
@@ -11623,7 +11626,8 @@ TEST(MetaServiceTest, SetSnapshotPropertyTest) {
AlterInstanceResponse res;
req.set_op(AlterInstanceRequest::SET_SNAPSHOT_PROPERTY);
req.set_instance_id("test_snapshot_instance");
- (*req.mutable_properties())["status"] = "invalid";
+ (*req.mutable_properties())[AlterInstanceRequest_SnapshotProperty_Name(
+ AlterInstanceRequest::ENABLE_SNAPSHOT)] = "invalid";
meta_service->alter_instance(reinterpret_cast<::google::protobuf::RpcController*>(&cntl),
&req, &res, nullptr);
@@ -11637,7 +11641,8 @@ TEST(MetaServiceTest, SetSnapshotPropertyTest) {
AlterInstanceResponse res;
req.set_op(AlterInstanceRequest::SET_SNAPSHOT_PROPERTY);
req.set_instance_id("test_snapshot_instance");
- (*req.mutable_properties())["max_reserved_snapshots"] = "0";
+ (*req.mutable_properties())[AlterInstanceRequest_SnapshotProperty_Name(
+ AlterInstanceRequest::MAX_RESERVED_SNAPSHOTS)] = "0";
meta_service->alter_instance(reinterpret_cast<::google::protobuf::RpcController*>(&cntl),
&req, &res, nullptr);
@@ -11651,7 +11656,8 @@ TEST(MetaServiceTest, SetSnapshotPropertyTest) {
AlterInstanceResponse res;
req.set_op(AlterInstanceRequest::SET_SNAPSHOT_PROPERTY);
req.set_instance_id("test_snapshot_instance");
- (*req.mutable_properties())["max_reserved_snapshots"] = "35";
+ (*req.mutable_properties())[AlterInstanceRequest_SnapshotProperty_Name(
+ AlterInstanceRequest::MAX_RESERVED_SNAPSHOTS)] = "35";
meta_service->alter_instance(reinterpret_cast<::google::protobuf::RpcController*>(&cntl),
&req, &res, nullptr);
@@ -11665,7 +11671,8 @@ TEST(MetaServiceTest, SetSnapshotPropertyTest) {
AlterInstanceResponse res;
req.set_op(AlterInstanceRequest::SET_SNAPSHOT_PROPERTY);
req.set_instance_id("test_snapshot_instance");
- (*req.mutable_properties())["max_reserved_snapshots"] = "10";
+ (*req.mutable_properties())[AlterInstanceRequest_SnapshotProperty_Name(
+ AlterInstanceRequest::MAX_RESERVED_SNAPSHOTS)] = "10";
meta_service->alter_instance(reinterpret_cast<::google::protobuf::RpcController*>(&cntl),
&req, &res, nullptr);
@@ -11679,7 +11686,8 @@ TEST(MetaServiceTest, SetSnapshotPropertyTest) {
AlterInstanceResponse res;
req.set_op(AlterInstanceRequest::SET_SNAPSHOT_PROPERTY);
req.set_instance_id("test_snapshot_instance");
- (*req.mutable_properties())["max_reserved_snapshots"] = "-1";
+ (*req.mutable_properties())[AlterInstanceRequest_SnapshotProperty_Name(
+ AlterInstanceRequest::MAX_RESERVED_SNAPSHOTS)] = "-1";
meta_service->alter_instance(reinterpret_cast<::google::protobuf::RpcController*>(&cntl),
&req, &res, nullptr);
@@ -11693,7 +11701,8 @@ TEST(MetaServiceTest, SetSnapshotPropertyTest) {
AlterInstanceResponse res;
req.set_op(AlterInstanceRequest::SET_SNAPSHOT_PROPERTY);
req.set_instance_id("test_snapshot_instance");
- (*req.mutable_properties())["max_reserved_snapshots"] = "36";
+ (*req.mutable_properties())[AlterInstanceRequest_SnapshotProperty_Name(
+ AlterInstanceRequest::MAX_RESERVED_SNAPSHOTS)] = "36";
meta_service->alter_instance(reinterpret_cast<::google::protobuf::RpcController*>(&cntl),
&req, &res, nullptr);
@@ -11707,7 +11716,8 @@ TEST(MetaServiceTest, SetSnapshotPropertyTest) {
AlterInstanceResponse res;
req.set_op(AlterInstanceRequest::SET_SNAPSHOT_PROPERTY);
req.set_instance_id("test_snapshot_instance");
- (*req.mutable_properties())["max_reserved_snapshots"] = "abc";
+ (*req.mutable_properties())[AlterInstanceRequest_SnapshotProperty_Name(
+ AlterInstanceRequest::MAX_RESERVED_SNAPSHOTS)] = "abc";
meta_service->alter_instance(reinterpret_cast<::google::protobuf::RpcController*>(&cntl),
&req, &res, nullptr);
@@ -11721,7 +11731,8 @@ TEST(MetaServiceTest, SetSnapshotPropertyTest) {
AlterInstanceResponse res;
req.set_op(AlterInstanceRequest::SET_SNAPSHOT_PROPERTY);
req.set_instance_id("test_snapshot_instance");
- (*req.mutable_properties())["snapshot_interval_seconds"] = "3600";
+ (*req.mutable_properties())[AlterInstanceRequest_SnapshotProperty_Name(
+ AlterInstanceRequest::SNAPSHOT_INTERVAL_SECONDS)] = "3600";
meta_service->alter_instance(reinterpret_cast<::google::protobuf::RpcController*>(&cntl),
&req, &res, nullptr);
@@ -11735,7 +11746,8 @@ TEST(MetaServiceTest, SetSnapshotPropertyTest) {
AlterInstanceResponse res;
req.set_op(AlterInstanceRequest::SET_SNAPSHOT_PROPERTY);
req.set_instance_id("test_snapshot_instance");
- (*req.mutable_properties())["snapshot_interval_seconds"] = "14400";
+ (*req.mutable_properties())[AlterInstanceRequest_SnapshotProperty_Name(
+ AlterInstanceRequest::SNAPSHOT_INTERVAL_SECONDS)] = "14400";
meta_service->alter_instance(reinterpret_cast<::google::protobuf::RpcController*>(&cntl),
&req, &res, nullptr);
@@ -11749,7 +11761,8 @@ TEST(MetaServiceTest, SetSnapshotPropertyTest) {
AlterInstanceResponse res;
req.set_op(AlterInstanceRequest::SET_SNAPSHOT_PROPERTY);
req.set_instance_id("test_snapshot_instance");
- (*req.mutable_properties())["snapshot_interval_seconds"] = "3599";
+ (*req.mutable_properties())[AlterInstanceRequest_SnapshotProperty_Name(
+ AlterInstanceRequest::SNAPSHOT_INTERVAL_SECONDS)] = "3599";
meta_service->alter_instance(reinterpret_cast<::google::protobuf::RpcController*>(&cntl),
&req, &res, nullptr);
@@ -11763,7 +11776,8 @@ TEST(MetaServiceTest, SetSnapshotPropertyTest) {
AlterInstanceResponse res;
req.set_op(AlterInstanceRequest::SET_SNAPSHOT_PROPERTY);
req.set_instance_id("test_snapshot_instance");
- (*req.mutable_properties())["snapshot_interval_seconds"] = "invalid";
+ (*req.mutable_properties())[AlterInstanceRequest_SnapshotProperty_Name(
+ AlterInstanceRequest::SNAPSHOT_INTERVAL_SECONDS)] = "invalid";
meta_service->alter_instance(reinterpret_cast<::google::protobuf::RpcController*>(&cntl),
&req, &res, nullptr);
@@ -11805,9 +11819,12 @@ TEST(MetaServiceTest, SetSnapshotPropertyTest) {
AlterInstanceResponse res;
req.set_op(AlterInstanceRequest::SET_SNAPSHOT_PROPERTY);
req.set_instance_id("test_snapshot_instance");
- (*req.mutable_properties())["status"] = "ENABLED";
- (*req.mutable_properties())["max_reserved_snapshots"] = "20";
- (*req.mutable_properties())["snapshot_interval_seconds"] = "12000";
+ (*req.mutable_properties())[AlterInstanceRequest_SnapshotProperty_Name(
+ AlterInstanceRequest::ENABLE_SNAPSHOT)] = "true";
+ (*req.mutable_properties())[AlterInstanceRequest_SnapshotProperty_Name(
+ AlterInstanceRequest::MAX_RESERVED_SNAPSHOTS)] = "20";
+ (*req.mutable_properties())[AlterInstanceRequest_SnapshotProperty_Name(
+ AlterInstanceRequest::SNAPSHOT_INTERVAL_SECONDS)] = "12000";
meta_service->alter_instance(reinterpret_cast<::google::protobuf::RpcController*>(&cntl),
&req, &res, nullptr);
@@ -11821,13 +11838,17 @@ TEST(MetaServiceTest, SetSnapshotPropertyTest) {
AlterInstanceResponse res;
req.set_op(AlterInstanceRequest::SET_SNAPSHOT_PROPERTY);
req.set_instance_id("test_snapshot_instance");
- (*req.mutable_properties())["status"] = "ENABLED";
- (*req.mutable_properties())["invalid_property"] = "value";
- (*req.mutable_properties())["max_reserved_snapshots"] = "10";
+ (*req.mutable_properties())[AlterInstanceRequest_SnapshotProperty_Name(
+ AlterInstanceRequest::ENABLE_SNAPSHOT)] = "true";
+ (*req.mutable_properties())[AlterInstanceRequest_SnapshotProperty_Name(
+ AlterInstanceRequest::MAX_RESERVED_SNAPSHOTS)] = "10";
+ (*req.mutable_properties())[AlterInstanceRequest_SnapshotProperty_Name(
+ AlterInstanceRequest::SNAPSHOT_INTERVAL_SECONDS)] = "12000";
+ (*req.mutable_properties())["unsupported_property"] = "value";
meta_service->alter_instance(reinterpret_cast<::google::protobuf::RpcController*>(&cntl),
&req, &res, nullptr);
- ASSERT_EQ(res.status().code(), MetaServiceCode::INVALID_ARGUMENT);
+ ASSERT_EQ(res.status().code(), MetaServiceCode::INVALID_ARGUMENT) <<
res.ShortDebugString();
}
// Test case 19: Non-existent instance ID
@@ -11837,7 +11858,8 @@ TEST(MetaServiceTest, SetSnapshotPropertyTest) {
AlterInstanceResponse res;
req.set_op(AlterInstanceRequest::SET_SNAPSHOT_PROPERTY);
req.set_instance_id("non_existent_instance");
- (*req.mutable_properties())["status"] = "ENABLED";
+ (*req.mutable_properties())[AlterInstanceRequest_SnapshotProperty_Name(
+ AlterInstanceRequest::ENABLE_SNAPSHOT)] = "true";
meta_service->alter_instance(reinterpret_cast<::google::protobuf::RpcController*>(&cntl),
&req, &res, nullptr);
@@ -11908,7 +11930,8 @@ TEST(MetaServiceTest, SnapshotConfigLimitsTest) {
AlterInstanceResponse alter_res;
alter_req.set_op(AlterInstanceRequest::SET_SNAPSHOT_PROPERTY);
alter_req.set_instance_id("test_snapshot_config_instance");
- (*alter_req.mutable_properties())["status"] = "enabled";
+
(*alter_req.mutable_properties())[AlterInstanceRequest_SnapshotProperty_Name(
+ AlterInstanceRequest::ENABLE_SNAPSHOT)] = "true";
meta_service->alter_instance(reinterpret_cast<::google::protobuf::RpcController*>(&cntl),
&alter_req, &alter_res, nullptr);
@@ -11930,7 +11953,8 @@ TEST(MetaServiceTest, SnapshotConfigLimitsTest) {
AlterInstanceResponse res;
req.set_op(AlterInstanceRequest::SET_SNAPSHOT_PROPERTY);
req.set_instance_id("test_snapshot_config_instance");
- (*req.mutable_properties())["snapshot_interval_seconds"] = "1799"; //
Below min limit
+ (*req.mutable_properties())[AlterInstanceRequest_SnapshotProperty_Name(
+ AlterInstanceRequest::SNAPSHOT_INTERVAL_SECONDS)] = "1799"; //
Below min limit
meta_service->alter_instance(reinterpret_cast<::google::protobuf::RpcController*>(&cntl),
&req, &res, nullptr);
@@ -11945,7 +11969,8 @@ TEST(MetaServiceTest, SnapshotConfigLimitsTest) {
AlterInstanceResponse res;
req.set_op(AlterInstanceRequest::SET_SNAPSHOT_PROPERTY);
req.set_instance_id("test_snapshot_config_instance");
- (*req.mutable_properties())["snapshot_interval_seconds"] = "3600"; //
Within limits
+ (*req.mutable_properties())[AlterInstanceRequest_SnapshotProperty_Name(
+ AlterInstanceRequest::SNAPSHOT_INTERVAL_SECONDS)] = "3600"; //
Within limits
meta_service->alter_instance(reinterpret_cast<::google::protobuf::RpcController*>(&cntl),
&req, &res, nullptr);
@@ -11959,7 +11984,8 @@ TEST(MetaServiceTest, SnapshotConfigLimitsTest) {
AlterInstanceResponse res;
req.set_op(AlterInstanceRequest::SET_SNAPSHOT_PROPERTY);
req.set_instance_id("test_snapshot_config_instance");
- (*req.mutable_properties())["max_reserved_snapshots"] = "21"; // Above
max limit
+ (*req.mutable_properties())[AlterInstanceRequest_SnapshotProperty_Name(
+ AlterInstanceRequest::MAX_RESERVED_SNAPSHOTS)] = "21"; //
Above max limit
meta_service->alter_instance(reinterpret_cast<::google::protobuf::RpcController*>(&cntl),
&req, &res, nullptr);
@@ -11974,13 +12000,30 @@ TEST(MetaServiceTest, SnapshotConfigLimitsTest) {
AlterInstanceResponse res;
req.set_op(AlterInstanceRequest::SET_SNAPSHOT_PROPERTY);
req.set_instance_id("test_snapshot_config_instance");
- (*req.mutable_properties())["max_reserved_snapshots"] = "10"; //
Within limits
+ (*req.mutable_properties())[AlterInstanceRequest_SnapshotProperty_Name(
+ AlterInstanceRequest::MAX_RESERVED_SNAPSHOTS)] = "10"; //
Within limits
meta_service->alter_instance(reinterpret_cast<::google::protobuf::RpcController*>(&cntl),
&req, &res, nullptr);
ASSERT_EQ(res.status().code(), MetaServiceCode::OK);
}
+ // Test case 5: unknown property should fail
+ {
+ brpc::Controller cntl;
+ AlterInstanceRequest req;
+ AlterInstanceResponse res;
+ req.set_op(AlterInstanceRequest::SET_SNAPSHOT_PROPERTY);
+ req.set_instance_id("test_snapshot_config_instance");
+ (*req.mutable_properties())["unknown_property"] = "value";
+
+
meta_service->alter_instance(reinterpret_cast<::google::protobuf::RpcController*>(&cntl),
+ &req, &res, nullptr);
+ ASSERT_EQ(res.status().code(), MetaServiceCode::INVALID_ARGUMENT);
+ ASSERT_TRUE(res.status().msg().find("unknown snapshot property:
unknown_property") !=
+ std::string::npos);
+ }
+
// Restore original config values
config::snapshot_min_interval_seconds = orig_min_interval;
config::snapshot_max_reserved_num = orig_max_reserved;
@@ -12045,7 +12088,8 @@ TEST(MetaServiceTest, SnapshotDefaultValuesTest) {
AlterInstanceResponse res;
req.set_op(AlterInstanceRequest::SET_SNAPSHOT_PROPERTY);
req.set_instance_id("test_instance");
- (*req.mutable_properties())["status"] = "ENABLED";
+ (*req.mutable_properties())[AlterInstanceRequest_SnapshotProperty_Name(
+ AlterInstanceRequest::ENABLE_SNAPSHOT)] = "true";
meta_service->alter_instance(reinterpret_cast<::google::protobuf::RpcController*>(&cntl),
&req, &res, nullptr);
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AdminSetAutoClusterSnapshotCommand.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AdminSetAutoClusterSnapshotCommand.java
index 3d61e1ad656..688de52cba4 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AdminSetAutoClusterSnapshotCommand.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AdminSetAutoClusterSnapshotCommand.java
@@ -68,7 +68,15 @@ public class AdminSetAutoClusterSnapshotCommand extends
Command implements Forwa
.setInstanceId(((CloudEnv)
Env.getCurrentEnv()).getCloudInstanceId())
.setOp(Cloud.AlterInstanceRequest.Operation.SET_SNAPSHOT_PROPERTY);
for (Map.Entry<String, String> entry : properties.entrySet()) {
- builder.putProperties(entry.getKey().toLowerCase(),
entry.getValue().toLowerCase());
+ String property;
+ if (entry.getKey().equalsIgnoreCase(PROP_MAX_RESERVED_SNAPSHOTS)) {
+ property =
Cloud.AlterInstanceRequest.SnapshotProperty.MAX_RESERVED_SNAPSHOTS.name();
+ } else if
(entry.getKey().equalsIgnoreCase(PROP_SNAPSHOT_INTERVAL_SECONDS)) {
+ property =
Cloud.AlterInstanceRequest.SnapshotProperty.SNAPSHOT_INTERVAL_SECONDS.name();
+ } else {
+ throw new RuntimeException("Unknown property: " +
entry.getKey());
+ }
+ builder.putProperties(property, entry.getValue().toLowerCase());
}
CloudSnapshotHandler cloudSnapshotHandler = ((CloudEnv)
ctx.getEnv()).getCloudSnapshotHandler();
cloudSnapshotHandler.alterInstance(builder.build());
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AdminSetClusterSnapshotFeatureSwitchCommand.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AdminSetClusterSnapshotFeatureSwitchCommand.java
index ceadd609746..371a3d14272 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AdminSetClusterSnapshotFeatureSwitchCommand.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AdminSetClusterSnapshotFeatureSwitchCommand.java
@@ -40,7 +40,6 @@ import org.apache.logging.log4j.Logger;
*/
public class AdminSetClusterSnapshotFeatureSwitchCommand extends Command
implements ForwardWithSync {
- public static final String PROP_ENABLED = "enabled";
private static final Logger LOG =
LogManager.getLogger(AdminSetClusterSnapshotFeatureSwitchCommand.class);
private boolean on;
@@ -59,7 +58,7 @@ public class AdminSetClusterSnapshotFeatureSwitchCommand
extends Command impleme
Cloud.AlterInstanceRequest.Builder builder =
Cloud.AlterInstanceRequest.newBuilder()
.setInstanceId(((CloudEnv)
Env.getCurrentEnv()).getCloudInstanceId())
.setOp(Cloud.AlterInstanceRequest.Operation.SET_SNAPSHOT_PROPERTY)
- .putProperties(PROP_ENABLED, String.valueOf(on));
+
.putProperties(Cloud.AlterInstanceRequest.SnapshotProperty.ENABLE_SNAPSHOT.name(),
String.valueOf(on));
CloudSnapshotHandler cloudSnapshotHandler = ((CloudEnv)
ctx.getEnv()).getCloudSnapshotHandler();
cloudSnapshotHandler.alterInstance(builder.build());
cloudSnapshotHandler.refreshAutoSnapshotJob();
diff --git a/gensrc/proto/cloud.proto b/gensrc/proto/cloud.proto
index 253ec7318b5..5234ac10eea 100644
--- a/gensrc/proto/cloud.proto
+++ b/gensrc/proto/cloud.proto
@@ -1374,11 +1374,19 @@ message AlterInstanceRequest {
SET_NORMAL = 7;
SET_SNAPSHOT_PROPERTY = 8;
}
+
+ enum SnapshotProperty {
+ UNKNOWN_PROPERTY = 0;
+ ENABLE_SNAPSHOT = 1; // true or false
+ MAX_RESERVED_SNAPSHOTS = 2; // in [0,
max_reserved_snapshots]
+ SNAPSHOT_INTERVAL_SECONDS = 3; // in [min_interval_seconds,
+inf)
+ }
+
optional string instance_id = 1;
optional Operation op = 2;
optional string name = 3;
optional string request_ip = 4;
- map<string, string> properties = 5;
+ map<string, string> properties = 5; // key is the Name of SnapshotProperty
value.
}
message AlterInstanceResponse {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
