Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: Some commands of Set do not have status return values from RocksDB #2185

Closed
Mixficsol opened this issue Dec 7, 2023 · 1 comment
Closed
Labels

Comments

@Mixficsol
Copy link
Collaborator

Mixficsol commented Dec 7, 2023

Is this a regression?

Yes

Description

pika/src/storage/src/redis_sets.cc 下,有一些命令没有对 RocksDB 的状态返回值做判断,包括 SRemCmd, SUnionCmd, SInterCmd, SDiffCmd,以SRem命令举例,这里的 s_ 是 RocksDB 的状态返回值,我们应该对这个进行判断。

void SRemCmd::Do(std::shared_ptr<Slot> slot) {
  s_ = slot->db()->SRem(key_, members_, &deleted_);
  res_.AppendInteger(deleted_);
}

void SUnionCmd::Do(std::shared_ptr<Slot> slot) {
  std::vector<std::string> members;
  slot->db()->SUnion(keys_, &members);
  res_.AppendArrayLenUint64(members.size());
  for (const auto& member : members) {
    res_.AppendStringLenUint64(member.size());
    res_.AppendContent(member);
  }
}

void SInterCmd::Do(std::shared_ptr<Slot> slot) {
  std::vector<std::string> members;
  slot->db()->SInter(keys_, &members);
  res_.AppendArrayLenUint64(members.size());
  for (const auto& member : members) {
    res_.AppendStringLenUint64(member.size());
    res_.AppendContent(member);
  }
}

void SDiffCmd::Do(std::shared_ptr<Slot> slot) {
  std::vector<std::string> members;
  slot->db()->SDiff(keys_, &members);
  res_.AppendArrayLenUint64(members.size());
  for (const auto& member : members) {
    res_.AppendStringLenUint64(member.size());
    res_.AppendContent(member);
  }
}

可以参照在 pika/src/storage/src/redis_zets.cc 下的 ZRem命令代码,这里对 s_ 的返回值进行了判断

void ZRemCmd::Do(std::shared_ptr<Slot> slot) {
  int32_t count = 0;
  s_ = slot->db()->ZRem(key_, members_, &count);
  if (s.ok() || s.IsNotFound()) {
    AddSlotKey("z", key_, slot);
    res_.AppendInteger(count);
  } else {
    res_.SetRes(CmdRes::kErrOther, s.ToString());
  }
}

至于状态值怎么判断,我们可以查看一下 stroage 的代码,pika/src/storage/src/redis_sets.cc下,我们以SRem命令举例

rocksdb::Status RedisSets::SRem(const Slice& key, const std::vector<std::string>& members, int32_t* ret) {
  *ret = 0;
  rocksdb::WriteBatch batch;
  ScopeRecordLock l(lock_mgr_, key);

  int32_t version = 0;
  uint32_t statistic = 0;
  std::string meta_value;
  rocksdb::Status s = db_->Get(default_read_options_, handles_[0], key, &meta_value);
  if (s.ok()) {
    ParsedSetsMetaValue parsed_sets_meta_value(&meta_value);
    if (parsed_sets_meta_value.IsStale()) {
      return rocksdb::Status::NotFound("stale");
    } else if (parsed_sets_meta_value.count() == 0) {
      return rocksdb::Status::NotFound();
    } else {
      int32_t cnt = 0;
      std::string member_value;
      version = parsed_sets_meta_value.version();
      for (const auto& member : members) {
        SetsMemberKey sets_member_key(key, version, member);
        s = db_->Get(default_read_options_, handles_[1], sets_member_key.Encode(), &member_value);
        if (s.ok()) {
          cnt++;
          statistic++;
          batch.Delete(handles_[1], sets_member_key.Encode());
        } else if (s.IsNotFound()) {
        } else {
          return s;
        }
      }
      *ret = cnt;
      if (!parsed_sets_meta_value.CheckModifyCount(-cnt)){
        return Status::InvalidArgument("set size overflow");
      }
      parsed_sets_meta_value.ModifyCount(-cnt);
      batch.Put(handles_[0], key, meta_value);
    }
  } else if (s.IsNotFound()) {
    *ret = 0;
    return rocksdb::Status::NotFound();
  } else {
    return s; // 对于返回状态不OK的值我们应该及时打印出报错
  }
  s = db_->Write(default_write_options_, &batch);
  UpdateSpecificKeyStatistics(key.ToString(), statistic);
  return s;
}
@Mixficsol Mixficsol changed the title Set的部分命令没有RocksDB的状态返回值 Set 的部分命令没有 RocksDB 的状态返回值 Dec 7, 2023
@AlexStocks AlexStocks changed the title Set 的部分命令没有 RocksDB 的状态返回值 bug: Some commands of Set do not have status return values from RocksDB Dec 7, 2023
@callme-taota
Copy link
Contributor

我来我来

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants