From e19b3e6dee6c1caf84a2be1bfaf597aacf34b87a Mon Sep 17 00:00:00 2001 From: songqing Date: Thu, 9 Apr 2026 18:51:02 +0800 Subject: [PATCH 1/2] fix(hyperloglog): use namespace-prefixed key in PFMERGE GetMetadata 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. --- src/types/redis_hyperloglog.cc | 2 +- .../unit/hyperloglog/hyperloglog_test.go | 27 +++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/types/redis_hyperloglog.cc b/src/types/redis_hyperloglog.cc index 63ee0b2b926..952177265f0 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 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 all_user_keys; diff --git a/tests/gocase/unit/hyperloglog/hyperloglog_test.go b/tests/gocase/unit/hyperloglog/hyperloglog_test.go index 62fdff40d66..2aa4e3d55fc 100644 --- a/tests/gocase/unit/hyperloglog/hyperloglog_test.go +++ b/tests/gocase/unit/hyperloglog/hyperloglog_test.go @@ -213,4 +213,31 @@ 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) + }) } From ba695016ce0123bb28eaed1613891bdda0a5c62b Mon Sep 17 00:00:00 2001 From: songqing Date: Fri, 10 Apr 2026 15:44:15 +0800 Subject: [PATCH 2/2] add new test --- .../unit/hyperloglog/hyperloglog_test.go | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/gocase/unit/hyperloglog/hyperloglog_test.go b/tests/gocase/unit/hyperloglog/hyperloglog_test.go index 2aa4e3d55fc..28fc949cf89 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" @@ -240,4 +241,29 @@ func TestHyperLogLog(t *testing.T) { 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)") + }) }