From dd865fd1b7792a5021b5dbb464ea394cead9954d Mon Sep 17 00:00:00 2001
From: Andrew Kryczka <andrew.kryczka2@gmail.com>
Date: Thu, 26 Sep 2019 13:32:37 -0700
Subject: [PATCH] Revert "Revert "c-deps: fix assertion-enabled builds""

This reverts commit 6d6b5c4d6ecb02519a1796db8dfca75538d3b687.

In the same PR I upgraded RocksDB to pull in
https://github.com/cockroachdb/rocksdb/pull/59, which relaxes the
assertion whose failure led to the original revert.

Release justification: run `make testrace` in teamcity with rocksdb
assertions enabled to increase our bug-finding chances

Release note: None
---
 Makefile                           | 7 ++++---
 build/teamcity-testrace.sh         | 3 ++-
 build/variables.mk                 | 3 ++-
 c-deps/libroach/ccl/key_manager.cc | 3 +--
 c-deps/libroach/options.cc         | 6 ++++++
 c-deps/rocksdb                     | 2 +-
 6 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/Makefile b/Makefile
index b011797ef488..6025e5aac549 100644
--- a/Makefile
+++ b/Makefile
@@ -424,7 +424,8 @@ use-stdmalloc          := $(findstring stdmalloc,$(TAGS))
 use-msan               := $(findstring msan,$(GOFLAGS))
 
 # User-requested build variants.
-USE_ROCKSDB_ASSERTIONS :=
+ENABLE_LIBROACH_ASSERTIONS ?=
+ENABLE_ROCKSDB_ASSERTIONS ?=
 
 BUILD_DIR := $(GOPATH)/native/$(TARGET_TRIPLE)
 
@@ -440,10 +441,10 @@ endif
 CRYPTOPP_DIR := $(BUILD_DIR)/cryptopp$(if $(use-msan),_msan)
 JEMALLOC_DIR := $(BUILD_DIR)/jemalloc$(if $(use-msan),_msan)
 PROTOBUF_DIR := $(BUILD_DIR)/protobuf$(if $(use-msan),_msan)
-ROCKSDB_DIR  := $(BUILD_DIR)/rocksdb$(if $(use-msan),_msan)$(if $(use-stdmalloc),_stdmalloc)$(if $(USE_ROCKSDB_ASSERTIONS),_assert)
+ROCKSDB_DIR  := $(BUILD_DIR)/rocksdb$(if $(use-msan),_msan)$(if $(use-stdmalloc),_stdmalloc)$(if $(ENABLE_ROCKSDB_ASSERTIONS),_assert)
 SNAPPY_DIR   := $(BUILD_DIR)/snappy$(if $(use-msan),_msan)
 LIBEDIT_DIR  := $(BUILD_DIR)/libedit$(if $(use-msan),_msan)
-LIBROACH_DIR := $(BUILD_DIR)/libroach$(if $(use-msan),_msan)
+LIBROACH_DIR := $(BUILD_DIR)/libroach$(if $(use-msan),_msan)$(if $(ENABLE_LIBROACH_ASSERTIONS),_assert)
 KRB5_DIR     := $(BUILD_DIR)/krb5$(if $(use-msan),_msan)
 # Can't share with protobuf because protoc is always built for the host.
 PROTOC_DIR := $(GOPATH)/native/$(HOST_TRIPLE)/protobuf
diff --git a/build/teamcity-testrace.sh b/build/teamcity-testrace.sh
index 8a7a109383aa..9c78595cb1e1 100755
--- a/build/teamcity-testrace.sh
+++ b/build/teamcity-testrace.sh
@@ -51,7 +51,8 @@ for pkg in $pkgspec; do
 		PKG="$pkg" \
 		TESTTIMEOUT=45m \
 		TESTFLAGS='-v' \
-		USE_ROCKSDB_ASSERTIONS=1 2>&1 \
+		ENABLE_ROCKSDB_ASSERTIONS=1 2>&1 \
+		ENABLE_LIBROACH_ASSERTIONS=1 2>&1 \
 		| tee -a artifacts/testrace.log \
 		| go-test-teamcity
 done
diff --git a/build/variables.mk b/build/variables.mk
index c43c87637bca..61068f4ba956 100644
--- a/build/variables.mk
+++ b/build/variables.mk
@@ -45,6 +45,8 @@ define VALID_VARS
   C_LIBS_OSS
   DOCGEN_TARGETS
   DUPLFLAGS
+  ENABLE_LIBROACH_ASSERTIONS
+  ENABLE_ROCKSDB_ASSERTIONS
   ERRORS_PATH
   ERRORS_PROTO
   EXECGEN_TARGETS
@@ -150,7 +152,6 @@ define VALID_VARS
   UI_TS_CCL
   UI_TS_OSS
   UNAME
-  USE_ROCKSDB_ASSERTIONS
   VERBOSE
   WEBPACK
   WEBPACK_DASHBOARD
diff --git a/c-deps/libroach/ccl/key_manager.cc b/c-deps/libroach/ccl/key_manager.cc
index 62b5c1ab91d4..ab8f69439061 100644
--- a/c-deps/libroach/ccl/key_manager.cc
+++ b/c-deps/libroach/ccl/key_manager.cc
@@ -268,8 +268,7 @@ rocksdb::Status DataKeyManager::LoadKeys() {
   std::unique_lock<std::mutex> l(mu_);
 
   // We should never have loaded keys before.
-  assert(data_keys_.size() == 0);
-  assert(store_keys_.size() == 0);
+  assert(current_key_ == nullptr);
   assert(registry_ == nullptr);
 
   std::unique_ptr<enginepbccl::DataKeysRegistry> registry(new enginepbccl::DataKeysRegistry());
diff --git a/c-deps/libroach/options.cc b/c-deps/libroach/options.cc
index 407fd1eed229..084cdfea14ca 100644
--- a/c-deps/libroach/options.cc
+++ b/c-deps/libroach/options.cc
@@ -119,6 +119,12 @@ class DBLogger : public rocksdb::Logger {
     }
   }
 
+  virtual void LogHeader(const char* format, va_list ap) override {
+    // RocksDB's `Logger::LogHeader()` implementation forgot to call the `Logv()` overload
+    // that takes severity info. Until it's fixed we can override their implementation.
+    Logv(rocksdb::InfoLogLevel::HEADER_LEVEL, format, ap);
+  }
+
   virtual void Logv(const char* format, va_list ap) override {
     // The RocksDB API tries to force us to separate the severity check (above function)
     // from the actual logging (this function) by making this function pure virtual.
diff --git a/c-deps/rocksdb b/c-deps/rocksdb
index 217d7a121d7a..e88209ad0a00 160000
--- a/c-deps/rocksdb
+++ b/c-deps/rocksdb
@@ -1 +1 @@
-Subproject commit 217d7a121d7ac8db4b742c3fef1e0605af536841
+Subproject commit e88209ad0a00bd0d0fdccd02e6ad0162c5286be5