Skip to content

Commit ff7d9b7

Browse files
authored
fix: potential crash with multi-sharded pfmerge (#5008)
Fixes #5004 Signed-off-by: Roman Gershman <roman@dragonflydb.io>
1 parent d5c3752 commit ff7d9b7

File tree

2 files changed

+7
-6
lines changed

2 files changed

+7
-6
lines changed

src/server/hll_family.cc

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -255,14 +255,14 @@ OpResult<int> PFMergeInternal(CmdArgList args, Transaction* tx, SinkReplyBuilder
255255
if (result.ok()) {
256256
hlls[sid] = std::move(result.value());
257257
} else {
258-
success = false;
258+
success.store(false, memory_order_relaxed);
259259
}
260-
return result.status();
260+
return OpStatus::OK;
261261
};
262262

263263
tx->Execute(std::move(cb), false);
264264

265-
if (!success) {
265+
if (!success.load(memory_order_relaxed)) {
266266
tx->Conclude();
267267
return OpStatus::INVALID_VALUE;
268268
}

src/server/hll_family_test.cc

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -194,11 +194,12 @@ TEST_F(HllFamilyTest, MergeOverlapping) {
194194
}
195195

196196
TEST_F(HllFamilyTest, MergeInvalid) {
197-
GTEST_SKIP() << "TBD: MergeInvalid test fails with multi-shard runs, see #5004";
197+
Run({"exists", "key1", "key4"});
198+
ASSERT_EQ(GetDebugInfo().shards_count, 2); // ensure 2 shards
198199

199200
EXPECT_EQ(CheckedInt({"pfadd", "key1", "1", "2", "3"}), 1);
200-
EXPECT_EQ(Run({"set", "key2", "..."}), "OK");
201-
EXPECT_THAT(Run({"pfmerge", "key1", "key2"}), ErrArg(HllFamily::kInvalidHllErr));
201+
EXPECT_EQ(Run({"set", "key4", "..."}), "OK");
202+
EXPECT_THAT(Run({"pfmerge", "key1", "key4"}), ErrArg(HllFamily::kInvalidHllErr));
202203
EXPECT_EQ(CheckedInt({"pfcount", "key1"}), 3);
203204
}
204205

0 commit comments

Comments
 (0)