Misc things for ASSERT_STATUS_CHECKED, also gcc 4.8.5 (#6871)

Summary:
* Print stack trace on status checked failure
* Make folly_synchronization_distributed_mutex_test a parallel test
* Disable ldb_test.py and rocksdb_dump_test.sh with
  ASSERT_STATUS_CHECKED (broken)
* Fix shadow warning in random_access_file_reader.h reported by gcc
  4.8.5 (ROCKSDB_NO_FBCODE), also https://github.com/facebook/rocksdb/issues/6866
* Work around compiler bug on max_align_t for gcc < 4.9
* Remove an apparently wrong comment in status.h
* Use check_some in Travis config (for proper diagnostic output)
* Fix ignored Status in loop in options_helper.cc
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6871

Test Plan: manual, CI

Reviewed By: ajkr

Differential Revision: D21706619

Pulled By: pdillinger

fbshipit-source-id: daf6364173d6689904eb394461a69a11f5bee2cb
main
Peter Dillinger 5 years ago committed by Facebook GitHub Bot
parent bcd32560dd
commit 803a517b48
  1. 2
      .travis.yml
  2. 11
      Makefile
  3. 4
      file/random_access_file_reader.h
  4. 18
      include/rocksdb/status.h
  5. 21
      options/options_helper.cc
  6. 9
      third-party/folly/folly/lang/Align.h

@ -300,7 +300,7 @@ script:
mkdir build && cd build && cmake -DJNI=1 .. -DCMAKE_BUILD_TYPE=Release $OPT && make -j4 rocksdb rocksdbjni mkdir build && cd build && cmake -DJNI=1 .. -DCMAKE_BUILD_TYPE=Release $OPT && make -j4 rocksdb rocksdbjni
;; ;;
status_checked) status_checked)
OPT=-DTRAVIS V=1 ASSERT_STATUS_CHECKED=1 make -j4 check OPT=-DTRAVIS V=1 ASSERT_STATUS_CHECKED=1 make -j4 check_some
;; ;;
esac esac
notifications: notifications:

@ -633,10 +633,6 @@ TESTS = \
timer_test \ timer_test \
db_with_timestamp_compaction_test \ db_with_timestamp_compaction_test \
ifeq ($(USE_FOLLY_DISTRIBUTED_MUTEX),1)
TESTS += folly_synchronization_distributed_mutex_test
endif
PARALLEL_TEST = \ PARALLEL_TEST = \
backupable_db_test \ backupable_db_test \
db_bloom_filter_test \ db_bloom_filter_test \
@ -660,6 +656,11 @@ PARALLEL_TEST = \
write_prepared_transaction_test \ write_prepared_transaction_test \
write_unprepared_transaction_test \ write_unprepared_transaction_test \
ifeq ($(USE_FOLLY_DISTRIBUTED_MUTEX),1)
TESTS += folly_synchronization_distributed_mutex_test
PARALLEL_TEST += folly_synchronization_distributed_mutex_test
endif
# options_settable_test doesn't pass with UBSAN as we use hack in the test # options_settable_test doesn't pass with UBSAN as we use hack in the test
ifdef COMPILE_WITH_UBSAN ifdef COMPILE_WITH_UBSAN
TESTS := $(shell echo $(TESTS) | sed 's/\boptions_settable_test\b//g') TESTS := $(shell echo $(TESTS) | sed 's/\boptions_settable_test\b//g')
@ -1034,9 +1035,11 @@ check: all
ifneq ($(PLATFORM), OS_AIX) ifneq ($(PLATFORM), OS_AIX)
$(PYTHON) tools/check_all_python.py $(PYTHON) tools/check_all_python.py
ifeq ($(filter -DROCKSDB_LITE,$(OPT)),) ifeq ($(filter -DROCKSDB_LITE,$(OPT)),)
ifndef ASSERT_STATUS_CHECKED # not yet working with these tests
$(PYTHON) tools/ldb_test.py $(PYTHON) tools/ldb_test.py
sh tools/rocksdb_dump_test.sh sh tools/rocksdb_dump_test.sh
endif endif
endif
endif endif
$(MAKE) check-format $(MAKE) check-format
$(MAKE) check-buck-targets $(MAKE) check-buck-targets

