diff --git a/HISTORY.md b/HISTORY.md index 4e1db503a..fc811d00d 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -12,6 +12,7 @@ * Fix false negative from the VerifyChecksum() API when there is a checksum mismatch in an index partition block in a BlockBasedTable format table file (index_type is kTwoLevelIndexSearch). * Fix sst_dump to return non-zero exit code if the specified file is not a recognized SST file or fails requested checks. * Fix incorrect results from batched MultiGet for duplicate keys, when the duplicate key matches the largest key of an SST file and the value type for the key in the file is a merge value. +* Fix "bad block type" error from persistent cache on Windows. ### Public API Change * Flush(..., column_family) may return Status::ColumnFamilyDropped() instead of Status::InvalidArgument() if column_family is dropped while processing the flush request. diff --git a/db/db_test2.cc b/db/db_test2.cc index 467492155..52370b136 100644 --- a/db/db_test2.cc +++ b/db/db_test2.cc @@ -2010,6 +2010,10 @@ class MockPersistentCache : public PersistentCache { return PersistentCache::StatsType(); } + uint64_t NewId() override { + return last_id_.fetch_add(1, std::memory_order_relaxed); + } + Status Insert(const Slice& page_key, const char* data, const size_t size) override { MutexLock _(&lock_); @@ -2050,6 +2054,7 @@ class MockPersistentCache : public PersistentCache { const bool is_compressed_ = true; size_t size_ = 0; const size_t max_size_ = 10 * 1024; // 10KiB + std::atomic last_id_{1}; }; #ifdef OS_LINUX @@ -2173,10 +2178,7 @@ TEST_F(DBTest2, TestPerfContextIterCpuTime) { } #endif // OS_LINUX -// GetUniqueIdFromFile is not implemented on these platforms. Persistent cache -// breaks when that function is not implemented and no regular block cache is -// provided. -#if !defined(OS_SOLARIS) && !defined(OS_WIN) +#if !defined OS_SOLARIS TEST_F(DBTest2, PersistentCache) { int num_iter = 80; @@ -2240,7 +2242,7 @@ TEST_F(DBTest2, PersistentCache) { } } } -#endif // !defined(OS_SOLARIS) && !defined(OS_WIN) +#endif // !defined OS_SOLARIS namespace { void CountSyncPoint() { diff --git a/include/rocksdb/persistent_cache.h b/include/rocksdb/persistent_cache.h index 9651812c8..e2dcfcac3 100644 --- a/include/rocksdb/persistent_cache.h +++ b/include/rocksdb/persistent_cache.h @@ -56,6 +56,12 @@ class PersistentCache { virtual StatsType Stats() = 0; virtual std::string GetPrintableOptions() const = 0; + + // Return a new numeric id. May be used by multiple clients who are + // sharding the same persistent cache to partition the key space. Typically + // the client will allocate a new id at startup and prepend the id to its + // cache keys. + virtual uint64_t NewId() = 0; }; // Factor method to create a new persistent cache diff --git a/table/block_based/block_based_table_builder.cc b/table/block_based/block_based_table_builder.cc index bc1b16ec2..70078a8c9 100644 --- a/table/block_based/block_based_table_builder.cc +++ b/table/block_based/block_based_table_builder.cc @@ -722,7 +722,7 @@ BlockBasedTableBuilder::BlockBasedTableBuilder( rep_->filter_builder->StartBlock(0); } if (table_options.block_cache_compressed.get() != nullptr) { - BlockBasedTable::GenerateCachePrefix( + BlockBasedTable::GenerateCachePrefix( table_options.block_cache_compressed.get(), file->writable_file(), &rep_->compressed_cache_key_prefix[0], &rep_->compressed_cache_key_prefix_size); diff --git a/table/block_based/block_based_table_reader.cc b/table/block_based/block_based_table_reader.cc index 7ce7ca55e..bb3bfd1d2 100644 --- a/table/block_based/block_based_table_reader.cc +++ b/table/block_based/block_based_table_reader.cc @@ -452,44 +452,21 @@ void BlockBasedTable::SetupCacheKeyPrefix(Rep* rep) { rep->cache_key_prefix_size = 0; rep->compressed_cache_key_prefix_size = 0; if (rep->table_options.block_cache != nullptr) { - GenerateCachePrefix(rep->table_options.block_cache.get(), rep->file->file(), - &rep->cache_key_prefix[0], &rep->cache_key_prefix_size); + GenerateCachePrefix( + rep->table_options.block_cache.get(), rep->file->file(), + &rep->cache_key_prefix[0], &rep->cache_key_prefix_size); } if (rep->table_options.persistent_cache != nullptr) { - GenerateCachePrefix(/*cache=*/nullptr, rep->file->file(), - &rep->persistent_cache_key_prefix[0], - &rep->persistent_cache_key_prefix_size); + GenerateCachePrefix( + rep->table_options.persistent_cache.get(), rep->file->file(), + &rep->persistent_cache_key_prefix[0], + &rep->persistent_cache_key_prefix_size); } if (rep->table_options.block_cache_compressed != nullptr) { - GenerateCachePrefix(rep->table_options.block_cache_compressed.get(), - rep->file->file(), &rep->compressed_cache_key_prefix[0], - &rep->compressed_cache_key_prefix_size); - } -} - -void BlockBasedTable::GenerateCachePrefix(Cache* cc, FSRandomAccessFile* file, - char* buffer, size_t* size) { - // generate an id from the file - *size = file->GetUniqueId(buffer, kMaxCacheKeyPrefixSize); - - // If the prefix wasn't generated or was too long, - // create one from the cache. - if (cc != nullptr && *size == 0) { - char* end = EncodeVarint64(buffer, cc->NewId()); - *size = static_cast(end - buffer); - } -} - -void BlockBasedTable::GenerateCachePrefix(Cache* cc, FSWritableFile* file, - char* buffer, size_t* size) { - // generate an id from the file - *size = file->GetUniqueId(buffer, kMaxCacheKeyPrefixSize); - - // If the prefix wasn't generated or was too long, - // create one from the cache. - if (cc != nullptr && *size == 0) { - char* end = EncodeVarint64(buffer, cc->NewId()); - *size = static_cast(end - buffer); + GenerateCachePrefix( + rep->table_options.block_cache_compressed.get(), rep->file->file(), + &rep->compressed_cache_key_prefix[0], + &rep->compressed_cache_key_prefix_size); } } diff --git a/table/block_based/block_based_table_reader.h b/table/block_based/block_based_table_reader.h index 94f659530..87bf82e53 100644 --- a/table/block_based/block_based_table_reader.h +++ b/table/block_based/block_based_table_reader.h @@ -439,10 +439,19 @@ class BlockBasedTable : public TableReader { static void SetupCacheKeyPrefix(Rep* rep); // Generate a cache key prefix from the file - static void GenerateCachePrefix(Cache* cc, FSRandomAccessFile* file, - char* buffer, size_t* size); - static void GenerateCachePrefix(Cache* cc, FSWritableFile* file, char* buffer, - size_t* size); + template + static void GenerateCachePrefix(TCache* cc, TFile* file, char* buffer, + size_t* size) { + // generate an id from the file + *size = file->GetUniqueId(buffer, kMaxCacheKeyPrefixSize); + + // If the prefix wasn't generated or was too long, + // create one from the cache. + if (cc != nullptr && *size == 0) { + char* end = EncodeVarint64(buffer, cc->NewId()); + *size = static_cast(end - buffer); + } + } // Size of all data blocks, maybe approximate uint64_t GetApproximateDataSize(); diff --git a/utilities/persistent_cache/persistent_cache_test.cc b/utilities/persistent_cache/persistent_cache_test.cc index e7d6a313b..92b39307d 100644 --- a/utilities/persistent_cache/persistent_cache_test.cc +++ b/utilities/persistent_cache/persistent_cache_test.cc @@ -6,10 +6,7 @@ // Copyright (c) 2011 The LevelDB Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. See the AUTHORS file for names of contributors. - -// GetUniqueIdFromFile is not implemented on Windows. Persistent cache -// breaks when that function is not implemented -#if !defined(ROCKSDB_LITE) && !defined(OS_WIN) +#if !defined ROCKSDB_LITE #include "utilities/persistent_cache/persistent_cache_test.h" @@ -472,6 +469,6 @@ int main(int argc, char** argv) { ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); } -#else // !defined(ROCKSDB_LITE) && !defined(OS_WIN) +#else // !defined ROCKSDB_LITE int main() { return 0; } -#endif // !defined(ROCKSDB_LITE) && !defined(OS_WIN) +#endif // !defined ROCKSDB_LITE diff --git a/utilities/persistent_cache/persistent_cache_tier.cc b/utilities/persistent_cache/persistent_cache_tier.cc index 3847a4ee9..54cbce8f7 100644 --- a/utilities/persistent_cache/persistent_cache_tier.cc +++ b/utilities/persistent_cache/persistent_cache_tier.cc @@ -99,6 +99,10 @@ PersistentCache::StatsType PersistentCacheTier::Stats() { return PersistentCache::StatsType{}; } +uint64_t PersistentCacheTier::NewId() { + return last_id_.fetch_add(1, std::memory_order_relaxed); +} + // // PersistentTieredCache implementation // diff --git a/utilities/persistent_cache/persistent_cache_tier.h b/utilities/persistent_cache/persistent_cache_tier.h index 3905957de..f8fa88c34 100644 --- a/utilities/persistent_cache/persistent_cache_tier.h +++ b/utilities/persistent_cache/persistent_cache_tier.h @@ -266,6 +266,8 @@ class PersistentCacheTier : public PersistentCache { virtual std::string GetPrintableOptions() const override = 0; + virtual uint64_t NewId() override; + // Return a reference to next tier virtual Tier& next_tier() { return next_tier_; } @@ -283,6 +285,7 @@ class PersistentCacheTier : public PersistentCache { private: Tier next_tier_; // next tier + std::atomic last_id_{1}; }; // PersistentTieredCache