Skip to content

Commit

Permalink
Add Iterator property "rocksdb.iterator.is-value-pinned" (#12659)
Browse files Browse the repository at this point in the history
Summary:
`ReadOptions::pin_data` already has the effect of pinning the `Slice` returned by `Iterator::value()` when the value is stored inline (e.g., `kTypeValue`). This PR adds a bit of visibility into that via a new `Iterator` property, "rocksdb.iterator.is-value-pinned", as well as some documentation and tests.

See also: #12658

Pull Request resolved: #12659

Reviewed By: cbi42

Differential Revision: D57391200

Pulled By: ajkr

fbshipit-source-id: 0caa8db27ca1aba86ee2addc3dfd6f0e003d32e2
  • Loading branch information
ajkr authored and facebook-github-bot committed May 16, 2024
1 parent 3ed46e0 commit 4eaf628
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 4 deletions.
10 changes: 10 additions & 0 deletions db/db_iter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,16 @@ Status DBIter::GetProperty(std::string prop_name, std::string* prop) {
*prop = "Iterator is not valid.";
}
return Status::OK();
} else if (prop_name == "rocksdb.iterator.is-value-pinned") {
if (valid_) {
*prop = (pin_thru_lifetime_ && iter_.Valid() &&
iter_.value().data() == value_.data())
? "1"
: "0";
} else {
*prop = "Iterator is not valid.";
}
return Status::OK();
} else if (prop_name == "rocksdb.iterator.internal-key") {
*prop = saved_key_.GetUserKey().ToString();
return Status::OK();
Expand Down
23 changes: 19 additions & 4 deletions db/db_iterator_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -133,11 +133,17 @@ TEST_P(DBIteratorTest, IteratorProperty) {
ASSERT_NOK(iter->GetProperty("non_existing.value", &prop_value));
ASSERT_OK(iter->GetProperty("rocksdb.iterator.is-key-pinned", &prop_value));
ASSERT_EQ("0", prop_value);
ASSERT_OK(
iter->GetProperty("rocksdb.iterator.is-value-pinned", &prop_value));
ASSERT_EQ("0", prop_value);
ASSERT_OK(iter->GetProperty("rocksdb.iterator.internal-key", &prop_value));
ASSERT_EQ("1", prop_value);
iter->Next();
ASSERT_OK(iter->GetProperty("rocksdb.iterator.is-key-pinned", &prop_value));
ASSERT_EQ("Iterator is not valid.", prop_value);
ASSERT_OK(
iter->GetProperty("rocksdb.iterator.is-value-pinned", &prop_value));
ASSERT_EQ("Iterator is not valid.", prop_value);

// Get internal key at which the iteration stopped (tombstone in this case).
ASSERT_OK(iter->GetProperty("rocksdb.iterator.internal-key", &prop_value));
Expand Down Expand Up @@ -1680,12 +1686,15 @@ TEST_P(DBIteratorTest, PinnedDataIteratorMultipleFiles) {
ro.pin_data = true;
auto iter = NewIterator(ro);

std::vector<std::pair<Slice, std::string>> results;
std::vector<std::pair<Slice, Slice>> results;
for (iter->SeekToFirst(); iter->Valid(); iter->Next()) {
std::string prop_value;
ASSERT_OK(iter->GetProperty("rocksdb.iterator.is-key-pinned", &prop_value));
ASSERT_EQ("1", prop_value);
results.emplace_back(iter->key(), iter->value().ToString());
ASSERT_OK(
iter->GetProperty("rocksdb.iterator.is-value-pinned", &prop_value));
ASSERT_EQ("1", prop_value);
results.emplace_back(iter->key(), iter->value());
}

ASSERT_EQ(results.size(), true_data.size());
Expand Down Expand Up @@ -1739,6 +1748,9 @@ TEST_P(DBIteratorTest, PinnedDataIteratorMergeOperator) {
std::string prop_value;
ASSERT_OK(iter->GetProperty("rocksdb.iterator.is-key-pinned", &prop_value));
ASSERT_EQ("1", prop_value);
ASSERT_OK(
iter->GetProperty("rocksdb.iterator.is-value-pinned", &prop_value));
ASSERT_EQ("0", prop_value);
results.emplace_back(iter->key(), iter->value().ToString());
}
ASSERT_OK(iter->status());
Expand Down Expand Up @@ -1792,12 +1804,15 @@ TEST_P(DBIteratorTest, PinnedDataIteratorReadAfterUpdate) {
}
}

std::vector<std::pair<Slice, std::string>> results;
std::vector<std::pair<Slice, Slice>> results;
for (iter->SeekToFirst(); iter->Valid(); iter->Next()) {
std::string prop_value;
ASSERT_OK(iter->GetProperty("rocksdb.iterator.is-key-pinned", &prop_value));
ASSERT_EQ("1", prop_value);
results.emplace_back(iter->key(), iter->value().ToString());
ASSERT_OK(
iter->GetProperty("rocksdb.iterator.is-value-pinned", &prop_value));
ASSERT_EQ("1", prop_value);
results.emplace_back(iter->key(), iter->value());
}
ASSERT_OK(iter->status());

Expand Down
6 changes: 6 additions & 0 deletions include/rocksdb/iterator.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,12 @@ class Iterator : public IteratorBase {
// - Iterator created with ReadOptions::pin_data = true
// - DB tables were created with
// BlockBasedTableOptions::use_delta_encoding = false.
// Property "rocksdb.iterator.is-value-pinned":
// If returning "1", this means that the Slice returned by value() is valid
// as long as the iterator is not deleted.
// It is guaranteed to always return "1" if
// - Iterator created with ReadOptions::pin_data = true
// - The value is found in a `kTypeValue` record
// Property "rocksdb.iterator.super-version-number":
// LSM version used by the iterator. The same format as DB Property
// kCurrentSuperVersionNumber. See its comment for more information.
Expand Down
4 changes: 4 additions & 0 deletions table/iterator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ Status Iterator::GetProperty(std::string prop_name, std::string* prop) {
*prop = "0";
return Status::OK();
}
if (prop_name == "rocksdb.iterator.is-value-pinned") {
*prop = "0";
return Status::OK();
}
return Status::InvalidArgument("Unidentified property.");
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* Added new `Iterator` property, "rocksdb.iterator.is-value-pinned", for checking whether the `Slice` returned by `Iterator::value()` can be used until the `Iterator` is destroyed.

0 comments on commit 4eaf628

Please sign in to comment.