chore: improve compatibility of EXPIRE functions with Redis

Also, provide a module name if stumbled upon module data that can not be loaded by dragonfly. Signed-off-by: Roman Gershman <[email protected]>
dragonflydb · Mar 8, 2024 · fae92be · fae92be
1 parent 292c5bc
commit fae92be
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 6 deletions.
diff --git a/src/server/common.h b/src/server/common.h
@@ -23,7 +23,7 @@ namespace dfly {
 enum class ListDir : uint8_t { LEFT, RIGHT };
 
 // Dependent on ExpirePeriod representation of the value.
-constexpr int64_t kMaxExpireDeadlineSec = (1u << 28) - 1;
+constexpr int64_t kMaxExpireDeadlineSec = (1u << 28) - 1;  // 8.5 years
 
 using DbIndex = uint16_t;
 using ShardId = uint16_t;

diff --git a/src/server/generic_family.cc b/src/server/generic_family.cc
@@ -743,11 +743,13 @@ void GenericFamily::Expire(CmdArgList args, ConnectionContext* cntx) {
     return cntx->SendError(kInvalidIntErr);
   }
 
-  if (int_arg > kMaxExpireDeadlineSec || int_arg < -kMaxExpireDeadlineSec) {
-    return cntx->SendError(InvalidExpireTime(cntx->cid->name()));
+  int_arg = std::max<int64_t>(int_arg, -1);
+
+  // silently cap the expire time to kMaxExpireDeadlineSec which is more than 8 years.
+  if (int_arg > kMaxExpireDeadlineSec) {
+    int_arg = kMaxExpireDeadlineSec;
   }
 
-  int_arg = std::max<int64_t>(int_arg, -1);
   auto expire_options = ParseExpireOptionsOrReply(args.subspan(2), cntx);
   if (!expire_options) {
     return;
@@ -851,7 +853,13 @@ void GenericFamily::Pexpire(CmdArgList args, ConnectionContext* cntx) {
   if (!absl::SimpleAtoi(msec, &int_arg)) {
     return cntx->SendError(kInvalidIntErr);
   }
-  int_arg = std::max<int64_t>(int_arg, 0L);
+  int_arg = std::max<int64_t>(int_arg, -1);
+
+  // to be more compatible with redis, we silently cap the expire time to kMaxExpireDeadlineSec
+  if (int_arg > kMaxExpireDeadlineSec * 1000) {
+    int_arg = kMaxExpireDeadlineSec * 1000;
+  }
+
   auto expire_options = ParseExpireOptionsOrReply(args.subspan(2), cntx);
   if (!expire_options) {
     return;

diff --git a/src/server/rdb_load.cc b/src/server/rdb_load.cc
@@ -203,6 +203,25 @@ int ziplistPairsConvertAndValidateIntegrity(const uint8_t* zl, size_t size, unsi
   return ret;
 }
 
+static string ModuleTypeName(uint64_t module_id) {
+  static const char ModuleNameSet[] =
+      "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
+      "abcdefghijklmnopqrstuvwxyz"
+      "0123456789-_";
+
+  char name[10];
+
+  name[9] = '\0';
+  char* p = name + 8;
+  module_id >>= 10;
+  for (int j = 0; j < 9; j++) {
+    *p-- = ModuleNameSet[module_id & 63];
+    module_id >>= 6;
+  }
+
+  return string{name};
+}
+
 }  // namespace
 
 class DecompressImpl {
@@ -1978,7 +1997,11 @@ error_code RdbLoader::Load(io::Source* src) {
     }
 
     if (type == RDB_OPCODE_MODULE_AUX) {
-      LOG(ERROR) << "Modules are not supported";
+      uint64_t module_id;
+      SET_OR_RETURN(LoadLen(nullptr), module_id);
+      string module_name = ModuleTypeName(module_id);
+
+      LOG(ERROR) << "Modules are not supported, error loading module " << module_name;
       return RdbError(errc::feature_not_supported);
     }