From 6d6b5c4d6ecb02519a1796db8dfca75538d3b687 Mon Sep 17 00:00:00 2001 From: Tobias Schottdorf Date: Mon, 12 Aug 2019 11:23:53 +0200 Subject: [PATCH] Revert "c-deps: fix assertion-enabled builds" This reverts commit 717c185830c393ef8c650808b6ba6cc587cf112b. Apparently we violate the assertions. This needs to be fixed, but until then, let's keep the ball rolling. One likely culprit is #38932, see: https://github.com/cockroachdb/cockroach/pull/39034#issuecomment-519950381 Release note: None --- Makefile | 7 +++---- build/teamcity-testrace.sh | 2 +- build/variables.mk | 3 +-- c-deps/libroach/ccl/key_manager.cc | 3 ++- c-deps/libroach/options.cc | 6 ------ 5 files changed, 7 insertions(+), 14 deletions(-) diff --git a/Makefile b/Makefile index d8aa62dc5a6f..9c7eabbed32a 100644 --- a/Makefile +++ b/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 diff --git a/build/teamcity-testrace.sh b/build/teamcity-testrace.sh index a722f5d74e1c..fea521e5d1a6 100755 --- a/build/teamcity-testrace.sh +++ b/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 diff --git a/build/variables.mk b/build/variables.mk index 61068f4ba956..c43c87637bca 100644 --- a/build/variables.mk +++ b/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 diff --git a/c-deps/libroach/ccl/key_manager.cc b/c-deps/libroach/ccl/key_manager.cc index ab8f69439061..62b5c1ab91d4 100644 --- a/c-deps/libroach/ccl/key_manager.cc +++ b/c-deps/libroach/ccl/key_manager.cc @@ -268,7 +268,8 @@ rocksdb::Status DataKeyManager::LoadKeys() { std::unique_lock 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 registry(new enginepbccl::DataKeysRegistry()); diff --git a/c-deps/libroach/options.cc b/c-deps/libroach/options.cc index d2463db199c3..39315f1cb2cb 100644 --- a/c-deps/libroach/options.cc +++ b/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.