From 8c54ad874be503fb14bb934080db8b373d62ca77 Mon Sep 17 00:00:00 2001 From: GavinMar Date: Thu, 13 Apr 2023 10:02:51 +0000 Subject: [PATCH 1/2] [BugFix] Add a `FIND_DEFAULT_PATH` option to control whether search dependent libraries from system default path first. Setting it to `OFF` can ensure find the dependent libraries from the given thirdparty path. Signed-off-by: GavinMar --- CMakeLists.txt | 47 ++++++++++++++++++++++++------------ build-scripts/cmake-build.sh | 1 + 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 42cee27..63e9b76 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -25,6 +25,7 @@ endif() option(WITH_TESTS "Build the starcache test" OFF) option(WITH_TOOLS "Build the starcache tools" OFF) option(WITH_COVERAGE "Enable code coverage build" OFF) +option(FIND_DEFAULT_PATH "Whether to find the library in default system library path" OFF) # set CMAKE_BUILD_TYPE if (NOT CMAKE_BUILD_TYPE) @@ -83,43 +84,57 @@ set(CMAKE_INSTALL_INCLUDEDIR ${CMAKE_INSTALL_PREFIX}/include) #### External Dependencies #### -FUNCTION(SEARCH_LIBRARY RESULT LIB_NAME LIB_ROOT) +FUNCTION(FIND_LIBRARY_IN_PATH RESULT LIB_FILE LIB_NAME LIB_PATH) + if(FIND_DEFAULT_PATH) + find_library(${RESULT} NAMES ${LIB_FILE} ${LIB_NAME} PATHS ${LIB_PATH}/lib) + else() + find_library(${RESULT} NAMES ${LIB_FILE} ${LIB_NAME} PATHS ${LIB_PATH}/lib NO_DEFAULT_PATH) + endif() + if (NOT ${RESULT}) + if(FIND_DEFAULT_PATH) + find_library(${RESULT} NAMES ${LIB_FILE} ${LIB_NAME} PATHS ${LIB_PATH}/lib64) + else() + find_library(${RESULT} NAMES ${LIB_FILE} ${LIB_NAME} PATHS ${LIB_PATH}/lib64 NO_DEFAULT_PATH) + endif() + endif() +ENDFUNCTION() + +FUNCTION(SEARCH_LIBRARY RESULT LIB_FILE LIB_NAME LIB_ROOT) if ("${LIB_ROOT}" STREQUAL "") set(SEARCH_PATH ${STARCACHE_THIRDPARTY_DIR}) else() set(SEARCH_PATH ${LIB_ROOT}) endif() - find_library(${RESULT} ${LIB_NAME} ${SEARCH_PATH}/lib) - if (NOT ${RESULT}) - find_library(${RESULT} ${LIB_NAME} ${SEARCH_PATH}/lib64) - endif() - + FIND_LIBRARY_IN_PATH(${RESULT} ${LIB_FILE} ${LIB_NAME} ${SEARCH_PATH}) if (${RESULT}) + message(STATUS "find library ${LIB_NAME}") if ((NOT "${LIB_ROOT}" STREQUAL "") AND (EXISTS "${LIB_ROOT}/include")) include_directories(${LIB_ROOT}/include) endif() + else() + message(ERROR "cannot find library ${LIB_NAME}") endif() ENDFUNCTION() ## PROTOBUF -SEARCH_LIBRARY(PROTOBUF_LIBPROTOBUF protobuf "${PROTOBUF_ROOT}") +SEARCH_LIBRARY(PROTOBUF_LIBPROTOBUF libprotobuf.a protobuf "${PROTOBUF_ROOT}") ## GFLAGS -SEARCH_LIBRARY(GFLAGS_LIB gflags "${GFLAGS_ROOT}") +SEARCH_LIBRARY(GFLAGS_LIB libgflags.a gflags "${GFLAGS_ROOT}") ## GLOG -SEARCH_LIBRARY(GLOG_LIB glog "${GLOG_ROOT}") +SEARCH_LIBRARY(GLOG_LIB libglog.a glog "${GLOG_ROOT}") ## BRPC -SEARCH_LIBRARY(BRPC_LIB brpc "${BRPC_ROOT}") +SEARCH_LIBRARY(BRPC_LIB libbrpc.a brpc "${BRPC_ROOT}") ## SSL -SEARCH_LIBRARY(SSL_LIB ssl "${SSL_ROOT}") -SEARCH_LIBRARY(CRYPTO_LIB crypto "${SSL_ROOT}") +SEARCH_LIBRARY(SSL_LIB libssl.a ssl "${SSL_ROOT}") +SEARCH_LIBRARY(CRYPTO_LIB libcrypto.a crypto "${SSL_ROOT}") ## FMT -SEARCH_LIBRARY(FMT_LIB fmt "${FMT_ROOT}") +SEARCH_LIBRARY(FMT_LIB libfmt.a fmt "${FMT_ROOT}") ## BOOST if ("${BOOST_ROOT}" STREQUAL "") @@ -203,11 +218,11 @@ target_link_libraries(starcache ${PROTOBUF_LIBPROTOBUF} ${SSL_LIB} ${CRYPTO_LIB} - ${BOOST_LIB} + ${BOOST_LIB} ${GLOG_LIB} ${GFLAGS_LIB} ${BRPC_LIB} - ${FMT_LIB} + ${FMT_LIB} -Wl,--end-group ) @@ -230,7 +245,7 @@ if(WITH_TESTS) endif() unset(CXX_INCLUDES) - SEARCH_LIBRARY(GTEST_LIB gtest "${GTEST_ROOT}") + SEARCH_LIBRARY(GTEST_LIB libgtest.a gtest "${GTEST_ROOT}") # TEST LIBRARY set(STARCACHE_TEST_SRCS diff --git a/build-scripts/cmake-build.sh b/build-scripts/cmake-build.sh index 6792a4d..15c4a13 100755 --- a/build-scripts/cmake-build.sh +++ b/build-scripts/cmake-build.sh @@ -174,6 +174,7 @@ $STARCACHE_CMAKE_CMD -B ${CMAKE_BUILD_DIR} -DCMAKE_CXX_COMPILER_LAUNCHER=ccache -DFMT_ROOT=${WITH_FMT_ROOT} \ -DGTEST_ROOT=${WITH_GTEST_ROOT} \ -DBOOST_ROOT=${WITH_BOOST_ROOT} \ + -DFIND_DEFAULT_PATH=${FIND_DEFAULT_PATH} \ ${STARCACHE_TEST_COVERAGE:+"-Dstarcache_BUILD_COVERAGE=$STARCACHE_TEST_COVERAGE"} \ . From c65acb638d270afa635699bb512d05ad4cfde298 Mon Sep 17 00:00:00 2001 From: GavinMar Date: Thu, 13 Apr 2023 10:47:31 +0000 Subject: [PATCH 2/2] [BugFix] Check the return value of `posix_memalign` before use the aligned buffer. Signed-off-by: GavinMar --- src/aux_funcs.cpp | 4 +++- src/block_file.cpp | 19 +++++++++++++------ src/cache_item.h | 1 + src/mem_space_manager.h | 1 + src/sharded_lock_manager.h | 1 + src/star_cache_impl.cpp | 4 ++++ src/tools/starcache_tester.cpp | 1 + 7 files changed, 24 insertions(+), 7 deletions(-) diff --git a/src/aux_funcs.cpp b/src/aux_funcs.cpp index d8c9061..f1a929f 100644 --- a/src/aux_funcs.cpp +++ b/src/aux_funcs.cpp @@ -25,7 +25,9 @@ uint64_t cachekey2id(const std::string& key) { size_t align_iobuf(const butil::IOBuf& buf, void** aligned_data) { size_t aligned_unit = config::FLAGS_io_align_unit_size; size_t aligned_size = round_up(buf.size(), aligned_unit); - posix_memalign(aligned_data, aligned_unit, aligned_size); + if (posix_memalign(aligned_data, aligned_unit, aligned_size) != 0) { + return 0; + } butil::IOBuf tmp_buf(buf); // IOBufCutter is a specialized utility to cut from IOBuf faster than using corresponding diff --git a/src/block_file.cpp b/src/block_file.cpp index 829cf28..f223197 100644 --- a/src/block_file.cpp +++ b/src/block_file.cpp @@ -48,7 +48,7 @@ Status BlockFile::close() { } Status BlockFile::write(off_t offset, const IOBuf& buf) { - ssize_t ret = 0; + ssize_t ret = -1; void* aligned_data = nullptr; size_t aligned_size = buf.size(); @@ -57,15 +57,19 @@ Status BlockFile::write(off_t offset, const IOBuf& buf) { auto data = buf.backing_block(0).data(); if (mem_need_align(data, buf.size())) { aligned_size = align_iobuf(buf, &aligned_data); - ret = ::pwrite(_fd, aligned_data, aligned_size, offset); - free(aligned_data); + if (aligned_size > 0) { + ret = ::pwrite(_fd, aligned_data, aligned_size, offset); + free(aligned_data); + } } else { ret = ::pwrite(_fd, data, aligned_size, offset); } } else if (!config::FLAGS_enable_os_page_cache) { aligned_size = align_iobuf(buf, &aligned_data); - ret = ::pwrite(_fd, aligned_data, aligned_size, offset); - free(aligned_data); + if (aligned_size > 0) { + ret = ::pwrite(_fd, aligned_data, aligned_size, offset); + free(aligned_data); + } } else { struct iovec iov[block_num]; for (size_t i = 0; i < block_num; ++i) { @@ -73,10 +77,13 @@ Status BlockFile::write(off_t offset, const IOBuf& buf) { } ret = ::pwritev(_fd, iov, block_num, offset); } + if (ret < 0) { + if (aligned_size == 0) { + errno = ENOMEM; + } return _report_io_error("fail to write block file"); } - STAR_VLOG << "write block file success, fd: " << _fd << ", path: " << _file_path << ", offset: " << offset << ", buf size: " << buf.size() << ", aligned_size: " << aligned_size << ", buf block num: " << block_num; return Status::OK(); diff --git a/src/cache_item.h b/src/cache_item.h index d061de0..a3c2df0 100644 --- a/src/cache_item.h +++ b/src/cache_item.h @@ -15,6 +15,7 @@ #pragma once #include +#include #include #include "block_item.h" diff --git a/src/mem_space_manager.h b/src/mem_space_manager.h index 4aaa0f4..c432e2f 100644 --- a/src/mem_space_manager.h +++ b/src/mem_space_manager.h @@ -18,6 +18,7 @@ #include #include +#include #include namespace starrocks::starcache { diff --git a/src/sharded_lock_manager.h b/src/sharded_lock_manager.h index 4c05b8c..72ca222 100644 --- a/src/sharded_lock_manager.h +++ b/src/sharded_lock_manager.h @@ -16,6 +16,7 @@ #include +#include #include #include "aux_funcs.h" diff --git a/src/star_cache_impl.cpp b/src/star_cache_impl.cpp index 1631769..6388a80 100644 --- a/src/star_cache_impl.cpp +++ b/src/star_cache_impl.cpp @@ -151,6 +151,10 @@ Status StarCacheImpl::_write_block(CacheItemPtr cache_item, const BlockKey& bloc // We allocate an aligned buffer here to avoid repeatedlly copying data to a new aligned buffer // when flush to disk file in `O_DIRECT` mode. size = align_iobuf(buf, &data); + if (size == 0) { + LOG(ERROR) << "align io buffer failed when write block"; + return Status(ENOMEM, "align io buffer failed"); + } } else { data = malloc(buf.size()); buf.copy_to(data); diff --git a/src/tools/starcache_tester.cpp b/src/tools/starcache_tester.cpp index 593d68a..e43b18b 100644 --- a/src/tools/starcache_tester.cpp +++ b/src/tools/starcache_tester.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include #include "star_cache.h"