Check iterator status after quit loop

[han-ian]

Search before asking

• I had searched in the issues and found no similar issues.

Motivation

From rocksdb doc, need handle error when iterating. Like bellow:

for (it->Seek("hello"); it->Valid(); it->Next()) {
  // Do something with it->key() and it->value().
}
if (!it->status().ok()) {
  // Handle error. it->status().ToString() contains error message.
}

However, the code in kvrocks does not do this check.
For instance:
https://github.com/apache/kvrocks/blob/unstable/src/types/redis_hash.cc#L344

rocksdb::Status Hash::GetAll(const Slice &user_key, std::vector<FieldValue> *field_values, HashFetchType type) {
  field_values->clear();

  std::string ns_key = AppendNamespacePrefix(user_key);
  HashMetadata metadata(false);
  LatestSnapShot ss(storage_);
  rocksdb::Status s = GetMetadata(GetOptions{.snapshot = ss.GetSnapShot()}, ns_key, &metadata);
  if (!s.ok()) return s.IsNotFound() ? rocksdb::Status::OK() : s;

  std::string prefix_key = InternalKey(ns_key, "", metadata.version, storage_->IsSlotIdEncoded()).Encode();
  std::string next_version_prefix_key =
      InternalKey(ns_key, "", metadata.version + 1, storage_->IsSlotIdEncoded()).Encode();

  rocksdb::ReadOptions read_options = storage_->DefaultScanOptions();
  read_options.snapshot = ss.GetSnapShot();
  rocksdb::Slice upper_bound(next_version_prefix_key);
  read_options.iterate_upper_bound = &upper_bound;

  auto iter = util::UniqueIterator(storage_, read_options);
  for (iter->Seek(prefix_key); iter->Valid() && iter->key().starts_with(prefix_key); iter->Next()) {
    if (type == HashFetchType::kOnlyKey) {
      InternalKey ikey(iter->key(), storage_->IsSlotIdEncoded());
      field_values->emplace_back(ikey.GetSubKey().ToString(), "");
    } else if (type == HashFetchType::kOnlyValue) {
      field_values->emplace_back("", iter->value().ToString());
    } else {
      InternalKey ikey(iter->key(), storage_->IsSlotIdEncoded());
      field_values->emplace_back(ikey.GetSubKey().ToString(), iter->value().ToString());
    }
  }
  return rocksdb::Status::OK();
}

Solution

check iterator status.

Are you willing to submit a PR?

• I'm willing to submit a PR!

[git-hulk]

@in-han Thanks for your feedback, would you like to submit a PR to improve this？

[han-ian]

@git-hulk Yes, I will do this next week.

@in-han Thanks for your feedback, would you like to submit a PR to improve this？

Yeah, I will do it next week.

[git-hulk]

@han-ian Thank you!

[LindaSummer]

Hi @git-hulk ,

This issue seems to be inactivated for 3 months.
Can this task be assigned to be?

Best Regards,
Edward

[git-hulk]

@LindaSummer Surely yes, I have assigned it to you, thank you!