From 9c0c8f5ff600044c46cd31f663e095cf4435414b Mon Sep 17 00:00:00 2001 From: Siying Dong Date: Thu, 16 Aug 2018 15:48:55 -0700 Subject: [PATCH] GetAllKeyVersions() to take an extra argument of `max_num_ikeys`. (#4271) Summary: Right now, `ldb idump` may have memory out of control if there is a big range of tombstones. Add an option to cut maxinum number of keys in GetAllKeyVersions(), and push down --max_num_ikeys from ldb. Pull Request resolved: https://github.com/facebook/rocksdb/pull/4271 Differential Revision: D9369149 Pulled By: siying fbshipit-source-id: 7cbb797b7d2fa16573495a7e84937456d3ff25bf --- HISTORY.md | 2 ++ include/rocksdb/utilities/debug.h | 6 +++++- tools/ldb_cmd.cc | 2 +- utilities/blob_db/blob_db_test.cc | 16 ++++++++++------ utilities/debug.cc | 5 +++++ .../write_prepared_transaction_test.cc | 4 +++- 6 files changed, 26 insertions(+), 9 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index abfd1a15c1..0e639fa94b 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -2,6 +2,8 @@ ## Unreleased ### Public API Change * The merge operands are passed to `MergeOperator::ShouldMerge` in the reversed order relative to how they were merged (passed to FullMerge or FullMergeV2) for performance reasons +* GetAllKeyVersions() to take an extra argument of `max_num_ikeys`. + ### New Features * Changes the format of index blocks by delta encoding the index values, which are the block handles. This saves the encoding of BlockHandle::offset of the non-head index entries in each restart interval. The feature is backward compatible but not forward compatible. It is disabled by default unless format_version 4 or above is used. * Add a new tool: trace_analyzer. Trace_analyzer analyzes the trace file generated by using trace_replay API. It can convert the binary format trace file to a human readable txt file, output the statistics of the analyzed query types such as access statistics and size statistics, combining the dumped whole key space file to analyze, support query correlation analyzing, and etc. Current supported query types are: Get, Put, Delete, SingleDelete, DeleteRange, Merge, Iterator (Seek, SeekForPrev only). diff --git a/include/rocksdb/utilities/debug.h b/include/rocksdb/utilities/debug.h index bc5b9bf03d..50645423d0 100644 --- a/include/rocksdb/utilities/debug.h +++ b/include/rocksdb/utilities/debug.h @@ -31,9 +31,13 @@ struct KeyVersion { }; // Returns listing of all versions of keys in the provided user key range. -// The range is inclusive-inclusive, i.e., [`begin_key`, `end_key`]. +// The range is inclusive-inclusive, i.e., [`begin_key`, `end_key`], or +// `max_num_ikeys` has been reached. Since all those keys returned will be +// copied to memory, if the range covers too many keys, the memory usage +// may be huge. `max_num_ikeys` can be used to cap the memory usage. // The result is inserted into the provided vector, `key_versions`. Status GetAllKeyVersions(DB* db, Slice begin_key, Slice end_key, + size_t max_num_ikeys, std::vector* key_versions); } // namespace rocksdb diff --git a/tools/ldb_cmd.cc b/tools/ldb_cmd.cc index 4492c63136..e0e24a73de 100644 --- a/tools/ldb_cmd.cc +++ b/tools/ldb_cmd.cc @@ -1237,7 +1237,7 @@ void InternalDumpCommand::DoCommand() { // Cast as DBImpl to get internal iterator std::vector key_versions; - Status st = GetAllKeyVersions(db_, from_, to_, &key_versions); + Status st = GetAllKeyVersions(db_, from_, to_, max_keys_, &key_versions); if (!st.ok()) { exec_state_ = LDBCommandExecuteResult::Failed(st.ToString()); return; diff --git a/utilities/blob_db/blob_db_test.cc b/utilities/blob_db/blob_db_test.cc index 5ee523260b..f00515c0c1 100644 --- a/utilities/blob_db/blob_db_test.cc +++ b/utilities/blob_db/blob_db_test.cc @@ -196,8 +196,9 @@ class BlobDBTest : public testing::Test { const std::map &expected_versions) { auto *bdb_impl = static_cast(blob_db_); DB *db = blob_db_->GetRootDB(); + const size_t kMaxKeys = 10000; std::vector versions; - GetAllKeyVersions(db, "", "", &versions); + GetAllKeyVersions(db, "", "", kMaxKeys, &versions); ASSERT_EQ(expected_versions.size(), versions.size()); size_t i = 0; for (auto &key_version : expected_versions) { @@ -1232,7 +1233,8 @@ TEST_F(BlobDBTest, FilterExpiredBlobIndex) { blob_db_->ReleaseSnapshot(snapshot); // Verify expired blob index are filtered. std::vector versions; - GetAllKeyVersions(blob_db_, "", "", &versions); + const size_t kMaxKeys = 10000; + GetAllKeyVersions(blob_db_, "", "", kMaxKeys, &versions); ASSERT_EQ(data_after_compact.size(), versions.size()); for (auto &version : versions) { ASSERT_TRUE(data_after_compact.count(version.user_key) > 0); @@ -1262,9 +1264,11 @@ TEST_F(BlobDBTest, FilterFileNotAvailable) { ASSERT_EQ(2, blob_files[1]->BlobFileNumber()); ASSERT_OK(blob_db_impl()->TEST_CloseBlobFile(blob_files[1])); + const size_t kMaxKeys = 10000; + DB *base_db = blob_db_->GetRootDB(); std::vector versions; - ASSERT_OK(GetAllKeyVersions(base_db, "", "", &versions)); + ASSERT_OK(GetAllKeyVersions(base_db, "", "", kMaxKeys, &versions)); ASSERT_EQ(2, versions.size()); ASSERT_EQ("bar", versions[0].user_key); ASSERT_EQ("foo", versions[1].user_key); @@ -1272,7 +1276,7 @@ TEST_F(BlobDBTest, FilterFileNotAvailable) { ASSERT_OK(blob_db_->Flush(FlushOptions())); ASSERT_OK(blob_db_->CompactRange(CompactRangeOptions(), nullptr, nullptr)); - ASSERT_OK(GetAllKeyVersions(base_db, "", "", &versions)); + ASSERT_OK(GetAllKeyVersions(base_db, "", "", kMaxKeys, &versions)); ASSERT_EQ(2, versions.size()); ASSERT_EQ("bar", versions[0].user_key); ASSERT_EQ("foo", versions[1].user_key); @@ -1282,7 +1286,7 @@ TEST_F(BlobDBTest, FilterFileNotAvailable) { blob_db_impl()->TEST_ObsoleteBlobFile(blob_files[0]); blob_db_impl()->TEST_DeleteObsoleteFiles(); ASSERT_OK(blob_db_->CompactRange(CompactRangeOptions(), nullptr, nullptr)); - ASSERT_OK(GetAllKeyVersions(base_db, "", "", &versions)); + ASSERT_OK(GetAllKeyVersions(base_db, "", "", kMaxKeys, &versions)); ASSERT_EQ(1, versions.size()); ASSERT_EQ("bar", versions[0].user_key); VerifyDB({{"bar", "v2"}}); @@ -1291,7 +1295,7 @@ TEST_F(BlobDBTest, FilterFileNotAvailable) { blob_db_impl()->TEST_ObsoleteBlobFile(blob_files[1]); blob_db_impl()->TEST_DeleteObsoleteFiles(); ASSERT_OK(blob_db_->CompactRange(CompactRangeOptions(), nullptr, nullptr)); - ASSERT_OK(GetAllKeyVersions(base_db, "", "", &versions)); + ASSERT_OK(GetAllKeyVersions(base_db, "", "", kMaxKeys, &versions)); ASSERT_EQ(0, versions.size()); VerifyDB({}); } diff --git a/utilities/debug.cc b/utilities/debug.cc index f9a69f4768..e0c5f5566e 100644 --- a/utilities/debug.cc +++ b/utilities/debug.cc @@ -12,6 +12,7 @@ namespace rocksdb { Status GetAllKeyVersions(DB* db, Slice begin_key, Slice end_key, + size_t max_num_ikeys, std::vector* key_versions) { assert(key_versions != nullptr); key_versions->clear(); @@ -30,6 +31,7 @@ Status GetAllKeyVersions(DB* db, Slice begin_key, Slice end_key, iter->SeekToFirst(); } + size_t num_keys = 0; for (; iter->Valid(); iter->Next()) { ParsedInternalKey ikey; if (!ParseInternalKey(iter->key(), &ikey)) { @@ -46,6 +48,9 @@ Status GetAllKeyVersions(DB* db, Slice begin_key, Slice end_key, iter->value().ToString() /* _value */, ikey.sequence /* _sequence */, static_cast(ikey.type) /* _type */); + if (++num_keys >= max_num_ikeys) { + break; + } } return Status::OK(); } diff --git a/utilities/transactions/write_prepared_transaction_test.cc b/utilities/transactions/write_prepared_transaction_test.cc index 1f980a3a7e..391d58defd 100644 --- a/utilities/transactions/write_prepared_transaction_test.cc +++ b/utilities/transactions/write_prepared_transaction_test.cc @@ -493,8 +493,10 @@ class WritePreparedTransactionTestBase : public TransactionTestBase { // Verify all versions of keys. void VerifyInternalKeys(const std::vector& expected_versions) { std::vector versions; + const size_t kMaxKeys = 100000; ASSERT_OK(GetAllKeyVersions(db, expected_versions.front().user_key, - expected_versions.back().user_key, &versions)); + expected_versions.back().user_key, kMaxKeys, + &versions)); ASSERT_EQ(expected_versions.size(), versions.size()); for (size_t i = 0; i < versions.size(); i++) { ASSERT_EQ(expected_versions[i].user_key, versions[i].user_key);