From e3d4e140753e19083c777b0c3d42d0ff94bae4bd Mon Sep 17 00:00:00 2001 From: sdong Date: Mon, 19 Oct 2015 23:40:15 -0700 Subject: [PATCH 1/3] DBCompactionTestWithParam.ManualCompaction to verify block cache is not filled in manual compaction Summary: Manual compaction should not fill block cache. Add the verification in unit test Test Plan: Run the test Reviewers: yhchiang, kradhakrishnan, rven, IslamAbdelRahman, anthony, igor Reviewed By: igor Subscribers: leveldb, dhruba Differential Revision: https://reviews.facebook.net/D49089 --- db/db_compaction_test.cc | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/db/db_compaction_test.cc b/db/db_compaction_test.cc index 615adbdc0..be53fdfde 100644 --- a/db/db_compaction_test.cc +++ b/db/db_compaction_test.cc @@ -1227,6 +1227,7 @@ TEST_F(DBCompactionTest, L0_CompactionBug_Issue44_b) { TEST_P(DBCompactionTestWithParam, ManualCompaction) { Options options = CurrentOptions(); options.max_subcompactions = max_subcompactions_; + options.statistics = rocksdb::CreateDBStatistics(); CreateAndReopenWithCF({"pikachu"}, options); // iter - 0 with 7 levels @@ -1258,7 +1259,14 @@ TEST_P(DBCompactionTestWithParam, ManualCompaction) { // Compact all MakeTables(1, "a", "z", 1); ASSERT_EQ("1,0,2", FilesPerLevel(1)); + + uint64_t prev_block_cache_add = + options.statistics->getTickerCount(BLOCK_CACHE_ADD); db_->CompactRange(CompactRangeOptions(), handles_[1], nullptr, nullptr); + // Verify manual compaction doesn't fill block cache + ASSERT_EQ(prev_block_cache_add, + options.statistics->getTickerCount(BLOCK_CACHE_ADD)); + ASSERT_EQ("0,0,1", FilesPerLevel(1)); if (iter == 0) { @@ -1266,6 +1274,7 @@ TEST_P(DBCompactionTestWithParam, ManualCompaction) { options.max_background_flushes = 0; options.num_levels = 3; options.create_if_missing = true; + options.statistics = rocksdb::CreateDBStatistics(); DestroyAndReopen(options); CreateAndReopenWithCF({"pikachu"}, options); } From e154ee08639baa0eddf952201655f6d1313b9b5d Mon Sep 17 00:00:00 2001 From: Dmitri Smirnov Date: Tue, 20 Oct 2015 13:35:08 -0700 Subject: [PATCH 2/3] Do not build test only code and unit tests in Release builds Test code errors are currently blocking Windows Release builew We do not want spend time building in Release what we can not run We want to eliminate a source of most frequent errors when people check-in test only code which can not be built in Release. This feature will work only if you invoke msbuild against rocksdb.sln Invoking it against ALL_BUILD target will attempt to build everything. --- CMakeLists.txt | 61 +++++++++++++++++++++++++++++++++++------- appveyor.yml | 2 +- appveyordailytests.yml | 2 +- 3 files changed, 53 insertions(+), 12 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 98efe3892..a05e7c887 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -14,10 +14,15 @@ # 3. Run cmake to generate project files for Windows, add more options to enable required third-party libraries. # See thirdparty.inc for more information. # sample command: cmake -G "Visual Studio 12 Win64" -DGFLAGS=1 -DSNAPPY=1 -DJEMALLOC=1 .. -# 4. Then build the project in debug mode (you may want to add /m: flag to run msbuild in parallel threads) -# msbuild ALL_BUILD.vcxproj +# 4. Then build the project in debug mode (you may want to add /m[:] flag to run msbuild in parallel threads +# or simply /m ot use all avail cores) +# msbuild rocksdb.sln +# +# rocksdb.sln build features exclusions of test only code in Release. If you build ALL_BUILD then everything +# will be attempted but test only code does not build in Release mode. +# # 5. And release mode (/m[:] is also supported) -# msbuild ALL_BUILD.vcxproj /p:Configuration=Release +# msbuild rocksdb.sln /p:Configuration=Release # cmake_minimum_required(VERSION 2.6) @@ -83,6 +88,7 @@ set(LIBS ${ROCKSDB_LIBS} ${THIRDPARTY_LIBS} ${SYSTEM_LIBS}) add_subdirectory(third-party/gtest-1.7.0/fused-src/gtest) +# Main library source code set(SOURCES db/builder.cc db/c.cc @@ -100,7 +106,6 @@ set(SOURCES db/db_impl_experimental.cc db/db_impl_readonly.cc db/db_iter.cc - db/db_test_util.cc db/event_helpers.cc db/experimental.cc db/filename.cc @@ -252,6 +257,12 @@ set(SOURCES utilities/write_batch_with_index/write_batch_with_index_internal.cc ) +# For test util library that is build only in DEBUG mode +# and linked to tests. Add test only code that is not #ifdefed for Release here. +set(TESTUTIL_SOURCE + db/db_test_util.cc +) + add_library(rocksdblib${ARTIFACT_SUFFIX} ${SOURCES}) set_target_properties(rocksdblib${ARTIFACT_SUFFIX} PROPERTIES COMPILE_FLAGS "/Fd${CMAKE_CFG_INTDIR}/rocksdblib${ARTIFACT_SUFFIX}.pdb") add_dependencies(rocksdblib${ARTIFACT_SUFFIX} GenerateBuildVersion) @@ -367,7 +378,7 @@ set(TESTS utilities/write_batch_with_index/write_batch_with_index_test.cc ) -set(EXES ${APPS} ${TESTS}) +set(EXES ${APPS}) foreach(sourcefile ${EXES}) string(REPLACE ".cc" "" exename ${sourcefile}) @@ -376,12 +387,42 @@ foreach(sourcefile ${EXES}) target_link_libraries(${exename}${ARTIFACT_SUFFIX} ${LIBS}) endforeach(sourcefile ${EXES}) -# C executables must link to a shared object -set(C_EXES ${C_TESTS}) +# test utilities are only build in debug +set(TESTUTILLIB testutillib${ARTIFACT_SUFFIX}) +add_library(${TESTUTILLIB} STATIC ${TESTUTIL_SOURCE}) +set_target_properties(${TESTUTILLIB} PROPERTIES COMPILE_FLAGS "/Fd${CMAKE_CFG_INTDIR}/testutillib${ARTIFACT_SUFFIX}.pdb") +set_target_properties(${TESTUTILLIB} + PROPERTIES EXCLUDE_FROM_DEFAULT_BUILD_RELEASE 1 + EXCLUDE_FROM_DEFAULT_BUILD_MINRELEASE 1 + EXCLUDE_FROM_DEFAULT_BUILD_RELWITHDEBINFO 1 + ) -foreach(sourcefile ${C_EXES}) +# Tests are excluded from Release builds +set(TEST_EXES ${TESTS}) + +foreach(sourcefile ${TEST_EXES}) + string(REPLACE ".cc" "" exename ${sourcefile}) + string(REGEX REPLACE "^((.+)/)+" "" exename ${exename}) + add_executable(${exename}${ARTIFACT_SUFFIX} ${sourcefile}) + set_target_properties(${exename}${ARTIFACT_SUFFIX} + PROPERTIES EXCLUDE_FROM_DEFAULT_BUILD_RELEASE 1 + EXCLUDE_FROM_DEFAULT_BUILD_MINRELEASE 1 + EXCLUDE_FROM_DEFAULT_BUILD_RELWITHDEBINFO 1 + ) + target_link_libraries(${exename}${ARTIFACT_SUFFIX} ${LIBS} testutillib${ARTIFACT_SUFFIX}) +endforeach(sourcefile ${TEST_EXES}) + +# C executables must link to a shared object +set(C_TEST_EXES ${C_TESTS}) + +foreach(sourcefile ${C_TEST_EXES}) string(REPLACE ".c" "" exename ${sourcefile}) string(REGEX REPLACE "^((.+)/)+" "" exename ${exename}) add_executable(${exename}${ARTIFACT_SUFFIX} ${sourcefile}) - target_link_libraries(${exename}${ARTIFACT_SUFFIX} rocksdb${ARTIFACT_SUFFIX}) -endforeach(sourcefile ${C_TESTS}) + set_target_properties(${exename}${ARTIFACT_SUFFIX} + PROPERTIES EXCLUDE_FROM_DEFAULT_BUILD_RELEASE 1 + EXCLUDE_FROM_DEFAULT_BUILD_MINRELEASE 1 + EXCLUDE_FROM_DEFAULT_BUILD_RELWITHDEBINFO 1 + ) + target_link_libraries(${exename}${ARTIFACT_SUFFIX} rocksdb${ARTIFACT_SUFFIX} testutillib${ARTIFACT_SUFFIX}) +endforeach(sourcefile ${C_TEST_EXES}) diff --git a/appveyor.yml b/appveyor.yml index e13e2d226..eb2f63172 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -5,7 +5,7 @@ before_build: - cmake -G "Visual Studio 12 Win64" .. - cd .. build: - project: build\ALL_BUILD.vcxproj + project: build\rocksdb.sln parallel: true verbosity: minimal test: off diff --git a/appveyordailytests.yml b/appveyordailytests.yml index a8b4af60c..0a35ac68b 100644 --- a/appveyordailytests.yml +++ b/appveyordailytests.yml @@ -5,7 +5,7 @@ before_build: - cmake -G "Visual Studio 12 Win64" -DOPTDBG=1 .. - cd .. build: - project: build\ALL_BUILD.vcxproj + project: build\rocksdb.sln parallel: true verbosity: minimal test: From 5678c05d865010e589105645248d33b814f3fcf7 Mon Sep 17 00:00:00 2001 From: Yueh-Hsuan Chiang Date: Tue, 20 Oct 2015 17:09:09 -0700 Subject: [PATCH 3/3] Use DEBUG_LEVEL=0 in make release and make clean Summary: Use DEBUG_LEVEL=0 in make release and make clean Test Plan: make clean make release -j32 Reviewers: MarkCallaghan, sdong, anthony, IslamAbdelRahman, igor Reviewed By: igor Subscribers: dhruba, leveldb Differential Revision: https://reviews.facebook.net/D49125 --- Makefile | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index fb14eab92..47b56cf3b 100644 --- a/Makefile +++ b/Makefile @@ -41,6 +41,14 @@ ifeq ($(MAKECMDGOALS),dbg) DEBUG_LEVEL=2 endif +ifeq ($(MAKECMDGOALS),clean) + DEBUG_LEVEL=0 +endif + +ifeq ($(MAKECMDGOALS),release) + DEBUG_LEVEL=0 +endif + ifeq ($(MAKECMDGOALS),shared_lib) DEBUG_LEVEL=0 endif @@ -404,7 +412,7 @@ dbg: $(LIBRARY) $(BENCHMARKS) tools $(TESTS) # creates static library and programs release: $(MAKE) clean - OPT="-DNDEBUG -O2" $(MAKE) static_lib tools db_bench + DEBUG_LEVEL=0 $(MAKE) static_lib tools db_bench coverage: $(MAKE) clean