@ -62,13 +62,13 @@ class RandomAccessFileReader {
public: public:
explicit RandomAccessFileReader( explicit RandomAccessFileReader(
std::unique_ptr<FSRandomAccessFile>&& raf, const std::string& _file_name, std::unique_ptr<FSRandomAccessFile>&& raf, const std::string& _file_name,
Env* env = nullptr, Statistics* stats = nullptr, uint32_t hist_type = 0, Env* _env = nullptr, Statistics* stats = nullptr, uint32_t hist_type = 0,
HistogramImpl* file_read_hist = nullptr, HistogramImpl* file_read_hist = nullptr,
RateLimiter* rate_limiter = nullptr, RateLimiter* rate_limiter = nullptr,
const std::vector<std::shared_ptr<EventListener>>& listeners = {}) const std::vector<std::shared_ptr<EventListener>>& listeners = {})
: file_(std::move(raf)), : file_(std::move(raf)),
file_name_(std::move(_file_name)), file_name_(std::move(_file_name)),
env_(env), env_(_env),
stats_(stats), stats_(stats),
hist_type_(hist_type), hist_type_(hist_type),
file_read_hist_(file_read_hist), file_read_hist_(file_read_hist),

@ -16,7 +16,17 @@
#pragma once #pragma once
#ifdef ROCKSDB_ASSERT_STATUS_CHECKED
#include <stdio.h>
#include <stdlib.h>
#endif
#include <string> #include <string>
#ifdef ROCKSDB_ASSERT_STATUS_CHECKED
#include "port/stack_trace.h"
#endif
#include "rocksdb/slice.h" #include "rocksdb/slice.h"
namespace ROCKSDB_NAMESPACE { namespace ROCKSDB_NAMESPACE {
@ -27,7 +37,11 @@ class Status {
Status() : code_(kOk), subcode_(kNone), sev_(kNoError), state_(nullptr) {} Status() : code_(kOk), subcode_(kNone), sev_(kNoError), state_(nullptr) {}
~Status() { ~Status() {
#ifdef ROCKSDB_ASSERT_STATUS_CHECKED #ifdef ROCKSDB_ASSERT_STATUS_CHECKED
assert(checked_); if (!checked_) {
fprintf(stderr, "Failed to check Status\n");
port::PrintStack();
abort();
}
#endif // ROCKSDB_ASSERT_STATUS_CHECKED #endif // ROCKSDB_ASSERT_STATUS_CHECKED
delete[] state_; delete[] state_;
} }
@ -511,8 +525,6 @@ inline Status::Status(const Status& s, Severity sev)
state_ = (s.state_ == nullptr) ? nullptr : CopyState(s.state_); state_ = (s.state_ == nullptr) ? nullptr : CopyState(s.state_);
} }
inline Status& Status::operator=(const Status& s) { inline Status& Status::operator=(const Status& s) {
// The following condition catches both aliasing (when this == &s),
// and the common case where both s and *this are ok.
if (this != &s) { if (this != &s) {
#ifdef ROCKSDB_ASSERT_STATUS_CHECKED #ifdef ROCKSDB_ASSERT_STATUS_CHECKED
s.checked_ = true; s.checked_ = true;

@ -1122,17 +1122,18 @@ Status OptionTypeInfo::ParseStruct(
// This option represents the entire struct // This option represents the entire struct
std::unordered_map<std::string, std::string> opt_map; std::unordered_map<std::string, std::string> opt_map;
status = StringToMap(opt_value, &opt_map); status = StringToMap(opt_value, &opt_map);
if (status.ok()) { for (const auto& map_iter : opt_map) {
for (const auto& map_iter : opt_map) { if (!status.ok()) {
const auto iter = struct_map->find(map_iter.first); break;
if (iter != struct_map->end()) { }
status = iter->second.Parse(config_options, map_iter.first, const auto iter = struct_map->find(map_iter.first);
map_iter.second, if (iter != struct_map->end()) {
opt_addr + iter->second.offset_); status =
} else { iter->second.Parse(config_options, map_iter.first, map_iter.second,
return Status::InvalidArgument("Unrecognized option: ", opt_addr + iter->second.offset_);
} else {
status = Status::InvalidArgument("Unrecognized option: ",
struct_name + "." + map_iter.first); struct_name + "." + map_iter.first);
}
} }
} }
} else if (StartsWith(opt_name, struct_name + ".")) { } else if (StartsWith(opt_name, struct_name + ".")) {

@ -22,6 +22,15 @@
#include <folly/Portability.h> #include <folly/Portability.h>
#include <folly/ConstexprMath.h> #include <folly/ConstexprMath.h>
// Work around bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56019
#ifdef __GNUC__
#if __GNUC__ < 4 || (__GNUC__ == 4 && __GNUC_MINOR__ < 9)
namespace std {
using ::max_align_t;
}
#endif
#endif
namespace folly { namespace folly {
// has_extended_alignment // has_extended_alignment

Loading…
Cancel
Save