From 7a7ca8eb5b7138a567ccaaefdb0b6c0af1a42d85 Mon Sep 17 00:00:00 2001 From: Levi Tamasi Date: Thu, 19 Dec 2019 18:03:24 -0800 Subject: [PATCH] BlobDB: only compare CF IDs when checking whether an API call is for the default CF (#6226) Summary: BlobDB currently only supports using the default column family. The earlier code enforces this by comparing the `ColumnFamilyHandle` passed to the `Get`/`Put`/etc. call with the handle returned by `DefaultColumnFamily` (which, at the end of the day, comes from `DBImpl::default_cf_handle_`). Since other `ColumnFamilyHandle`s can also point to the default column family, this can reject legitimate requests as well. (As an example, with the earlier code, the handle returned by `BlobDB::Open` cannot actually be used in API calls.) The patch fixes this by comparing only the IDs of the column family handles instead of the pointers themselves. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6226 Test Plan: `make check` Differential Revision: D19187461 Pulled By: ltamasi fbshipit-source-id: 54ce2e12ebb1f07e6d1e70e3b1e0213dfa94bda2 --- HISTORY.md | 1 + utilities/blob_db/blob_db.h | 14 +++++++------- utilities/blob_db/blob_db_impl.cc | 2 +- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index 0f35a44d9..24f4aea04 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -11,6 +11,7 @@ * Fixed two performance issues related to memtable history trimming. First, a new SuperVersion is now created only if some memtables were actually trimmed. Second, trimming is only scheduled if there is at least one flushed memtable that is kept in memory for the purposes of transaction conflict checking. * Fix a bug in WriteBatchWithIndex::MultiGetFromBatchAndDB, which is called by Transaction::MultiGet, that causes due to stale pointer access when the number of keys is > 32 * BlobDB no longer updates the SST to blob file mapping upon failed compactions. +* Fixed a bug where BlobDB was comparing the `ColumnFamilyHandle` pointers themselves instead of only the column family IDs when checking whether an API call uses the default column family or not. ### New Features * It is now possible to enable periodic compactions for the base DB when using BlobDB. diff --git a/utilities/blob_db/blob_db.h b/utilities/blob_db/blob_db.h index 5a950134e..25f10f14a 100644 --- a/utilities/blob_db/blob_db.h +++ b/utilities/blob_db/blob_db.h @@ -91,7 +91,7 @@ class BlobDB : public StackableDB { virtual Status Put(const WriteOptions& options, ColumnFamilyHandle* column_family, const Slice& key, const Slice& value) override { - if (column_family != DefaultColumnFamily()) { + if (column_family->GetID() != DefaultColumnFamily()->GetID()) { return Status::NotSupported( "Blob DB doesn't support non-default column family."); } @@ -102,7 +102,7 @@ class BlobDB : public StackableDB { virtual Status Delete(const WriteOptions& options, ColumnFamilyHandle* column_family, const Slice& key) override { - if (column_family != DefaultColumnFamily()) { + if (column_family->GetID() != DefaultColumnFamily()->GetID()) { return Status::NotSupported( "Blob DB doesn't support non-default column family."); } @@ -115,7 +115,7 @@ class BlobDB : public StackableDB { virtual Status PutWithTTL(const WriteOptions& options, ColumnFamilyHandle* column_family, const Slice& key, const Slice& value, uint64_t ttl) { - if (column_family != DefaultColumnFamily()) { + if (column_family->GetID() != DefaultColumnFamily()->GetID()) { return Status::NotSupported( "Blob DB doesn't support non-default column family."); } @@ -129,7 +129,7 @@ class BlobDB : public StackableDB { virtual Status PutUntil(const WriteOptions& options, ColumnFamilyHandle* column_family, const Slice& key, const Slice& value, uint64_t expiration) { - if (column_family != DefaultColumnFamily()) { + if (column_family->GetID() != DefaultColumnFamily()->GetID()) { return Status::NotSupported( "Blob DB doesn't support non-default column family."); } @@ -161,7 +161,7 @@ class BlobDB : public StackableDB { const std::vector& keys, std::vector* values) override { for (auto column_family : column_families) { - if (column_family != DefaultColumnFamily()) { + if (column_family->GetID() != DefaultColumnFamily()->GetID()) { return std::vector( column_families.size(), Status::NotSupported( @@ -201,7 +201,7 @@ class BlobDB : public StackableDB { virtual Iterator* NewIterator(const ReadOptions& options) override = 0; virtual Iterator* NewIterator(const ReadOptions& options, ColumnFamilyHandle* column_family) override { - if (column_family != DefaultColumnFamily()) { + if (column_family->GetID() != DefaultColumnFamily()->GetID()) { // Blob DB doesn't support non-default column family. return nullptr; } @@ -221,7 +221,7 @@ class BlobDB : public StackableDB { const int output_path_id = -1, std::vector* const output_file_names = nullptr, CompactionJobInfo* compaction_job_info = nullptr) override { - if (column_family != DefaultColumnFamily()) { + if (column_family->GetID() != DefaultColumnFamily()->GetID()) { return Status::NotSupported( "Blob DB doesn't support non-default column family."); } diff --git a/utilities/blob_db/blob_db_impl.cc b/utilities/blob_db/blob_db_impl.cc index de5aebc27..d98c4f231 100644 --- a/utilities/blob_db/blob_db_impl.cc +++ b/utilities/blob_db/blob_db_impl.cc @@ -1513,7 +1513,7 @@ Status BlobDBImpl::Get(const ReadOptions& read_options, Status BlobDBImpl::GetImpl(const ReadOptions& read_options, ColumnFamilyHandle* column_family, const Slice& key, PinnableSlice* value, uint64_t* expiration) { - if (column_family != DefaultColumnFamily()) { + if (column_family->GetID() != DefaultColumnFamily()->GetID()) { return Status::NotSupported( "Blob DB doesn't support non-default column family."); }