Skip to content

Commit 92651f7

Browse files
Yukang-LianYukang-Lian
authored
[improve](snapshot) Modify some snapshot interface (#56393)
1 parent 9f9f1eb commit 92651f7

File tree

8 files changed

+326
-52
lines changed

8 files changed

+326
-52
lines changed

‎cloud/src/common/config.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,4 +366,8 @@ CONF_mBool(enable_logging_conflict_keys, "false");
366366
// Default is 1 hour (3600 seconds).
367367
CONF_Int64(prune_aborted_snapshot_seconds, "3600"); // 1h
368368

369+
// Snapshot configuration limits
370+
CONF_Int32(snapshot_min_interval_seconds, "3600"); // 1h min interval limit
371+
CONF_Int32(snapshot_max_reserved_num, "35"); // max reserved snapshots limit
372+
369373
} // namespace doris::cloud::config

‎cloud/src/meta-service/meta_service_http.cpp‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -687,12 +687,18 @@ static HttpResponse process_set_snapshot_property(MetaServiceImpl* service,
687687
static HttpResponse process_set_multi_version_status(MetaServiceImpl* service,
688688
brpc::Controller* ctrl) {
689689
auto& uri = ctrl->http_request().uri();
690-
std::string cloud_unique_id(http_query(uri, "cloud_unique_id"));
691690
std::string instance_id(http_query(uri, "instance_id"));
691+
std::string cloud_unique_id(http_query(uri, "cloud_unique_id"));
692692
std::string multi_version_status_str(http_query(uri, "multi_version_status"));
693693

694-
if (cloud_unique_id.empty() || instance_id.empty() || multi_version_status_str.empty()) {
695-
return http_json_reply(MetaServiceCode::INVALID_ARGUMENT, "missing required arguments");
694+
// Prefer instance_id if provided, fallback to cloud_unique_id
695+
if (instance_id.empty()) {
696+
return http_json_reply(MetaServiceCode::INVALID_ARGUMENT, "empty instance id");
697+
}
698+
699+
if (multi_version_status_str.empty()) {
700+
return http_json_reply(MetaServiceCode::INVALID_ARGUMENT,
701+
"multi_version_status is required");
696702
}
697703

698704
// Parse multi_version_status from string to enum
@@ -716,8 +722,9 @@ static HttpResponse process_set_multi_version_status(MetaServiceImpl* service,
716722
"MULTI_VERSION_WRITE_ONLY, MULTI_VERSION_READ_WRITE, MULTI_VERSION_ENABLED");
717723
}
718724
// Call snapshot manager directly
719-
auto [code, msg] = service->snapshot_manager()->set_multi_version_status(
720-
instance_id, cloud_unique_id, multi_version_status);
725+
auto [code, msg] = service->snapshot_manager()->set_multi_version_status(instance_id,
726+
multi_version_status);
727+
721728
return http_json_reply(code, msg);
722729
}
723730

@@ -751,19 +758,19 @@ static HttpResponse process_get_snapshot_property(MetaServiceImpl* service,
751758
std::string switch_status;
752759
switch (instance.snapshot_switch_status()) {
753760
case SNAPSHOT_SWITCH_DISABLED:
754-
switch_status = "disabled";
761+
switch_status = "UNSUPPORTED";
755762
break;
756763
case SNAPSHOT_SWITCH_OFF:
757-
switch_status = "false";
764+
switch_status = "DISABLED";
758765
break;
759766
case SNAPSHOT_SWITCH_ON:
760-
switch_status = "true";
767+
switch_status = "ENABLED";
761768
break;
762769
default:
763-
switch_status = "unknown";
770+
switch_status = "UNKNOWN";
764771
break;
765772
}
766-
properties.AddMember("enabled", rapidjson::Value(switch_status.c_str(), allocator), allocator);
773+
properties.AddMember("status", rapidjson::Value(switch_status.c_str(), allocator), allocator);
767774

768775
// Max reserved snapshots
769776
if (instance.has_max_reserved_snapshot()) {

‎cloud/src/meta-service/meta_service_resource.cpp‎

Lines changed: 48 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include <gen_cpp/cloud.pb.h>
2222

2323
#include <algorithm>
24+
#include <cctype>
2425
#include <charconv>
2526
#include <chrono>
2627
#include <numeric>
@@ -47,7 +48,7 @@ using namespace std::chrono;
4748
namespace {
4849
constexpr char pattern_str[] = "^[a-zA-Z][0-9a-zA-Z_]*$";
4950

50-
constexpr char SNAPSHOT_ENABLED_KEY[] = "enabled";
51+
constexpr char SNAPSHOT_STATUS_KEY[] = "status";
5152
constexpr char SNAPSHOT_MAX_RESERVED_KEY[] = "max_reserved_snapshots";
5253
constexpr char SNAPSHOT_INTERVAL_SECONDS_KEY[] = "snapshot_interval_seconds";
5354

@@ -1820,30 +1821,50 @@ std::pair<MetaServiceCode, std::string> handle_snapshot_switch(const std::string
18201821
const std::string& key,
18211822
const std::string& value,
18221823
InstanceInfoPB* instance) {
1823-
if (value != "true" && value != "false") {
1824+
// Only allow "ENABLED" and "DISABLED" values (case insensitive)
1825+
std::string value_upper = value;
1826+
std::ranges::transform(value_upper, value_upper.begin(), ::toupper);
1827+
1828+
if (value_upper != "ENABLED" && value_upper != "DISABLED") {
18241829
return std::make_pair(MetaServiceCode::INVALID_ARGUMENT,
18251830
"Invalid value for enabled property: " + value +
1826-
", expected 'true' or 'false'" +
1831+
", expected 'ENABLED' or 'DISABLED' (case insensitive)" +
18271832
", instance_id: " + instance_id);
18281833
}
1834+
1835+
// Check if snapshot is not ready (UNSUPPORTED state)
18291836
if (instance->snapshot_switch_status() == SNAPSHOT_SWITCH_DISABLED) {
18301837
return std::make_pair(MetaServiceCode::INVALID_ARGUMENT,
18311838
"Snapshot not ready, instance_id: " + instance_id);
18321839
}
1833-
if (value == "true" && instance->snapshot_switch_status() == SNAPSHOT_SWITCH_ON) {
1834-
return std::make_pair(
1835-
MetaServiceCode::INVALID_ARGUMENT,
1836-
"Snapshot is already set to SNAPSHOT_SWITCH_ON, instance_id: " + instance_id);
1837-
}
1838-
if (value == "false" && instance->snapshot_switch_status() == SNAPSHOT_SWITCH_OFF) {
1840+
1841+
// Determine target status
1842+
SnapshotSwitchStatus target_status =
1843+
(value_upper == "ENABLED") ? SNAPSHOT_SWITCH_ON : SNAPSHOT_SWITCH_OFF;
1844+
1845+
// Check if the status is already set to the target value
1846+
if (instance->snapshot_switch_status() == target_status) {
1847+
std::string status_name = (target_status == SNAPSHOT_SWITCH_ON) ? "ENABLED" : "DISABLED";
18391848
return std::make_pair(
18401849
MetaServiceCode::INVALID_ARGUMENT,
1841-
"Snapshot is already set to SNAPSHOT_SWITCH_OFF, instance_id: " + instance_id);
1850+
"Snapshot is already set to " + status_name + ", instance_id: " + instance_id);
18421851
}
1843-
if (value == "true") {
1844-
instance->set_snapshot_switch_status(SNAPSHOT_SWITCH_ON);
1845-
} else {
1846-
instance->set_snapshot_switch_status(SNAPSHOT_SWITCH_OFF);
1852+
1853+
// Set the new status
1854+
instance->set_snapshot_switch_status(target_status);
1855+
1856+
// Set default values when first enabling snapshot
1857+
if (target_status == SNAPSHOT_SWITCH_ON) {
1858+
if (!instance->has_snapshot_interval_seconds() ||
1859+
instance->snapshot_interval_seconds() == 0) {
1860+
instance->set_snapshot_interval_seconds(3600);
1861+
LOG(INFO) << "Set default snapshot_interval_seconds to 3600 for instance "
1862+
<< instance_id;
1863+
}
1864+
if (!instance->has_max_reserved_snapshot() || instance->max_reserved_snapshot() == 0) {
1865+
instance->set_max_reserved_snapshot(1);
1866+
LOG(INFO) << "Set default max_reserved_snapshots to 1 for instance " << instance_id;
1867+
}
18471868
}
18481869

18491870
std::string msg = "Set snapshot enabled to " + value + " for instance " + instance_id;
@@ -1862,9 +1883,11 @@ std::pair<MetaServiceCode, std::string> handle_max_reserved_snapshots(
18621883
return std::make_pair(MetaServiceCode::INVALID_ARGUMENT,
18631884
"max_reserved_snapshots must be non-negative, got: " + value);
18641885
}
1865-
if (max_snapshots > 35) {
1886+
if (max_snapshots > config::snapshot_max_reserved_num) {
18661887
return std::make_pair(MetaServiceCode::INVALID_ARGUMENT,
1867-
"max_reserved_snapshots too large, maximum is 35, got: " + value);
1888+
"max_reserved_snapshots too large, maximum is " +
1889+
std::to_string(config::snapshot_max_reserved_num) +
1890+
", got: " + value);
18681891
}
18691892
} catch (const std::exception& e) {
18701893
return std::make_pair(MetaServiceCode::INVALID_ARGUMENT,
@@ -1886,10 +1909,11 @@ std::pair<MetaServiceCode, std::string> handle_snapshot_intervals(const std::str
18861909
int intervals;
18871910
try {
18881911
intervals = std::stoi(value);
1889-
if (intervals < 3600) {
1890-
return std::make_pair(
1891-
MetaServiceCode::INVALID_ARGUMENT,
1892-
"snapshot_intervals too small, minimum is 3600 seconds, got: " + value);
1912+
if (intervals < config::snapshot_min_interval_seconds) {
1913+
return std::make_pair(MetaServiceCode::INVALID_ARGUMENT,
1914+
"snapshot_intervals too small, minimum is " +
1915+
std::to_string(config::snapshot_min_interval_seconds) +
1916+
" seconds, got: " + value);
18931917
}
18941918
} catch (const std::exception& e) {
18951919
return std::make_pair(MetaServiceCode::INVALID_ARGUMENT,
@@ -2097,8 +2121,8 @@ void MetaServiceImpl::alter_instance(google::protobuf::RpcController* controller
20972121
* Handle SET_SNAPSHOT_PROPERTY operation - configures snapshot-related properties for an instance.
20982122
*
20992123
* Supported property keys and their expected values:
2100-
* - "enabled": "true" | "false"
2101-
* Controls whether snapshot functionality is enabled for the instance
2124+
* - "status": "UNSUPPORTED" | "ENABLED" | "DISABLED"
2125+
* Controls the snapshot functionality status for the instance
21022126
*
21032127
* - "max_reserved_snapshots": numeric string (0-35)
21042128
* Sets the maximum number of snapshots to retain for the instance
@@ -2124,7 +2148,7 @@ void MetaServiceImpl::alter_instance(google::protobuf::RpcController* controller
21242148

21252149
std::pair<MetaServiceCode, std::string> result;
21262150

2127-
if (key == SNAPSHOT_ENABLED_KEY) {
2151+
if (key == SNAPSHOT_STATUS_KEY) {
21282152
result = handle_snapshot_switch(request->instance_id(), key, value, instance);
21292153
} else if (key == SNAPSHOT_MAX_RESERVED_KEY) {
21302154
result = handle_max_reserved_snapshots(request->instance_id(), key, value,
@@ -4277,6 +4301,7 @@ void MetaServiceImpl::get_cluster_status(google::protobuf::RpcController* contro
42774301
void notify_refresh_instance(std::shared_ptr<TxnKv> txn_kv, const std::string& instance_id,
42784302
KVStats* stats) {
42794303
LOG(INFO) << "begin notify_refresh_instance";
4304+
TEST_SYNC_POINT_RETURN_WITH_VOID("notify_refresh_instance_return");
42804305
std::unique_ptr<Transaction> txn;
42814306
TxnErrorCode err = txn_kv->create_txn(&txn);
42824307
if (err != TxnErrorCode::TXN_OK) {

‎cloud/src/meta-service/meta_service_snapshot.cpp‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -101,16 +101,20 @@ void MetaServiceImpl::list_snapshot(::google::protobuf::RpcController* controlle
101101
ListSnapshotResponse* response,
102102
::google::protobuf::Closure* done) {
103103
RPC_PREPROCESS(list_snapshot, get, put, del);
104-
if (!request->has_cloud_unique_id() || request->cloud_unique_id().empty()) {
105-
code = MetaServiceCode::INVALID_ARGUMENT;
106-
msg = "cloud_unique_id not set";
107-
return;
108-
}
109104

110-
instance_id = get_instance_id(resource_mgr_, request->cloud_unique_id());
111-
if (instance_id.empty()) {
105+
// Prefer instance_id if provided, fallback to cloud_unique_id
106+
if (request->has_instance_id() && !request->instance_id().empty()) {
107+
instance_id = request->instance_id();
108+
} else if (request->has_cloud_unique_id() && !request->cloud_unique_id().empty()) {
109+
instance_id = get_instance_id(resource_mgr_, request->cloud_unique_id());
110+
if (instance_id.empty()) {
111+
code = MetaServiceCode::INVALID_ARGUMENT;
112+
msg = "empty instance_id";
113+
return;
114+
}
115+
} else {
112116
code = MetaServiceCode::INVALID_ARGUMENT;
113-
msg = "empty instance_id";
117+
msg = "either instance_id or cloud_unique_id must be provided";
114118
return;
115119
}
116120
RPC_RATE_LIMIT(list_snapshot);

‎cloud/src/snapshot/snapshot_manager.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,7 @@ void SnapshotManager::clone_instance(const CloneInstanceRequest& request,
6464
}
6565

6666
std::pair<MetaServiceCode, std::string> SnapshotManager::set_multi_version_status(
67-
std::string_view instance_id, std::string_view cloud_unique_id,
68-
MultiVersionStatus multi_version_status) {
67+
std::string_view instance_id, MultiVersionStatus multi_version_status) {
6968
return {MetaServiceCode::UNDEFINED_ERR, "Not implemented"};
7069
}
7170

‎cloud/src/snapshot/snapshot_manager.h‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,7 @@ class SnapshotManager {
4848
CloneInstanceResponse* response);
4949

5050
virtual std::pair<MetaServiceCode, std::string> set_multi_version_status(
51-
std::string_view instance_id, std::string_view cloud_unique_id,
52-
MultiVersionStatus multi_version_status);
51+
std::string_view instance_id, MultiVersionStatus multi_version_status);
5352

5453
virtual int check_snapshots(InstanceChecker* checker);
5554

0 commit comments

Comments
 (0)