From a4c796fc61754f8cbf019c66701bf1ec23be9948 Mon Sep 17 00:00:00 2001
From: ptrus <peter@u-s.si>
Date: Fri, 10 Nov 2023 16:19:12 +0100
Subject: [PATCH] rocksdb: use ptcmalloc in dockerfile

---
 docker/oasis-core-dev/Dockerfile           | 22 +++++++++++++++++-----
 go/storage/mkvs/db/rocksdb/batch.go        |  7 ++++---
 go/storage/mkvs/db/rocksdb/rocksdb_test.go |  2 ++
 3 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/docker/oasis-core-dev/Dockerfile b/docker/oasis-core-dev/Dockerfile
index faa9066bcfe..bafa38af070 100644
--- a/docker/oasis-core-dev/Dockerfile
+++ b/docker/oasis-core-dev/Dockerfile
@@ -36,7 +36,9 @@ RUN apt-get update -qq && apt-get upgrade -qq && apt-get install -qq \
     libseccomp-dev \
     bubblewrap \
     # Compression libs for RocksDB.
-    libsnappy-dev libbz2-dev liblz4-dev libzstd-dev && \
+    libsnappy-dev libbz2-dev liblz4-dev libzstd-dev \
+    # tcmalloc for RocksDB.
+    libgoogle-perftools-dev && \
     apt-get autoclean && apt-get autoremove && rm -rf /var/cache/apt/archives/* && \
     # for linting Git commits
     pip install gitlint
@@ -85,7 +87,7 @@ RUN wget https://dl.google.com/go/go${GO_VERSION}.linux-amd64.tar.gz && \
     go install mvdan.cc/gofumpt@${GOFUMPT_VERSION} && \
     go install golang.org/x/tools/cmd/goimports@${GOIMPORTS_VERSION}
 
-# Install jemalloc (used by BadgerDB).
+# Install jemalloc for BadgerDB ('je_' API prefix).
 RUN wget -O jemalloc.tar.bz2 \
     https://github.com/jemalloc/jemalloc/releases/download/${JEMALLOC_VERSION}/jemalloc-${JEMALLOC_VERSION}.tar.bz2 && \
     # Ensure checksum matches.
@@ -101,15 +103,25 @@ RUN wget -O jemalloc.tar.bz2 \
     cd .. && rm jemalloc.tar.bz2 && rm -rf jemalloc-${JEMALLOC_VERSION}
 
 # Install RocksDB.
+# https://github.com/facebook/rocksdb/blob/main/INSTALL.md
 RUN wget -q https://github.com/facebook/rocksdb/archive/v${ROCKSDB_VERSION}.tar.gz && \
   # Ensure checksum matches.
     echo "${ROCKSDB_CHECKSUM}  v${ROCKSDB_VERSION}.tar.gz" | sha256sum -c && \
     tar -zxf v${ROCKSDB_VERSION}.tar.gz && \
     cd rocksdb-${ROCKSDB_VERSION} && \
-    # TODO: clashes with jemalloc used by BadgerDB.
+    # In production it's recomended to use either tcmalloc or jemalloc instead of ptmalloc:
+    # https://blog.cloudflare.com/the-effect-of-switching-to-tcmalloc-on-rocksdb-memory-use/
+    # However RocksDB jemalloc requirement clashes with BadgerDB (RocksDB requires it without 'je_' prefix).
+    # Using tcmalloc next to badgerdb's jemalloc also causes problems. Therefore we default to ptmalloc for
+    # the docker container. If/when RocksDB becomes the prefered database, we should default to building
+    # BadgerDB without jemalloc and use tcmalloc/jemalloc here.
+    #
+    # Disable jemalloc as it is used by default if found on the system.
+    ROCKSDB_DISABLE_JEMALLOC=1 \
     # For 64-bit x86 the `PORTABLE=haswell` is a reasonable compromise, which supports many or most
     # of the available optimizations while still being compatible with most processors made since
-    # roughly 2013. https://github.com/facebook/rocksdb/blob/main/INSTALL.md
-    DEBUG_LEVEL=0 ROCKSDB_DISABLE_JEMALLOC=1 PORTABLE=haswell make -j4 shared_lib && \
+    # roughly 2013.
+    PORTABLE=haswell \
+    DEBUG_LEVEL=0 make -j4 shared_lib && \
     make install-shared && ldconfig && \
     cd .. && rm -rf v${ROCKSDB_VERSION}.tar.gz rocksdb-${ROCKSDB_VERSION}
diff --git a/go/storage/mkvs/db/rocksdb/batch.go b/go/storage/mkvs/db/rocksdb/batch.go
index b80666c75b4..38582032d90 100644
--- a/go/storage/mkvs/db/rocksdb/batch.go
+++ b/go/storage/mkvs/db/rocksdb/batch.go
@@ -106,8 +106,9 @@ func (ba *rocksdbBatch) Commit(root node.Root) error {
 			if err != nil {
 				return err
 			}
-			// Check if oldRootsMeta was updated in this batch.
-			// TODO: could this be avoided?
+
+			// Check if oldRootsMeta was updated in this batch. Since we don't see the
+			// batch updates, until the batch is applied.
 			wbIter := ba.bat.NewIterator()
 			for {
 				if !wbIter.Next() {
@@ -115,7 +116,7 @@ func (ba *rocksdbBatch) Commit(root node.Root) error {
 				}
 				rec := wbIter.Record()
 				if bytes.Equal(rec.Key, rootsMetadataKeyFmt.Encode(ba.oldRoot.Version)) {
-					if rec.Type == grocksdb.WriteBatchValueRecord {
+					if rec.Type == grocksdb.WriteBatchCFValueRecord {
 						if err = cbor.Unmarshal(rec.Value, &oldRootsMeta); err != nil {
 							panic(err)
 						}
diff --git a/go/storage/mkvs/db/rocksdb/rocksdb_test.go b/go/storage/mkvs/db/rocksdb/rocksdb_test.go
index 567d2f2cdd9..fb87096c043 100644
--- a/go/storage/mkvs/db/rocksdb/rocksdb_test.go
+++ b/go/storage/mkvs/db/rocksdb/rocksdb_test.go
@@ -121,6 +121,7 @@ func createCheckpoint(ctx context.Context, require *require.Assertions, dir stri
 					nodeKeys[string(it.Key())] = struct{}{}
 				}
 			}
+			return nil
 		})
 	}
 	loadNodes(rocksdb.cfIOTree)
@@ -148,6 +149,7 @@ func verifyNodes(require *require.Assertions, rocksdb *rocksdbNodeDB, version ui
 				require.Equal(true, ok, "unexpected node in db")
 				delete(notVisited, string(key))
 			}
+			return nil
 		})
 	}
 	checkNodes(rocksdb.cfIOTree)