diff --git a/db/column_family.cc b/db/column_family.cc index 0534d563a..d0a16dd48 100644 --- a/db/column_family.cc +++ b/db/column_family.cc @@ -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"); } } diff --git a/db/compaction/compaction_iterator.cc b/db/compaction/compaction_iterator.cc index 9655d8a99..4555ec568 100644 --- a/db/compaction/compaction_iterator.cc +++ b/db/compaction/compaction_iterator.cc @@ -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()); } diff --git a/db/compaction/compaction_job_test.cc b/db/compaction/compaction_job_test.cc index cee43554a..a94e3a2b8 100644 --- a/db/compaction/compaction_job_test.cc +++ b/db/compaction/compaction_job_test.cc @@ -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()) { diff --git a/db/db_compaction_filter_test.cc b/db/db_compaction_filter_test.cc index 538dc3ade..fb9186cae 100644 --- a/db/db_compaction_filter_test.cc +++ b/db/db_compaction_filter_test.cc @@ -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++; diff --git a/db/db_iter.cc b/db/db_iter.cc index dc8f7f0f6..66937cc38 100644 --- a/db/db_iter.cc +++ b/db/db_iter.cc @@ -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", diff --git a/db/db_iter_test.cc b/db/db_iter_test.cc index ddbea8d17..1c9680da2 100644 --- a/db/db_iter_test.cc +++ b/db/db_iter_test.cc @@ -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) { diff --git a/db/db_test_util.cc b/db/db_test_util.cc index 611892d21..2b0ae2d50 100644 --- a/db/db_test_util.cc +++ b/db/db_test_util.cc @@ -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(); diff --git a/db/db_with_timestamp_basic_test.cc b/db/db_with_timestamp_basic_test.cc index 8d71a3d91..a0984bc2f 100644 --- a/db/db_with_timestamp_basic_test.cc +++ b/db/db_with_timestamp_basic_test.cc @@ -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) { diff --git a/db/dbformat.cc b/db/dbformat.cc index 40e9917ec..ada35f1fb 100644 --- a/db/dbformat.cc +++ b/db/dbformat.cc @@ -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)"; diff --git a/db/dbformat.h b/db/dbformat.h index 6911d4297..158d9c5dd 100644 --- a/db/dbformat.h +++ b/db/dbformat.h @@ -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(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. diff --git a/db/dbformat_test.cc b/db/dbformat_test.cc index a2c67795a..09ee4a38b 100644 --- a/db/dbformat_test.cc +++ b/db/dbformat_test.cc @@ -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); diff --git a/db/external_sst_file_ingestion_job.cc b/db/external_sst_file_ingestion_job.cc index 21601f617..42f246150 100644 --- a/db/external_sst_file_ingestion_job.cc +++ b/db/external_sst_file_ingestion_job.cc @@ -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()); diff --git a/db/import_column_family_job.cc b/db/import_column_family_job.cc index 6dcfc8874..9e9073483 100644 --- a/db/import_column_family_job.cc +++ b/db/import_column_family_job.cc @@ -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); diff --git a/db/merge_helper.cc b/db/merge_helper.cc index c87d525cb..61ff22506 100644 --- a/db/merge_helper.cc +++ b/db/merge_helper.cc @@ -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( diff --git a/db/range_del_aggregator.cc b/db/range_del_aggregator.cc index 1f6a7b139..20616c22e 100644 --- a/db/range_del_aggregator.cc +++ b/db/range_del_aggregator.cc @@ -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. diff --git a/db/range_del_aggregator.h b/db/range_del_aggregator.h index b47cf31d3..d5d79d5a8 100644 --- a/db/range_del_aggregator.h +++ b/db/range_del_aggregator.h @@ -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, diff --git a/db/repair.cc b/db/repair.cc index 8b50ba5c3..671c105a1 100644 --- a/db/repair.cc +++ b/db/repair.cc @@ -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()); diff --git a/db/table_properties_collector.cc b/db/table_properties_collector.cc index d98ff5e9b..b43afdc5c 100644 --- a/db/table_properties_collector.cc +++ b/db/table_properties_collector.cc @@ -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), diff --git a/db/version_set.cc b/db/version_set.cc index e94f5870d..4eef86cb7 100644 --- a/db/version_set.cc +++ b/db/version_set.cc @@ -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) { diff --git a/db/write_batch_test.cc b/db/write_batch_test.cc index 007fb66db..5a210b3d6 100644 --- a/db/write_batch_test.cc +++ b/db/write_batch_test.cc @@ -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("); diff --git a/java/rocksjni/write_batch_test.cc b/java/rocksjni/write_batch_test.cc index 114eac42c..c517afcc1 100644 --- a/java/rocksjni/write_batch_test.cc +++ b/java/rocksjni/write_batch_test.cc @@ -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("); diff --git a/table/block_based/block_based_table_reader.cc b/table/block_based/block_based_table_reader.cc index e12c32813..dc4f6139c 100644 --- a/table/block_based/block_based_table_reader.cc +++ b/table/block_based/block_based_table_reader.cc @@ -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()) { diff --git a/table/cuckoo/cuckoo_table_builder.cc b/table/cuckoo/cuckoo_table_builder.cc index 1e63bf85d..f42e87bdf 100644 --- a/table/cuckoo/cuckoo_table_builder.cc +++ b/table/cuckoo/cuckoo_table_builder.cc @@ -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; } diff --git a/table/cuckoo/cuckoo_table_builder_test.cc b/table/cuckoo/cuckoo_table_builder_test.cc index fd6aaef1d..322dbf0e4 100644 --- a/table/cuckoo/cuckoo_table_builder_test.cc +++ b/table/cuckoo/cuckoo_table_builder_test.cc @@ -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++; } } diff --git a/table/cuckoo/cuckoo_table_reader.cc b/table/cuckoo/cuckoo_table_reader.cc index 66b77249a..275649ea8 100644 --- a/table/cuckoo/cuckoo_table_reader.cc +++ b/table/cuckoo/cuckoo_table_reader.cc @@ -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); } diff --git a/table/mock_table.cc b/table/mock_table.cc index eb198a44b..fc867fa97 100644 --- a/table/mock_table.cc +++ b/table/mock_table.cc @@ -189,7 +189,7 @@ Status MockTableReader::Get(const ReadOptions&, const Slice& key, std::unique_ptr 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(); diff --git a/table/plain/plain_table_builder.cc b/table/plain/plain_table_builder.cc index e3ef67da6..faebcfe2f 100644 --- a/table/plain/plain_table_builder.cc +++ b/table/plain/plain_table_builder.cc @@ -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; } diff --git a/table/plain/plain_table_key_coding.cc b/table/plain/plain_table_key_coding.cc index 0f3b37fd1..39feb8dd0 100644 --- a/table/plain/plain_table_key_coding.cc +++ b/table/plain/plain_table_key_coding.cc @@ -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")); } diff --git a/table/plain/plain_table_reader.cc b/table/plain/plain_table_reader.cc index 0b734308b..e08174948 100644 --- a/table/plain/plain_table_reader.cc +++ b/table/plain/plain_table_reader.cc @@ -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); diff --git a/table/sst_file_dumper.cc b/table/sst_file_dumper.cc index af96154c4..cff9dd970 100644 --- a/table/sst_file_dumper.cc +++ b/table/sst_file_dumper.cc @@ -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; diff --git a/table/table_test.cc b/table/table_test.cc index 80dba1271..f736f3458 100644 --- a/table/table_test.cc +++ b/table/table_test.cc @@ -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); diff --git a/tools/block_cache_analyzer/block_cache_trace_analyzer.h b/tools/block_cache_analyzer/block_cache_trace_analyzer.h index 48a544813..74fc22b10 100644 --- a/tools/block_cache_analyzer/block_cache_trace_analyzer.h +++ b/tools/block_cache_analyzer/block_cache_trace_analyzer.h @@ -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]++; diff --git a/tools/sst_dump_tool.cc b/tools/sst_dump_tool.cc index ac1a5e36a..a703b23c3 100644 --- a/tools/sst_dump_tool.cc +++ b/tools/sst_dump_tool.cc @@ -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; diff --git a/utilities/blob_db/blob_compaction_filter.cc b/utilities/blob_db/blob_compaction_filter.cc index 7fe43d114..c86ae7c88 100644 --- a/utilities/blob_db/blob_compaction_filter.cc +++ b/utilities/blob_db/blob_compaction_filter.cc @@ -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; } diff --git a/utilities/debug.cc b/utilities/debug.cc index 12fab0f08..7521213cc 100644 --- a/utilities/debug.cc +++ b/utilities/debug.cc @@ -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!"); }