Skip to content

Commit 886b0bb

Browse files
authored
[fix](memory-tracker) Avoid nested task attach in cloud snapshot manager (#63189)
Cloud download callback already attaches the `SnapshotLoader` resource context. During cloud restore,` CloudSnapshotLoader::download()` calls `CloudSnapshotMgr::make_snapshot()`, which previously used `SCOPED_ATTACH_TASK` again and could hit `ThreadContext::attach_task() `DCHECK because task attach is not nestable. Use `SCOPED_SWITCH_THREAD_MEM_TRACKER_LIMITER` in `CloudSnapshotMgr` and`SnapshotMgr` snapshot operations avoiding the nested-attach pattern. These methods only need to account memory to the manager tracker, and limiter switching works both with and without an outer attached task context.
1 parent c02bd65 commit 886b0bb

2 files changed

Lines changed: 5 additions & 5 deletions

File tree

be/src/cloud/cloud_snapshot_mgr.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ CloudSnapshotMgr::CloudSnapshotMgr(CloudStorageEngine& engine) : _engine(engine)
6161
Status CloudSnapshotMgr::make_snapshot(int64_t target_tablet_id, StorageResource& storage_resource,
6262
std::unordered_map<std::string, std::string>& file_mapping,
6363
bool is_restore, const Slice* slice) {
64-
SCOPED_ATTACH_TASK(_mem_tracker);
64+
SCOPED_SWITCH_THREAD_MEM_TRACKER_LIMITER(_mem_tracker);
6565
if (is_restore && slice == nullptr) {
6666
return Status::Error<INVALID_ARGUMENT>("slice cannot be null in restore.");
6767
}
@@ -105,7 +105,7 @@ Status CloudSnapshotMgr::make_snapshot(int64_t target_tablet_id, StorageResource
105105
}
106106

107107
Status CloudSnapshotMgr::commit_snapshot(int64_t tablet_id) {
108-
SCOPED_ATTACH_TASK(_mem_tracker);
108+
SCOPED_SWITCH_THREAD_MEM_TRACKER_LIMITER(_mem_tracker);
109109
CloudTabletSPtr tablet = DORIS_TRY(_engine.tablet_mgr().get_tablet(tablet_id));
110110
if (tablet == nullptr) {
111111
return Status::Error<TABLE_NOT_FOUND>("failed to get tablet. tablet={}", tablet_id);
@@ -117,7 +117,7 @@ Status CloudSnapshotMgr::commit_snapshot(int64_t tablet_id) {
117117
}
118118

119119
Status CloudSnapshotMgr::release_snapshot(int64_t tablet_id, bool is_completed) {
120-
SCOPED_ATTACH_TASK(_mem_tracker);
120+
SCOPED_SWITCH_THREAD_MEM_TRACKER_LIMITER(_mem_tracker);
121121
RETURN_IF_ERROR(_engine.meta_mgr().finish_restore_job(tablet_id, is_completed));
122122
LOG(INFO) << "success to release snapshot. [tablet_id=" << tablet_id << "]";
123123
return Status::OK();

be/src/storage/snapshot/snapshot_manager.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ SnapshotManager::~SnapshotManager() = default;
105105

106106
Status SnapshotManager::make_snapshot(const TSnapshotRequest& request, string* snapshot_path,
107107
bool* allow_incremental_clone) {
108-
SCOPED_ATTACH_TASK(_mem_tracker);
108+
SCOPED_SWITCH_THREAD_MEM_TRACKER_LIMITER(_mem_tracker);
109109
Status res = Status::OK();
110110
if (snapshot_path == nullptr) {
111111
return Status::Error<INVALID_ARGUMENT>("output parameter cannot be null");
@@ -151,7 +151,7 @@ Status SnapshotManager::release_snapshot(const string& snapshot_path) {
151151

152152
// If the requested snapshot_path is located in the root/snapshot folder, it is considered legal and can be deleted.
153153
// Otherwise, it is considered an illegal request and returns an error result.
154-
SCOPED_ATTACH_TASK(_mem_tracker);
154+
SCOPED_SWITCH_THREAD_MEM_TRACKER_LIMITER(_mem_tracker);
155155
auto stores = _engine.get_stores();
156156
for (auto* store : stores) {
157157
std::string abs_path;

0 commit comments

Comments
 (0)