Skip to content

Commit 4959bef

Browse files
author
shahar
committed
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.
1 parent 4785767 commit 4959bef

File tree

2 files changed

+24
-6
lines changed

2 files changed

+24
-6
lines changed

src/server/list_family_test.cc

+9
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,15 @@ TEST_F(ListFamilyTest, LRem) {
372372
ASSERT_THAT(Run({"lrem", "nexists", "0", "elem"}), IntArg(0));
373373
}
374374

375+
TEST_F(ListFamilyTest, DumpRestorePlain) {
376+
const string kValue(10'000, '#');
377+
EXPECT_EQ(CheckedInt({"LPUSH", kKey1, kValue}), 1);
378+
auto buffer = Run({"DUMP", kKey1}).GetBuf();
379+
EXPECT_EQ(Run({"RESTORE", kKey2, "0", ToSV(buffer)}), "OK");
380+
EXPECT_EQ(CheckedInt({"LLEN", kKey2}), 1);
381+
EXPECT_EQ(Run({"LRANGE", kKey2, "0", "1"}), kValue);
382+
}
383+
375384
TEST_F(ListFamilyTest, LTrim) {
376385
Run({"rpush", kKey1, "a", "b", "c", "d"});
377386
ASSERT_EQ(Run({"ltrim", kKey1, "-2", "-1"}), "OK");

src/server/rdb_load.cc

+15-6
Original file line numberDiff line numberDiff line change
@@ -722,13 +722,19 @@ void RdbLoaderBase::OpaqueObjLoader::CreateList(const LoadTrace* ltrace) {
722722
if (ec_)
723723
return false;
724724

725+
uint8_t* lp = nullptr;
726+
727+
auto load_plain = [&]() {
728+
lp = (uint8_t*)zmalloc(sv.size());
729+
::memcpy(lp, (uint8_t*)sv.data(), sv.size());
730+
quicklistAppendPlainNode(ql, lp, sv.size());
731+
};
732+
725733
if (container == QUICKLIST_NODE_CONTAINER_PLAIN) {
726-
quicklistAppendPlainNode(ql, (uint8_t*)sv.data(), sv.size());
734+
load_plain();
727735
return true;
728736
}
729737

730-
uint8_t* lp = nullptr;
731-
732738
if (rdb_type_ == RDB_TYPE_LIST_QUICKLIST_2) {
733739
uint8_t* src = (uint8_t*)sv.data();
734740
if (!lpValidateIntegrity(src, sv.size(), 0, nullptr, nullptr)) {
@@ -747,10 +753,13 @@ void RdbLoaderBase::OpaqueObjLoader::CreateList(const LoadTrace* ltrace) {
747753
lp = lpNew(sv.size());
748754
if (!ziplistValidateIntegrity((uint8_t*)sv.data(), sv.size(), 1,
749755
ziplistEntryConvertAndValidate, &lp)) {
750-
LOG(ERROR) << "Ziplist integrity check failed: " << sv.size();
751756
zfree(lp);
752-
ec_ = RdbError(errc::rdb_file_corrupted);
753-
return false;
757+
// TODO: Revert loading plain format here once we fix serialization size.
758+
// LOG(ERROR) << "Ziplist integrity check failed: " << sv.size();
759+
// ec_ = RdbError(errc::rdb_file_corrupted);
760+
// return false;
761+
load_plain();
762+
return true;
754763
}
755764

756765
/* Silently skip empty ziplists, if we'll end up with empty quicklist we'll fail later. */

0 commit comments

Comments
 (0)