From 7e9c9f3b6388dc1f30a9fab723aabd9c8a963028 Mon Sep 17 00:00:00 2001 From: CalvinNeo Date: Fri, 19 Jul 2024 10:51:36 +0800 Subject: [PATCH 1/4] pick Signed-off-by: CalvinNeo --- dbms/src/Storages/KVStore/TMTContext.cpp | 2 +- libs/libcommon/cmake/find_jemalloc.cmake | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dbms/src/Storages/KVStore/TMTContext.cpp b/dbms/src/Storages/KVStore/TMTContext.cpp index 953f6024639..825d8d7d8cc 100644 --- a/dbms/src/Storages/KVStore/TMTContext.cpp +++ b/dbms/src/Storages/KVStore/TMTContext.cpp @@ -171,10 +171,10 @@ TMTContext::TMTContext( void TMTContext::initS3GCManager(const TiFlashRaftProxyHelper * proxy_helper) { - kvstore->fetchProxyConfig(proxy_helper); if (!raftproxy_config.pd_addrs.empty() && S3::ClientFactory::instance().isEnabled() && !context.getSharedContextDisagg()->isDisaggregatedComputeMode()) { + kvstore->fetchProxyConfig(proxy_helper); if (kvstore->getProxyConfigSummay().valid) { LOG_INFO( diff --git a/libs/libcommon/cmake/find_jemalloc.cmake b/libs/libcommon/cmake/find_jemalloc.cmake index 8768a7e3350..794c351e172 100644 --- a/libs/libcommon/cmake/find_jemalloc.cmake +++ b/libs/libcommon/cmake/find_jemalloc.cmake @@ -14,7 +14,7 @@ option (ENABLE_JEMALLOC "Set to TRUE to use jemalloc" ON) # TODO: Make ENABLE_JEMALLOC_PROF default value to ON after https://github.com/pingcap/tics/issues/3236 get fixed. -option (ENABLE_JEMALLOC_PROF "Set to ON to enable jemalloc profiling" OFF) +option (ENABLE_JEMALLOC_PROF "Set to ON to enable jemalloc profiling" ON) option (USE_INTERNAL_JEMALLOC_LIBRARY "Set to FALSE to use system jemalloc library instead of bundled" ${NOT_UNBUNDLED}) if (ENABLE_JEMALLOC AND (CMAKE_BUILD_TYPE_UC STREQUAL "ASAN" OR CMAKE_BUILD_TYPE_UC STREQUAL "UBSAN" OR CMAKE_BUILD_TYPE_UC STREQUAL "TSAN")) From 195e21b23a61285bb303c5020402676242520290 Mon Sep 17 00:00:00 2001 From: CalvinNeo Date: Fri, 19 Jul 2024 12:26:47 +0800 Subject: [PATCH 2/4] a Signed-off-by: CalvinNeo --- dbms/src/Interpreters/AsynchronousMetrics.cpp | 1 + .../src/Storages/Page/PageStorageMemorySummary.h | 3 ++- .../Page/V3/Universal/UniversalWriteBatchImpl.h | 16 ++++++++++++++-- 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/dbms/src/Interpreters/AsynchronousMetrics.cpp b/dbms/src/Interpreters/AsynchronousMetrics.cpp index b0fd613b170..dd8df736a72 100644 --- a/dbms/src/Interpreters/AsynchronousMetrics.cpp +++ b/dbms/src/Interpreters/AsynchronousMetrics.cpp @@ -302,6 +302,7 @@ void AsynchronousMetrics::update() set("LogDiskBytes", usage.total_log_disk_size); set("PagesInMem", usage.num_pages); set("VersionedEntries", DB::PS::PageStorageMemorySummary::versioned_entry_or_delete_count.load()); + set("UniversalWrite", DB::PS::PageStorageMemorySummary::universal_write_count.load()); } if (context.getSharedContextDisagg()->isDisaggregatedStorageMode()) diff --git a/dbms/src/Storages/Page/PageStorageMemorySummary.h b/dbms/src/Storages/Page/PageStorageMemorySummary.h index 9c1717fed48..8a53e60b0b6 100644 --- a/dbms/src/Storages/Page/PageStorageMemorySummary.h +++ b/dbms/src/Storages/Page/PageStorageMemorySummary.h @@ -23,6 +23,7 @@ struct PageStorageMemorySummary static inline std::atomic_int64_t uni_page_id_bytes{0}; static inline std::atomic_int64_t versioned_entry_or_delete_bytes{0}; static inline std::atomic_int64_t versioned_entry_or_delete_count{0}; + static inline std::atomic_int64_t universal_write_count{0}; }; -} // namespace DB::PS \ No newline at end of file +} // namespace DB::PS diff --git a/dbms/src/Storages/Page/V3/Universal/UniversalWriteBatchImpl.h b/dbms/src/Storages/Page/V3/Universal/UniversalWriteBatchImpl.h index d7a81dda5aa..88c6eb70570 100644 --- a/dbms/src/Storages/Page/V3/Universal/UniversalWriteBatchImpl.h +++ b/dbms/src/Storages/Page/V3/Universal/UniversalWriteBatchImpl.h @@ -53,6 +53,8 @@ class UniversalWriteBatch : private boost::noncopyable : prefix(std::move(prefix_)) {} + ~UniversalWriteBatch() { PS::PageStorageMemorySummary::universal_write_count.fetch_sub(writes.size()); } + void putPage( PageIdU64 page_id, UInt64 tag, @@ -126,6 +128,7 @@ class UniversalWriteBatch : private boost::noncopyable Write w{WriteBatchWriteType::PUT, page_id, tag, read_buffer, size, "", std::move(offsets)}; total_data_size += size; writes.emplace_back(std::move(w)); + PS::PageStorageMemorySummary::universal_write_count.fetch_add(1); } void putPage( @@ -156,6 +159,7 @@ class UniversalWriteBatch : private boost::noncopyable data_location}; writes.emplace_back(std::move(w)); has_writes_from_remote = true; + PS::PageStorageMemorySummary::universal_write_count.fetch_add(1); } void updateRemotePage(const UniversalPageId & page_id, const ReadBufferPtr & read_buffer, PageSize size) @@ -164,6 +168,7 @@ class UniversalWriteBatch : private boost::noncopyable total_data_size += size; writes.emplace_back(std::move(w)); // This is use for update local page data from remote, don't need to set `has_writes_from_remote` + PS::PageStorageMemorySummary::universal_write_count.fetch_add(1); } void putExternal(const UniversalPageId & page_id, UInt64 tag) @@ -171,6 +176,7 @@ class UniversalWriteBatch : private boost::noncopyable // External page's data is not managed by PageStorage, which means data is empty. Write w{WriteBatchWriteType::PUT_EXTERNAL, page_id, tag, nullptr, 0, "", {}}; writes.emplace_back(std::move(w)); + PS::PageStorageMemorySummary::universal_write_count.fetch_add(1); } void putRemoteExternal(const UniversalPageId & page_id, const PS::V3::CheckpointLocation & data_location) @@ -179,6 +185,7 @@ class UniversalWriteBatch : private boost::noncopyable Write w{WriteBatchWriteType::PUT_EXTERNAL, page_id, /*tag*/ 0, nullptr, 0, "", {}, data_location}; writes.emplace_back(std::move(w)); has_writes_from_remote = true; + PS::PageStorageMemorySummary::universal_write_count.fetch_add(1); } // Add RefPage{ref_id} -> Page{page_id} @@ -186,12 +193,14 @@ class UniversalWriteBatch : private boost::noncopyable { Write w{WriteBatchWriteType::REF, ref_id, 0, nullptr, 0, page_id, {}}; writes.emplace_back(std::move(w)); + PS::PageStorageMemorySummary::universal_write_count.fetch_add(1); } void delPage(const UniversalPageId & page_id) { Write w{WriteBatchWriteType::DEL, page_id, 0, nullptr, 0, "", {}}; writes.emplace_back(std::move(w)); + PS::PageStorageMemorySummary::universal_write_count.fetch_add(1); } bool empty() const { return writes.empty(); } @@ -276,11 +285,14 @@ class UniversalWriteBatch : private boost::noncopyable UniversalWriteBatch(UniversalWriteBatch && rhs) noexcept : prefix(std::move(rhs.prefix)) - , writes(std::move(rhs.writes)) , total_data_size(rhs.total_data_size) , has_writes_from_remote(rhs.has_writes_from_remote) , remote_lock_disabled(rhs.remote_lock_disabled) - {} + { + PS::PageStorageMemorySummary::universal_write_count.fetch_sub(writes.size()); + PS::PageStorageMemorySummary::universal_write_count.fetch_add(rhs.writes.size()); + writes = std::move(rhs.writes); + } void swap(UniversalWriteBatch & o) { From 3dcbac1940be75083290b25391b0a569e8f5c726 Mon Sep 17 00:00:00 2001 From: CalvinNeo Date: Fri, 19 Jul 2024 13:16:57 +0800 Subject: [PATCH 3/4] a Signed-off-by: CalvinNeo --- .../V3/Universal/UniversalWriteBatchImpl.h | 1 - .../tests/gtest_universal_page_storage.cpp | 40 +++++++++++++++++++ .../Storages/Page/V3/tests/gtest_wal_log.cpp | 1 + 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/dbms/src/Storages/Page/V3/Universal/UniversalWriteBatchImpl.h b/dbms/src/Storages/Page/V3/Universal/UniversalWriteBatchImpl.h index 88c6eb70570..d5caf135475 100644 --- a/dbms/src/Storages/Page/V3/Universal/UniversalWriteBatchImpl.h +++ b/dbms/src/Storages/Page/V3/Universal/UniversalWriteBatchImpl.h @@ -290,7 +290,6 @@ class UniversalWriteBatch : private boost::noncopyable , remote_lock_disabled(rhs.remote_lock_disabled) { PS::PageStorageMemorySummary::universal_write_count.fetch_sub(writes.size()); - PS::PageStorageMemorySummary::universal_write_count.fetch_add(rhs.writes.size()); writes = std::move(rhs.writes); } diff --git a/dbms/src/Storages/Page/V3/Universal/tests/gtest_universal_page_storage.cpp b/dbms/src/Storages/Page/V3/Universal/tests/gtest_universal_page_storage.cpp index 06865d4cdee..3015c2d6132 100644 --- a/dbms/src/Storages/Page/V3/Universal/tests/gtest_universal_page_storage.cpp +++ b/dbms/src/Storages/Page/V3/Universal/tests/gtest_universal_page_storage.cpp @@ -568,18 +568,58 @@ TEST(UniPageStorageIdTest, UniversalPageIdMemoryTrace) auto u_id = UniversalPageIdFormat::toFullPageId("aaa", 100); auto page1_mem = PS::PageStorageMemorySummary::uni_page_id_bytes.load(); auto ps = page1_mem - prim_mem; + // copy construct auto u_id_cpy = u_id; ASSERT_EQ(PS::PageStorageMemorySummary::uni_page_id_bytes.load(), prim_mem + ps * 2); + // move assignment UniversalPageId u_id_mv = UniversalPageIdFormat::toFullPageId("aaa", 100); ASSERT_EQ(PS::PageStorageMemorySummary::uni_page_id_bytes.load(), prim_mem + ps * 3); u_id_mv = std::move(u_id_cpy); ASSERT_EQ(PS::PageStorageMemorySummary::uni_page_id_bytes.load(), prim_mem + ps * 2); + // copy assignment UniversalPageId u_id_cpy2 = UniversalPageIdFormat::toFullPageId("aaa", 100); u_id_cpy2 = u_id_mv; ASSERT_EQ(PS::PageStorageMemorySummary::uni_page_id_bytes.load(), prim_mem + ps * 3); + // move construct + auto u_id_mv2 = std::move(u_id_cpy2); + ASSERT_EQ(PS::PageStorageMemorySummary::uni_page_id_bytes.load(), prim_mem + ps * 3); } ASSERT_EQ(PS::PageStorageMemorySummary::uni_page_id_bytes.load(), prim_mem); } + +TEST(UniPageStorageIdTest, UniversalWriteBatchMemory) +{ + const String prefix = "aaa"; + const UInt64 tag = 0; + static constexpr size_t buf_sz = 1024; + char c_buff[buf_sz] = {}; + for (size_t i = 0; i < buf_sz; ++i) + { + c_buff[i] = i % 0xff; + } + { + UniversalWriteBatch wb; + wb.putPage( + UniversalPageIdFormat::toFullPageId(prefix, 0), + tag, + std::make_shared(c_buff, buf_sz), + buf_sz); + ASSERT_EQ(PageStorageMemorySummary::universal_write_count.load(), 1); + } + ASSERT_EQ(PageStorageMemorySummary::universal_write_count.load(), 0); + { + UniversalWriteBatch wb; + wb.putPage( + UniversalPageIdFormat::toFullPageId(prefix, 0), + tag, + std::make_shared(c_buff, buf_sz), + buf_sz); + UniversalWriteBatch wb2 = std::move(wb); + ASSERT_EQ(PageStorageMemorySummary::universal_write_count.load(), 1); + } + ASSERT_EQ(PageStorageMemorySummary::universal_write_count.load(), 0); +} + } // namespace PS::universal::tests } // namespace DB diff --git a/dbms/src/Storages/Page/V3/tests/gtest_wal_log.cpp b/dbms/src/Storages/Page/V3/tests/gtest_wal_log.cpp index b2c1b486420..a06a9094c7a 100644 --- a/dbms/src/Storages/Page/V3/tests/gtest_wal_log.cpp +++ b/dbms/src/Storages/Page/V3/tests/gtest_wal_log.cpp @@ -858,4 +858,5 @@ TEST(LogFileRWTest2, ManuallySync) ASSERT_EQ(scratch, payload); } } + } // namespace DB::PS::V3::tests From bcd734f86828f4ae78b43cb941b906ca0ccc7754 Mon Sep 17 00:00:00 2001 From: CalvinNeo Date: Fri, 19 Jul 2024 13:50:34 +0800 Subject: [PATCH 4/4] a Signed-off-by: CalvinNeo --- libs/libcommon/cmake/find_jemalloc.cmake | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libs/libcommon/cmake/find_jemalloc.cmake b/libs/libcommon/cmake/find_jemalloc.cmake index 794c351e172..b03bca63b37 100644 --- a/libs/libcommon/cmake/find_jemalloc.cmake +++ b/libs/libcommon/cmake/find_jemalloc.cmake @@ -13,7 +13,9 @@ # limitations under the License. option (ENABLE_JEMALLOC "Set to TRUE to use jemalloc" ON) -# TODO: Make ENABLE_JEMALLOC_PROF default value to ON after https://github.com/pingcap/tics/issues/3236 get fixed. +# 1. The deadlock mentioned in https://github.com/pingcap/tics/issues/3236 is not related to ENABLE_JEMALLOC_PROF. +# 2. It is also expected to be eliminated even if the heap profiling is activated, with a newer version of pprof-rs. +# TODO: Enable continuous heap profiling after we make sure statement 2. option (ENABLE_JEMALLOC_PROF "Set to ON to enable jemalloc profiling" ON) option (USE_INTERNAL_JEMALLOC_LIBRARY "Set to FALSE to use system jemalloc library instead of bundled" ${NOT_UNBUNDLED})