From d3690256f2058bbdce0b68dae24c3f484e88a142 Mon Sep 17 00:00:00 2001 From: shahar Date: Thu, 10 Oct 2024 16:16:03 +0300 Subject: [PATCH] fix: Temporary fix for loading plain lists **The Issue** We do not serialize lists in PLAIN format well today. We're working on getting it right, but existing wire format simply doesn't work. **The (temporary) fix** This PR attempts to load lists as usual, but upon failure it reverts to loading a plain list format. This is not a perfect solution, and in some extreme edge cases it could fail. But.. it's temporary. --- src/server/list_family_test.cc | 9 +++++++++ src/server/rdb_load.cc | 16 ++++++++++++---- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/server/list_family_test.cc b/src/server/list_family_test.cc index b9ef3ab716e8..95089a2c799a 100644 --- a/src/server/list_family_test.cc +++ b/src/server/list_family_test.cc @@ -388,6 +388,15 @@ TEST_F(ListFamilyTest, DumpRestorePlain) { EXPECT_EQ(Run({"LRANGE", kKey2, "0", "1"}), kValue); } +TEST_F(ListFamilyTest, DumpRestorePlain) { + const string kValue(10'000, '#'); + EXPECT_EQ(CheckedInt({"LPUSH", kKey1, kValue}), 1); + auto buffer = Run({"DUMP", kKey1}).GetBuf(); + EXPECT_EQ(Run({"RESTORE", kKey2, "0", ToSV(buffer)}), "OK"); + EXPECT_EQ(CheckedInt({"LLEN", kKey2}), 1); + EXPECT_EQ(Run({"LRANGE", kKey2, "0", "1"}), kValue); +} + TEST_F(ListFamilyTest, LTrim) { Run({"rpush", kKey1, "a", "b", "c", "d"}); ASSERT_EQ(Run({"ltrim", kKey1, "-2", "-1"}), "OK"); diff --git a/src/server/rdb_load.cc b/src/server/rdb_load.cc index 60e433dfca3a..4d825ccaf7c9 100644 --- a/src/server/rdb_load.cc +++ b/src/server/rdb_load.cc @@ -734,10 +734,15 @@ void RdbLoaderBase::OpaqueObjLoader::CreateList(const LoadTrace* ltrace) { return false; uint8_t* lp = nullptr; - if (container == QUICKLIST_NODE_CONTAINER_PLAIN) { + + auto load_plain = [&]() { lp = (uint8_t*)zmalloc(sv.size()); ::memcpy(lp, (uint8_t*)sv.data(), sv.size()); quicklistAppendPlainNode(ql, lp, sv.size()); + }; + + if (container == QUICKLIST_NODE_CONTAINER_PLAIN) { + load_plain(); return true; } @@ -759,10 +764,13 @@ void RdbLoaderBase::OpaqueObjLoader::CreateList(const LoadTrace* ltrace) { lp = lpNew(sv.size()); if (!ziplistValidateIntegrity((uint8_t*)sv.data(), sv.size(), 1, ziplistEntryConvertAndValidate, &lp)) { - LOG(ERROR) << "Ziplist integrity check failed: " << sv.size(); zfree(lp); - ec_ = RdbError(errc::rdb_file_corrupted); - return false; + // TODO: Revert loading plain format here once we fix serialization size. + // LOG(ERROR) << "Ziplist integrity check failed: " << sv.size(); + // ec_ = RdbError(errc::rdb_file_corrupted); + // return false; + load_plain(); + return true; } /* Silently skip empty ziplists, if we'll end up with empty quicklist we'll fail later. */