andreacavalli/rocksdb
0
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Change ParseInternalKey() to return Status instead of bool (#7457)

Browse Source 
Summary:
Fixes https://github.com/facebook/rocksdb/issues/7430

Change ParseInternalKey() to return Status instead of bool.

db_bench (seekrandom) based before/after results with value size of 100 bytes and 16 bytes can be found at (tests ran on an udb server):
https://www.dropbox.com/s/47bwamdy5ozngph/PIK_ret_Status_results.xlsx?dl=0

![db_bench_results](https://user-images.githubusercontent.com/62277872/94642825-2a21a800-029a-11eb-88f2-124136c83fd3.png)

Pull Request resolved: https://github.com/facebook/rocksdb/pull/7457

Reviewed By: ajkr

Differential Revision: D24002433

Pulled By: ramvadiv

fbshipit-source-id: ac253ecf577a29044c47c3fe254a01e71404c44c
This commit is contained in:
Ramkumar Vadivelu 2020-09-30 19:15:42 -07:00 committed by Facebook GitHub Bot
parent e127fe18c3
commit e04a50923d
35 changed files with 120 additions and 102 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
db/column_family.cc
View File

@ -1118,7 +1118,8 @@ Status ColumnFamilyData::RangesOverlapWithMemtables(
ParsedInternalKey seek_result;
if (status.ok()) {
if (memtable_iter->Valid() &&
!ParseInternalKey(memtable_iter->key(), &seek_result)) {
ParseInternalKey(memtable_iter->key(), &seek_result) !=
Status::OK()) {
status = Status::Corruption("DB have corrupted keys");
}
}

24
db/compaction/compaction_iterator.cc
View File

@ -151,11 +151,11 @@ void CompactionIterator::Next() {
if (merge_out_iter_.Valid()) {
key_ = merge_out_iter_.key();
value_ = merge_out_iter_.value();
bool valid_key = ParseInternalKey(key_, &ikey_);
Status s = ParseInternalKey(key_, &ikey_);
// MergeUntil stops when it encounters a corrupt key and does not
// include them in the result, so we expect the keys here to be valid.
assert(valid_key);
if (!valid_key) {
assert(s.ok());
if (!s.ok()) {
ROCKS_LOG_FATAL(info_log_, "Invalid key (%s) in compaction",
key_.ToString(true).c_str());
}
@ -270,7 +270,8 @@ void CompactionIterator::NextFromInput() {
value_ = input_->value();
iter_stats_.num_input_records++;
if (!ParseInternalKey(key_, &ikey_)) {
Status pikStatus = ParseInternalKey(key_, &ikey_);
if (!pikStatus.ok()) {
iter_stats_.num_input_corrupt_records++;
// If `expect_valid_internal_key_` is false, return the corrupted key
@ -437,7 +438,8 @@ void CompactionIterator::NextFromInput() {
// Check whether the next key exists, is not corrupt, and is the same key
// as the single delete.
if (input_->Valid() && ParseInternalKey(input_->key(), &next_ikey) &&
if (input_->Valid() &&
ParseInternalKey(input_->key(), &next_ikey) == Status::OK() &&
cmp_->Equal(ikey_.user_key, next_ikey.user_key)) {
// Check whether the next key belongs to the same snapshot as the
// SingleDelete.
@ -584,7 +586,8 @@ void CompactionIterator::NextFromInput() {
// Skip over all versions of this key that happen to occur in the same snapshot
// range as the delete
while (!IsPausingManualCompaction() && !IsShuttingDown() &&
input_->Valid() && ParseInternalKey(input_->key(), &next_ikey) &&
input_->Valid() &&
(ParseInternalKey(input_->key(), &next_ikey) == Status::OK()) &&
cmp_->Equal(ikey_.user_key, next_ikey.user_key) &&
(prev_snapshot == 0 ||
DEFINITELY_NOT_IN_SNAPSHOT(next_ikey.sequence, prev_snapshot))) {
@ -592,7 +595,8 @@ void CompactionIterator::NextFromInput() {
}
// If you find you still need to output a row with this key, we need to output the
// delete too
if (input_->Valid() && ParseInternalKey(input_->key(), &next_ikey) &&
if (input_->Valid() &&
(ParseInternalKey(input_->key(), &next_ikey) == Status::OK()) &&
cmp_->Equal(ikey_.user_key, next_ikey.user_key)) {
valid_ = true;
at_next_ = true;
@ -621,11 +625,11 @@ void CompactionIterator::NextFromInput() {
// These will be correctly set below.
key_ = merge_out_iter_.key();
value_ = merge_out_iter_.value();
bool valid_key = ParseInternalKey(key_, &ikey_);
pikStatus = ParseInternalKey(key_, &ikey_);
// MergeUntil stops when it encounters a corrupt key and does not
// include them in the result, so we expect the keys here to valid.
assert(valid_key);
if (!valid_key) {
assert(pikStatus.ok());
if (!pikStatus.ok()) {
ROCKS_LOG_FATAL(info_log_, "Invalid key (%s) in compaction",
key_.ToString(true).c_str());
}

4
db/compaction/compaction_job_test.cc
View File

@ -144,7 +144,7 @@ class CompactionJobTest : public testing::Test {
std::string skey;
std::string value;
std::tie(skey, value) = kv;
bool parsed = ParseInternalKey(skey, &key);
const Status pikStatus = ParseInternalKey(skey, &key);
smallest_seqno = std::min(smallest_seqno, key.sequence);
largest_seqno = std::max(largest_seqno, key.sequence);
@ -162,7 +162,7 @@ class CompactionJobTest : public testing::Test {
first_key = false;
if (parsed && key.type == kTypeBlobIndex) {
if (pikStatus.ok() && key.type == kTypeBlobIndex) {
BlobIndex blob_index;
const Status s = blob_index.DecodeFrom(value);
if (!s.ok()) {

6
db/db_compaction_filter_test.cc
View File

@ -314,7 +314,7 @@ TEST_F(DBTestCompactionFilter, CompactionFilter) {
ASSERT_OK(iter->status());
while (iter->Valid()) {
ParsedInternalKey ikey(Slice(), 0, kTypeValue);
ASSERT_EQ(ParseInternalKey(iter->key(), &ikey), true);
ASSERT_OK(ParseInternalKey(iter->key(), &ikey));
total++;
if (ikey.sequence != 0) {
count++;
@ -405,7 +405,7 @@ TEST_F(DBTestCompactionFilter, CompactionFilter) {
ASSERT_OK(iter->status());
while (iter->Valid()) {
ParsedInternalKey ikey(Slice(), 0, kTypeValue);
ASSERT_EQ(ParseInternalKey(iter->key(), &ikey), true);
ASSERT_OK(ParseInternalKey(iter->key(), &ikey));
ASSERT_NE(ikey.sequence, (unsigned)0);
count++;
iter->Next();
@ -624,7 +624,7 @@ TEST_F(DBTestCompactionFilter, CompactionFilterContextManual) {
ASSERT_OK(iter->status());
while (iter->Valid()) {
ParsedInternalKey ikey(Slice(), 0, kTypeValue);
ASSERT_EQ(ParseInternalKey(iter->key(), &ikey), true);
ASSERT_OK(ParseInternalKey(iter->key(), &ikey));
total++;
if (ikey.sequence != 0) {
count++;

2
db/db_iter.cc
View File

@ -107,7 +107,7 @@ Status DBIter::GetProperty(std::string prop_name, std::string* prop) {
}
bool DBIter::ParseKey(ParsedInternalKey* ikey) {
if (!ParseInternalKey(iter_.key(), ikey)) {
if (ParseInternalKey(iter_.key(), ikey) != Status::OK()) {
status_ = Status::Corruption("corrupted internal key in DBIter");
valid_ = false;
ROCKS_LOG_ERROR(logger_, "corrupted internal key in DBIter: %s",

7
db/db_iter_test.cc
View File

@ -99,9 +99,10 @@ class TestIterator : public InternalIterator {
}
for (auto it = data_.begin(); it != data_.end(); ++it) {
ParsedInternalKey ikey;
bool ok __attribute__((__unused__)) = ParseInternalKey(it->first, &ikey);
assert(ok);
if (ikey.user_key != _key) {
Status pikStatus = ParseInternalKey(it->first, &ikey);
pikStatus.PermitUncheckedError();
assert(pikStatus.ok());
if (!pikStatus.ok() || ikey.user_key != _key) {
continue;
}
if (valid_ && data_.begin() + iter_ > it) {

6
db/db_test_util.cc
View File

@ -962,7 +962,7 @@ std::string DBTestBase::AllEntriesFor(const Slice& user_key, int cf) {
bool first = true;
while (iter->Valid()) {
ParsedInternalKey ikey(Slice(), 0, kTypeValue);
if (!ParseInternalKey(iter->key(), &ikey)) {
if (ParseInternalKey(iter->key(), &ikey) != Status::OK()) {
result += "CORRUPTED";
} else {
if (!last_options_.comparator->Equal(ikey.user_key, user_key)) {
@ -1374,7 +1374,7 @@ void DBTestBase::validateNumberOfEntries(int numValues, int cf) {
while (iter->Valid()) {
ParsedInternalKey ikey;
ikey.clear();
ASSERT_EQ(ParseInternalKey(iter->key(), &ikey), true);
ASSERT_OK(ParseInternalKey(iter->key(), &ikey));
// checks sequence number for updates
ASSERT_EQ(ikey.sequence, (unsigned)seq--);
@ -1579,7 +1579,7 @@ void DBTestBase::VerifyDBInternal(
for (auto p : true_data) {
ASSERT_TRUE(iter->Valid());
ParsedInternalKey ikey;
ASSERT_TRUE(ParseInternalKey(iter->key(), &ikey));
ASSERT_OK(ParseInternalKey(iter->key(), &ikey));
ASSERT_EQ(p.first, ikey.user_key);
ASSERT_EQ(p.second, iter->value());
iter->Next();

4
db/db_with_timestamp_basic_test.cc
View File

@ -141,7 +141,7 @@ class DBBasicTestWithTimestampBase : public DBTestBase {
ukey_and_ts.assign(expected_ukey.data(), expected_ukey.size());
ukey_and_ts.append(expected_ts.data(), expected_ts.size());
ParsedInternalKey parsed_ikey;
ASSERT_TRUE(ParseInternalKey(it->key(), &parsed_ikey));
ASSERT_OK(ParseInternalKey(it->key(), &parsed_ikey));
ASSERT_EQ(ukey_and_ts, parsed_ikey.user_key);
ASSERT_EQ(expected_val_type, parsed_ikey.type);
ASSERT_EQ(expected_seq, parsed_ikey.sequence);
@ -161,7 +161,7 @@ class DBBasicTestWithTimestampBase : public DBTestBase {
ukey_and_ts.append(expected_ts.data(), expected_ts.size());
ParsedInternalKey parsed_ikey;
ASSERT_TRUE(ParseInternalKey(it->key(), &parsed_ikey));
ASSERT_OK(ParseInternalKey(it->key(), &parsed_ikey));
ASSERT_EQ(expected_val_type, parsed_ikey.type);
ASSERT_EQ(Slice(ukey_and_ts), parsed_ikey.user_key);
if (expected_val_type == kTypeValue) {

4
db/dbformat.cc
View File

@ -49,7 +49,7 @@ EntryType GetEntryType(ValueType value_type) {
bool ParseFullKey(const Slice& internal_key, FullKey* fkey) {
ParsedInternalKey ikey;
if (!ParseInternalKey(internal_key, &ikey)) {
if (ParseInternalKey(internal_key, &ikey) != Status::OK()) {
return false;
}
fkey->user_key = ikey.user_key;
@ -90,7 +90,7 @@ std::string ParsedInternalKey::DebugString(bool hex) const {
std::string InternalKey::DebugString(bool hex) const {
std::string result;
ParsedInternalKey parsed;
if (ParseInternalKey(rep_, &parsed)) {
if (ParseInternalKey(rep_, &parsed) == Status::OK()) {
result = parsed.DebugString(hex);
} else {
result = "(bad)";

22
db/dbformat.h
View File

@ -104,8 +104,9 @@ struct ParsedInternalKey {
ValueType type;
ParsedInternalKey()
: sequence(kMaxSequenceNumber) // Make code analyzer happy
{} // Intentionally left uninitialized (for speed)
: sequence(kMaxSequenceNumber),
type(kTypeDeletion) // Make code analyzer happy
{} // Intentionally left uninitialized (for speed)
// u contains timestamp if user timestamp feature is enabled.
ParsedInternalKey(const Slice& u, const SequenceNumber& seq, ValueType t)
: user_key(u), sequence(seq), type(t) {}
@ -162,8 +163,8 @@ extern void AppendInternalKeyFooter(std::string* result, SequenceNumber s,
// stores the parsed data in "*result", and returns true.
//
// On error, returns false, leaves "*result" in an undefined state.
extern bool ParseInternalKey(const Slice& internal_key,
ParsedInternalKey* result);
extern Status ParseInternalKey(const Slice& internal_key,
ParsedInternalKey* result);
// Returns the user key portion of an internal key.
inline Slice ExtractUserKey(const Slice& internal_key) {
@ -281,7 +282,8 @@ class InternalKey {
bool Valid() const {
ParsedInternalKey parsed;
return ParseInternalKey(Slice(rep_), &parsed);
return (ParseInternalKey(Slice(rep_), &parsed) == Status::OK()) ? true
: false;
}
void DecodeFrom(const Slice& s) { rep_.assign(s.data(), s.size()); }
@ -322,17 +324,19 @@ inline int InternalKeyComparator::Compare(const InternalKey& a,
return Compare(a.Encode(), b.Encode());
}
inline bool ParseInternalKey(const Slice& internal_key,
ParsedInternalKey* result) {
inline Status ParseInternalKey(const Slice& internal_key,
ParsedInternalKey* result) {
const size_t n = internal_key.size();
if (n < 8) return false;
if (n < 8) return Status::Corruption("Internal Key too small");
uint64_t num = DecodeFixed64(internal_key.data() + n - 8);
unsigned char c = num & 0xff;
result->sequence = num >> 8;
result->type = static_cast<ValueType>(c);
assert(result->type <= ValueType::kMaxValue);
result->user_key = Slice(internal_key.data(), n - 8);
return IsExtendedValueType(result->type);
return IsExtendedValueType(result->type)
? Status::OK()
: Status::Corruption("Invalid Key Type");
}
// Update the sequence number in the internal key.

6
db/dbformat_test.cc
View File

@ -41,12 +41,12 @@ static void TestKey(const std::string& key,
Slice in(encoded);
ParsedInternalKey decoded("", 0, kTypeValue);
ASSERT_TRUE(ParseInternalKey(in, &decoded));
ASSERT_OK(ParseInternalKey(in, &decoded));
ASSERT_EQ(key, decoded.user_key.ToString());
ASSERT_EQ(seq, decoded.sequence);
ASSERT_EQ(vt, decoded.type);
ASSERT_TRUE(!ParseInternalKey(Slice("bar"), &decoded));
ASSERT_NOK(ParseInternalKey(Slice("bar"), &decoded));
}
class FormatTest : public testing::Test {};
@ -186,7 +186,7 @@ TEST_F(FormatTest, UpdateInternalKey) {
Slice in(ikey);
ParsedInternalKey decoded;
ASSERT_TRUE(ParseInternalKey(in, &decoded));
ASSERT_OK(ParseInternalKey(in, &decoded));
ASSERT_EQ(user_key, decoded.user_key.ToString());
ASSERT_EQ(new_seq, decoded.sequence);
ASSERT_EQ(new_val_type, decoded.type);

6
db/external_sst_file_ingestion_job.cc
View File

@ -601,7 +601,7 @@ Status ExternalSstFileIngestionJob::GetIngestedFileInfo(
bool bounds_set = false;
iter->SeekToFirst();
if (iter->Valid()) {
if (!ParseInternalKey(iter->key(), &key)) {
if (ParseInternalKey(iter->key(), &key) != Status::OK()) {
return Status::Corruption("external file have corrupted keys");
}
if (key.sequence != 0) {
@ -610,7 +610,7 @@ Status ExternalSstFileIngestionJob::GetIngestedFileInfo(
file_to_ingest->smallest_internal_key.SetFrom(key);
iter->SeekToLast();
if (!ParseInternalKey(iter->key(), &key)) {
if (ParseInternalKey(iter->key(), &key) != Status::OK()) {
return Status::Corruption("external file have corrupted keys");
}
if (key.sequence != 0) {
@ -627,7 +627,7 @@ Status ExternalSstFileIngestionJob::GetIngestedFileInfo(
if (range_del_iter != nullptr) {
for (range_del_iter->SeekToFirst(); range_del_iter->Valid();
range_del_iter->Next()) {
if (!ParseInternalKey(range_del_iter->key(), &key)) {
if (ParseInternalKey(range_del_iter->key(), &key) != Status::OK()) {
return Status::Corruption("external file have corrupted keys");
}
RangeTombstone tombstone(key, range_del_iter->value());

4
db/import_column_family_job.cc
View File

@ -252,14 +252,14 @@ Status ImportColumnFamilyJob::GetIngestedFileInfo(
// Get first (smallest) key from file
iter->SeekToFirst();
if (!ParseInternalKey(iter->key(), &key)) {
if (ParseInternalKey(iter->key(), &key) != Status::OK()) {
return Status::Corruption("external file have corrupted keys");
}
file_to_import->smallest_internal_key.SetFrom(key);
// Get last (largest) key from file
iter->SeekToLast();
if (!ParseInternalKey(iter->key(), &key)) {
if (ParseInternalKey(iter->key(), &key) != Status::OK()) {
return Status::Corruption("external file have corrupted keys");
}
file_to_import->largest_internal_key.SetFrom(key);

17
db/merge_helper.cc
View File

@ -138,13 +138,10 @@ Status MergeHelper::MergeUntil(InternalIterator* iter,
// orig_ikey is backed by original_key if keys_.empty()
// orig_ikey is backed by keys_.back() if !keys_.empty()
ParsedInternalKey orig_ikey;
bool succ = ParseInternalKey(original_key, &orig_ikey);
assert(succ);
Status s;
if (!succ) {
s = Status::Corruption("Cannot parse key in MergeUntil");
return s;
}
Status s = ParseInternalKey(original_key, &orig_ikey);
assert(s.ok());
if (!s.ok()) return s;
bool hit_the_next_user_key = false;
for (; iter->Valid(); iter->Next(), original_key_is_iter = false) {
@ -156,7 +153,7 @@ Status MergeHelper::MergeUntil(InternalIterator* iter,
ParsedInternalKey ikey;
assert(keys_.size() == merge_context_.GetNumOperands());
if (!ParseInternalKey(iter->key(), &ikey)) {
if (ParseInternalKey(iter->key(), &ikey) != Status::OK()) {
// stop at corrupted key
if (assert_valid_internal_key_) {
assert(!"Corrupted internal key not expected.");
@ -271,7 +268,9 @@ Status MergeHelper::MergeUntil(InternalIterator* iter,
if (keys_.size() == 1) {
// we need to re-anchor the orig_ikey because it was anchored by
// original_key before
ParseInternalKey(keys_.back(), &orig_ikey);
Status pikStatus = ParseInternalKey(keys_.back(), &orig_ikey);
pikStatus.PermitUncheckedError();
assert(pikStatus.ok());
}
if (filter == CompactionFilter::Decision::kKeep) {
merge_context_.PushOperand(

15
db/range_del_aggregator.cc
View File

@ -33,17 +33,20 @@ TruncatedRangeDelIterator::TruncatedRangeDelIterator(
if (smallest != nullptr) {
pinned_bounds_.emplace_back();
auto& parsed_smallest = pinned_bounds_.back();
if (!ParseInternalKey(smallest->Encode(), &parsed_smallest)) {
assert(false);
}
Status pikStatus = ParseInternalKey(smallest->Encode(), &parsed_smallest);
pikStatus.PermitUncheckedError();
assert(pikStatus.ok());
smallest_ = &parsed_smallest;
}
if (largest != nullptr) {
pinned_bounds_.emplace_back();
auto& parsed_largest = pinned_bounds_.back();
if (!ParseInternalKey(largest->Encode(), &parsed_largest)) {
assert(false);
}
Status pikStatus = ParseInternalKey(largest->Encode(), &parsed_largest);
pikStatus.PermitUncheckedError();
assert(pikStatus.ok());
if (parsed_largest.type == kTypeRangeDeletion &&
parsed_largest.sequence == kMaxSequenceNumber) {
// The file boundary has been artificially extended by a range tombstone.

6
db/range_del_aggregator.h
View File

@ -283,9 +283,13 @@ class RangeDelAggregator {
bool ShouldDelete(const Slice& key, RangeDelPositioningMode mode) {
ParsedInternalKey parsed;
if (!ParseInternalKey(key, &parsed)) {
Status pikStatus = ParseInternalKey(key, &parsed);
assert(pikStatus.ok());
if (!pikStatus.ok()) {
return false;
}
return ShouldDelete(parsed, mode);
}
virtual bool ShouldDelete(const ParsedInternalKey& parsed,

2
db/repair.cc
View File

@ -554,7 +554,7 @@ class Repairer {
ParsedInternalKey parsed;
for (iter->SeekToFirst(); iter->Valid(); iter->Next()) {
Slice key = iter->key();
if (!ParseInternalKey(key, &parsed)) {
if (ParseInternalKey(key, &parsed) != Status::OK()) {
ROCKS_LOG_ERROR(db_options_.info_log,
"Table #%" PRIu64 ": unparsable key %s",
t->meta.fd.GetNumber(), EscapeString(key).c_str());

5
db/table_properties_collector.cc
View File

@ -33,8 +33,9 @@ Status UserKeyTablePropertiesCollector::InternalAdd(const Slice& key,
const Slice& value,
uint64_t file_size) {
ParsedInternalKey ikey;
if (!ParseInternalKey(key, &ikey)) {
return Status::InvalidArgument("Invalid internal key");
Status s = ParseInternalKey(key, &ikey);
if (s != Status::OK()) {
return s;
}
return collector_->AddUserKey(ikey.user_key, value, GetEntryType(ikey.type),

5
db/version_set.cc
View File

@ -92,9 +92,8 @@ Status OverlapWithIterator(const Comparator* ucmp,
*overlap = false;
if (iter->Valid()) {
ParsedInternalKey seek_result;
if (!ParseInternalKey(iter->key(), &seek_result)) {
return Status::Corruption("DB have corrupted keys");
}
Status s = ParseInternalKey(iter->key(), &seek_result);
if (!s.ok()) return s;
if (ucmp->CompareWithoutTimestamp(seek_result.user_key, largest_user_key) <=
0) {

2
db/write_batch_test.cc
View File

@ -62,7 +62,7 @@ static std::string PrintContents(WriteBatch* b) {
for (iter->SeekToFirst(); iter->Valid(); iter->Next()) {
ParsedInternalKey ikey;
ikey.clear();
EXPECT_TRUE(ParseInternalKey(iter->key(), &ikey));
EXPECT_OK(ParseInternalKey(iter->key(), &ikey));
switch (ikey.type) {
case kTypeValue:
state.append("Put(");

8
java/rocksjni/write_batch_test.cc
View File

@ -63,10 +63,10 @@ jbyteArray Java_org_rocksdb_WriteBatchTest_getContents(JNIEnv* env,
for (iter->SeekToFirst(); iter->Valid(); iter->Next()) {
ROCKSDB_NAMESPACE::ParsedInternalKey ikey;
ikey.clear();
bool parsed = ROCKSDB_NAMESPACE::ParseInternalKey(iter->key(), &ikey);
if (!parsed) {
assert(parsed);
}
ROCKSDB_NAMESPACE::Status pikStatus =
ROCKSDB_NAMESPACE::ParseInternalKey(iter->key(), &ikey);
pikStatus.PermitUncheckedError();
assert(pikStatus.ok());
switch (ikey.type) {
case ROCKSDB_NAMESPACE::kTypeValue:
state.append("Put(");

4
table/block_based/block_based_table_reader.cc
View File

@ -2321,7 +2321,7 @@ Status BlockBasedTable::Get(const ReadOptions& read_options, const Slice& key,
// Call the *saver function on each entry/block until it returns false
for (; biter.Valid(); biter.Next()) {
ParsedInternalKey parsed_key;
if (!ParseInternalKey(biter.key(), &parsed_key)) {
if (ParseInternalKey(biter.key(), &parsed_key) != Status::OK()) {
s = Status::Corruption(Slice());
}
@ -2638,7 +2638,7 @@ void BlockBasedTable::MultiGet(const ReadOptions& read_options,
ParsedInternalKey parsed_key;
Cleanable dummy;
Cleanable* value_pinner = nullptr;
if (!ParseInternalKey(biter->key(), &parsed_key)) {
if (ParseInternalKey(biter->key(), &parsed_key) != Status::OK()) {
s = Status::Corruption(Slice());
}
if (biter->IsValuePinned()) {

2
table/cuckoo/cuckoo_table_builder.cc
View File

@ -90,7 +90,7 @@ void CuckooTableBuilder::Add(const Slice& key, const Slice& value) {
return;
}
ParsedInternalKey ikey;
if (!ParseInternalKey(key, &ikey)) {
if (ParseInternalKey(key, &ikey) != Status::OK()) {
status_ = Status::Corruption("Unable to parse key into inernal key.");
return;
}

3
table/cuckoo/cuckoo_table_builder_test.cc
View File

@ -47,7 +47,8 @@ class CuckooBuilderTest : public testing::Test {
uint64_t num_deletions = 0;
for (const auto& key : keys) {
ParsedInternalKey parsed;
if (ParseInternalKey(key, &parsed) && parsed.type == kTypeDeletion) {
if (ParseInternalKey(key, &parsed) == Status::OK() &&
parsed.type == kTypeDeletion) {
num_deletions++;
}
}

3
table/cuckoo/cuckoo_table_reader.cc
View File

@ -172,7 +172,8 @@ Status CuckooTableReader::Get(const ReadOptions& /*readOptions*/,
} else {
Slice full_key(bucket, key_length_);
ParsedInternalKey found_ikey;
ParseInternalKey(full_key, &found_ikey);
Status s = ParseInternalKey(full_key, &found_ikey);
if (!s.ok()) return s;
bool dont_care __attribute__((__unused__));
get_context->SaveValue(found_ikey, value, &dont_care);
}

4
table/mock_table.cc
View File

@ -189,7 +189,7 @@ Status MockTableReader::Get(const ReadOptions&, const Slice& key,
std::unique_ptr<MockTableIterator> iter(new MockTableIterator(table_));
for (iter->Seek(key); iter->Valid(); iter->Next()) {
ParsedInternalKey parsed_key;
if (!ParseInternalKey(iter->key(), &parsed_key)) {
if (ParseInternalKey(iter->key(), &parsed_key) != Status::OK()) {
return Status::Corruption(Slice());
}
@ -287,7 +287,7 @@ void MockTableFactory::AssertLatestFile(
ParsedInternalKey ikey;
std::string key, value;
std::tie(key, value) = kv;
ParseInternalKey(Slice(key), &ikey);
ASSERT_OK(ParseInternalKey(Slice(key), &ikey));
std::cout << ikey.DebugString(false) << " -> " << value << std::endl;
}
FAIL();

2
table/plain/plain_table_builder.cc
View File

@ -128,7 +128,7 @@ void PlainTableBuilder::Add(const Slice& key, const Slice& value) {
size_t meta_bytes_buf_size = 0;
ParsedInternalKey internal_key;
if (!ParseInternalKey(key, &internal_key)) {
if (ParseInternalKey(key, &internal_key) != Status::OK()) {
assert(false);
return;
}

4
table/plain/plain_table_key_coding.cc
View File

@ -85,7 +85,7 @@ IOStatus PlainTableKeyEncoder::AppendKey(const Slice& key,
uint64_t* offset, char* meta_bytes_buf,
size_t* meta_bytes_buf_size) {
ParsedInternalKey parsed_key;
if (!ParseInternalKey(key, &parsed_key)) {
if (ParseInternalKey(key, &parsed_key) != Status::OK()) {
return IOStatus::Corruption(Slice());
}
@ -279,7 +279,7 @@ Status PlainTableKeyDecoder::ReadInternalKey(
return file_reader_.status();
}
*internal_key_valid = true;
if (!ParseInternalKey(*internal_key, parsed_key)) {
if (ParseInternalKey(*internal_key, parsed_key) != Status::OK()) {
return Status::Corruption(
Slice("Incorrect value type found when reading the next key"));
}

15
table/plain/plain_table_reader.cc
View File

@ -457,16 +457,15 @@ Status PlainTableReader::GetOffset(PlainTableKeyDecoder* decoder,
uint32_t high = upper_bound;
ParsedInternalKey mid_key;
ParsedInternalKey parsed_target;
if (!ParseInternalKey(target, &parsed_target)) {
return Status::Corruption(Slice());
}
Status s = ParseInternalKey(target, &parsed_target);
if (!s.ok()) return s;
// The key is between [low, high). Do a binary search between it.
while (high - low > 1) {
uint32_t mid = (high + low) / 2;
uint32_t file_offset = GetFixed32Element(base_ptr, mid);
uint32_t tmp;
Status s = decoder->NextKeyNoValue(file_offset, &mid_key, nullptr, &tmp);
s = decoder->NextKeyNoValue(file_offset, &mid_key, nullptr, &tmp);
if (!s.ok()) {
return s;
}
@ -491,7 +490,7 @@ Status PlainTableReader::GetOffset(PlainTableKeyDecoder* decoder,
ParsedInternalKey low_key;
uint32_t tmp;
uint32_t low_key_offset = GetFixed32Element(base_ptr, low);
Status s = decoder->NextKeyNoValue(low_key_offset, &low_key, nullptr, &tmp);
s = decoder->NextKeyNoValue(low_key_offset, &low_key, nullptr, &tmp);
if (!s.ok()) {
return s;
}
@ -594,9 +593,9 @@ Status PlainTableReader::Get(const ReadOptions& /*ro*/, const Slice& target,
}
ParsedInternalKey found_key;
ParsedInternalKey parsed_target;
if (!ParseInternalKey(target, &parsed_target)) {
return Status::Corruption(Slice());
}
s = ParseInternalKey(target, &parsed_target);
if (!s.ok()) return Status::Corruption(Slice());
Slice found_value;
while (offset < file_info_.data_end_offset) {
s = Next(&decoder, &offset, &found_key, nullptr, &found_value);

2
table/sst_file_dumper.cc
View File

@ -441,7 +441,7 @@ Status SstFileDumper::ReadSequential(bool print_kv, uint64_t read_num,
if (read_num > 0 && i > read_num) break;
ParsedInternalKey ikey;
if (!ParseInternalKey(key, &ikey)) {
if (ParseInternalKey(key, &ikey) != Status::OK()) {
std::cerr << "Internal Key [" << key.ToString(true /* in hex*/)
<< "] parse error!\n";
continue;

14
table/table_test.cc
View File

@ -248,7 +248,7 @@ class KeyConvertingIterator : public InternalIterator {
Slice key() const override {
assert(Valid());
ParsedInternalKey parsed_key;
if (!ParseInternalKey(iter_->key(), &parsed_key)) {
if (ParseInternalKey(iter_->key(), &parsed_key) != Status::OK()) {
status_ = Status::Corruption("malformed internal key");
return Slice("corrupted key");
}
@ -1552,7 +1552,7 @@ TEST_P(BlockBasedTableTest, RangeDelBlock) {
for (size_t i = 0; i < expected_tombstones.size(); i++) {
ASSERT_TRUE(iter->Valid());
ParsedInternalKey parsed_key;
ASSERT_TRUE(ParseInternalKey(iter->key(), &parsed_key));
ASSERT_OK(ParseInternalKey(iter->key(), &parsed_key));
RangeTombstone t(parsed_key, iter->value());
const auto& expected_t = expected_tombstones[i];
ASSERT_EQ(t.start_key_, expected_t.start_key_);
@ -4102,7 +4102,7 @@ TEST_P(BlockBasedTableTest, DISABLED_TableWithGlobalSeqno) {
char current_c = 'a';
for (iter->SeekToFirst(); iter->Valid(); iter->Next()) {
ParsedInternalKey pik;
ASSERT_TRUE(ParseInternalKey(iter->key(), &pik));
ASSERT_OK(ParseInternalKey(iter->key(), &pik));
ASSERT_EQ(pik.type, ValueType::kTypeValue);
ASSERT_EQ(pik.sequence, 0);
@ -4123,7 +4123,7 @@ TEST_P(BlockBasedTableTest, DISABLED_TableWithGlobalSeqno) {
current_c = 'a';
for (iter->SeekToFirst(); iter->Valid(); iter->Next()) {
ParsedInternalKey pik;
ASSERT_TRUE(ParseInternalKey(iter->key(), &pik));
ASSERT_OK(ParseInternalKey(iter->key(), &pik));
ASSERT_EQ(pik.type, ValueType::kTypeValue);
ASSERT_EQ(pik.sequence, 10);
@ -4141,7 +4141,7 @@ TEST_P(BlockBasedTableTest, DISABLED_TableWithGlobalSeqno) {
ASSERT_TRUE(iter->Valid());
ParsedInternalKey pik;
ASSERT_TRUE(ParseInternalKey(iter->key(), &pik));
ASSERT_OK(ParseInternalKey(iter->key(), &pik));
ASSERT_EQ(pik.type, ValueType::kTypeValue);
ASSERT_EQ(pik.sequence, 10);
@ -4160,7 +4160,7 @@ TEST_P(BlockBasedTableTest, DISABLED_TableWithGlobalSeqno) {
current_c = 'a';
for (iter->SeekToFirst(); iter->Valid(); iter->Next()) {
ParsedInternalKey pik;
ASSERT_TRUE(ParseInternalKey(iter->key(), &pik));
ASSERT_OK(ParseInternalKey(iter->key(), &pik));
ASSERT_EQ(pik.type, ValueType::kTypeValue);
ASSERT_EQ(pik.sequence, 3);
@ -4179,7 +4179,7 @@ TEST_P(BlockBasedTableTest, DISABLED_TableWithGlobalSeqno) {
ASSERT_TRUE(iter->Valid());
ParsedInternalKey pik;
ASSERT_TRUE(ParseInternalKey(iter->key(), &pik));
ASSERT_OK(ParseInternalKey(iter->key(), &pik));
ASSERT_EQ(pik.type, ValueType::kTypeValue);
ASSERT_EQ(pik.sequence, 3);

3
tools/block_cache_analyzer/block_cache_trace_analyzer.h
View File

@ -103,7 +103,8 @@ struct BlockAccessInfo {
num_referenced_key_exist_in_block++;
if (referenced_data_size > block_size && block_size != 0) {
ParsedInternalKey internal_key;
ParseInternalKey(access.referenced_key, &internal_key);
Status s = ParseInternalKey(access.referenced_key, &internal_key);
assert(s.ok()); // TODO
}
} else {
non_exist_key_num_access_map[access.referenced_key][access.caller]++;

2
tools/sst_dump_tool.cc
View File

@ -238,7 +238,7 @@ int SSTDumpTool::Run(int argc, char const* const* argv, Options options) {
Slice sl_key = ROCKSDB_NAMESPACE::Slice(in_key);
ParsedInternalKey ikey;
int retc = 0;
if (!ParseInternalKey(sl_key, &ikey)) {
if (ParseInternalKey(sl_key, &ikey) != Status::OK()) {
std::cerr << "Internal Key [" << sl_key.ToString(true /* in hex*/)
<< "] parse error!\n";
retc = -1;

4
utilities/blob_db/blob_compaction_filter.cc
View File

@ -66,7 +66,7 @@ CompactionFilter::Decision BlobIndexCompactionFilterBase::FilterV2(
// Hack: Internal key is passed to BlobIndexCompactionFilter for it to
// get sequence number.
ParsedInternalKey ikey;
if (!ParseInternalKey(key, &ikey)) {
if (ParseInternalKey(key, &ikey) != Status::OK()) {
assert(false);
return Decision::kKeep;
}
@ -83,7 +83,7 @@ CompactionFilter::Decision BlobIndexCompactionFilterBase::FilterV2(
// Hack: Internal key is passed to BlobIndexCompactionFilter for it to
// get sequence number.
ParsedInternalKey ikey;
if (!ParseInternalKey(key, &ikey)) {
if (ParseInternalKey(key, &ikey) != Status::OK()) {
assert(false);
return Decision::kKeep;
}

2
utilities/debug.cc
View File

@ -55,7 +55,7 @@ Status GetAllKeyVersions(DB* db, ColumnFamilyHandle* cfh, Slice begin_key,
size_t num_keys = 0;
for (; iter->Valid(); iter->Next()) {
ParsedInternalKey ikey;
if (!ParseInternalKey(iter->key(), &ikey)) {
if (ParseInternalKey(iter->key(), &ikey) != Status::OK()) {
return Status::Corruption("Internal Key [" + iter->key().ToString() +
"] parse error!");
}