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 16, 2024
1 parent 749c7be commit 28b8601
Show file tree
Hide file tree
Showing 7 changed files with 55 additions and 56 deletions.
38 changes: 19 additions & 19 deletions src/commands/cmd_hash.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ class CommandHRandField : public Commander {
bool no_parameters_ = true;
};

class CommandFiledExpireBase : public Commander {
class CommandFieldExpireBase : public Commander {
protected:
Status commonParse(const std::vector<std::string> &args, int start_idx) {
int idx = start_idx;
Expand Down Expand Up @@ -500,70 +500,70 @@ class CommandFiledExpireBase : public Commander {
std::vector<Slice> fields_;
};

class CommandHExpire : public CommandFiledExpireBase {
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 };

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

Status Execute(Server *srv, Connection *conn, std::string *output) override {
return expireFieldExecute(srv, conn, output, false);
}
};

class CommandHExpireAt : public CommandFiledExpireBase {
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 };

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

Status Execute(Server *srv, Connection *conn, std::string *output) override {
return expireFieldExecute(srv, conn, output, false);
}
};

class CommandHPExpire : public CommandFiledExpireBase {
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 };

expire_ = *parse_result + util::GetTimeStampMS();
return CommandFiledExpireBase::commonParse(args, 3);
return CommandFieldExpireBase::commonParse(args, 3);
}

Status Execute(Server *srv, Connection *conn, std::string *output) override {
return expireFieldExecute(srv, conn, output, false);
}
};

class CommandHPExpireAt : public CommandFiledExpireBase {
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 };

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

Status Execute(Server *srv, Connection *conn, std::string *output) override {
return expireFieldExecute(srv, conn, output, false);
}
};

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

Status Execute(Server *srv, Connection *conn, std::string *output) override {
Expand All @@ -585,10 +585,10 @@ class CommandHExpireTime : public CommandFiledExpireBase {
}
};

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

Status Execute(Server *srv, Connection *conn, std::string *output) override {
Expand All @@ -610,10 +610,10 @@ class CommandHPExpireTime : public CommandFiledExpireBase {
}
};

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

Status Execute(Server *srv, Connection *conn, std::string *output) override {
Expand All @@ -630,10 +630,10 @@ class CommandHTTL : public CommandFiledExpireBase {
}
};

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

Status Execute(Server *srv, Connection *conn, std::string *output) override {
Expand All @@ -650,10 +650,10 @@ class CommandHPTTL : public CommandFiledExpireBase {
}
};

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

