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.

Sign up for GitHub

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

Jump to bottom

Fix blob db compression bug #2447

Closed
wants to merge 4 commits into from
+18 −17
Closed

Fix blob db compression bug #2447

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
41f31ce
Fix blob db compression bug
Jun 14, 2017
35c8113
address comment
Jun 14, 2017
c99e4f1
Fix compile error
Jun 14, 2017
aa9728a
enable compresison test
Jun 14, 2017
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
30 changes: 14 additions & 16 deletions utilities/blob_db/blob_db_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -897,15 +897,8 @@ Status BlobDBImpl::Write(const WriteOptions& opts, WriteBatch* updates) {
last_file_ = bfile;
has_put_ = true;

Slice value = value_unc;
std::string compression_output;
if (impl_->bdb_options_.compression != kNoCompression) {
CompressionType ct = impl_->bdb_options_.compression;
CompressionOptions compression_opts;
value = CompressBlock(value_unc, compression_opts, &ct,
kBlockBasedTableVersionFormat, Slice(),
&compression_output);
}
Slice value = impl_->GetCompressedSlice(value_unc, &compression_output);

std::string headerbuf;
Writer::ConstructBlobHeader(&headerbuf, key, value, expiration, -1);
Expand Down Expand Up @@ -1018,6 +1011,18 @@ Status BlobDBImpl::PutWithTTL(const WriteOptions& options,
: -1);
}

Slice BlobDBImpl::GetCompressedSlice(const Slice& raw,
std::string* compression_output) const {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe you just overlooked this issue while trying to make changes rapidly 😃
No need to pass in compression_output as you are returning it; just declare it locally in this function. Also lines 925 and 1061 can be removed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to go once this change is made.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

compression_output is needed. It backs the return Slice, and have to be destroy after we finish using the value. I need to fix compile error though..

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, didn't realize that the storage is external to Slice. Good point.

if (bdb_options_.compression == kNoCompression) {
return raw;
}
CompressionType ct = bdb_options_.compression;
CompressionOptions compression_opts;
CompressBlock(raw, compression_opts, &ct, kBlockBasedTableVersionFormat,
Slice(), compression_output);
return *compression_output;
}

Status BlobDBImpl::PutUntil(const WriteOptions& options,
ColumnFamilyHandle* column_family, const Slice& key,
const Slice& value_unc, int32_t expiration) {
Expand All @@ -1028,15 +1033,8 @@ Status BlobDBImpl::PutUntil(const WriteOptions& options,

if (!bfile) return Status::NotFound("Blob file not found");

Slice value = value_unc;
std::string compression_output;
if (bdb_options_.compression != kNoCompression) {
CompressionType ct = bdb_options_.compression;
CompressionOptions compression_opts;
value = CompressBlock(value_unc, compression_opts, &ct,
kBlockBasedTableVersionFormat, Slice(),
&compression_output);
}
Slice value = GetCompressedSlice(value_unc, &compression_output);

std::string headerbuf;
Writer::ConstructBlobHeader(&headerbuf, key, value, expiration, -1);
Expand Down
3 changes: 3 additions & 0 deletions utilities/blob_db/blob_db_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,9 @@ class BlobDBImpl : public BlobDB {
const std::string& index_entry, std::string* value,
SequenceNumber* sequence = nullptr);

Slice GetCompressedSlice(const Slice& raw,
std::string* compression_output) const;

// Just before flush starts acting on memtable files,
// this handler is called.
void OnFlushBeginHandler(DB* db, const FlushJobInfo& info);
Expand Down
2 changes: 1 addition & 1 deletion utilities/blob_db/blob_db_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ TEST_F(BlobDBTest, Override) {
}

#ifdef SNAPPY
TEST_F(BlobDBTest, DISABLED_Compression) {
TEST_F(BlobDBTest, Compression) {
Random rnd(301);
BlobDBOptionsImpl bdb_options;
bdb_options.disable_background_tasks = true;
Expand Down