propagate changes for `hsetex` with `PXAT` absolute timestamps by frostzt · Pull Request #8 · ranshid/valkey

frostzt · 2026-01-14T06:05:04Z

Created this so that you can actively review this @ranshid

ranshid · 2026-01-14T13:19:22Z

@frostzt PTAL. I changed the implementation foir hincr/hset/hsetnx to not propagate hdel. Still in our case ONLY when hsetex has the KEEPTTL flag, it will have to propagate. Also my changes around hsetex can be used to help your case

…fix-propagate-hsetex-when-expired

frostzt · 2026-01-14T14:26:30Z

@ranshid can you check once? I am now doing it in 3 stages:

If the field does not exist we use HDEL
If the field has KEEPTTL and has an expiry I send the command with PXAT
For field without expiry its just HSET

ranshid · 2026-01-14T16:58:55Z

@frostzt this is a big change you placed there :) I am not even sure why you need to go that far.
The way I see it is this:

We only need to collect the fields pointers (just the field name from the argument - c->argv[i]) into an array which could have been pre-allocated (you can also place it on the stack, but risk a stack overflow) according to the command number of fields (not fields+values)
What we need to do now s that in the place where I already placed the "hexpired" report:

else {
            if (expired_overriten) {
                server.stat_expiredfields += expired_overriten;
                notifyKeyspaceEvent(NOTIFY_HASH, "hexpired", c->argv[1], c->db->id);
            }

we need to run a loop calling propagateFieldsDeletion on the collected items and loop to the next collected items based on the value it returned. That is it :)

frostzt · 2026-01-14T17:01:15Z

@frostzt this is a big change you placed there :) I am not even sure why you need to go that far. The way I see it is this:
1. We only need to collect the fields pointers (just the field name from the argument - c->argv[i]) into an array which could have been pre-allocated (you can also place it on the stack, but risk a stack overflow) according to the command number of fields (not fields+values)

2. What we need to do now s that in the place where I already placed the "hexpired" report:
else {
            if (expired_overriten) {
                server.stat_expiredfields += expired_overriten;
                notifyKeyspaceEvent(NOTIFY_HASH, "hexpired", c->argv[1], c->db->id);
            }
we need to run a loop calling propagateFieldsDeletion on the collected items and loop to the next collected items based on the value it returned. That is it :)

Ah forgive me for that I kinda thought of it in a sequential way lemme update this, and thanks a lot for bearing with me 😭

ranshid

dropped some comments