Status Execute(Server *srv, Connection *conn, std::string *output) override {
Expand Down
2 changes: 1 addition & 1 deletion src/storage/compact_filter.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ bool SubKeyFilter::Filter(int level, const Slice &key, const Slice &value, std::

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

} // namespace engine
8 changes: 4 additions & 4 deletions src/storage/redis_db.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -580,15 +580,15 @@ 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 feild
if (type == kRedisHash && (static_cast<HashMetadata*>(&metadata))->IsEncodedFieldExpire()) {
// filter expired hash field
if (type == kRedisHash && (static_cast<HashMetadata*>(&metadata))->IsEncodedFieldHasTTL()) {
uint64_t expire = 0;
rocksdb::Slice data(value.data(), value.size());
GetFixed64(&data, &expire);
value = data.ToString();
if (expire != 0 && expire <= now) {
continue;
}
value = data.ToString();
}
keys->emplace_back(ikey.GetSubKey().ToString());
if (values != nullptr) {
Expand Down Expand Up @@ -662,7 +662,7 @@ 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))->IsEncodedFieldExpire()) {
} else if (metadata.Type() == kRedisHash && (static_cast<HashMetadata*>(&metadata))->IsEncodedFieldHasTTL()) {
redis::Hash hash_db(storage_, namespace_);
auto [_, user_key] = ExtractNamespaceKey(key, storage_->IsSlotIdEncoded());
std::vector<FieldValue> field_values;
Expand Down
2 changes: 1 addition & 1 deletion src/storage/redis_metadata.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ bool Metadata::IsEmptyableType() const {

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

bool HashMetadata::IsEncodedFieldExpire() const {
bool HashMetadata::IsEncodedFieldHasTTL() const {
return flags & METADATA_HASH_FIELD_EXPIRE_MASK;
}

Expand Down
2 changes: 1 addition & 1 deletion src/storage/redis_metadata.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ class HashMetadata : public Metadata {
public:
explicit HashMetadata(bool generate_version = true) : Metadata(kRedisHash, generate_version) {}

bool IsEncodedFieldExpire() const;
bool IsEncodedFieldHasTTL() const;
};

class SetMetadata : public Metadata {
Expand Down
53 changes: 26 additions & 27 deletions src/types/redis_hash.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ rocksdb::Status Hash::Size(const Slice &user_key, uint64_t *size) {
HashMetadata metadata(false);
rocksdb::Status s = GetMetadata(Database::GetOptions{}, ns_key, &metadata);
if (!s.ok()) return s;
if (!metadata.IsEncodedFieldExpire()) {
if (!metadata.IsEncodedFieldHasTTL()) {
*size = metadata.size;
} else {
std::vector<FieldValue> field_values;
Expand All @@ -68,7 +68,7 @@ rocksdb::Status Hash::Get(const Slice &user_key, const Slice &field, std::string
s = storage_->Get(read_options, sub_key, value);
if (!s.ok()) return s;
uint64_t expire = 0;
return decodeFieldValue(metadata, value, expire);
return decodeFieldAndTTL(metadata, value, expire);
}

rocksdb::Status Hash::IncrBy(const Slice &user_key, const Slice &field, int64_t increment, int64_t *new_value) {
Expand All @@ -88,7 +88,7 @@ rocksdb::Status Hash::IncrBy(const Slice &user_key, const Slice &field, int64_t
std::string value_bytes;
s = storage_->Get(rocksdb::ReadOptions(), sub_key, &value_bytes);
if (!s.ok() && !s.IsNotFound()) return s;
if (s.ok() && decodeFieldValue(metadata, &value_bytes, expire).ok()) {
if (s.ok() && decodeFieldAndTTL(metadata, &value_bytes, expire).ok()) {
auto parse_result = ParseInt<int64_t>(value_bytes, 10);
if (!parse_result) {
return rocksdb::Status::InvalidArgument(parse_result.Msg());
Expand All @@ -113,8 +113,8 @@ rocksdb::Status Hash::IncrBy(const Slice &user_key, const Slice &field, int64_t
WriteBatchLogData log_data(kRedisHash);
batch->PutLogData(log_data.Encode());
auto value_str = std::to_string(*new_value);
if (metadata.IsEncodedFieldExpire()) {
encodeValueExpire(&value_str, expire);
if (metadata.IsEncodedFieldHasTTL()) {
encodeFieldAndTTL(&value_str, expire);
}
batch->Put(sub_key, value_str);
if (!exists) {
Expand Down Expand Up @@ -143,7 +143,7 @@ rocksdb::Status Hash::IncrByFloat(const Slice &user_key, const Slice &field, dou
std::string value_bytes;
s = storage_->Get(rocksdb::ReadOptions(), sub_key, &value_bytes);
if (!s.ok() && !s.IsNotFound()) return s;
if (s.ok() && decodeFieldValue(metadata, &value_bytes, expire).ok()) {
if (s.ok() && decodeFieldAndTTL(metadata, &value_bytes, expire).ok()) {
auto value_stat = ParseFloat(value_bytes);
if (!value_stat || isspace(value_bytes[0])) {
return rocksdb::Status::InvalidArgument("value is not a number");
Expand All @@ -164,8 +164,8 @@ rocksdb::Status Hash::IncrByFloat(const Slice &user_key, const Slice &field, dou
WriteBatchLogData log_data(kRedisHash);
batch->PutLogData(log_data.Encode());
auto value_str = std::to_string(*new_value);
if (metadata.IsEncodedFieldExpire()) {
encodeValueExpire(&value_str, expire);
if (metadata.IsEncodedFieldHasTTL()) {
encodeFieldAndTTL(&value_str, expire);
}
batch->Put(sub_key, value_str);
if (!exists) {
Expand Down Expand Up @@ -216,7 +216,7 @@ rocksdb::Status Hash::MGet(const Slice &user_key, const std::vector<Slice> &fiel
auto status = statuses_vector[i];
if (!status.IsNotFound()) {
uint64_t expire = 0;
status = decodeFieldValue(metadata, &value, expire);
status = decodeFieldAndTTL(metadata, &value, expire);
}
values->emplace_back(value);
statuses->emplace_back(status);
Expand Down Expand Up @@ -252,8 +252,7 @@ rocksdb::Status Hash::Delete(const Slice &user_key, const std::vector<Slice> &fi
return s;
}
uint64_t expire = 0;
s = decodeFieldValue(metadata, &value, expire);
if (s.ok()) {
if (decodeFieldAndTTL(metadata, &value, expire).ok()) {
*deleted_cnt += 1;
batch->Delete(sub_key);
}
Expand Down Expand Up @@ -299,7 +298,7 @@ rocksdb::Status Hash::MSet(const Slice &user_key, const std::vector<FieldValue>

if (s.ok()) {
if (nx || field_value == it->value) continue;
exists = decodeFieldValue(metadata, &field_value, expire).ok();
exists = decodeFieldAndTTL(metadata, &field_value, expire).ok();
}
}

Expand All @@ -309,8 +308,8 @@ rocksdb::Status Hash::MSet(const Slice &user_key, const std::vector<FieldValue>
}

auto value = it->value;
if (metadata.IsEncodedFieldExpire()) {
encodeValueExpire(&value, expire);
if (metadata.IsEncodedFieldHasTTL()) {
encodeFieldAndTTL(&value, expire);
}
batch->Put(sub_key, value);
}
Expand Down Expand Up @@ -381,7 +380,7 @@ rocksdb::Status Hash::RangeByLex(const Slice &user_key, const RangeLexSpec &spec
// filte expired field
auto value = iter->value().ToString();
uint64_t expire = 0;
if (!decodeFieldValue(metadata, &value, expire).ok()) {
if (!decodeFieldAndTTL(metadata, &value, expire).ok()) {
continue;
}
field_values->emplace_back(ikey.GetSubKey().ToString(), value);
Expand Down Expand Up @@ -413,7 +412,7 @@ rocksdb::Status Hash::GetAll(const Slice &user_key, std::vector<FieldValue> *fie
// filte expired field
uint64_t expire = 0;
auto value = iter->value().ToString();
if (!decodeFieldValue(metadata, &value, expire).ok()) {
if (!decodeFieldAndTTL(metadata, &value, expire).ok()) {
continue;
}
if (type == HashFetchType::kOnlyKey) {
Expand Down Expand Up @@ -529,9 +528,9 @@ rocksdb::Status Hash::ExpireFields(const Slice &user_key, uint64_t expire_ms,

auto value = values_vector[i].ToString();
uint64_t field_expire = 0;
decodeFieldValue(metadata, &value, field_expire);
decodeFieldAndTTL(metadata, &value, field_expire);
if (isMeetCondition(type, expire_ms, field_expire)) {
encodeValueExpire(&value, expire_ms);
encodeFieldAndTTL(&value, expire_ms);
batch->Put(sub_ikey.Encode(), value);
if (is_persist && field_expire == 0) {
// for hpersist command, -1 if the field exists but has no associated expiration
Expand All @@ -546,7 +545,7 @@ rocksdb::Status Hash::ExpireFields(const Slice &user_key, uint64_t expire_ms,
}

// convert rest field encoding
if (!metadata.IsEncodedFieldExpire()) {
if (!metadata.IsEncodedFieldHasTTL()) {
metadata.flags |= METADATA_HASH_FIELD_EXPIRE_MASK;

std::unordered_set<std::string_view> field_set;
Expand All @@ -568,7 +567,7 @@ rocksdb::Status Hash::ExpireFields(const Slice &user_key, uint64_t expire_ms,
InternalKey sub_ikey(iter->key(), storage_->IsSlotIdEncoded());
auto value = iter->value().ToString();
if (field_set.find(sub_ikey.GetSubKey().ToStringView()) == field_set.end()) {
encodeValueExpire(&value, 0);
encodeFieldAndTTL(&value, 0);
batch->Put(sub_ikey.Encode(), value);
}
}
Expand Down Expand Up @@ -624,7 +623,7 @@ rocksdb::Status Hash::TTLFields(const Slice &user_key, const std::vector<Slice>
}

uint64_t expire = 0;
status = decodeFieldValue(metadata, &value, expire);
status = decodeFieldAndTTL(metadata, &value, expire);
if (status.IsNotFound()) {
ret->emplace_back(-2);
} else if (expire == 0) {
Expand All @@ -636,8 +635,8 @@ rocksdb::Status Hash::TTLFields(const Slice &user_key, const std::vector<Slice>
return rocksdb::Status::OK();
}

bool Hash::IsExpiredField(Metadata &metadata, const Slice &value) {
if (!(static_cast<HashMetadata*>(&metadata))->IsEncodedFieldExpire()) {
bool Hash::IsFieldExpired(Metadata &metadata, const Slice &value) {
if (!(static_cast<HashMetadata*>(&metadata))->IsEncodedFieldHasTTL()) {
return false;
}
uint64_t expire = 0;
Expand All @@ -646,8 +645,8 @@ bool Hash::IsExpiredField(Metadata &metadata, const Slice &value) {
return expire != 0 && expire < util::GetTimeStampMS();
}

rocksdb::Status Hash::decodeFieldValue(const HashMetadata &metadata, std::string *value, uint64_t &expire) {
if (!metadata.IsEncodedFieldExpire()) {
rocksdb::Status Hash::decodeFieldAndTTL(const HashMetadata &metadata, std::string *value, uint64_t &expire) {
if (!metadata.IsEncodedFieldHasTTL()) {
return rocksdb::Status::OK();
}
rocksdb::Slice data(value->data(), value->size());
Expand All @@ -656,7 +655,7 @@ rocksdb::Status Hash::decodeFieldValue(const HashMetadata &metadata, std::string
return (expire == 0 || expire > util::GetTimeStampMS()) ? rocksdb::Status::OK() : rocksdb::Status::NotFound();
}

rocksdb::Status Hash::encodeValueExpire(std::string *value, uint64_t expire) {
rocksdb::Status Hash::encodeFieldAndTTL(std::string *value, uint64_t expire) {
std::string buf;
PutFixed64(&buf, expire);
buf.append(*value);
Expand All @@ -668,7 +667,7 @@ bool Hash::isMeetCondition(HashFieldExpireType type, uint64_t new_expire, uint64
if (type == HashFieldExpireType::None) return true;
if (type == HashFieldExpireType::NX && old_expire == 0) return true;
if (type == HashFieldExpireType::XX && old_expire != 0) return true;
// if a filed has no associated expiration, we treated it expiration is infinite
// if a field has no associated expiration, we treated it expiration is infinite
auto expire = old_expire == 0 ? UINT64_MAX : old_expire;
if (type == HashFieldExpireType::GT && new_expire > expire) return true;
if (type == HashFieldExpireType::LT && new_expire < expire) return true;
Expand Down
Loading

0 comments on commit 28b8601

Please sign in to comment.