From 20225a41c5f3c9d9c3d44bf50a1ac5995ccfcb67 Mon Sep 17 00:00:00 2001 From: Spade A <71589810+SpadeA-Tang@users.noreply.github.com> Date: Sat, 9 Sep 2023 13:04:45 +0800 Subject: [PATCH] Avoid chaos between NotifyOnMemTableSealed and NotifyOnFlushCompleted (#347) Signed-off-by: SpadeA-Tang --- db/db_impl/db_impl_write.cc | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/db/db_impl/db_impl_write.cc b/db/db_impl/db_impl_write.cc index b6b6dd22d5c..dd7f65f03f7 100644 --- a/db/db_impl/db_impl_write.cc +++ b/db/db_impl/db_impl_write.cc @@ -2254,11 +2254,16 @@ Status DBImpl::SwitchMemtable(ColumnFamilyData* cfd, WriteContext* context) { mutable_cf_options); #ifndef ROCKSDB_LITE - mutex_.Unlock(); + // From above, the memtable has been converted to imm memtable and apended to + // memlist. If we unlock here, it has possibility to be picked by a concurrent + // flush job. Furthermore, that flush job may even be completed and calls + // NotifyOnFlushCompleted before the following NotifyOnMemTableSealed which + // can cause troubles. + // So, unlike Facebook/Rocksdb, we does not unlock here. + // Notify client that memtable is sealed, now that we have successfully // installed a new memtable NotifyOnMemTableSealed(cfd, memtable_info); - mutex_.Lock(); #endif // ROCKSDB_LITE // It is possible that we got here without checking the value of i_os, but // that is okay. If we did, it most likely means that s was already an error.