Skip to content

Commit

Permalink
Revert "Revert "c-deps: fix assertion-enabled builds""
Browse files Browse the repository at this point in the history
This reverts commit 6d6b5c4.

In the same PR I upgraded RocksDB to pull in
cockroachdb/rocksdb#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
  • Loading branch information
ajkr committed Sep 28, 2019
1 parent ece7b8b commit dd865fd
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 8 deletions.
7 changes: 4 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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
Expand Down
3 changes: 2 additions & 1 deletion build/teamcity-testrace.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion build/variables.mk
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -150,7 +152,6 @@ define VALID_VARS
UI_TS_CCL
UI_TS_OSS
UNAME
USE_ROCKSDB_ASSERTIONS
VERBOSE
WEBPACK
WEBPACK_DASHBOARD
Expand Down
3 changes: 1 addition & 2 deletions c-deps/libroach/ccl/key_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
6 changes: 6 additions & 0 deletions c-deps/libroach/options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion c-deps/rocksdb
Submodule rocksdb updated 1 files
+23 −16 db/version_builder.cc

0 comments on commit dd865fd

Please sign in to comment.