Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check return value of TableBuilder::Add() in compaction routine. #1179

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Check return value of TableBuilder::Add() in compaction routine.
This is a prospective fix for the crash described in
crbug.com/326566884. Inspection of the crash dump suggests that
`Add()` is failing, but the compaction routine is continuing to
run. While it's not entirely clear how precisely this leads to
the observed failure mode, it seems safer to check the status
of the TableBuilder and abort early as needed.
  • Loading branch information
Evan Stade committed Apr 9, 2024
commit 80d6c09fa4664f38c1252a6d03ef3b8148665941
7 changes: 6 additions & 1 deletion db/db_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -998,7 +998,12 @@ Status DBImpl::DoCompactionWork(CompactionState* compact) {
compact->current_output()->smallest.DecodeFrom(key);
}
compact->current_output()->largest.DecodeFrom(key);
compact->builder->Add(key, input->value());

status = compact->builder->Add(key, input->value());
if (!status.ok()) {
FinishCompactionOutputFile(compact, input);
break;
}

// Close output file if it is big enough
if (compact->builder->FileSize() >=
Expand Down
2 changes: 1 addition & 1 deletion include/leveldb/table_builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class LEVELDB_EXPORT TableBuilder {
// Add key,value to the table being constructed.
// REQUIRES: key is after any previously added key according to comparator.
// REQUIRES: Finish(), Abandon() have not been called
void Add(const Slice& key, const Slice& value);
Status Add(const Slice& key, const Slice& value);

// Advanced operation: flush any buffered key/value pairs to file.
// Can be used to ensure that two adjacent entries never live in
Expand Down
7 changes: 5 additions & 2 deletions table/table_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,12 @@ Status TableBuilder::ChangeOptions(const Options& options) {
return Status::OK();
}

void TableBuilder::Add(const Slice& key, const Slice& value) {
Status TableBuilder::Add(const Slice& key, const Slice& value) {
Rep* r = rep_;
assert(!r->closed);
if (!ok()) return;
if (!ok()) {
return status();
}
if (r->num_entries > 0) {
assert(r->options.comparator->Compare(key, Slice(r->last_key)) > 0);
}
Expand All @@ -120,6 +122,7 @@ void TableBuilder::Add(const Slice& key, const Slice& value) {
if (estimated_block_size >= r->options.block_size) {
Flush();
}
return status();
}

void TableBuilder::Flush() {
Expand Down