From b0e20acae3e7bbfb2f0d74e6e1318188db00d133 Mon Sep 17 00:00:00 2001 From: Kaijie Chen Date: Fri, 27 Sep 2024 11:06:38 +0800 Subject: [PATCH] Revert "[enhancement](mem-tracker) Use thread local mem tracker to track s3 file buffer memory usage" (#41360) Reverts apache/doris#40597 This PR cased memtable memtracker check in cloud_p0 to fail. ``` *** Check failure stack trace: *** [17:31:12 ](http://43.132.222.7:8111/buildConfiguration/Doris_DorisRegression_CloudP0/536070?buildTab=log&linesState=43518&logView=flowAware&focusLine=43518) F20240926 17:27:30.342573 15767 memtable.cpp:145] memtable flush success but cosumption is not 0, it is 5242880F20240926 17:27:30.357038 15763 memtable.cpp:145] memtable flush success but cosumption is not 0, it is 5242880F20240926 17:27:30.357043 15762 [17:31:12 ](http://43.132.222.7:8111/buildConfiguration/Doris_DorisRegression_CloudP0/536070?buildTab=log&linesState=43519&logView=flowAware&focusLine=43519) mtable.cpp:145] memtable flush success but cosumption is not 0, it is 5242880 [17:31:12 ](http://43.132.222.7:8111/buildConfiguration/Doris_DorisRegression_CloudP0/536070?buildTab=log&linesState=43520&logView=flowAware&focusLine=43520) *** Check failure stack trace: *** [17:31:12 ](http://43.132.222.7:8111/buildConfiguration/Doris_DorisRegression_CloudP0/536070?buildTab=log&linesState=43521&logView=flowAware&focusLine=43521) F20240926 17:27:30.342573 15767 memtable.cpp:145] memtable flush success but cosumption is not 0, it is 5242880F20240926 17:27:30.357038 15763 memtable.cpp:145] memtable flush success but cosumption is not 0, it is 5242880F20240926 17:27:30.357043 15762 [17:31:12 ](http://43.132.222.7:8111/buildConfiguration/Doris_DorisRegression_CloudP0/536070?buildTab=log&linesState=43522&logView=flowAware&focusLine=43522) mtable.cpp:145] memtable flush success but cosumption is not 0, it is 5242880F20240926 17:27:30.358129 15765 memtable.cpp:145] memtable flush success but cosumption is not 0, it is 5242880 [17:31:12 ](http://43.132.222.7:8111/buildConfiguration/Doris_DorisRegression_CloudP0/536070?buildTab=log&linesState=43523&logView=flowAware&focusLine=43523) *** Check failure stack trace: *** [17:31:12 ](http://43.132.222.7:8111/buildConfiguration/Doris_DorisRegression_CloudP0/536070?buildTab=log&linesState=43524&logView=flowAware&focusLine=43524) F20240926 17:27:30.342573 15767 memtable.cpp:145] memtable flush success but cosumption is not 0, it is 5242880F20240926 17:27:30.357038 15763 memtable.cpp:145] memtable flush success but cosumption is not 0, it is 5242880F20240926 17:27:30.357043 15762 [17:31:12 ](http://43.132.222.7:8111/buildConfiguration/Doris_DorisRegression_CloudP0/536070?buildTab=log&linesState=43525&logView=flowAware&focusLine=43525) mtable.cpp:145] memtable flush success but cosumption is not 0, it is 5242880F20240926 17:27:30.358129 15765 memtable.cpp:145] memtable flush success but cosumption is not 0, it is 5242880F20240926 17:27:30.358175 15766 memtable.cpp:145] memtable flush success but cosumption is not 0, it is 5242880 [17:31:12 ](http://43.132.222.7:8111/buildConfiguration/Doris_DorisRegression_CloudP0/536070?buildTab=log&linesState=43526&logView=flowAware&focusLine=43526) *** Check failure stack trace: *** ``` http://43.132.222.7:8111/buildConfiguration/Doris_DorisRegression_CloudP0/536070?buildTab=log&linesState=25290&logView=flowAware&focusLine=44023 --- be/src/io/fs/s3_file_bufferpool.cpp | 25 ++++++------------------- be/src/io/fs/s3_file_bufferpool.h | 14 +++++--------- be/src/olap/tablet.cpp | 6 ------ be/src/olap/tablet.h | 3 --- be/src/runtime/snapshot_loader.cpp | 5 ----- be/src/runtime/snapshot_loader.h | 2 -- 6 files changed, 11 insertions(+), 44 deletions(-) diff --git a/be/src/io/fs/s3_file_bufferpool.cpp b/be/src/io/fs/s3_file_bufferpool.cpp index 0d59ea0ed88263..f1f90ea7f2ef06 100644 --- a/be/src/io/fs/s3_file_bufferpool.cpp +++ b/be/src/io/fs/s3_file_bufferpool.cpp @@ -31,7 +31,6 @@ #include "io/cache/file_cache_common.h" #include "io/fs/s3_common.h" #include "runtime/exec_env.h" -#include "runtime/memory/mem_tracker_limiter.h" #include "runtime/thread_context.h" #include "util/defer_op.h" #include "util/slice.h" @@ -78,19 +77,17 @@ Slice FileBuffer::get_slice() const { } FileBuffer::FileBuffer(BufferType type, std::function alloc_holder, - size_t offset, OperationState state, - std::shared_ptr mem_tracker) + size_t offset, OperationState state) : _type(type), _alloc_holder(std::move(alloc_holder)), _offset(offset), _size(0), _state(std::move(state)), _inner_data(std::make_unique()), - _capacity(_inner_data->size()), - _mem_tracker(std::move(mem_tracker)) {} + _capacity(_inner_data->size()) {} FileBuffer::~FileBuffer() { - SCOPED_SWITCH_THREAD_MEM_TRACKER_LIMITER(_mem_tracker); + SCOPED_SWITCH_THREAD_MEM_TRACKER_LIMITER(ExecEnv::GetInstance()->s3_file_buffer_tracker()); _inner_data.reset(); } @@ -243,22 +240,13 @@ FileBufferBuilder& FileBufferBuilder::set_allocate_file_blocks_holder( } Status FileBufferBuilder::build(std::shared_ptr* buf) { - auto mem_tracker = ExecEnv::GetInstance()->s3_file_buffer_tracker(); - auto* thread_ctx = doris::thread_context(true); - if (thread_ctx != nullptr) { - // if thread local mem tracker is set, use it instead. - auto curr_tracker = thread_ctx->thread_mem_tracker_mgr->limiter_mem_tracker(); - if (curr_tracker != ExecEnv::GetInstance()->orphan_mem_tracker()) { - mem_tracker = std::move(curr_tracker); - } - } - SCOPED_SWITCH_THREAD_MEM_TRACKER_LIMITER(mem_tracker); + SCOPED_SWITCH_THREAD_MEM_TRACKER_LIMITER(ExecEnv::GetInstance()->s3_file_buffer_tracker()); OperationState state(_sync_after_complete_task, _is_cancelled); if (_type == BufferType::UPLOAD) { RETURN_IF_CATCH_EXCEPTION(*buf = std::make_shared( std::move(_upload_cb), std::move(state), _offset, - std::move(_alloc_holder_cb), std::move(mem_tracker))); + std::move(_alloc_holder_cb))); return Status::OK(); } if (_type == BufferType::DOWNLOAD) { @@ -266,8 +254,7 @@ Status FileBufferBuilder::build(std::shared_ptr* buf) { std::move(_download), std::move(_write_to_local_file_cache), std::move(_write_to_use_buffer), std::move(state), - _offset, std::move(_alloc_holder_cb), - std::move(mem_tracker))); + _offset, std::move(_alloc_holder_cb))); return Status::OK(); } // should never come here diff --git a/be/src/io/fs/s3_file_bufferpool.h b/be/src/io/fs/s3_file_bufferpool.h index a603c3cb29a4cb..1b552850ae3af8 100644 --- a/be/src/io/fs/s3_file_bufferpool.h +++ b/be/src/io/fs/s3_file_bufferpool.h @@ -27,7 +27,6 @@ #include "common/status.h" #include "io/cache/file_block.h" -#include "runtime/memory/mem_tracker_limiter.h" #include "util/crc32c.h" #include "util/slice.h" #include "util/threadpool.h" @@ -78,7 +77,7 @@ struct OperationState { struct FileBuffer { FileBuffer(BufferType type, std::function alloc_holder, size_t offset, - OperationState state, std::shared_ptr mem_tracker); + OperationState state); virtual ~FileBuffer(); /** * submit the correspoding task to async executor @@ -128,16 +127,14 @@ struct FileBuffer { struct PartData; std::unique_ptr _inner_data; size_t _capacity; - std::shared_ptr _mem_tracker; }; struct DownloadFileBuffer final : public FileBuffer { DownloadFileBuffer(std::function download, std::function write_to_cache, std::function write_to_use_buffer, OperationState state, - size_t offset, std::function alloc_holder, - std::shared_ptr mem_tracker) - : FileBuffer(BufferType::DOWNLOAD, alloc_holder, offset, state, std::move(mem_tracker)), + size_t offset, std::function alloc_holder) + : FileBuffer(BufferType::DOWNLOAD, alloc_holder, offset, state), _download(std::move(download)), _write_to_local_file_cache(std::move(write_to_cache)), _write_to_use_buffer(std::move(write_to_use_buffer)) {} @@ -156,9 +153,8 @@ struct DownloadFileBuffer final : public FileBuffer { struct UploadFileBuffer final : public FileBuffer { UploadFileBuffer(std::function upload_cb, OperationState state, - size_t offset, std::function alloc_holder, - std::shared_ptr mem_tracker) - : FileBuffer(BufferType::UPLOAD, alloc_holder, offset, state, std::move(mem_tracker)), + size_t offset, std::function alloc_holder) + : FileBuffer(BufferType::UPLOAD, alloc_holder, offset, state), _upload_to_remote(std::move(upload_cb)) {} ~UploadFileBuffer() override = default; Status append_data(const Slice& s) override; diff --git a/be/src/olap/tablet.cpp b/be/src/olap/tablet.cpp index f7d9dfea2d4fda..c1c1baebd2ce9d 100644 --- a/be/src/olap/tablet.cpp +++ b/be/src/olap/tablet.cpp @@ -106,7 +106,6 @@ #include "olap/txn_manager.h" #include "olap/types.h" #include "olap/utils.h" -#include "runtime/memory/mem_tracker_limiter.h" #include "segment_loader.h" #include "service/point_query_executor.h" #include "tablet.h" @@ -269,9 +268,6 @@ Tablet::Tablet(StorageEngine& engine, TabletMetaSharedPtr tablet_meta, DataDir* _tablet_path = fmt::format("{}/{}/{}/{}/{}", _data_dir->path(), DATA_PREFIX, _tablet_meta->shard_id(), tablet_id(), schema_hash()); } - _upload_cooldown_meta_tracker = MemTrackerLimiter::create_shared( - MemTrackerLimiter::Type::OTHER, - fmt::format("UploadCoolDownMeta#tablet_id={}", tablet_id())); } bool Tablet::set_tablet_schema_into_rowset_meta() { @@ -2101,8 +2097,6 @@ Status Tablet::write_cooldown_meta() { _cooldown_conf.cooldown_replica_id, tablet_id()); } - SCOPED_SWITCH_THREAD_MEM_TRACKER_LIMITER(_upload_cooldown_meta_tracker); - auto storage_resource = DORIS_TRY(get_resource_by_storage_policy_id(storage_policy_id())); std::vector cooldowned_rs_metas; diff --git a/be/src/olap/tablet.h b/be/src/olap/tablet.h index 71af08e61cdba4..33253e82ced2b5 100644 --- a/be/src/olap/tablet.h +++ b/be/src/olap/tablet.h @@ -46,7 +46,6 @@ #include "olap/rowset/rowset_reader.h" #include "olap/rowset/segment_v2/segment.h" #include "olap/version_graph.h" -#include "runtime/memory/mem_tracker_limiter.h" #include "segment_loader.h" #include "util/metrics.h" #include "util/once.h" @@ -609,8 +608,6 @@ class Tablet final : public BaseTablet { std::shared_ptr _visible_version; std::atomic_bool _is_full_compaction_running = false; - - std::shared_ptr _upload_cooldown_meta_tracker; }; inline CumulativeCompactionPolicy* Tablet::cumulative_compaction_policy() { diff --git a/be/src/runtime/snapshot_loader.cpp b/be/src/runtime/snapshot_loader.cpp index 0a13ac6085ce69..d04a5463879c9e 100644 --- a/be/src/runtime/snapshot_loader.cpp +++ b/be/src/runtime/snapshot_loader.cpp @@ -52,7 +52,6 @@ #include "olap/tablet_manager.h" #include "runtime/client_cache.h" #include "runtime/exec_env.h" -#include "runtime/memory/mem_tracker_limiter.h" #include "util/s3_uri.h" #include "util/s3_util.h" #include "util/thrift_rpc_helper.h" @@ -116,9 +115,6 @@ Status SnapshotLoader::init(TStorageBackendType::type type, const std::string& l } else { return Status::InternalError("Unknown storage type: {}", type); } - _mem_tracker = MemTrackerLimiter::create_shared( - MemTrackerLimiter::Type::OTHER, - fmt::format("SnapShotLoader#job_id={}#task_id={}", _job_id, _task_id)); return Status::OK(); } @@ -129,7 +125,6 @@ Status SnapshotLoader::upload(const std::map& src_to_d if (!_remote_fs) { return Status::InternalError("Storage backend not initialized."); } - SCOPED_SWITCH_THREAD_MEM_TRACKER_LIMITER(_mem_tracker); LOG(INFO) << "begin to upload snapshot files. num: " << src_to_dest_path.size() << ", broker addr: " << _broker_addr << ", job: " << _job_id << ", task" << _task_id; diff --git a/be/src/runtime/snapshot_loader.h b/be/src/runtime/snapshot_loader.h index b6da1a2adae5dc..7b1d5a0d9428f8 100644 --- a/be/src/runtime/snapshot_loader.h +++ b/be/src/runtime/snapshot_loader.h @@ -26,7 +26,6 @@ #include "common/status.h" #include "olap/tablet_fwd.h" -#include "runtime/memory/mem_tracker_limiter.h" namespace doris { namespace io { @@ -112,7 +111,6 @@ class SnapshotLoader { const TNetworkAddress _broker_addr; const std::map _prop; std::shared_ptr _remote_fs; - std::shared_ptr _mem_tracker; }; } // end namespace doris