diff --git a/README.md b/README.md index 27351170b..f317fbcbb 100644 --- a/README.md +++ b/README.md @@ -193,4 +193,17 @@ Design config support. ~140 commands overall... ## Milestone Molt API 5,6 - without cluster and modules. Streams support. ~80 commands overall. ## Milestone Adult -TBD. \ No newline at end of file +TBD. + + +## Design decisions along the way +### Expiration deadlines with relative accuracy +I decided to limit the expiration range to 180 days. Moreover, expiration deadlines +with millisecond precision (PEXPIRE/PSETEX etc) will be rounded to closest second +**for deadlines greater than 16777215ms (approximately 280 minutes). In other words, +expiries of `PEXPIRE key 10010` will expire exactly after 10 seconds and 10ms. However, +`PEXPIRE key 16777300` will expire after 16777 seconds (i.e. 300ms earlier). Similarly, +`PEXPIRE key 16777800` will expire after 16778 seconds, i.e. 200ms later. + +Such rounding has at most 0.006% error which I hope is acceptable for ranges so big. +If you it breaks your use-cases - talk to me or open an issue and explain your case. \ No newline at end of file diff --git a/src/facade/error.h b/src/facade/error.h index 4e85ad795..f04382718 100644 --- a/src/facade/error.h +++ b/src/facade/error.h @@ -20,6 +20,6 @@ extern const char kDbIndOutOfRangeErr[]; extern const char kInvalidDbIndErr[]; extern const char kScriptNotFound[]; extern const char kAuthRejected[]; - +extern const char kExpiryOutOfRange[]; } // namespace dfly diff --git a/src/facade/facade.cc b/src/facade/facade.cc index 6db9ce11f..6d8691180 100644 --- a/src/facade/facade.cc +++ b/src/facade/facade.cc @@ -49,6 +49,7 @@ const char kDbIndOutOfRangeErr[] = "DB index is out of range"; const char kInvalidDbIndErr[] = "invalid DB index"; const char kScriptNotFound[] = "-NOSCRIPT No matching script. Please use EVAL."; const char kAuthRejected[] = "-WRONGPASS invalid username-password pair or user is disabled."; +const char kExpiryOutOfRange[] = "expiry is out of range"; const char* RespExpr::TypeName(Type t) { switch (t) { diff --git a/src/server/common_types.h b/src/server/common_types.h index 760a1406c..ba8f47b6b 100644 --- a/src/server/common_types.h +++ b/src/server/common_types.h @@ -16,6 +16,8 @@ namespace dfly { enum class ListDir : uint8_t { LEFT, RIGHT }; +constexpr uint64_t kMaxExpireDeadlineSec = (1u << 24) - 1; + using DbIndex = uint16_t; using ShardId = uint16_t; using TxId = uint64_t; diff --git a/src/server/generic_family.cc b/src/server/generic_family.cc index facaf81e5..caff80166 100644 --- a/src/server/generic_family.cc +++ b/src/server/generic_family.cc @@ -5,7 +5,7 @@ #include "server/generic_family.h" extern "C" { - #include "redis/object.h" +#include "redis/object.h" } #include "base/logging.h" @@ -21,6 +21,7 @@ DEFINE_uint32(dbnum, 16, "Number of databases"); namespace dfly { using namespace std; using facade::Protocol; +using facade::kExpiryOutOfRange; namespace { @@ -244,8 +245,11 @@ void GenericFamily::Expire(CmdArgList args, ConnectionContext* cntx) { return OpExpire(OpArgs{shard, t->db_index()}, key, params); }; OpStatus status = cntx->transaction->ScheduleSingleHop(move(cb)); - - (*cntx)->SendLong(status == OpStatus::OK); + if (status == OpStatus::OUT_OF_RANGE) { + return (*cntx)->SendError(kExpiryOutOfRange); + } else { + (*cntx)->SendLong(status == OpStatus::OK); + } } void GenericFamily::ExpireAt(CmdArgList args, ConnectionContext* cntx) { @@ -263,7 +267,12 @@ void GenericFamily::ExpireAt(CmdArgList args, ConnectionContext* cntx) { return OpExpire(OpArgs{shard, t->db_index()}, key, params); }; OpStatus status = cntx->transaction->ScheduleSingleHop(std::move(cb)); - (*cntx)->SendLong(status == OpStatus::OK); + + if (status == OpStatus::OUT_OF_RANGE) { + return (*cntx)->SendError(kExpiryOutOfRange); + } else { + (*cntx)->SendLong(status == OpStatus::OK); + } } void GenericFamily::Rename(CmdArgList args, ConnectionContext* cntx) { @@ -424,7 +433,6 @@ void GenericFamily::Scan(CmdArgList args, ConnectionContext* cntx) { } } - OpStatus GenericFamily::OpExpire(const OpArgs& op_args, string_view key, const ExpireParams& params) { auto& db_slice = op_args.shard->db_slice(); @@ -432,18 +440,20 @@ OpStatus GenericFamily::OpExpire(const OpArgs& op_args, string_view key, if (!IsValid(it)) return OpStatus::KEY_NOTFOUND; - int64_t abs_msec = (params.unit == TimeUnit::SEC) ? params.ts * 1000 : params.ts; + int64_t msec = (params.unit == TimeUnit::SEC) ? params.ts * 1000 : params.ts; + int64_t now_msec = db_slice.Now(); + int64_t rel_msec = params.absolute ? msec - now_msec : msec; - if (!params.absolute) { - abs_msec += db_slice.Now(); + if (rel_msec > int64_t(kMaxExpireDeadlineSec * 1000)) { + return OpStatus::OUT_OF_RANGE; } - if (abs_msec <= int64_t(db_slice.Now())) { + if (rel_msec <= 0) { CHECK(db_slice.Del(op_args.db_ind, it)); } else if (IsValid(expire_it)) { - expire_it->second = abs_msec; + expire_it->second = rel_msec + now_msec; } else { - db_slice.Expire(op_args.db_ind, it, abs_msec); + db_slice.Expire(op_args.db_ind, it, rel_msec + now_msec); } return OpStatus::OK; @@ -491,8 +501,8 @@ OpResult GenericFamily::OpExists(const OpArgs& op_args, ArgSlice keys) return res; } -OpResult GenericFamily::OpRen(const OpArgs& op_args, string_view from, - string_view to, bool skip_exists) { +OpResult GenericFamily::OpRen(const OpArgs& op_args, string_view from, string_view to, + bool skip_exists) { auto& db_slice = op_args.shard->db_slice(); auto [from_it, expire_it] = db_slice.FindExt(op_args.db_ind, from); if (!IsValid(from_it)) @@ -538,8 +548,8 @@ void GenericFamily::OpScan(const OpArgs& op_args, uint64_t* cursor, vectorSendError(kInvalidIntErr); } - if (int_arg <= 0 || (!is_ms && int_arg >= 500000000)) { - return builder->SendError("invalid expire time in set"); + + if (int_arg <= 0 || (!is_ms && int_arg >= int64_t(kMaxExpireDeadlineSec))) { + return builder->SendError(facade::kExpiryOutOfRange); } if (!is_ms) { int_arg *= 1000; } + if (int_arg >= int64_t(kMaxExpireDeadlineSec * 1000)) { + return builder->SendError(facade::kExpiryOutOfRange); + } sparams.expire_after_ms = int_arg; } else if (cur_arg == "NX") { sparams.how = SetCmd::SET_IF_NOTEXIST;