Fix stress/crash test handling of SST unique IDs (#9054)

Summary:
Was not handling the case of OnTableFileCreated invoked for
table file NOT created.

Also improved error reporting and caught a missing status check.

Also strengthened the db_stress listener to require file_size > 0 when
status.ok(). We would be violating the API contract if status is OK and
we didn't create a valid SST file.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/9054

Test Plan: make blackbox_crash_test for a while

Reviewed By: zhichao-cao

Differential Revision: D31765200

Pulled By: pdillinger

fbshipit-source-id: 7c527f5531bc239a5efd7a7b018545d480f926e2
main
Peter Dillinger 3 years ago committed by Facebook GitHub Bot
parent 8fb3fe8d39
commit 0534393fc8
  1. 9
      db_stress_tool/db_stress_listener.cc
  2. 17
      db_stress_tool/db_stress_listener.h

@ -123,10 +123,15 @@ void UniqueIdVerifier::Verify(const std::string& id) {
} }
void DbStressListener::VerifyTableFileUniqueId( void DbStressListener::VerifyTableFileUniqueId(
const TableProperties& new_file_properties) { const TableProperties& new_file_properties, const std::string& file_path) {
// Verify unique ID // Verify unique ID
std::string id; std::string id;
GetUniqueIdFromTableProperties(new_file_properties, &id); Status s = GetUniqueIdFromTableProperties(new_file_properties, &id);
if (!s.ok()) {
fprintf(stderr, "Error getting SST unique id for %s: %s\n",
file_path.c_str(), s.ToString().c_str());
assert(false);
}
unique_ids_.Verify(id); unique_ids_.Verify(id);
} }

@ -101,19 +101,17 @@ class DbStressListener : public EventListener {
void OnTableFileCreated(const TableFileCreationInfo& info) override { void OnTableFileCreated(const TableFileCreationInfo& info) override {
assert(info.db_name == db_name_); assert(info.db_name == db_name_);
assert(IsValidColumnFamilyName(info.cf_name)); assert(IsValidColumnFamilyName(info.cf_name));
if (info.file_size) {
VerifyFilePath(info.file_path);
}
assert(info.job_id > 0 || FLAGS_compact_files_one_in > 0); assert(info.job_id > 0 || FLAGS_compact_files_one_in > 0);
if (info.status.ok() && info.file_size > 0) { if (info.status.ok()) {
assert(info.file_size > 0);
VerifyFilePath(info.file_path);
assert(info.table_properties.data_size > 0 || assert(info.table_properties.data_size > 0 ||
info.table_properties.num_range_deletions > 0); info.table_properties.num_range_deletions > 0);
assert(info.table_properties.raw_key_size > 0); assert(info.table_properties.raw_key_size > 0);
assert(info.table_properties.num_entries > 0); assert(info.table_properties.num_entries > 0);
VerifyTableFileUniqueId(info.table_properties, info.file_path);
} }
--num_pending_file_creations_; --num_pending_file_creations_;
VerifyTableFileUniqueId(info.table_properties);
} }
void OnMemTableSealed(const MemTableInfo& /*info*/) override { void OnMemTableSealed(const MemTableInfo& /*info*/) override {
@ -130,7 +128,7 @@ class DbStressListener : public EventListener {
RandomSleep(); RandomSleep();
// Here we assume that each generated external file is ingested // Here we assume that each generated external file is ingested
// exactly once (or thrown away in case of crash) // exactly once (or thrown away in case of crash)
VerifyTableFileUniqueId(info.table_properties); VerifyTableFileUniqueId(info.table_properties, info.internal_file_path);
} }
void OnBackgroundError(BackgroundErrorReason /* reason */, void OnBackgroundError(BackgroundErrorReason /* reason */,
@ -248,7 +246,10 @@ class DbStressListener : public EventListener {
#endif // !NDEBUG #endif // !NDEBUG
} }
void VerifyTableFileUniqueId(const TableProperties& new_file_properties); // Unique id is verified using the TableProperties. file_path is only used
// for reporting.
void VerifyTableFileUniqueId(const TableProperties& new_file_properties,
const std::string& file_path);
void RandomSleep() { void RandomSleep() {
std::this_thread::sleep_for( std::this_thread::sleep_for(

Loading…
Cancel
Save