From: Levi Tamasi <ltamasi@fb.com>
Date: Fri, 13 Dec 2019 20:45:49 +0000 (-0800)
Subject: Do not create/install new SuperVersion if nothing was deleted during memtable trim... 
X-Git-Tag: v6.5.3~4
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=afbfb3f56783a0ce426a7af06898e2178f7b2694;p=rocksdb.git

Do not create/install new SuperVersion if nothing was deleted during memtable trim (#6169)

Summary:
We have observed an increase in CPU load caused by frequent calls to
`ColumnFamilyData::InstallSuperVersion` from `DBImpl::TrimMemtableHistory`
when using `max_write_buffer_size_to_maintain` to limit the amount of
memtable history maintained for transaction conflict checking. As it turns out,
this is caused by the code creating and installing a new `SuperVersion` even if
no memtables were actually trimmed. The patch adds a check to avoid this.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6169

Test Plan:
Compared `perf` output for

```
./db_bench -benchmarks=randomtransaction -optimistic_transaction_db=1 -statistics -stats_interval_seconds=1 -duration=90 -num=500000 --max_write_buffer_size_to_maintain=16000000 --transaction_set_snapshot=1 --threads=32
```

before and after the change. With the fix, the call chain `rocksdb::DBImpl::TrimMemtableHistory` ->
`rocksdb::ColumnFamilyData::InstallSuperVersion` -> `rocksdb::ThreadLocalPtr::StaticMeta::Scrape`
no longer registers in the `perf` report.

Differential Revision: D19031509

Pulled By: ltamasi

fbshipit-source-id: 02686fce594e5b50eba0710e4b28a9b808c8aa20
---

diff --git a/db/db_impl/db_impl_write.cc b/db/db_impl/db_impl_write.cc
index 2f6d35d1..456129a7 100644
--- a/db/db_impl/db_impl_write.cc
+++ b/db/db_impl/db_impl_write.cc
@@ -1499,12 +1499,14 @@ Status DBImpl::TrimMemtableHistory(WriteContext* context) {
   for (auto& cfd : cfds) {
     autovector<MemTable*> to_delete;
     cfd->imm()->TrimHistory(&to_delete, cfd->mem()->ApproximateMemoryUsage());
-    for (auto m : to_delete) {
-      delete m;
+    if (!to_delete.empty()) {
+      for (auto m : to_delete) {
+        delete m;
+      }
+      context->superversion_context.NewSuperVersion();
+      assert(context->superversion_context.new_superversion.get() != nullptr);
+      cfd->InstallSuperVersion(&context->superversion_context, &mutex_);
     }
-    context->superversion_context.NewSuperVersion();
-    assert(context->superversion_context.new_superversion.get() != nullptr);
-    cfd->InstallSuperVersion(&context->superversion_context, &mutex_);
 
     if (cfd->Unref()) {
       delete cfd;