Merge #39562

39562: Revert "c-deps: fix assertion-enabled builds" r=knz a=tbg

This reverts commit 717c185.

Apparently we violate the assertions. This needs to be fixed, but until
then, let's keep the ball rolling.

The assertion failures typically take the form

> L0 file with seqno 90 90 vs. file with global_seqno 90
SIGABRT: abort

See for example #39559

One likely culprit is #38932, see:

#39034 (comment)

Release note: None

Co-authored-by: Tobias Schottdorf <tobias.schottdorf@gmail.com>

Makefile

@@ -421,8 +421,7 @@ use-stdmalloc          := $(findstring stdmalloc,$(TAGS))
 use-msan               := $(findstring msan,$(GOFLAGS))
 
 # User-requested build variants.
-ENABLE_LIBROACH_ASSERTIONS ?=
-ENABLE_ROCKSDB_ASSERTIONS ?=
+USE_ROCKSDB_ASSERTIONS :=
 
 BUILD_DIR := $(GOPATH)/native/$(TARGET_TRIPLE)
 
@@ -438,10 +437,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 $(ENABLE_ROCKSDB_ASSERTIONS),_assert)
+ROCKSDB_DIR  := $(BUILD_DIR)/rocksdb$(if $(use-msan),_msan)$(if $(use-stdmalloc),_stdmalloc)$(if $(USE_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)$(if $(ENABLE_LIBROACH_ASSERTIONS),_assert)
+LIBROACH_DIR := $(BUILD_DIR)/libroach$(if $(use-msan),_msan)
 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

build/teamcity-testrace.sh

@@ -49,7 +49,7 @@ for pkg in $pkgspec; do
 		PKG="$pkg" \
 		TESTTIMEOUT=45m \
 		TESTFLAGS='-v' \
-		ENABLE_ROCKSDB_ASSERTIONS=1 2>&1 \
+		USE_ROCKSDB_ASSERTIONS=1 2>&1 \
 		| tee -a artifacts/testrace.log \
 		| go-test-teamcity
 done

build/variables.mk

@@ -45,8 +45,6 @@ define VALID_VARS
   C_LIBS_OSS
   DOCGEN_TARGETS
   DUPLFLAGS
-  ENABLE_LIBROACH_ASSERTIONS
-  ENABLE_ROCKSDB_ASSERTIONS
   ERRORS_PATH
   ERRORS_PROTO
   EXECGEN_TARGETS
@@ -152,6 +150,7 @@ define VALID_VARS
   UI_TS_CCL
   UI_TS_OSS
   UNAME
+  USE_ROCKSDB_ASSERTIONS
   VERBOSE
   WEBPACK
   WEBPACK_DASHBOARD

c-deps/libroach/ccl/key_manager.cc

@@ -268,7 +268,8 @@ rocksdb::Status DataKeyManager::LoadKeys() {
   std::unique_lock<std::mutex> l(mu_);
 
   // We should never have loaded keys before.
-  assert(current_key_ == nullptr);
+  assert(data_keys_.size() == 0);
+  assert(store_keys_.size() == 0);
   assert(registry_ == nullptr);
 
   std::unique_ptr<enginepbccl::DataKeysRegistry> registry(new enginepbccl::DataKeysRegistry());

c-deps/libroach/options.cc

@@ -121,12 +121,6 @@ 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.