ranshid · 2026-01-14T18:53:20Z

        } else {
-            if (expired_overriten) {
+            /* Propagate deletions for expired/non-existent fields in batches */
+            if (keepttl_count > 0) {


so keepttl_count and expired_overriten are always identical (if we decide to increment the expired_overriten whenever expired is set by hashTypeSet). I think we need not use 2 different values and we can simply use expired_overriten. I think the propagation logic should be applied here, but only in case keepttl_fields is not NULL.
The reason is that when KEEPTTL is provided the flag is set for ALL fields.

ranshid · 2026-01-14T20:55:42Z

@frostzt mostly LGTM. can you please add a test in hashexopire.tcl (you can do something in the inspiration of valkey-io#3036 and place it in the same place I placed all the other tests in valkey-io#3060

frostzt · 2026-01-14T21:49:16Z

            if (expired) expired_overriten++;
            changes++;
-            if (need_rewrite_argv) {
+
+            /* When KEEPTTL is used, we need to track all fields to propagate them
+             * individually with their actual timestamps */
+            if ((flags & ARGS_KEEPTTL) && expired) {
+                if (keepttl_fields == NULL) {
+                    keepttl_fields = zmalloc(sizeof(robj *) * num_fields);
+                }
+                keepttl_fields[expired_overriten] = c->argv[i];
+                incrRefCount(c->argv[i]);


@ranshid I think I messed up here! Caught it while running tests locally I increased expired_overriten however we're using that as index which is causing the server to panic, I think a simple post increase should be good?

Yeh it should be increased after we use it to index the position

ranshid · 2026-01-15T09:53:28Z

@frostzt 2 issues:

in order to support the KEEPTTL we need to fix how hashTypeSet function manages expirations.
This means that instead of doing:

is_expired = timestampIsExpired(entry_expiry);

we should do:

is_expired = entry_expiry != EXPIRY_NONE && checkAlreadyExpired(entry_expiry);

this is in order to make sure replicas/import mode and slot migration are operating correctly by forcing the logic instructed by the replication/import/migration stream
I will make this change in my PR since it will also require some of my tests to change.

The test is not really testing what we wanted.
We have 2 issues with KEEPTTL:
a. the case of a primary HSETEX overwrites an expired field which was not yet reclaimed by the ActiveExpiration
b. the case of primary HSETEX overwrites an NOT expired field BUT when the command reaches the replica it will use the old way (as explained in #(1) - timestampIsExpired) and thus will do something different than what the primary was doing.
The propagation of the hdel which you are working on, is required to solve case 'a' AFTER we solve case 'b' :)
I mean the fix in #(1) will introduce a problem in the case of 'a' which is why we will propagate hdel in order to make sure the replica will not KEPPTTL of a field which the primary ALOS expired by overwrting it.

I suggest 2 tests for each of the cases a/b:

 test {HSETEX KEEPTTL replica should preserve ttl when field is not expired on primary} {
            lassign [setup_replication_test $primary $replica $primary_host $primary_port] primary_initial_expired replica_initial_expired
            $primary debug set-active-expire 0

            $primary hset myhash f1 v1

            wait_for_ofs_sync $primary $replica

            pause_process $replica_pid
            
            $primary multi
            $primary hpexpire myhash 1 fields 1 f1
            $primary hsetex myhash KEEPTTL fields 1 f1 v2
            $primary exec

            # wait for f1 to expired
            wait_for_condition 50 100 {
                [$primary httl myhash fields 1 f1] == -2
            } else {
                fail "Field was not logically expired on primary"
            }

            resume_process $replica_pid

            wait_for_ofs_sync $primary $replica

            assert_equal {-2} [$primary httl myhash fields 1 f1]
            assert_equal {-2} [$replica httl myhash fields 1 f1]
            $primary debug set-active-expire 1
        } {OK} {needs:debug}

        test {HSETEX KEEPTTL replica should NOT preserve ttl when field is expired on primary} {
            lassign [setup_replication_test $primary $replica $primary_host $primary_port] primary_initial_expired replica_initial_expired
            $primary debug set-active-expire 0

            # write a short lived field on the primary and wait for the propagation
            $primary hsetex myhash PX 1 fields 1 f1 v1
        
            # wait for f1 to expired
            wait_for_condition 50 100 {
                [$primary httl myhash fields 1 f1] == -2
            } else {
                fail "Field was not logically expired on primary"
            }

            # Now overite the expired field on the primary and wait for it to propagate to the replica
            $primary hsetex myhash KEEPTTL fields 1 f1 v2
            wait_for_ofs_sync $primary $replica

            assert_equal {v2} [$primary hget myhash f1]
            assert_equal {v2} [$replica hget myhash f1]
            $primary debug set-active-expire 1
        } {OK} {needs:debug}

ranshid · 2026-01-15T09:57:59Z

@frostzt for time constraints I made some of the changes I suggested in my PR. please merge it to yours and focus on the new hdel propagation.
Currently in my latest changes the test HSETEX KEEPTTL replica should NOT preserve ttl when field is expired on primary will fail. and your fix is the one that should make it work again

…e-expired-element' into fix-propagate-hsetex-when-expired

frostzt · 2026-01-15T10:21:11Z

@frostzt for time constraints I made some of the changes I suggested in my PR. please merge it to yours and focus on the new hdel propagation. Currently in my latest changes the test HSETEX KEEPTTL replica should NOT preserve ttl when field is expired on primary will fail. and your fix is the one that should make it work again

Thanks @ranshid just took a pull!
Question: With the above by doing this

is_expired = entry_expiry != EXPIRY_NONE && checkAlreadyExpired(entry_expiry);

This essentially is for fields that don't define a TTL. My question now is for hdel propagations this would impact the above that I have written right? What should my logical flow for tackling this be?

ranshid · 2026-01-15T10:26:29Z

+                }
+                keepttl_fields[expired_overriten] = c->argv[i];
+                incrRefCount(c->argv[i]);
+            } else if (need_rewrite_argv) {


we also rewrite the argv when we have the FXX/FNX and NX/XX flags, so we cannot "else if" here

ok this fixed the test, @ranshid if you don't mind (I'll push the changes) but can you explain this to me a little more?

more about what?

ranshid · 2026-01-15T10:33:27Z

@frostzt for time constraints I made some of the changes I suggested in my PR. please merge it to yours and focus on the new hdel propagation. Currently in my latest changes the test HSETEX KEEPTTL replica should NOT preserve ttl when field is expired on primary will fail. and your fix is the one that should make it work again

Thanks @ranshid just took a pull! Question: With the above by doing this
is_expired = entry_expiry != EXPIRY_NONE && checkAlreadyExpired(entry_expiry);
This essentially is for fields that don't define a TTL. My question now is for hdel propagations this would impact the above that I have written right? What should my logical flow for tackling this be?

Your currently implemented logic is fine. In hsetex when we got an indication that we overwritten an expired field and KEEPTTL is used (ONLY THEN) we should ALSO propagate hdel the command itself will be propagated right after

ranshid · 2026-01-15T10:37:05Z

        } else {
-            if (expired_overriten) {
+            /* Propagate deletions for expired/non-existent fields in batches */
+            if (keepttl_fields != NULL) {


this will mask the next 2 lines which SHOULD be executed even if keepttl_fields is null. keepttl_fields is not NULL only when we overwritten expired fields AND KEEPTTL flag was provided. however even if KEEPTTL was not provided, we might have overwritten some fields which were found to be expired

oh snap yeah, that makes sense

I think I just attached my thinking that overritten == overritten with KEEPTTL

no, so this is why I split the effort. the overwriting of expired keys is a general problem/bug I fixed in my PR. the specific case of KEEPTTL is another issue

ranshid · 2026-01-15T10:39:37Z

Thank you sooooo much @frostzt !

ranshid · 2026-01-15T10:42:22Z

+                for (int i = 0; i < expired_overriten; i++) {
+                    decrRefCount(keepttl_fields[i]);
+                }


still not sure why this is needed?

oh checking the code for propagateFieldDeletion I get it my bad again I didn't knew that it does that already!

yes, the ref count ownership is passed to this function.
I suggest always compile and test with ASAN:
make -j noopt SANITIZER=address OPTIMIZATION=-O0

ranshid · 2026-01-15T10:47:11Z

Great @frostzt I will now merge it and take it from here! much much Thank you!

frostzt · 2026-01-15T10:47:57Z

Thank you sooooo much @frostzt !

I honestly don't know if I deserve that but all thanks to you I really wanted to work in Valkey and have been just mesmerizing its code I can assure you the effort you put in to help me means a lot honestly I learned so much I'll keep up and keep contributing into this and hopefully will be able to write much better PRs (that will be easier for you to review 😛 )

…lkey-io#3174) I was working on ASAN large memory tests when I countered this issue. The issue was that the hardcoded `999` key could land in an early bucket. Then shrink rehash could finish early, and later inserts could trigger a new expansion rehash, resetting rehash_idx low. The test now picks the survivor key dynamically as the key mapped to the highest bucket index. ``` [test_hashtable.c] Memory leak detected of 336 bytes ================================================================= ==3901==ERROR: LeakSanitizer: detected memory leaks Direct leak of 80 byte(s) in 1 object(s) allocated from: #0 0x7fb0556fd9c7 in malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69 #1 0x563bfdf4c47d in ztrymalloc_usable_internal /home/runner/work/valkey/valkey/src/zmalloc.c:156 #2 0x563bfdf4c47d in valkey_malloc /home/runner/work/valkey/valkey/src/zmalloc.c:185 #3 0x563bfdd42eaf in hashtableCreate /home/runner/work/valkey/valkey/src/hashtable.c:1217 #4 0x563bfdaa1cbf in test_empty_buckets_rehashing unit/test_hashtable.c:232 #5 0x563bfdae772b in runTestSuite unit/test_main.c:36 #6 0x563bfda86b20 in main unit/test_main.c:108 #7 0x7fb05522a1c9 (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e) #8 0x7fb05522a28a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e) valkey-io#9 0x563bfda8a5c4 in _start (/home/runner/work/valkey/valkey/src/valkey-unit-tests+0x17c5c4) (BuildId: 44cfc183e6e82e499bcc9f6adc094d7f774ee9d2) Indirect leak of 128 byte(s) in 1 object(s) allocated from: #0 0x7fb0556fd340 in calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:77 #1 0x563bfdf4c922 in ztrycalloc_usable_internal /home/runner/work/valkey/valkey/src/zmalloc.c:214 #2 0x563bfdf4c922 in valkey_calloc /home/runner/work/valkey/valkey/src/zmalloc.c:257 #3 0x563bfdd40967 in resize /home/runner/work/valkey/valkey/src/hashtable.c:741 #4 0x563bfdd45eb1 in hashtableExpandIfNeeded /home/runner/work/valkey/valkey/src/hashtable.c:1446 #5 0x563bfdd45eb1 in hashtableExpandIfNeeded /home/runner/work/valkey/valkey/src/hashtable.c:1433 #6 0x563bfdd45eb1 in insert /home/runner/work/valkey/valkey/src/hashtable.c:1041 #7 0x563bfdd45eb1 in hashtableAddOrFind /home/runner/work/valkey/valkey/src/hashtable.c:1554 #8 0x563bfdd45eb1 in hashtableAdd /home/runner/work/valkey/valkey/src/hashtable.c:1539 valkey-io#9 0x563bfdaa1e3b in test_empty_buckets_rehashing unit/test_hashtable.c:254 valkey-io#10 0x563bfdae772b in runTestSuite unit/test_main.c:36 valkey-io#11 0x563bfda86b20 in main unit/test_main.c:108 valkey-io#12 0x7fb05522a1c9 (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e) valkey-io#13 0x7fb05522a28a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e) valkey-io#14 0x563bfda8a5c4 in _start (/home/runner/work/valkey/valkey/src/valkey-unit-tests+0x17c5c4) (BuildId: 44cfc183e6e82e499bcc9f6adc094d7f774ee9d2) Indirect leak of 64 byte(s) in 1 object(s) allocated from: #0 0x7fb0556fd340 in calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:77 #1 0x563bfdf4c922 in ztrycalloc_usable_internal /home/runner/work/valkey/valkey/src/zmalloc.c:214 #2 0x563bfdf4c922 in valkey_calloc /home/runner/work/valkey/valkey/src/zmalloc.c:257 #3 0x563bfdd3f553 in bucketConvertToChained /home/runner/work/valkey/valkey/src/hashtable.c:908 #4 0x563bfdd3f553 in findBucketForInsert /home/runner/work/valkey/valkey/src/hashtable.c:1021 #5 0x563bfdd45d9e in insert /home/runner/work/valkey/valkey/src/hashtable.c:1045 #6 0x563bfdd45d9e in hashtableAddOrFind /home/runner/work/valkey/valkey/src/hashtable.c:1554 #7 0x563bfdd45d9e in hashtableAdd /home/runner/work/valkey/valkey/src/hashtable.c:1539 #8 0x563bfdaa1e3b in test_empty_buckets_rehashing unit/test_hashtable.c:254 valkey-io#9 0x563bfdae772b in runTestSuite unit/test_main.c:36 valkey-io#10 0x563bfda86b20 in main unit/test_main.c:108 valkey-io#11 0x7fb05522a1c9 (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e) valkey-io#12 0x7fb05522a28a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e) valkey-io#13 0x563bfda8a5c4 in _start (/home/runner/work/valkey/valkey/src/valkey-unit-tests+0x17c5c4) (BuildId: 44cfc183e6e82e499bcc9f6adc094d7f774ee9d2) Indirect leak of 64 byte(s) in 1 object(s) allocated from: #0 0x7fb0556fd340 in calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:77 #1 0x563bfdf4c922 in ztrycalloc_usable_internal /home/runner/work/valkey/valkey/src/zmalloc.c:214 #2 0x563bfdf4c922 in valkey_calloc /home/runner/work/valkey/valkey/src/zmalloc.c:257 #3 0x563bfdd40967 in resize /home/runner/work/valkey/valkey/src/hashtable.c:741 #4 0x563bfdaa1df8 in test_empty_buckets_rehashing unit/test_hashtable.c:248 #5 0x563bfdae772b in runTestSuite unit/test_main.c:36 #6 0x563bfda86b20 in main unit/test_main.c:108 #7 0x7fb05522a1c9 (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e) #8 0x7fb05522a28a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e) valkey-io#9 0x563bfda8a5c4 in _start (/home/runner/work/valkey/valkey/src/valkey-unit-tests+0x17c5c4) (BuildId: 44cfc183e6e82e499bcc9f6adc094d7f774ee9d2) SUMMARY: AddressSanitizer: 336 byte(s) leaked in 4 allocation(s). ``` Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>

…y-io#2257) **Current state** During `hashtableScanDefrag`, rehashing is paused to prevent entries from moving, but the scan callback can still delete entries which triggers `hashtableShrinkIfNeeded`. For example, the `expireScanCallback` can delete expired entries. **Issue** This can cause the table to be resized and the old memory to be freed while the scan is still accessing it, resulting in the following memory access violation: ``` [err]: Sanitizer error: ================================================================= ==46774==ERROR: AddressSanitizer: heap-use-after-free on address 0x611000003100 at pc 0x0000004704d3 bp 0x7fffcb062000 sp 0x7fffcb061ff0 READ of size 1 at 0x611000003100 thread T0 #0 0x4704d2 in isPositionFilled /home/gusakovy/Projects/valkey/src/hashtable.c:422 #1 0x478b45 in hashtableScanDefrag /home/gusakovy/Projects/valkey/src/hashtable.c:1768 #2 0x4789c2 in hashtableScan /home/gusakovy/Projects/valkey/src/hashtable.c:1729 #3 0x47e3ca in kvstoreScan /home/gusakovy/Projects/valkey/src/kvstore.c:402 #4 0x6d9040 in activeExpireCycle /home/gusakovy/Projects/valkey/src/expire.c:297 #5 0x4859d2 in databasesCron /home/gusakovy/Projects/valkey/src/server.c:1269 #6 0x486e92 in serverCron /home/gusakovy/Projects/valkey/src/server.c:1577 #7 0x4637dd in processTimeEvents /home/gusakovy/Projects/valkey/src/ae.c:370 #8 0x4643e3 in aeProcessEvents /home/gusakovy/Projects/valkey/src/ae.c:513 valkey-io#9 0x4647ea in aeMain /home/gusakovy/Projects/valkey/src/ae.c:543 valkey-io#10 0x4a61fc in main /home/gusakovy/Projects/valkey/src/server.c:7291 valkey-io#11 0x7f471957c139 in __libc_start_main (/lib64/libc.so.6+0x21139) valkey-io#12 0x452e39 in _start (/local/home/gusakovy/Projects/valkey/src/valkey-server+0x452e39) 0x611000003100 is located 0 bytes inside of 256-byte region [0x611000003100,0x611000003200) freed by thread T0 here: #0 0x7f471a34a1e5 in __interceptor_free (/lib64/libasan.so.4+0xd81e5) #1 0x4aefbc in zfree_internal /home/gusakovy/Projects/valkey/src/zmalloc.c:400 #2 0x4aeff5 in valkey_free /home/gusakovy/Projects/valkey/src/zmalloc.c:415 #3 0x4707d2 in rehashingCompleted /home/gusakovy/Projects/valkey/src/hashtable.c:456 #4 0x471b5b in resize /home/gusakovy/Projects/valkey/src/hashtable.c:656 #5 0x475bff in hashtableShrinkIfNeeded /home/gusakovy/Projects/valkey/src/hashtable.c:1272 #6 0x47704b in hashtablePop /home/gusakovy/Projects/valkey/src/hashtable.c:1448 #7 0x47716f in hashtableDelete /home/gusakovy/Projects/valkey/src/hashtable.c:1459 #8 0x480038 in kvstoreHashtableDelete /home/gusakovy/Projects/valkey/src/kvstore.c:847 valkey-io#9 0x50c12c in dbGenericDeleteWithDictIndex /home/gusakovy/Projects/valkey/src/db.c:490 valkey-io#10 0x515f28 in deleteExpiredKeyAndPropagateWithDictIndex /home/gusakovy/Projects/valkey/src/db.c:1831 valkey-io#11 0x516103 in deleteExpiredKeyAndPropagate /home/gusakovy/Projects/valkey/src/db.c:1844 valkey-io#12 0x6d8642 in activeExpireCycleTryExpire /home/gusakovy/Projects/valkey/src/expire.c:70 valkey-io#13 0x6d8706 in expireScanCallback /home/gusakovy/Projects/valkey/src/expire.c:139 valkey-io#14 0x478bd8 in hashtableScanDefrag /home/gusakovy/Projects/valkey/src/hashtable.c:1770 valkey-io#15 0x4789c2 in hashtableScan /home/gusakovy/Projects/valkey/src/hashtable.c:1729 valkey-io#16 0x47e3ca in kvstoreScan /home/gusakovy/Projects/valkey/src/kvstore.c:402 valkey-io#17 0x6d9040 in activeExpireCycle /home/gusakovy/Projects/valkey/src/expire.c:297 valkey-io#18 0x4859d2 in databasesCron /home/gusakovy/Projects/valkey/src/server.c:1269 valkey-io#19 0x486e92 in serverCron /home/gusakovy/Projects/valkey/src/server.c:1577 valkey-io#20 0x4637dd in processTimeEvents /home/gusakovy/Projects/valkey/src/ae.c:370 valkey-io#21 0x4643e3 in aeProcessEvents /home/gusakovy/Projects/valkey/src/ae.c:513 valkey-io#22 0x4647ea in aeMain /home/gusakovy/Projects/valkey/src/ae.c:543 valkey-io#23 0x4a61fc in main /home/gusakovy/Projects/valkey/src/server.c:7291 valkey-io#24 0x7f471957c139 in __libc_start_main (/lib64/libc.so.6+0x21139) previously allocated by thread T0 here: #0 0x7f471a34a753 in __interceptor_calloc (/lib64/libasan.so.4+0xd8753) #1 0x4ae48c in ztrycalloc_usable_internal /home/gusakovy/Projects/valkey/src/zmalloc.c:214 #2 0x4ae757 in valkey_calloc /home/gusakovy/Projects/valkey/src/zmalloc.c:257 #3 0x4718fc in resize /home/gusakovy/Projects/valkey/src/hashtable.c:645 #4 0x475bff in hashtableShrinkIfNeeded /home/gusakovy/Projects/valkey/src/hashtable.c:1272 #5 0x47704b in hashtablePop /home/gusakovy/Projects/valkey/src/hashtable.c:1448 #6 0x47716f in hashtableDelete /home/gusakovy/Projects/valkey/src/hashtable.c:1459 #7 0x480038 in kvstoreHashtableDelete /home/gusakovy/Projects/valkey/src/kvstore.c:847 #8 0x50c12c in dbGenericDeleteWithDictIndex /home/gusakovy/Projects/valkey/src/db.c:490 valkey-io#9 0x515f28 in deleteExpiredKeyAndPropagateWithDictIndex /home/gusakovy/Projects/valkey/src/db.c:1831 valkey-io#10 0x516103 in deleteExpiredKeyAndPropagate /home/gusakovy/Projects/valkey/src/db.c:1844 valkey-io#11 0x6d8642 in activeExpireCycleTryExpire /home/gusakovy/Projects/valkey/src/expire.c:70 valkey-io#12 0x6d8706 in expireScanCallback /home/gusakovy/Projects/valkey/src/expire.c:139 valkey-io#13 0x478bd8 in hashtableScanDefrag /home/gusakovy/Projects/valkey/src/hashtable.c:1770 valkey-io#14 0x4789c2 in hashtableScan /home/gusakovy/Projects/valkey/src/hashtable.c:1729 valkey-io#15 0x47e3ca in kvstoreScan /home/gusakovy/Projects/valkey/src/kvstore.c:402 valkey-io#16 0x6d9040 in activeExpireCycle /home/gusakovy/Projects/valkey/src/expire.c:297 valkey-io#17 0x4859d2 in databasesCron /home/gusakovy/Projects/valkey/src/server.c:1269 valkey-io#18 0x486e92 in serverCron /home/gusakovy/Projects/valkey/src/server.c:1577 valkey-io#19 0x4637dd in processTimeEvents /home/gusakovy/Projects/valkey/src/ae.c:370 valkey-io#20 0x4643e3 in aeProcessEvents /home/gusakovy/Projects/valkey/src/ae.c:513 valkey-io#21 0x4647ea in aeMain /home/gusakovy/Projects/valkey/src/ae.c:543 valkey-io#22 0x4a61fc in main /home/gusakovy/Projects/valkey/src/server.c:7291 valkey-io#23 0x7f471957c139 in __libc_start_main (/lib64/libc.so.6+0x21139) SUMMARY: AddressSanitizer: heap-use-after-free /home/gusakovy/Projects/valkey/src/hashtable.c:422 in isPositionFilled Shadow bytes around the buggy address: 0x0c227fff85d0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c227fff85e0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c227fff85f0: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd 0x0c227fff8600: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c227fff8610: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa =>0x0c227fff8620:[fd]fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c227fff8630: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c227fff8640: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c227fff8650: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c227fff8660: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c227fff8670: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb ==46774==ABORTING ``` **Solution** Suggested solution is to also pause auto shrinking during `hashtableScanDefrag`. I noticed that there was already a `hashtablePauseAutoShrink` method and `pause_auto_shrink` counter, but it wasn't actually used in `hashtableShrinkIfNeeded` so I fixed that. **Testing** I created a simple tcl test that (most of the times) triggers this error, but it's a little clunky so I didn't add it as part of the PR: ``` start_server {tags {"expire hashtable defrag"}} { test {hashtable scan defrag on expiry} { r config set hz 100 set num_keys 20 for {set i 0} {$i < $num_keys} {incr i} { r set "key_$i" "value_$i" } for {set j 0} {$j < 50} {incr j} { set expire_keys 100 for {set i 0} {$i < $expire_keys} {incr i} { # Short expiry time to ensure they expire quickly r psetex "expire_key_${i}_${j}" 100 "expire_value_${i}_${j}" } # Verify keys are set set initial_size [r dbsize] assert_equal $initial_size [expr $num_keys + $expire_keys] after 150 for {set i 0} {$i < 10} {incr i} { r get "expire_key_${i}_${j}" after 10 } } set remaining_keys [r dbsize] assert_equal $remaining_keys $num_keys # Verify server is still responsive assert_equal [r ping] {PONG} } {} } ``` Compiling with ASAN using `make noopt SANITIZER=address valkey-server` and running the test causes error above. Applying the fix resolves the issue. Signed-off-by: Yakov Gusakov <yaakov0015@gmail.com>

simple draft for propagate changes for hsetex deletions

103f6a3

frostzt commented Jan 14, 2026

View reviewed changes

Comment thread src/t_hash.c Outdated

frostzt mentioned this pull request Jan 14, 2026

Fix how hash is handling overriding of expired fields overwrite valkey-io/valkey#3060

Merged

frostzt added 3 commits January 14, 2026 18:52

Merge branch 'fix-propagate-hdel-when-override-expired-element' into …

becef3e

…fix-propagate-hsetex-when-expired

send KEEPTTL Commands with PXAT timestamps

89a1037

free up if no changes were done

158f0be

frostzt requested a review from ranshid January 14, 2026 14:26

chore: remove extra whitespaces

11702da

frostzt changed the title ~~simple draft for propagate changes for hsetex deletions~~ propagate changes for hsetex with PXAT absolute timestamps Jan 14, 2026

chore: propagate deleted fields

74b8282

ranshid reviewed Jan 14, 2026

View reviewed changes

Comment thread src/t_hash.c Outdated

Comment thread src/t_hash.c Outdated

Comment thread src/t_hash.c Outdated

Comment thread src/t_hash.c Outdated

Comment thread src/t_hash.c Outdated

chore: review changes

7a9b20a

frostzt requested a review from ranshid January 14, 2026 18:42

ranshid reviewed Jan 14, 2026

View reviewed changes

use expired_overriten and combine with keepttl_count

7c16acf

frostzt requested a review from ranshid January 14, 2026 20:44

frostzt commented Jan 14, 2026

View reviewed changes

added a simple test

01f502b

Merge remote-tracking branch 'ranshid/fix-propagate-hdel-when-overrid…

393bc85

…e-expired-element' into fix-propagate-hsetex-when-expired

ranshid reviewed Jan 15, 2026

View reviewed changes

Comment thread tests/unit/hashexpire.tcl Outdated

remove redundant test

f433059

ranshid reviewed Jan 15, 2026

View reviewed changes

fix rewrite argv for flags

0ad4a7e

ranshid reviewed Jan 15, 2026

View reviewed changes

add expired overriten for overwritten fields

10018ce

ranshid reviewed Jan 15, 2026

View reviewed changes

propagateFieldDeletion alredy handles refcount

8119b29

ranshid merged commit 7faa9a2 into ranshid:fix-propagate-hdel-when-override-expired-element Jan 15, 2026

Conversation

frostzt commented Jan 14, 2026

Uh oh!

Uh oh!

ranshid commented Jan 14, 2026

Uh oh!

frostzt commented Jan 14, 2026

Uh oh!

ranshid commented Jan 14, 2026

Uh oh!

frostzt commented Jan 14, 2026

Uh oh!

ranshid left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ranshid commented Jan 14, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ranshid commented Jan 15, 2026

Uh oh!

ranshid commented Jan 15, 2026

Uh oh!

frostzt commented Jan 15, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ranshid commented Jan 15, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ranshid commented Jan 15, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ranshid commented Jan 15, 2026

Uh oh!

frostzt commented Jan 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants