Skip to content

Commit

Permalink
fix for review
Browse files Browse the repository at this point in the history
  • Loading branch information
@jjz921024
jjz921024 committed Jul 17, 2024
1 parent 4b141d2 commit cb1ff54
Show file tree
Hide file tree
Showing 8 changed files with 224 additions and 98 deletions.
80 changes: 35 additions & 45 deletions src/commands/cmd_hash.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,7 @@ class CommandFieldExpireBase : public Commander {
}

if (args.size() != idx + fields_num) {
return { Status::RedisParseErr, errWrongNumOfArguments };
return {Status::RedisParseErr, errWrongNumOfArguments};
}

for (size_t i = idx; i < args.size(); i++) {
Expand All @@ -479,7 +479,7 @@ class CommandFieldExpireBase : public Commander {
}

*output = redis::MultiLen(ret.size());
for (const auto i : ret) {
for (const auto &i : ret) {
output->append(redis::Integer(i));
}

Expand All @@ -504,8 +504,8 @@ class CommandHExpire : public CommandFieldExpireBase {
public:
Status Parse(const std::vector<std::string> &args) override {
auto parse_result = ParseInt<uint64_t>(args[2], 10);
if (!parse_result) return { Status::RedisParseErr, errValueNotInteger };
if (!parse_result) return {Status::RedisParseErr, errValueNotInteger};

expire_ = *parse_result * 1000 + util::GetTimeStampMS();
return CommandFieldExpireBase::commonParse(args, 3);
}
Expand All @@ -519,12 +519,12 @@ class CommandHExpireAt : public CommandFieldExpireBase {
public:
Status Parse(const std::vector<std::string> &args) override {
auto parse_result = ParseInt<uint64_t>(args[2], 10);
if (!parse_result) return { Status::RedisParseErr, errValueNotInteger };
if (!parse_result) return {Status::RedisParseErr, errValueNotInteger};

expire_ = *parse_result * 1000;
return CommandFieldExpireBase::commonParse(args, 3);
}

Status Execute(Server *srv, Connection *conn, std::string *output) override {
return expireFieldExecute(srv, conn, output, false);
}
Expand All @@ -534,8 +534,8 @@ class CommandHPExpire : public CommandFieldExpireBase {
public:
Status Parse(const std::vector<std::string> &args) override {
auto parse_result = ParseInt<uint64_t>(args[2], 10);
if (!parse_result) return { Status::RedisParseErr, errValueNotInteger };
if (!parse_result) return {Status::RedisParseErr, errValueNotInteger};

expire_ = *parse_result + util::GetTimeStampMS();
return CommandFieldExpireBase::commonParse(args, 3);
}
Expand All @@ -549,8 +549,8 @@ class CommandHPExpireAt : public CommandFieldExpireBase {
public:
Status Parse(const std::vector<std::string> &args) override {
auto parse_result = ParseInt<uint64_t>(args[2], 10);
if (!parse_result) return { Status::RedisParseErr, errValueNotInteger };
if (!parse_result) return {Status::RedisParseErr, errValueNotInteger};

expire_ = *parse_result;
return CommandFieldExpireBase::commonParse(args, 3);
}
Expand All @@ -562,9 +562,7 @@ class CommandHPExpireAt : public CommandFieldExpireBase {

class CommandHExpireTime : public CommandFieldExpireBase {
public:
Status Parse(const std::vector<std::string> &args) override {
return CommandFieldExpireBase::commonParse(args, 2);
}
Status Parse(const std::vector<std::string> &args) override { return CommandFieldExpireBase::commonParse(args, 2); }

Status Execute(Server *srv, Connection *conn, std::string *output) override {
std::vector<int64_t> ret;
Expand All @@ -574,22 +572,20 @@ class CommandHExpireTime : public CommandFieldExpireBase {
}
auto now = util::GetTimeStampMS();
*output = redis::MultiLen(ret.size());
for (const auto ttl : ret) {
uint64_t expire = ttl;
for (const auto &ttl : ret) {
if (ttl > 0) {
expire = (now + expire) / 1000;
output->append(redis::Integer((now + ttl) / 1000));
} else {
output->append(redis::Integer(ttl));
}
output->append(redis::Integer(expire));
}
return Status::OK();
}
};

class CommandHPExpireTime : public CommandFieldExpireBase {
public:
Status Parse(const std::vector<std::string> &args) override {
return CommandFieldExpireBase::commonParse(args, 2);
}
Status Parse(const std::vector<std::string> &args) override { return CommandFieldExpireBase::commonParse(args, 2); }

Status Execute(Server *srv, Connection *conn, std::string *output) override {
std::vector<int64_t> ret;
Expand All @@ -599,22 +595,20 @@ class CommandHPExpireTime : public CommandFieldExpireBase {
}
auto now = util::GetTimeStampMS();
*output = redis::MultiLen(ret.size());
for (const auto ttl : ret) {
uint64_t expire = ttl;
for (const auto &ttl : ret) {
if (ttl > 0) {
expire = now + expire;
output->append(redis::Integer(now + ttl));
} else {
output->append(redis::Integer(ttl));
}
output->append(redis::Integer(expire));
}
return Status::OK();
}
};

class CommandHTTL : public CommandFieldExpireBase {
public:
Status Parse(const std::vector<std::string> &args) override {
return CommandFieldExpireBase::commonParse(args, 2);
}
Status Parse(const std::vector<std::string> &args) override { return CommandFieldExpireBase::commonParse(args, 2); }

Status Execute(Server *srv, Connection *conn, std::string *output) override {
std::vector<int64_t> ret;
Expand All @@ -623,7 +617,7 @@ class CommandHTTL : public CommandFieldExpireBase {
return {Status::RedisExecErr, s.Msg()};
}
*output = redis::MultiLen(ret.size());
for (const auto ttl : ret) {
for (const auto &ttl : ret) {
output->append(redis::Integer(ttl > 0 ? ttl / 1000 : ttl));
}
return Status::OK();
Expand All @@ -632,9 +626,7 @@ class CommandHTTL : public CommandFieldExpireBase {

class CommandHPTTL : public CommandFieldExpireBase {
public:
Status Parse(const std::vector<std::string> &args) override {
return CommandFieldExpireBase::commonParse(args, 2);
}
Status Parse(const std::vector<std::string> &args) override { return CommandFieldExpireBase::commonParse(args, 2); }

Status Execute(Server *srv, Connection *conn, std::string *output) override {
std::vector<int64_t> ret;
Expand All @@ -643,7 +635,7 @@ class CommandHPTTL : public CommandFieldExpireBase {
return {Status::RedisExecErr, s.Msg()};
}
*output = redis::MultiLen(ret.size());
for (const auto ttl : ret) {
for (const auto &ttl : ret) {
output->append(redis::Integer(ttl));
}
return Status::OK();
Expand All @@ -652,9 +644,7 @@ class CommandHPTTL : public CommandFieldExpireBase {

class CommandHPersist : public CommandFieldExpireBase {
public:
Status Parse(const std::vector<std::string> &args) override {
return CommandFieldExpireBase::commonParse(args, 2);
}
Status Parse(const std::vector<std::string> &args) override { return CommandFieldExpireBase::commonParse(args, 2); }

Status Execute(Server *srv, Connection *conn, std::string *output) override {
// equivalent to setting the expiration time to 0
Expand All @@ -678,15 +668,15 @@ REDIS_REGISTER_COMMANDS(MakeCmdAttr<CommandHGet>("hget", 3, "read-only", 1, 1, 1
MakeCmdAttr<CommandHGetAll>("hgetall", 2, "read-only", 1, 1, 1),
MakeCmdAttr<CommandHScan>("hscan", -3, "read-only", 1, 1, 1),
MakeCmdAttr<CommandHRangeByLex>("hrangebylex", -4, "read-only", 1, 1, 1),
MakeCmdAttr<CommandHRandField>("hrandfield", -2, "read-only", 1, 1, 1),
MakeCmdAttr<CommandHExpire>("hexpire", -6, "write", 1, 1, 1),
MakeCmdAttr<CommandHExpireAt>("hexpireat", -6, "write", 1, 1, 1),
MakeCmdAttr<CommandHExpireTime>("hexpiretime", -5, "read-only", 1, 1, 1),
MakeCmdAttr<CommandHPExpire>("hpexpire", -6, "write", 1, 1, 1),
MakeCmdAttr<CommandHPExpireAt>("hpexpireat", -6, "write", 1, 1, 1),
MakeCmdAttr<CommandHPExpireTime>("hpexpiretime", -5, "read-only", 1, 1, 1),
MakeCmdAttr<CommandHPersist>("hpersist", -5, "write", 1, 1, 1),
MakeCmdAttr<CommandHTTL>("httl", -5, "read-only", 1, 1, 1),
MakeCmdAttr<CommandHRandField>("hrandfield", -2, "read-only", 1, 1, 1),
MakeCmdAttr<CommandHExpire>("hexpire", -6, "write", 1, 1, 1),
MakeCmdAttr<CommandHExpireAt>("hexpireat", -6, "write", 1, 1, 1),
MakeCmdAttr<CommandHExpireTime>("hexpiretime", -5, "read-only", 1, 1, 1),
MakeCmdAttr<CommandHPExpire>("hpexpire", -6, "write", 1, 1, 1),
MakeCmdAttr<CommandHPExpireAt>("hpexpireat", -6, "write", 1, 1, 1),
MakeCmdAttr<CommandHPExpireTime>("hpexpiretime", -5, "read-only", 1, 1, 1),
MakeCmdAttr<CommandHPersist>("hpersist", -5, "write", 1, 1, 1),
MakeCmdAttr<CommandHTTL>("httl", -5, "read-only", 1, 1, 1),
MakeCmdAttr<CommandHPTTL>("hpttl", -5, "read-only", 1, 1, 1), )

} // namespace redis
4 changes: 2 additions & 2 deletions src/storage/compact_filter.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,8 @@ bool SubKeyFilter::Filter(int level, const Slice &key, const Slice &value, std::
return false;
}

return IsMetadataExpired(ikey, metadata) ||
(metadata.Type() == kRedisBitmap && redis::Bitmap::IsEmptySegment(value)) ||
return IsMetadataExpired(ikey, metadata) ||
(metadata.Type() == kRedisBitmap && redis::Bitmap::IsEmptySegment(value)) ||
(metadata.Type() == kRedisHash && redis::Hash::IsFieldExpired(metadata, value));
}

Expand Down
44 changes: 31 additions & 13 deletions src/storage/redis_db.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -558,10 +558,24 @@ rocksdb::Status SubKeyScanner::Scan(RedisType type, const Slice &user_key, const
uint64_t cnt = 0;
std::string ns_key = AppendNamespacePrefix(user_key);
Metadata metadata(type, false);
std::string raw_value;
Slice rest;
LatestSnapShot ss(storage_);
rocksdb::Status s = GetMetadata(GetOptions{ss.GetSnapShot()}, {type}, ns_key, &metadata);
rocksdb::Status s = GetMetadata(GetOptions{ss.GetSnapShot()}, {type}, ns_key, &raw_value, &metadata, &rest);
if (!s.ok()) return s;

// for hash type, we should filter expired field if encoding is with_ttl
bool filter_expired_field = false;
if (metadata.Type() == kRedisHash && !rest.empty()) {
uint8_t field_encoding = 0;
if (!GetFixed8(&rest, reinterpret_cast<uint8_t *>(&field_encoding))) {
return rocksdb::Status::InvalidArgument();
}
if (field_encoding == 1) {
filter_expired_field = true;
}
}

rocksdb::ReadOptions read_options = storage_->DefaultScanOptions();
auto iter = util::UniqueIterator(storage_, read_options);
std::string match_prefix_key =
Expand All @@ -585,8 +599,7 @@ rocksdb::Status SubKeyScanner::Scan(RedisType type, const Slice &user_key, const
}
InternalKey ikey(iter->key(), storage_->IsSlotIdEncoded());
auto value = iter->value().ToString();
// filter expired hash field
if (type == kRedisHash && (static_cast<HashMetadata*>(&metadata))->is_ttl_field_encoded) {
if (filter_expired_field) {
uint64_t expire = 0;
rocksdb::Slice data(value.data(), value.size());
GetFixed64(&data, &expire);
Expand Down Expand Up @@ -667,17 +680,22 @@ rocksdb::Status Database::typeInternal(const Slice &key, RedisType *type) {
if (!s.ok()) return s;
if (metadata.Expired()) {
*type = kRedisNone;
} else if (metadata.Type() == kRedisHash && (static_cast<HashMetadata*>(&metadata))->is_ttl_field_encoded) {
redis::Hash hash_db(storage_, namespace_);
auto [_, user_key] = ExtractNamespaceKey(key, storage_->IsSlotIdEncoded());
std::vector<FieldValue> field_values;
s = hash_db.GetAll(user_key, &field_values, HashFetchType::kOnlyKey);
if (!s.ok()) {
return s;
}
if (!field_values.empty()) {
} else if (metadata.Type() == kRedisHash) {
HashMetadata hash_metadata(false);
s = hash_metadata.Decode(value);
if (!s.ok()) return s;
if (!hash_metadata.IsTTLFieldEncoded()) {
*type = metadata.Type();
}
} else {
redis::Hash hash_db(storage_, namespace_);
auto [_, user_key] = ExtractNamespaceKey(key, storage_->IsSlotIdEncoded());
std::vector<FieldValue> field_values;
s = hash_db.GetAll(user_key, &field_values, HashFetchType::kOnlyKey);
if (!s.ok()) return s;
if (!field_values.empty()) {
*type = metadata.Type();
}
}
} else {
*type = metadata.Type();
}
Expand Down
14 changes: 7 additions & 7 deletions src/storage/redis_metadata.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -334,12 +334,12 @@ bool Metadata::IsEmptyableType() const {

bool Metadata::Expired() const { return ExpireAt(util::GetTimeStampMS()); }

bool HashMetadata::IsTTLFieldEncoded() const { return field_encoding == HashFieldEncoding::WITH_TTL; }

void HashMetadata::Encode(std::string *dst) const {
Metadata::Encode(dst);
if (is_ttl_field_encoded) {
uint8_t flag = 0;
flag |= METADATA_HASH_FIELD_EXPIRE_MASK;
PutFixed8(dst, flag);
if (IsTTLFieldEncoded()) {
PutFixed8(dst, uint8_t(field_encoding));
}
}

Expand All @@ -349,9 +349,9 @@ rocksdb::Status HashMetadata::Decode(Slice *input) {
}

if (input->size() >= 1) {
uint8_t flag = 0;
GetFixed8(input, &flag);
is_ttl_field_encoded = flag &= METADATA_HASH_FIELD_EXPIRE_MASK;
if (!GetFixed8(input, reinterpret_cast<uint8_t *>(&field_encoding))) {
return rocksdb::Status::InvalidArgument(kErrMetadataTooShort);
}
}

return rocksdb::Status::OK();
Expand Down
11 changes: 9 additions & 2 deletions src/storage/redis_metadata.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,6 @@ class InternalKey {

constexpr uint8_t METADATA_64BIT_ENCODING_MASK = 0x80;
constexpr uint8_t METADATA_TYPE_MASK = 0x0f;
constexpr uint8_t METADATA_HASH_FIELD_EXPIRE_MASK = 0x01;

class Metadata {
public:
Expand Down Expand Up @@ -201,14 +200,22 @@ class Metadata {
static uint64_t generateVersion();
};

enum class HashFieldEncoding : uint8_t {
RAW = 0,
WITH_TTL = 1,
};

class HashMetadata : public Metadata {
public:
bool is_ttl_field_encoded;
HashFieldEncoding field_encoding = HashFieldEncoding::RAW;

explicit HashMetadata(bool generate_version = true) : Metadata(kRedisHash, generate_version) {}

void Encode(std::string *dst) const override;
using Metadata::Decode;
rocksdb::Status Decode(Slice *input) override;

bool IsTTLFieldEncoded() const;
};

class SetMetadata : public Metadata {
Expand Down
Loading

0 comments on commit cb1ff54

Please sign in to comment.