andreacavalli/rocksdb
0
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Don't call OnTableFileCreated with OK for empty+deleted file (#9118)

Browse Source 
Summary:
EventListener::OnTableFileCreated was previously called with OK
status and file_size==0 in cases of no SST file contents written
(because there was no content to add) and the empty file deleted before
calling the listener. This could lead to a stress test assertion failure
added in https://github.com/facebook/rocksdb/issues/9054.

This changes the status to Aborted, to align with the API doc:
"... if the file is successfully created. Now it will also be called on
failure case. User can check info.status to see if it succeeded or not."
For internal purposes, this case is considered "success" but for
listener purposes, no SST file is (successfully) created.

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

Test Plan: test case added + existing db_stress

Reviewed By: ajkr, riversand963

Differential Revision: D32120232

Pulled By: pdillinger

fbshipit-source-id: a804e2e0a52598018d3b182da97804d402ffcdfa
This commit is contained in:
Peter Dillinger 2021-11-03 08:42:08 -07:00 committed by Facebook GitHub Bot
parent 21f8a57f2a
commit 2b60621f16
3 changed files with 19 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
HISTORY.md
View File

@ -8,6 +8,7 @@
* Prevent a `CompactRange()` with `CompactRangeOptions::change_level == true` from possibly causing corruption to the LSM state (overlapping files within a level) when run in parallel with another manual compaction. Note that setting `force_consistency_checks == true` (the default) would cause the DB to enter read-only mode in this scenario and return `Status::Corruption`, rather than committing any corruption.
* Fixed a bug in CompactionIterator when write-prepared transaction is used. A released earliest write conflict snapshot may cause assertion failure in dbg mode and unexpected key in opt mode.
* Fix ticker WRITE_WITH_WAL("rocksdb.write.wal"), this bug is caused by a bad extra `RecordTick(stats_, WRITE_WITH_WAL)` (at 2 place), this fix remove the extra `RecordTick`s and fix the corresponding test case.
* EventListener::OnTableFileCreated was previously called with OK status and file_size==0 in cases of no SST file contents written (because there was no content to add) and the empty file deleted before calling the listener. Now the status is Aborted.
### Behavior Changes
* `NUM_FILES_IN_SINGLE_COMPACTION` was only counting the first input level files, now it's including all input files.

7
db/builder.cc
View File

@ -380,14 +380,19 @@ Status BuildTable(
}
}
Status status_for_listener = s;
if (meta->fd.GetFileSize() == 0) {
fname = "(nil)";
if (s.ok()) {
status_for_listener = Status::Aborted("Empty SST file not kept");
}
}
// Output to event logger and fire events.
EventHelpers::LogAndNotifyTableFileCreationFinished(
event_logger, ioptions.listeners, dbname, tboptions.column_family_name,
fname, job_id, meta->fd, meta->oldest_blob_file_number, tp,
tboptions.reason, s, file_checksum, file_checksum_func_name);
tboptions.reason, status_for_listener, file_checksum,
file_checksum_func_name);
return s;
}

12
db/listener_test.cc
View File

@ -772,6 +772,7 @@ class TableFileCreationListener : public EventListener {
} else {
if (idx >= 0) {
failure_[idx]++;
last_failure_ = info.status;
}
}
}
@ -779,6 +780,7 @@ class TableFileCreationListener : public EventListener {
int started_[2];
int finished_[2];
int failure_[2];
Status last_failure_;
};
TEST_F(EventListenerTest, TableFileCreationListenersTest) {
@ -801,6 +803,7 @@ TEST_F(EventListenerTest, TableFileCreationListenersTest) {
test_env->SetStatus(Status::NotSupported("not supported"));
ASSERT_NOK(Flush());
listener->CheckAndResetCounters(1, 1, 1, 0, 0, 0);
ASSERT_TRUE(listener->last_failure_.IsNotSupported());
test_env->SetStatus(Status::OK());
Reopen(options);
@ -817,6 +820,14 @@ TEST_F(EventListenerTest, TableFileCreationListenersTest) {
ASSERT_OK(dbfull()->TEST_WaitForCompact());
listener->CheckAndResetCounters(0, 0, 0, 1, 1, 0);
// Verify that an empty table file that is immediately deleted gives Aborted
// status to listener.
ASSERT_OK(Put("baz", "z"));
ASSERT_OK(SingleDelete("baz"));
ASSERT_OK(Flush());
listener->CheckAndResetCounters(1, 1, 1, 0, 0, 0);
ASSERT_TRUE(listener->last_failure_.IsAborted());
ASSERT_OK(Put("foo", "aaa3"));
ASSERT_OK(Put("bar", "bbb3"));
ASSERT_OK(Flush());
@ -825,6 +836,7 @@ TEST_F(EventListenerTest, TableFileCreationListenersTest) {
dbfull()->CompactRange(CompactRangeOptions(), &kRangeStart, &kRangeEnd));
ASSERT_NOK(dbfull()->TEST_WaitForCompact());
listener->CheckAndResetCounters(1, 1, 0, 1, 1, 1);
ASSERT_TRUE(listener->last_failure_.IsNotSupported());
Close();
}