From b708b166dc7093e08dcd432edcff0b39c581ec21 Mon Sep 17 00:00:00 2001 From: Levi Tamasi Date: Fri, 12 Mar 2021 16:42:30 -0800 Subject: [PATCH] Fix a harmless data race affecting two test cases (#8055) Summary: `DBTest.GetLiveBlobFiles` and `ObsoleteFilesTest.BlobFiles` both modify the current `Version` in their setup phase, implicitly assuming that no other threads would touch the `Version` while this is happening. The periodic stats dumper thread violates this assumption; the patch fixes this by disabling it in the affected test cases. (Note: the data race is harmless in the sense that it only affects test code.) Pull Request resolved: https://github.com/facebook/rocksdb/pull/8055 Test Plan: ``` COMPILE_WITH_TSAN=1 make db_test -j24 gtest-parallel --repeat=10000 ./db_test --gtest_filter="*GetLiveBlobFiles" COMPILE_WITH_TSAN=1 make obsolete_files_test -j24 gtest-parallel --repeat=10000 ./obsolete_files_test --gtest_filter="*BlobFiles" ``` Reviewed By: riversand963 Differential Revision: D27022715 Pulled By: ltamasi fbshipit-source-id: b6cc77ed63d8bc1cbe0603522ff1a572182fc9ab --- db/db_test.cc | 7 +++++++ db/obsolete_files_test.cc | 8 ++++++++ 2 files changed, 15 insertions(+) diff --git a/db/db_test.cc b/db/db_test.cc index dbe4a161b..7afe317db 100644 --- a/db/db_test.cc +++ b/db/db_test.cc @@ -2332,6 +2332,13 @@ TEST_F(DBTest, ReadonlyDBGetLiveManifestSize) { } TEST_F(DBTest, GetLiveBlobFiles) { + // Note: the following prevents an otherwise harmless data race between the + // test setup code (AddBlobFile) below and the periodic stat dumping thread. + Options options = CurrentOptions(); + options.stats_dump_period_sec = 0; + + Reopen(options); + VersionSet* const versions = dbfull()->TEST_GetVersionSet(); assert(versions); assert(versions->GetColumnFamilySet()); diff --git a/db/obsolete_files_test.cc b/db/obsolete_files_test.cc index 8159581a3..ee6b07639 100644 --- a/db/obsolete_files_test.cc +++ b/db/obsolete_files_test.cc @@ -94,6 +94,12 @@ class ObsoleteFilesTest : public DBTestBase { options.WAL_ttl_seconds = 300; // Used to test log files options.WAL_size_limit_MB = 1024; // Used to test log files options.wal_dir = wal_dir_; + + // Note: the following prevents an otherwise harmless data race between the + // test setup code (AddBlobFile) in ObsoleteFilesTest.BlobFiles and the + // periodic stat dumping thread. + options.stats_dump_period_sec = 0; + Destroy(options); Reopen(options); } @@ -192,6 +198,8 @@ TEST_F(ObsoleteFilesTest, DeleteObsoleteOptionsFile) { } TEST_F(ObsoleteFilesTest, BlobFiles) { + ReopenDB(); + VersionSet* const versions = dbfull()->TEST_GetVersionSet(); assert(versions); assert(versions->GetColumnFamilySet());