Skip to content

Commit

Permalink
fix: Temporary fix for loading plain lists
Browse files Browse the repository at this point in the history
**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.
  • Loading branch information
chakaz authored and adiholden committed Nov 27, 2024
1 parent e4208e9 commit d369025
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 4 deletions.
9 changes: 9 additions & 0 deletions src/server/list_family_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
16 changes: 12 additions & 4 deletions src/server/rdb_load.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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. */
Expand Down

0 comments on commit d369025

Please sign in to comment.