From 931876e86e7173ec32f274993200e7a057bbddf2 Mon Sep 17 00:00:00 2001 From: chenyou-fdu Date: Fri, 17 Jan 2020 15:53:04 -0800 Subject: [PATCH] Separate enable-WAL and disable-WAL writer to avoid unwanted data in log files (#6290) Summary: When we do concurrently writes, and different write operations will have WAL enable or disable. But the data from write operation with WAL disabled will still be logged into log files, which will lead to extra disk write/sync since we do not want any guarantee for these part of data. Detail can be found in https://github.com/facebook/rocksdb/issues/6280. This PR avoid mixing the two types in a write group. The advantage is simpler reasoning about the write group content Pull Request resolved: https://github.com/facebook/rocksdb/pull/6290 Differential Revision: D19448598 Pulled By: maysamyabandeh fbshipit-source-id: 3d990a0f79a78ea1bfc90773f6ebafc1884c20de --- db/db_write_test.cc | 36 ++++++++++++++++++++++++++++++++++++ db/write_thread.cc | 4 ++-- 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/db/db_write_test.cc b/db/db_write_test.cc index 9eca823c2..8931f1752 100644 --- a/db/db_write_test.cc +++ b/db/db_write_test.cc @@ -7,6 +7,7 @@ #include #include #include +#include #include "db/db_test_util.h" #include "db/write_batch_internal.h" #include "db/write_thread.h" @@ -185,6 +186,41 @@ TEST_P(DBWriteTest, LockWalInEffect) { ASSERT_OK(dbfull()->UnlockWAL()); } +TEST_P(DBWriteTest, ConcurrentlyDisabledWAL) { + Options options = GetOptions(); + options.statistics = rocksdb::CreateDBStatistics(); + options.statistics->set_stats_level(StatsLevel::kAll); + Reopen(options); + std::string wal_key_prefix = "WAL_KEY_"; + std::string no_wal_key_prefix = "K_"; + // 100 KB value each for NO-WAL operation + std::string no_wal_value(1024 * 100, 'X'); + // 1B value each for WAL operation + std::string wal_value = "0"; + std::thread threads[10]; + for (int t = 0; t < 10; t++) { + threads[t] = std::thread([t, wal_key_prefix, wal_value, no_wal_key_prefix, no_wal_value, this] { + for(int i = 0; i < 10; i++) { + rocksdb::WriteOptions write_option_disable; + write_option_disable.disableWAL = true; + rocksdb::WriteOptions write_option_default; + std::string no_wal_key = no_wal_key_prefix + std::to_string(t) + "_" + std::to_string(i); + this->Put(no_wal_key, no_wal_value, write_option_disable); + std::string wal_key = wal_key_prefix + std::to_string(i) + "_" + std::to_string(i); + this->Put(wal_key, wal_value, write_option_default); + dbfull()->SyncWAL(); + } + return 0; + }); + } + for (auto& t: threads) { + t.join(); + } + uint64_t bytes_num = options.statistics->getTickerCount(rocksdb::Tickers::WAL_FILE_BYTES); + // written WAL size should less than 100KB (even included HEADER & FOOTER overhead) + ASSERT_LE(bytes_num, 1024 * 100); +} + INSTANTIATE_TEST_CASE_P(DBWriteTestInstance, DBWriteTest, testing::Values(DBTestBase::kDefault, DBTestBase::kConcurrentWALWrites, diff --git a/db/write_thread.cc b/db/write_thread.cc index 1ded68fde..46859e814 100644 --- a/db/write_thread.cc +++ b/db/write_thread.cc @@ -444,8 +444,8 @@ size_t WriteThread::EnterAsBatchGroupLeader(Writer* leader, break; } - if (!w->disable_wal && leader->disable_wal) { - // Do not include a write that needs WAL into a batch that has + if (w->disable_wal != leader->disable_wal) { + // Do not mix writes that enable WAL with the ones whose // WAL disabled. break; }