This is an automated email from the ASF dual-hosted git repository.
twice pushed a commit to branch unstable
in repository https://gitbox.apache.org/repos/asf/kvrocks.git
The following commit(s) were added to refs/heads/unstable by this push:
new 5ebde751f fix(hyperloglog): use namespace-prefixed key in PFMERGE
GetMetadata (#3441)
5ebde751f is described below
commit 5ebde751f0e15058e9823978a02b062b3a20c115
Author: Songqing Zhang <[email protected]>
AuthorDate: Thu Apr 16 22:26:12 2026 +0800
fix(hyperloglog): use namespace-prefixed key in PFMERGE GetMetadata (#3441)
Merge called GetMetadata with the raw user key instead of the
namespace-prefixed key, causing the destination's existing metadata to
never be found and potentially orphaning old sub-keys.
---------
Co-authored-by: 纪华裕 <[email protected]>
---
src/types/redis_hyperloglog.cc | 2 +-
tests/gocase/unit/hyperloglog/hyperloglog_test.go | 53 +++++++++++++++++++++++
2 files changed, 54 insertions(+), 1 deletion(-)
diff --git a/src/types/redis_hyperloglog.cc b/src/types/redis_hyperloglog.cc
index 63ee0b2b9..952177265 100644
--- a/src/types/redis_hyperloglog.cc
+++ b/src/types/redis_hyperloglog.cc
@@ -240,7 +240,7 @@ rocksdb::Status HyperLogLog::Merge(engine::Context &ctx,
const Slice &dest_user_
std::vector<std::string> registers;
HyperLogLogMetadata metadata;
- rocksdb::Status s = GetMetadata(ctx, dest_user_key, &metadata);
+ rocksdb::Status s = GetMetadata(ctx, dest_key, &metadata);
if (!s.ok() && !s.IsNotFound()) return s;
{
std::vector<Slice> all_user_keys;
diff --git a/tests/gocase/unit/hyperloglog/hyperloglog_test.go
b/tests/gocase/unit/hyperloglog/hyperloglog_test.go
index 62fdff40d..28fc949cf 100644
--- a/tests/gocase/unit/hyperloglog/hyperloglog_test.go
+++ b/tests/gocase/unit/hyperloglog/hyperloglog_test.go
@@ -23,6 +23,7 @@ import (
"context"
"fmt"
"testing"
+ "time"
"github.com/apache/kvrocks/tests/gocase/util"
"github.com/stretchr/testify/require"
@@ -213,4 +214,56 @@ func TestHyperLogLog(t *testing.T) {
require.NoError(t, err)
require.EqualValues(t, 10, card)
})
+
+ t.Run("PFMERGE into existing dest preserves dest data", func(t
*testing.T) {
+ require.NoError(t, rdb.Do(ctx, "DEL", "dest", "src1",
"src2").Err())
+
+ _, err := rdb.PFAdd(ctx, "dest", "a", "b", "c").Result()
+ require.NoError(t, err)
+ _, err = rdb.PFAdd(ctx, "src1", "d", "e").Result()
+ require.NoError(t, err)
+ _, err = rdb.PFAdd(ctx, "src2", "f").Result()
+ require.NoError(t, err)
+
+ // Merge src1 and src2 into existing dest
+ _, err = rdb.PFMerge(ctx, "dest", "src1", "src2").Result()
+ require.NoError(t, err)
+
+ // dest should have all 6 elements
+ card, err := rdb.PFCount(ctx, "dest").Result()
+ require.NoError(t, err)
+ require.EqualValues(t, 6, card)
+
+ // Merge again into dest to verify idempotency
+ _, err = rdb.PFMerge(ctx, "dest", "src1").Result()
+ require.NoError(t, err)
+ card, err = rdb.PFCount(ctx, "dest").Result()
+ require.NoError(t, err)
+ require.EqualValues(t, 6, card)
+ })
+
+ // PFMERGE must load dest metadata using the namespace-prefixed key
(same as storage).
+ // If GetMetadata used the raw user key, it would miss the record, keep
default Metadata (expire=0),
+ // and the following PFMERGE would rewrite metadata without preserving
TTL (TTL becomes "no expiry").
+ t.Run("PFMERGE into existing dest preserves TTL", func(t *testing.T) {
+ require.NoError(t, rdb.Do(ctx, "DEL", "dest_ttl",
"src_ttl").Err())
+
+ _, err := rdb.PFAdd(ctx, "dest_ttl", "a", "b").Result()
+ require.NoError(t, err)
+ ok, err := rdb.Expire(ctx, "dest_ttl",
3600*time.Second).Result()
+ require.NoError(t, err)
+ require.True(t, ok)
+
+ ttlBefore := rdb.TTL(ctx, "dest_ttl").Val()
+ require.Greater(t, ttlBefore, time.Duration(0))
+
+ _, err = rdb.PFAdd(ctx, "src_ttl", "c").Result()
+ require.NoError(t, err)
+ _, err = rdb.PFMerge(ctx, "dest_ttl", "src_ttl").Result()
+ require.NoError(t, err)
+
+ ttlAfter := rdb.TTL(ctx, "dest_ttl").Val()
+ require.Greater(t, ttlAfter, time.Duration(0),
+ "PFMERGE must keep dest TTL when dest already exists
(metadata must be read with ns-prefixed key)")
+ })
}
