From b01a884175c3886d93d6115a2b538dd14b104acb Mon Sep 17 00:00:00 2001 From: Yingchun Lai <laiyingchun@apache.org> Date: Wed, 22 Nov 2023 12:03:59 +0800 Subject: [PATCH] refactor(build): Improve the project CMake (#1687) This patch includes these changes: - Move `cmake_minimum_required` to the top of CMakeLists.txt files. - Do not pass useless cmake options when build thirdparties. - Generate a make parallel option to speed up building thirdparty libraries. - Add `DOWNLOAD_EXTRACT_TIMESTAMP` and `DOWNLOAD_NO_PROGRESS` when build thirdparties (See https://cmake.org/cmake/help/latest/module/ExternalProject.html). - Remove the useless `gflags` thirdparty library. - Add a patch from offcial repository to build `rocksdb`, so it's possible to build thirdparty directly in `thirdparty` diretory. --- .licenserc.yaml | 2 +- CMakeLists.txt | 2 +- run.sh | 14 +- thirdparty/CMakeLists.txt | 114 +++++++--- .../fix_rocksdb-cmake-PORTABLE-option.patch | 75 ++++++ .../rocksdb_fix_atomic_flush_0879c240.patch | 213 ------------------ 6 files changed, 162 insertions(+), 258 deletions(-) create mode 100644 thirdparty/fix_rocksdb-cmake-PORTABLE-option.patch delete mode 100644 thirdparty/rocksdb_fix_atomic_flush_0879c240.patch diff --git a/.licenserc.yaml b/.licenserc.yaml index 2cea94f325..ccda24093d 100644 --- a/.licenserc.yaml +++ b/.licenserc.yaml @@ -69,9 +69,9 @@ header: - 'thirdparty/fix_jemalloc_for_m1_on_macos.patch' - 'thirdparty/fix_libevent_for_macos.patch' - 'thirdparty/fix_prometheus-cpp_limits.patch' + - 'thirdparty/fix_rocksdb-cmake-PORTABLE-option.patch' - 'thirdparty/fix_s2_for_aarch64.patch' - 'thirdparty/fix_thrift_for_cpp11.patch' - - 'thirdparty/rocksdb_fix_atomic_flush_0879c240.patch' # TODO(yingchun): shell/* files are import from thirdparties, we can move them to thirdparty later. # Copyright (c) 2016, Adi Shavit - 'src/shell/argh.h' diff --git a/CMakeLists.txt b/CMakeLists.txt index fee1807892..12c87dadd2 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -15,8 +15,8 @@ # specific language governing permissions and limitations # under the License. -project(pegasus) cmake_minimum_required(VERSION 3.11.0) +project(pegasus) if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU") # require at least gcc 7.0.0 diff --git a/run.sh b/run.sh index ab5e9852df..ad0206f8eb 100755 --- a/run.sh +++ b/run.sh @@ -211,12 +211,7 @@ function run_build() CMAKE_OPTIONS="-DCMAKE_C_COMPILER=${C_COMPILER} -DCMAKE_CXX_COMPILER=${CXX_COMPILER} - -DUSE_JEMALLOC=${USE_JEMALLOC} - -DENABLE_GCOV=${ENABLE_GCOV} - -DENABLE_GPERF=${ENABLE_GPERF} - -DBoost_NO_BOOST_CMAKE=ON - -DBOOST_ROOT=${THIRDPARTY_ROOT}/output - -DBoost_NO_SYSTEM_PATHS=ON" + -DUSE_JEMALLOC=${USE_JEMALLOC}" echo "BUILD_TYPE=$BUILD_TYPE" if [ "$BUILD_TYPE" == "debug" ] @@ -258,6 +253,13 @@ function run_build() cd .. fi + CMAKE_OPTIONS="${CMAKE_OPTIONS} + -DENABLE_GCOV=${ENABLE_GCOV} + -DENABLE_GPERF=${ENABLE_GPERF} + -DBoost_NO_BOOST_CMAKE=ON + -DBOOST_ROOT=${THIRDPARTY_ROOT}/output + -DBoost_NO_SYSTEM_PATHS=ON" + echo "INFO: start build Pegasus..." BUILD_DIR="${BUILD_ROOT_DIR}/${BUILD_TYPE}_${SANITIZER}" BUILD_DIR=${BUILD_DIR%_*} diff --git a/thirdparty/CMakeLists.txt b/thirdparty/CMakeLists.txt index 8645ee1a1c..967fd48857 100644 --- a/thirdparty/CMakeLists.txt +++ b/thirdparty/CMakeLists.txt @@ -17,8 +17,8 @@ # under the License. ############################################################################## -project(pegasus_thirdparties) cmake_minimum_required(VERSION 3.11.0) +project(pegasus_thirdparties) if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU") # require at least gcc 7.0.0 @@ -44,6 +44,12 @@ set(TP_OUTPUT ${PROJECT_SOURCE_DIR}/output) # LOG_DIR = <STAMP_DIR> set_property(DIRECTORY PROPERTY EP_BASE ${TP_DIR}/build) +include(ProcessorCount) +ProcessorCount(PARALLEL) +if(PARALLEL EQUAL 0) + set(PARALLEL 1) +endif() + set(OSS_URL_PREFIX "http://pegasus-thirdparty-package.oss-cn-beijing.aliyuncs.com") message(STATUS "Setting up third-parties...") @@ -59,7 +65,9 @@ ExternalProject_Add(boost BUILD_COMMAND ./b2 toolset=gcc cxxflags=-fPIC cxxstd=11 install INSTALL_COMMAND cp -R include/boost ${TP_OUTPUT}/include && cp -R lib ${TP_OUTPUT}/ BUILD_IN_SOURCE 1 - ) + DOWNLOAD_EXTRACT_TIMESTAMP true + DOWNLOAD_NO_PROGRESS true +) # header-only file(MAKE_DIRECTORY ${TP_OUTPUT}/include/concurrentqueue) @@ -71,7 +79,9 @@ ExternalProject_Add(concurrentqueue BUILD_COMMAND "" INSTALL_COMMAND cp -R blockingconcurrentqueue.h concurrentqueue.h lightweightsemaphore.h internal/ ${TP_OUTPUT}/include/concurrentqueue BUILD_IN_SOURCE 1 - ) + DOWNLOAD_EXTRACT_TIMESTAMP true + DOWNLOAD_NO_PROGRESS true +) # header-only file(MAKE_DIRECTORY ${TP_OUTPUT}/include/readerwriterqueue) @@ -83,7 +93,9 @@ ExternalProject_Add(readerwriterqueue BUILD_COMMAND "" INSTALL_COMMAND cp -R atomicops.h readerwriterqueue.h ${TP_OUTPUT}/include/readerwriterqueue BUILD_IN_SOURCE 1 - ) + DOWNLOAD_EXTRACT_TIMESTAMP true + DOWNLOAD_NO_PROGRESS true +) ExternalProject_Add(googletest URL ${OSS_URL_PREFIX}/googletest-release-1.8.0.tar.gz @@ -92,7 +104,9 @@ ExternalProject_Add(googletest CMAKE_ARGS -DCMAKE_INSTALL_PREFIX=${TP_OUTPUT} -DCMAKE_C_COMPILER=${CMAKE_C_COMPILER} -DCMAKE_CXX_COMPILER=${CMAKE_CXX_COMPILER} - ) + DOWNLOAD_EXTRACT_TIMESTAMP true + DOWNLOAD_NO_PROGRESS true +) ExternalProject_Get_property(googletest SOURCE_DIR) set(googletest_SRC ${SOURCE_DIR}) @@ -102,7 +116,9 @@ ExternalProject_Add(gperftools URL_MD5 4e218a40a354748c50d054c285caaae8 CONFIGURE_COMMAND ./configure --prefix=${TP_OUTPUT} --enable-static=no --enable-frame-pointers=yes BUILD_IN_SOURCE 1 - ) + DOWNLOAD_EXTRACT_TIMESTAMP true + DOWNLOAD_NO_PROGRESS true +) set(HDFS_CLIENT_DIR "hadoop-hdfs-project/hadoop-hdfs-native-client") ExternalProject_Add(hadoop @@ -115,7 +131,9 @@ ExternalProject_Add(hadoop CONFIGURE_COMMAND "" BUILD_COMMAND "" INSTALL_COMMAND "" - ) + DOWNLOAD_EXTRACT_TIMESTAMP true + DOWNLOAD_NO_PROGRESS true +) # header-only ExternalProject_Add(rapidjson @@ -126,7 +144,9 @@ ExternalProject_Add(rapidjson BUILD_COMMAND "" INSTALL_COMMAND cp -R include/rapidjson ${TP_OUTPUT}/include BUILD_IN_SOURCE 1 - ) + DOWNLOAD_EXTRACT_TIMESTAMP true + DOWNLOAD_NO_PROGRESS true +) ExternalProject_Add(thrift URL ${OSS_URL_PREFIX}/thrift-0.9.3.tar.gz @@ -153,7 +173,9 @@ ExternalProject_Add(thrift -DCMAKE_C_COMPILER=${CMAKE_C_COMPILER} -DCMAKE_CXX_COMPILER=${CMAKE_CXX_COMPILER} DEPENDS boost - ) + DOWNLOAD_EXTRACT_TIMESTAMP true + DOWNLOAD_NO_PROGRESS true +) check_cxx_compiler_flag(-Wformat-overflow COMPILER_SUPPORTS_FORMAT_OVERFLOW) if (COMPILER_SUPPORTS_FORMAT_OVERFLOW) @@ -172,13 +194,15 @@ ExternalProject_Add(zookeeper URL_MD5 44c2a33e01931aed94ef7f3d39d0963e PATCH_COMMAND "" COMMAND cd zookeeper-jute && mvn compile && cd ../zookeeper-client/zookeeper-client-c && cmake -DCMAKE_BUILD_TYPE=release -DWANT_CPPUNIT=OFF -DWITH_OPENSSL=OFF -DWITH_CYRUS_SASL=${ZOOKEEPER_WITH_CYRUS_SASL} -DCMAKE_INSTALL_PREFIX=${TP_OUTPUT} - COMMAND cd zookeeper-client/zookeeper-client-c && make + COMMAND cd zookeeper-client/zookeeper-client-c && make -j${PARALLEL} COMMAND cp -R zookeeper-client/zookeeper-client-c/include/. ${TP_OUTPUT}/include/zookeeper && cp zookeeper-client/zookeeper-client-c/generated/zookeeper.jute.h ${TP_OUTPUT}/include/zookeeper && cp zookeeper-client/zookeeper-client-c/libzookeeper.a ${TP_OUTPUT}/lib && cp zookeeper-client/zookeeper-client-c/libhashtable.a ${TP_OUTPUT}/lib CONFIGURE_COMMAND "" BUILD_COMMAND "" INSTALL_COMMAND "" BUILD_IN_SOURCE 1 - ) + DOWNLOAD_EXTRACT_TIMESTAMP true + DOWNLOAD_NO_PROGRESS true +) ExternalProject_Add(libevent URL ${OSS_URL_PREFIX}/libevent-release-2.1.8-stable.tar.gz @@ -192,8 +216,9 @@ ExternalProject_Add(libevent -DEVENT__DISABLE_SAMPLES=On -DCMAKE_POSITION_INDEPENDENT_CODE=ON -DCMAKE_C_COMPILER=${CMAKE_C_COMPILER} - -DCMAKE_CXX_COMPILER=${CMAKE_CXX_COMPILER} - ) + DOWNLOAD_EXTRACT_TIMESTAMP true + DOWNLOAD_NO_PROGRESS true +) ExternalProject_Add(poco URL ${OSS_URL_PREFIX}/poco-1.11.1-release.tar.gz @@ -217,7 +242,9 @@ ExternalProject_Add(poco -DCMAKE_C_COMPILER=${CMAKE_C_COMPILER} -DCMAKE_CXX_COMPILER=${CMAKE_CXX_COMPILER} DEPENDS boost - ) + DOWNLOAD_EXTRACT_TIMESTAMP true + DOWNLOAD_NO_PROGRESS true +) ExternalProject_Add(fds URL ${OSS_URL_PREFIX}/fds-1.0.0.tar.gz @@ -237,7 +264,9 @@ ExternalProject_Add(fds UPDATE_COMMAND "" # do not update DEPENDS googletest poco boost BUILD_IN_SOURCE 1 - ) + DOWNLOAD_EXTRACT_TIMESTAMP true + DOWNLOAD_NO_PROGRESS true +) ExternalProject_Add(fmt URL https://github.com/fmtlib/fmt/archive/refs/tags/10.1.1.tar.gz @@ -248,16 +277,9 @@ ExternalProject_Add(fmt -DCMAKE_POSITION_INDEPENDENT_CODE=ON -DCMAKE_C_COMPILER=${CMAKE_C_COMPILER} -DCMAKE_CXX_COMPILER=${CMAKE_CXX_COMPILER} - ) - -ExternalProject_Add(gflags - URL ${OSS_URL_PREFIX}/gflags-2.2.1.zip - https://github.com/gflags/gflags/archive/v2.2.1.zip - URL_MD5 2d988ef0b50939fb50ada965dafce96b - CMAKE_ARGS -DCMAKE_INSTALL_PREFIX=${TP_OUTPUT} - -DCMAKE_C_COMPILER=${CMAKE_C_COMPILER} - -DCMAKE_CXX_COMPILER=${CMAKE_CXX_COMPILER} - ) + DOWNLOAD_EXTRACT_TIMESTAMP true + DOWNLOAD_NO_PROGRESS true +) # civetweb is one of the dependencies of promemetheus-cpp, do not build & install ExternalProject_Add(civetweb @@ -267,7 +289,9 @@ ExternalProject_Add(civetweb CONFIGURE_COMMAND "" BUILD_COMMAND "" INSTALL_COMMAND "" - ) + DOWNLOAD_EXTRACT_TIMESTAMP true + DOWNLOAD_NO_PROGRESS true +) ExternalProject_Get_property(civetweb SOURCE_DIR) set(civetweb_SRC ${SOURCE_DIR}) @@ -310,7 +334,9 @@ ExternalProject_Add(curl CONFIGURE_COMMAND ./configure --prefix=${TP_OUTPUT} ${CURL_OPTIONS} BUILD_IN_SOURCE 1 - ) + DOWNLOAD_EXTRACT_TIMESTAMP true + DOWNLOAD_NO_PROGRESS true +) ExternalProject_Add(prometheus-cpp URL ${OSS_URL_PREFIX}/prometheus-cpp-0.7.0.tar.gz @@ -323,7 +349,9 @@ ExternalProject_Add(prometheus-cpp -DENABLE_TESTING=OFF -DCMAKE_C_COMPILER=${CMAKE_C_COMPILER} -DCMAKE_CXX_COMPILER=${CMAKE_CXX_COMPILER} - ) + DOWNLOAD_EXTRACT_TIMESTAMP true + DOWNLOAD_NO_PROGRESS true +) # header-only ExternalProject_Add(nlohmann_json @@ -334,7 +362,9 @@ ExternalProject_Add(nlohmann_json BUILD_COMMAND "" INSTALL_COMMAND cp -R include/nlohmann ${TP_OUTPUT}/include BUILD_IN_SOURCE 1 - ) + DOWNLOAD_EXTRACT_TIMESTAMP true + DOWNLOAD_NO_PROGRESS true +) ExternalProject_Add(s2geometry URL ${OSS_URL_PREFIX}/s2geometry-e8d16637a467d9f096a92a6d81a50a9b747ca828.zip @@ -350,7 +380,9 @@ ExternalProject_Add(s2geometry -DCMAKE_C_COMPILER=${CMAKE_C_COMPILER} -DCMAKE_CXX_COMPILER=${CMAKE_CXX_COMPILER} DEPENDS googletest - ) + DOWNLOAD_EXTRACT_TIMESTAMP true + DOWNLOAD_NO_PROGRESS true +) option(USE_JEMALLOC "use jemalloc" OFF) message(STATUS "USE_JEMALLOC = ${USE_JEMALLOC}") @@ -361,13 +393,14 @@ ExternalProject_Add(jemalloc URL_MD5 3d41fbf006e6ebffd489bdb304d009ae PATCH_COMMAND patch -p0 < ${TP_DIR}/fix_jemalloc_for_m1_on_macos.patch CONFIGURE_COMMAND ./configure --prefix=${TP_OUTPUT} --enable-cxx --enable-stats --enable-prof - BUILD_COMMAND make + BUILD_COMMAND make -j${PARALLEL} INSTALL_COMMAND make install BUILD_IN_SOURCE 1 - ) + DOWNLOAD_EXTRACT_TIMESTAMP true + DOWNLOAD_NO_PROGRESS true +) option(ROCKSDB_PORTABLE "Minimum CPU arch to support, or 0 = current CPU, 1 = baseline CPU" 0) - set(ROCKSDB_OPTIONS -DFAIL_ON_WARNINGS=OFF -DWITH_BENCHMARK_TOOLS=OFF @@ -396,12 +429,15 @@ ExternalProject_Add(rocksdb URL ${OSS_URL_PREFIX}/rocksdb-v8.5.3.tar.gz https://github.com/facebook/rocksdb/archive/refs/tags/v8.5.3.tar.gz URL_MD5 f03eac50ec958a21a7cb56183afb8fe4 - PATCH_COMMAND "" + PATCH_COMMAND patch -p1 < ${TP_DIR}/fix_rocksdb-cmake-PORTABLE-option.patch COMMAND rm -rf ${TP_DIR}/build/Source/rocksdb/plugin/encfs COMMAND git clone -b main --depth=1 https://github.com/pegasus-kv/encfs.git ${TP_DIR}/build/Source/rocksdb/plugin/encfs DEPENDS googletest jemalloc CMAKE_ARGS -DCMAKE_INSTALL_PREFIX=${TP_OUTPUT} - ${ROCKSDB_OPTIONS}) + ${ROCKSDB_OPTIONS} + DOWNLOAD_EXTRACT_TIMESTAMP true + DOWNLOAD_NO_PROGRESS true +) ExternalProject_Add(http-parser URL ${OSS_URL_PREFIX}/http-parser-2.9.4.zip @@ -411,7 +447,9 @@ ExternalProject_Add(http-parser BUILD_COMMAND "" INSTALL_COMMAND cp -R http_parser.h ${TP_OUTPUT}/include/nodejs/http_parser.h BUILD_IN_SOURCE 1 - ) + DOWNLOAD_EXTRACT_TIMESTAMP true + DOWNLOAD_NO_PROGRESS true +) ExternalProject_Add(abseil URL ${OSS_URL_PREFIX}/abseil-20230802.1.zip @@ -420,4 +458,6 @@ ExternalProject_Add(abseil CMAKE_ARGS -DCMAKE_INSTALL_PREFIX=${TP_OUTPUT} -DABSL_FIND_GOOGLETEST=OFF -DCMAKE_CXX_STANDARD=14 - ) + DOWNLOAD_EXTRACT_TIMESTAMP true + DOWNLOAD_NO_PROGRESS true +) diff --git a/thirdparty/fix_rocksdb-cmake-PORTABLE-option.patch b/thirdparty/fix_rocksdb-cmake-PORTABLE-option.patch new file mode 100644 index 0000000000..6fede3e3d0 --- /dev/null +++ b/thirdparty/fix_rocksdb-cmake-PORTABLE-option.patch @@ -0,0 +1,75 @@ +From 67233eaf66ee067867265bff87e9d89fb7c46086 Mon Sep 17 00:00:00 2001 +From: Kefu Chai <tchaikov@gmail.com> +Date: Mon, 18 Sep 2023 12:11:15 -0700 +Subject: [PATCH] cmake: check PORTABLE for well-known boolean representations + (#11724) + +Summary: +before 459969e9, we were using CMake `option()` to represent `PORTABLE`. so the CMake boolean representations like ON, OFF, 0 and 1 are supported. this is also the downstream package maintainers were using before v8.3.2. + +in 459969e9, this option is expanded to specify the argument of `-march` passed to compiler in order to be more flexible and hence allows user to specify CPU type directly. but in the typical use cases, user would just want to use "ON" for the best performance on the building host, and "OFF" for a portable build when it comes to a distro package maintainer. + +so, in this change, let's check for the boolean representations supported by CMake. + +Fixes https://github.com/facebook/rocksdb/issues/11558 +Signed-off-by: Kefu Chai <tchaikov@gmail.com> + +Pull Request resolved: https://github.com/facebook/rocksdb/pull/11724 + +Reviewed By: anand1976 + +Differential Revision: D48827447 + +Pulled By: ajkr + +fbshipit-source-id: b2fef7076b2e90ad13a1fbec80e197841fa06d38 +--- + CMakeLists.txt | 18 +++++++++--------- + 1 file changed, 9 insertions(+), 9 deletions(-) + +diff --git a/CMakeLists.txt b/CMakeLists.txt +index 4e30f6631..98cbf33ea 100644 +--- a/CMakeLists.txt ++++ b/CMakeLists.txt +@@ -254,7 +254,7 @@ if(CMAKE_SYSTEM_PROCESSOR MATCHES "loongarch64") + endif(CMAKE_SYSTEM_PROCESSOR MATCHES "loongarch64") + + set(PORTABLE 0 CACHE STRING "Minimum CPU arch to support, or 0 = current CPU, 1 = baseline CPU") +-if(PORTABLE STREQUAL 1) ++if(PORTABLE MATCHES "1|ON|YES|TRUE|Y") + # Usually nothing to do; compiler default is typically the most general + if(NOT MSVC) + if(CMAKE_SYSTEM_PROCESSOR MATCHES "^s390x") +@@ -264,14 +264,7 @@ if(PORTABLE STREQUAL 1) + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -march=loongarch64") + endif() + endif() +-elseif(PORTABLE MATCHES [^0]+) +- # Name of a CPU arch spec or feature set to require +- if(MSVC) +- set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /arch:${PORTABLE}") +- else() +- set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -march=${PORTABLE}") +- endif() +-else() ++elseif(PORTABLE MATCHES "0|OFF|NO|FALSE|N") + if(MSVC) + # NOTE: No auto-detection of current CPU, but instead assume some useful + # level of optimization is supported +@@ -285,6 +278,13 @@ else() + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -march=native") + endif() + endif() ++else() ++ # Name of a CPU arch spec or feature set to require ++ if(MSVC) ++ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /arch:${PORTABLE}") ++ else() ++ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -march=${PORTABLE}") ++ endif() + endif() + + include(CheckCXXSourceCompiles) +-- +2.42.1 + diff --git a/thirdparty/rocksdb_fix_atomic_flush_0879c240.patch b/thirdparty/rocksdb_fix_atomic_flush_0879c240.patch deleted file mode 100644 index 9f90da13e0..0000000000 --- a/thirdparty/rocksdb_fix_atomic_flush_0879c240.patch +++ /dev/null @@ -1,213 +0,0 @@ -diff --git a/db/db_impl/db_impl_compaction_flush.cc b/db/db_impl/db_impl_compaction_flush.cc -index f399b97d1..04760f832 100644 ---- a/db/db_impl/db_impl_compaction_flush.cc -+++ b/db/db_impl/db_impl_compaction_flush.cc -@@ -466,6 +466,8 @@ Status DBImpl::AtomicFlushMemTablesToOutputFiles( - autovector<const autovector<MemTable*>*> mems_list; - autovector<const MutableCFOptions*> mutable_cf_options_list; - autovector<FileMetaData*> tmp_file_meta; -+ autovector<std::list<std::unique_ptr<FlushJobInfo>>*> -+ committed_flush_jobs_info; - for (int i = 0; i != num_cfs; ++i) { - const auto& mems = jobs[i]->GetMemTables(); - if (!cfds[i]->IsDropped() && !mems.empty()) { -@@ -473,13 +475,18 @@ Status DBImpl::AtomicFlushMemTablesToOutputFiles( - mems_list.emplace_back(&mems); - mutable_cf_options_list.emplace_back(&all_mutable_cf_options[i]); - tmp_file_meta.emplace_back(&file_meta[i]); -+#ifndef ROCKSDB_LITE -+ committed_flush_jobs_info.emplace_back( -+ jobs[i]->GetCommittedFlushJobsInfo()); -+#endif //! ROCKSDB_LITE - } - } - - s = InstallMemtableAtomicFlushResults( - nullptr /* imm_lists */, tmp_cfds, mutable_cf_options_list, mems_list, - versions_.get(), &mutex_, tmp_file_meta, -- &job_context->memtables_to_free, directories_.GetDbDir(), log_buffer); -+ committed_flush_jobs_info, &job_context->memtables_to_free, -+ directories_.GetDbDir(), log_buffer); - } - - if (s.ok()) { -diff --git a/db/flush_job_test.cc b/db/flush_job_test.cc -index fec737942..d55645696 100644 ---- a/db/flush_job_test.cc -+++ b/db/flush_job_test.cc -@@ -388,10 +388,18 @@ TEST_F(FlushJobTest, FlushMemtablesMultipleColumnFamilies) { - for (auto cfd : all_cfds) { - mutable_cf_options_list.push_back(cfd->GetLatestMutableCFOptions()); - } -+ autovector<std::list<std::unique_ptr<FlushJobInfo>>*> -+ committed_flush_jobs_info; -+#ifndef ROCKSDB_LITE -+ for (auto& job : flush_jobs) { -+ committed_flush_jobs_info.push_back(job->GetCommittedFlushJobsInfo()); -+ } -+#endif //! ROCKSDB_LITE - - Status s = InstallMemtableAtomicFlushResults( - nullptr /* imm_lists */, all_cfds, mutable_cf_options_list, mems_list, -- versions_.get(), &mutex_, file_meta_ptrs, &job_context.memtables_to_free, -+ versions_.get(), &mutex_, file_meta_ptrs, -+ committed_flush_jobs_info, &job_context.memtables_to_free, - nullptr /* db_directory */, nullptr /* log_buffer */); - ASSERT_OK(s); - -diff --git a/db/listener_test.cc b/db/listener_test.cc -index 0e8bae407..8ba5e0060 100644 ---- a/db/listener_test.cc -+++ b/db/listener_test.cc -@@ -350,32 +350,38 @@ TEST_F(EventListenerTest, MultiCF) { - #ifdef ROCKSDB_USING_THREAD_STATUS - options.enable_thread_tracking = true; - #endif // ROCKSDB_USING_THREAD_STATUS -- TestFlushListener* listener = new TestFlushListener(options.env, this); -- options.listeners.emplace_back(listener); -- options.table_properties_collector_factories.push_back( -- std::make_shared<TestPropertiesCollectorFactory>()); -- std::vector<std::string> cf_names = { -- "pikachu", "ilya", "muromec", "dobrynia", -- "nikitich", "alyosha", "popovich"}; -- CreateAndReopenWithCF(cf_names, options); -- -- ASSERT_OK(Put(1, "pikachu", std::string(90000, 'p'))); -- ASSERT_OK(Put(2, "ilya", std::string(90000, 'i'))); -- ASSERT_OK(Put(3, "muromec", std::string(90000, 'm'))); -- ASSERT_OK(Put(4, "dobrynia", std::string(90000, 'd'))); -- ASSERT_OK(Put(5, "nikitich", std::string(90000, 'n'))); -- ASSERT_OK(Put(6, "alyosha", std::string(90000, 'a'))); -- ASSERT_OK(Put(7, "popovich", std::string(90000, 'p'))); -- for (int i = 1; i < 8; ++i) { -- ASSERT_OK(Flush(i)); -- ASSERT_EQ(listener->flushed_dbs_.size(), i); -- ASSERT_EQ(listener->flushed_column_family_names_.size(), i); -- } -+ for (auto atomic_flush : {false, true}) { -+ options.atomic_flush = atomic_flush; -+ options.create_if_missing = true; -+ DestroyAndReopen(options); -+ TestFlushListener* listener = new TestFlushListener(options.env, this); -+ options.listeners.emplace_back(listener); -+ options.table_properties_collector_factories.push_back( -+ std::make_shared<TestPropertiesCollectorFactory>()); -+ std::vector<std::string> cf_names = {"pikachu", "ilya", "muromec", -+ "dobrynia", "nikitich", "alyosha", -+ "popovich"}; -+ CreateAndReopenWithCF(cf_names, options); -+ -+ ASSERT_OK(Put(1, "pikachu", std::string(90000, 'p'))); -+ ASSERT_OK(Put(2, "ilya", std::string(90000, 'i'))); -+ ASSERT_OK(Put(3, "muromec", std::string(90000, 'm'))); -+ ASSERT_OK(Put(4, "dobrynia", std::string(90000, 'd'))); -+ ASSERT_OK(Put(5, "nikitich", std::string(90000, 'n'))); -+ ASSERT_OK(Put(6, "alyosha", std::string(90000, 'a'))); -+ ASSERT_OK(Put(7, "popovich", std::string(90000, 'p'))); -+ for (int i = 1; i < 8; ++i) { -+ ASSERT_OK(Flush(i)); -+ ASSERT_EQ(listener->flushed_dbs_.size(), i); -+ ASSERT_EQ(listener->flushed_column_family_names_.size(), i); -+ } - -- // make sure callback functions are called in the right order -- for (size_t i = 0; i < cf_names.size(); i++) { -- ASSERT_EQ(listener->flushed_dbs_[i], db_); -- ASSERT_EQ(listener->flushed_column_family_names_[i], cf_names[i]); -+ // make sure callback functions are called in the right order -+ for (size_t i = 0; i < cf_names.size(); i++) { -+ ASSERT_EQ(listener->flushed_dbs_[i], db_); -+ ASSERT_EQ(listener->flushed_column_family_names_[i], cf_names[i]); -+ } -+ Close(); - } - } - -diff --git a/db/memtable_list.cc b/db/memtable_list.cc -index 3281c96d0..6901e45f4 100644 ---- a/db/memtable_list.cc -+++ b/db/memtable_list.cc -@@ -641,6 +641,8 @@ Status InstallMemtableAtomicFlushResults( - const autovector<const MutableCFOptions*>& mutable_cf_options_list, - const autovector<const autovector<MemTable*>*>& mems_list, VersionSet* vset, - InstrumentedMutex* mu, const autovector<FileMetaData*>& file_metas, -+ const autovector<std::list<std::unique_ptr<FlushJobInfo>>*>& -+ committed_flush_jobs_info, - autovector<MemTable*>* to_delete, Directory* db_directory, - LogBuffer* log_buffer) { - AutoThreadOperationStageUpdater stage_updater( -@@ -666,6 +668,17 @@ Status InstallMemtableAtomicFlushResults( - (*mems_list[k])[i]->SetFlushCompleted(true); - (*mems_list[k])[i]->SetFileNumber(file_metas[k]->fd.GetNumber()); - } -+#ifndef ROCKSDB_LITE -+ if (committed_flush_jobs_info[k]) { -+ assert(!mems_list[k]->empty()); -+ assert((*mems_list[k])[0]); -+ std::unique_ptr<FlushJobInfo> flush_job_info = -+ (*mems_list[k])[0]->ReleaseFlushJobInfo(); -+ committed_flush_jobs_info[k]->push_back(std::move(flush_job_info)); -+ } -+#else //! ROCKSDB_LITE -+ (void)committed_flush_jobs_info; -+#endif // ROCKSDB_LITE - } - - Status s; -diff --git a/db/memtable_list.h b/db/memtable_list.h -index cbf42ac08..2bd605089 100644 ---- a/db/memtable_list.h -+++ b/db/memtable_list.h -@@ -137,6 +137,8 @@ class MemTableListVersion { - const autovector<const autovector<MemTable*>*>& mems_list, - VersionSet* vset, InstrumentedMutex* mu, - const autovector<FileMetaData*>& file_meta, -+ const autovector<std::list<std::unique_ptr<FlushJobInfo>>*>& -+ committed_flush_jobs_info, - autovector<MemTable*>* to_delete, Directory* db_directory, - LogBuffer* log_buffer); - -@@ -375,6 +377,8 @@ class MemTableList { - const autovector<const autovector<MemTable*>*>& mems_list, - VersionSet* vset, InstrumentedMutex* mu, - const autovector<FileMetaData*>& file_meta, -+ const autovector<std::list<std::unique_ptr<FlushJobInfo>>*>& -+ committed_flush_jobs_info, - autovector<MemTable*>* to_delete, Directory* db_directory, - LogBuffer* log_buffer); - -@@ -417,6 +421,8 @@ extern Status InstallMemtableAtomicFlushResults( - const autovector<const MutableCFOptions*>& mutable_cf_options_list, - const autovector<const autovector<MemTable*>*>& mems_list, VersionSet* vset, - InstrumentedMutex* mu, const autovector<FileMetaData*>& file_meta, -+ const autovector<std::list<std::unique_ptr<FlushJobInfo>>*>& -+ committed_flush_jobs_info, - autovector<MemTable*>* to_delete, Directory* db_directory, - LogBuffer* log_buffer); - } // namespace rocksdb -diff --git a/db/memtable_list_test.cc b/db/memtable_list_test.cc -index 32a227f4b..b88d37997 100644 ---- a/db/memtable_list_test.cc -+++ b/db/memtable_list_test.cc -@@ -175,11 +175,20 @@ class MemTableListTest : public testing::Test { - for (auto& meta : file_metas) { - file_meta_ptrs.push_back(&meta); - } -+ std::vector<std::list<std::unique_ptr<FlushJobInfo>>> -+ committed_flush_jobs_info_storage(cf_ids.size()); -+ autovector<std::list<std::unique_ptr<FlushJobInfo>>*> -+ committed_flush_jobs_info; -+ for (int i = 0; i < static_cast<int>(cf_ids.size()); ++i) { -+ committed_flush_jobs_info.push_back( -+ &committed_flush_jobs_info_storage[i]); -+ } -+ - InstrumentedMutex mutex; - InstrumentedMutexLock l(&mutex); - return InstallMemtableAtomicFlushResults( - &lists, cfds, mutable_cf_options_list, mems_list, &versions, &mutex, -- file_meta_ptrs, to_delete, nullptr, &log_buffer); -+ file_meta_ptrs, committed_flush_jobs_info, to_delete, nullptr, &log_buffer); - } - }; -