Passover cleanups.

1. Add ttl with reload test.
2. Removed several LOG(FATAL) messages and replaced them with error propagation.
3. Added scan test for all the options.

src/facade/resp_expr.h

@@ -20,7 +20,7 @@ class RespExpr {
 
   using Vec = std::vector<RespExpr>;
   Type type;
-  bool has_support;  // whether pointers in this item are supported by external storage.
+  bool has_support;  // whether pointers in this item are supported by the external storage.
 
   std::variant<int64_t, Buffer, Vec*> u;
 

src/server/error.h

@@ -33,7 +33,7 @@ namespace rdb {
 enum errc {
   wrong_signature = 1,
   bad_version = 2,
-  module_not_supported = 3,
+  feature_not_supported = 3,
   duplicate_key = 4,
   rdb_file_corrupted = 5,
   bad_checksum = 6,

src/server/generic_family_test.cc

@@ -153,4 +153,33 @@ TEST_F(GenericFamilyTest, RenameBinary) {
   EXPECT_THAT(Run({"get", kKey2}), RespEq("bar"));
 }
 
+using testing::Each;
+using testing::StartsWith;
+using testing::AnyOf;
+
+TEST_F(GenericFamilyTest, Scan) {
+  for (unsigned i = 0; i < 10; ++i)
+    Run({"set", absl::StrCat("key", i), "bar"});
+
+  for (unsigned i = 0; i < 10; ++i)
+    Run({"set", absl::StrCat("str", i), "bar"});
+
+  for (unsigned i = 0; i < 10; ++i)
+    Run({"sadd", absl::StrCat("set", i), "bar"});
+
+  for (unsigned i = 0; i < 10; ++i)
+    Run({"zadd", absl::StrCat("zset", i), "0", "bar"});
+
+  auto resp = Run({"scan", "0", "count", "20", "type", "string"});
+  EXPECT_EQ(2, resp.size());
+  auto vec = StrArray(resp[1]);
+  EXPECT_GT(vec.size(), 10);
+  EXPECT_THAT(vec, Each(AnyOf(StartsWith("str"), StartsWith("key"))));
+
+  resp = Run({"scan", "0", "count", "20", "match", "zset*"});
+  vec = StrArray(resp[1]);
+  EXPECT_EQ(10, vec.size());
+  EXPECT_THAT(vec, Each(StartsWith("zset")));
+}
+
 }  // namespace dfly

src/server/list_family.cc

@@ -380,23 +380,24 @@ void ListFamily::BPopGeneric(ListDir dir, CmdArgList args, ConnectionContext* cn
   BPopper popper(dir);
   OpStatus result = popper.Run(transaction, unsigned(timeout * 1000));
 
+  if (result == OpStatus::OK) {
+    CHECK(popper.found());
+    VLOG(1) << "BLPop returned ";
+
+    auto res = popper.result();
+    std::string_view str_arr[2] = {res.first, res.second};
+    return (*cntx)->SendStringArr(str_arr);
+  }
+
   switch (result) {
     case OpStatus::WRONG_TYPE:
       return (*cntx)->SendError(kWrongTypeErr);
-    case OpStatus::OK:
-      break;
     case OpStatus::TIMED_OUT:
       return (*cntx)->SendNullArray();
     default:
-      LOG(FATAL) << "Unexpected error " << result;
+      LOG(ERROR) << "Unexpected error " << result;
   }
-
-  CHECK(popper.found());
-  VLOG(1) << "BLPop returned ";
-
-  auto res = popper.result();
-  std::string_view str_arr[2] = {res.first, res.second};
-  return (*cntx)->SendStringArr(str_arr);
+  return (*cntx)->SendNullArray();
 }
 
 void ListFamily::PushGeneric(ListDir dir, bool skip_notexists, CmdArgList args,

src/server/rdb_load.cc

@@ -332,7 +332,8 @@ error_code RdbLoader::Load(io::Source* src) {
     }
 
     if (type == RDB_OPCODE_MODULE_AUX) {
-      return RdbError(errc::module_not_supported);
+      LOG(ERROR) << "Modules are not supported";
+      return RdbError(errc::feature_not_supported);
     }
 
     if (!rdbIsObjectType(type)) {
@@ -416,7 +417,8 @@ auto RdbLoader::LoadLen(bool* is_encoded) -> io::Result<uint64_t> {
     res = absl::big_endian::Load64(mem_buf_.InputBuffer().data());
     mem_buf_.ConsumeInput(8);
   } else {
-    LOG(FATAL) << "Unknown length encoding " << type << " in rdbLoadLen()";
+    LOG(ERROR) << "Bad length encoding " << type << " in rdbLoadLen()";
+    return Unexpected(errc::rdb_file_corrupted);
   }
 
   return res;
@@ -501,7 +503,8 @@ error_code RdbLoader::HandleAux() {
   } else if (!strcasecmp((char*)auxkey->ptr, "repl-offset")) {
     // TODO
   } else if (!strcasecmp((char*)auxkey->ptr, "lua")) {
-    LOG(FATAL) << "Lua scripts are not supported";
+    LOG(ERROR) << "Lua scripts are not supported";
+    return RdbError(errc::feature_not_supported);
   } else if (!strcasecmp((char*)auxkey->ptr, "redis-ver")) {
     LOG(INFO) << "Loading RDB produced by version " << (char*)auxval->ptr;
   } else if (!strcasecmp((char*)auxkey->ptr, "ctime")) {
@@ -575,7 +578,8 @@ auto RdbLoader::FetchGenericString(int flags) -> io::Result<OpaqueBuf> {
       case RDB_ENC_LZF:
         return FetchLzfStringObject(flags);
       default:
-        LOG(FATAL) << "Unknown RDB string encoding type " << len;
+        LOG(ERROR) << "Unknown RDB string encoding len " << len;
+        return Unexpected(errc::rdb_file_corrupted);
     }
   }
 

src/server/rdb_save.cc

@@ -216,8 +216,8 @@ error_code RdbSerializer::SaveObject(const PrimeValue& pv) {
     return SaveZSetObject(pv.AsRObj());
   }
 
-  LOG(FATAL) << "Not implemented " << obj_type;
-  return error_code{};
+  LOG(ERROR) << "Not implemented " << obj_type;
+  return make_error_code(errc::function_not_supported);
 }
 
 error_code RdbSerializer::SaveListObject(const robj* obj) {

src/server/rdb_test.cc

@@ -83,7 +83,7 @@ TEST_F(RdbTest, LoadSmall6) {
   auto ec = loader.Load(&fs);
   CHECK(!ec);
   auto resp = Run({"scan", "0"});
-  EXPECT_THAT(Array(resp[1]),
+  EXPECT_THAT(StrArray(resp[1]),
               UnorderedElementsAre("list1", "hset_zl", "list2", "zset_sl", "intset", "set1",
                                    "zset_zl", "hset_ht", "intkey", "strkey"));
   resp = Run({"smembers", "intset"});
@@ -125,4 +125,11 @@ TEST_F(RdbTest, Reload) {
   EXPECT_EQ(2, CheckedInt({"ZCARD", "zs2"}));
 }
 
+TEST_F(RdbTest, ReloadTtl) {
+  Run({"set", "key", "val"});
+  Run({"expire", "key", "1000"});
+  Run({"debug", "reload"});
+  EXPECT_LT(990, CheckedInt({"ttl", "key"}));
+}
+
 }  // namespace dfly

src/server/test_utils.cc

@@ -287,13 +287,18 @@ auto BaseFamilyTest::AddFindConn(Protocol proto, std::string_view id) -> TestCon
   return it->second.get();
 }
 
-RespVec BaseFamilyTest::Array(const RespExpr& expr) {
+vector<string> BaseFamilyTest::StrArray(const RespExpr& expr) {
   CHECK(expr.type == RespExpr::ARRAY || expr.type == RespExpr::NIL_ARRAY);
   if (expr.type == RespExpr::NIL_ARRAY)
-    return RespVec{};
+    return vector<string>{};
 
   const RespVec* src = get<RespVec*>(expr.u);
-  return *src;
+  vector<string> res(src->size());
+  for (size_t i = 0; i < src->size(); ++i) {
+    res[i] = ToSV(src->at(i).GetBuf());
+  }
+  
+  return res;
 }
 
 }  // namespace dfly

src/server/test_utils.h

@@ -66,7 +66,7 @@ class BaseFamilyTest : public ::testing::Test {
   }
 
   TestConnWrapper* AddFindConn(Protocol proto, std::string_view id);
-  static RespVec Array(const RespExpr& expr);
+  static std::vector<std::string> StrArray(const RespExpr& expr);
 
   // ts is ms
   void UpdateTime(uint64_t ms);

src/server/zset_family.cc

@@ -193,7 +193,8 @@ void IntervalVisitor::ActionRange(unsigned start, unsigned end) {
 
       Next(zl, &eptr, &sptr);
     }
-  } else if (zobj_->encoding == OBJ_ENCODING_SKIPLIST) {
+  } else {
+    CHECK_EQ(zobj_->encoding, OBJ_ENCODING_SKIPLIST);
     zset* zs = (zset*)zobj_->ptr;
     zskiplist* zsl = zs->zsl;
     zskiplistNode* ln;
@@ -216,18 +217,15 @@ void IntervalVisitor::ActionRange(unsigned start, unsigned end) {
       result_.emplace_back(string(ele, sdslen(ele)), ln->score);
       ln = params_.reverse ? ln->backward : ln->level[0].forward;
     }
-  } else {
-    LOG(FATAL) << "Unknown sorted set encoding" << zobj_->encoding;
   }
 }
 
 void IntervalVisitor::ActionRange(const zrangespec& range) {
   if (zobj_->encoding == OBJ_ENCODING_LISTPACK) {
     ExtractListPack(range);
-  } else if (zobj_->encoding == OBJ_ENCODING_SKIPLIST) {
-    ExtractSkipList(range);
   } else {
-    LOG(FATAL) << "Unknown sorted set encoding " << zobj_->encoding;
+    CHECK_EQ(zobj_->encoding, OBJ_ENCODING_SKIPLIST);
+    ExtractSkipList(range);
   }
 }
 
@@ -238,11 +236,10 @@ void IntervalVisitor::ActionRem(unsigned start, unsigned end) {
     removed_ = (end - start) + 1;
     zl = lpDeleteRange(zl, 2 * start, 2 * removed_);
     zobj_->ptr = zl;
-  } else if (zobj_->encoding == OBJ_ENCODING_SKIPLIST) {
+  } else {
+    CHECK_EQ(OBJ_ENCODING_SKIPLIST, zobj_->encoding);
     zset* zs = (zset*)zobj_->ptr;
     removed_ = zslDeleteRangeByRank(zs->zsl, start + 1, end + 1, zs->dict);
-  } else {
-    LOG(FATAL) << "Unknown sorted set encoding" << zobj_->encoding;
   }
 }
 
@@ -253,11 +250,10 @@ void IntervalVisitor::ActionRem(const zrangespec& range) {
     zl = zzlDeleteRangeByScore(zl, &range, &deleted);
     zobj_->ptr = zl;
     removed_ = deleted;
-  } else if (zobj_->encoding == OBJ_ENCODING_SKIPLIST) {
+  } else {
+    CHECK_EQ(OBJ_ENCODING_SKIPLIST, zobj_->encoding);
     zset* zs = (zset*)zobj_->ptr;
     removed_ = zslDeleteRangeByScore(zs->zsl, &range, zs->dict);
-  } else {
-    LOG(FATAL) << "Unknown sorted set encoding" << zobj_->encoding;
   }
 }