Allow partial sync after loading AOF with preamble #2366

arthurkiller · 2025-07-17T14:48:40Z

The AOF preamble mechanism replaces the traditional AOF base file with an RDB snapshot during rewrite operations, which reduces I/O overhead and improves loading performance.
However, when valkey loads the RDB-formatted preamble from the base AOF file, it does not process the replication ID (replid) information within the RDB AUX fields. This omission has two limitations:

On a primary, it prevents the primary from accepting PSYNC continue requests after restarting with a preamble-enabled AOF file.
On a replica, it prevents the replica from successfully performing partial sync requests (avoiding full sync) after restarting with a preamble-enabled AOF file.

To resolve this, this commit aligns the AOF preamble handling with the logic used for standalone RDB files, by storing the replication ID and replication offset in the AOF preamble and restoring them when loading the AOF file.

Resolves #2677

secwall · 2025-07-25T09:59:59Z

./runtest-cluster fails on my machine on new assert with backlog (serverAssert(server.repl_backlog)).
Could we also write some tests on this? For example for positive scenario?

arthurkiller · 2025-07-26T04:47:54Z

./runtest-cluster fails on my machine on new assert with backlog (serverAssert(server.repl_backlog)). Could we also write some tests on this? For example for positive scenario?

thx for your interest. The main purpose of this PR is to clarify why we missed this in preamble-aof, and make sure adding this will not break the consistency of data and replication.

I will try add more test cases.

arthurkiller · 2025-07-28T03:22:48Z

./runtest-cluster fails on my machine on new assert with backlog (serverAssert(server.repl_backlog)). Could we also write some tests on this? For example for positive scenario?

@secwall I still can not get an error after a weekend of testing. Can you give some hints on reproducing?

secwall · 2025-07-28T08:07:58Z

Here is how I get an error:

git clone https://github.com/arthurkiller/valkey.git
cd valkey
git checkout fix/load-replid-from-preamble-base-aof
make -j
./runtest-cluster

It fails on failover loop test:

*** Crash report found in valkey_6/log.txt ***
=== VALKEY BUG REPORT START: Cut & paste starting from here ===
1555848:M 28 Jul 2025 11:02:33.690 # === ASSERTION FAILED ===
1555848:M 28 Jul 2025 11:02:33.690 # ==> aof.c:1488 'server.repl_backlog' is not true

------ STACK TRACE ------

1555848 valkey-server *
../../../src/valkey-server *:30006 [cluster](loadSingleAppendOnlyFile+0xf39)[0x5d6ce14d5af9]
../../../src/valkey-server *:30006 [cluster](loadAppendOnlyFiles+0x112)[0x5d6ce14d5cd2]
../../../src/valkey-server *:30006 [cluster](loadDataFromDisk+0x114)[0x5d6ce1443434]
../../../src/valkey-server *:30006 [cluster](main+0x378)[0x5d6ce1415778]
/lib/x86_64-linux-gnu/libc.so.6(+0x2a1ca)[0x73b7bf42a1ca]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0x8b)[0x73b7bf42a28b]
../../../src/valkey-server *:30006 [cluster](_start+0x25)[0x5d6ce14171a5]

1555849 bio_close_file
/lib/x86_64-linux-gnu/libc.so.6(+0x98d71)[0x73b7bf498d71]
/lib/x86_64-linux-gnu/libc.so.6(pthread_cond_wait+0x20d)[0x73b7bf49b7ed]
../../../src/valkey-server *:30006 [cluster](bioProcessBackgroundJobs+0x1af)[0x5d6ce1516a9f]
/lib/x86_64-linux-gnu/libc.so.6(+0x9caa4)[0x73b7bf49caa4]
/lib/x86_64-linux-gnu/libc.so.6(+0x129c3c)[0x73b7bf529c3c]

1555850 bio_aof
/lib/x86_64-linux-gnu/libc.so.6(+0x98d71)[0x73b7bf498d71]
/lib/x86_64-linux-gnu/libc.so.6(pthread_cond_wait+0x20d)[0x73b7bf49b7ed]
../../../src/valkey-server *:30006 [cluster](bioProcessBackgroundJobs+0x1af)[0x5d6ce1516a9f]
/lib/x86_64-linux-gnu/libc.so.6(+0x9caa4)[0x73b7bf49caa4]
/lib/x86_64-linux-gnu/libc.so.6(+0x129c3c)[0x73b7bf529c3c]

1555851 bio_lazy_free
/lib/x86_64-linux-gnu/libc.so.6(+0x98d71)[0x73b7bf498d71]
/lib/x86_64-linux-gnu/libc.so.6(pthread_cond_wait+0x20d)[0x73b7bf49b7ed]
../../../src/valkey-server *:30006 [cluster](bioProcessBackgroundJobs+0x1af)[0x5d6ce1516a9f]
/lib/x86_64-linux-gnu/libc.so.6(+0x9caa4)[0x73b7bf49caa4]
/lib/x86_64-linux-gnu/libc.so.6(+0x129c3c)[0x73b7bf529c3c]

4/4 expected stacktraces.

------ STACK TRACE DONE ------

codecov · 2025-08-06T06:58:06Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.37%. Comparing base (54da834) to head (1d03246).
⚠️ Report is 43 commits behind head on unstable.

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable    #2366      +/-   ##
============================================
- Coverage     72.64%   72.37%   -0.27%     
============================================
  Files           128      128              
  Lines         71278    70284     -994     
============================================
- Hits          51777    50866     -911     
+ Misses        19501    19418      -83

Files with missing lines	Coverage Δ
src/aof.c	`81.19% <100.00%> (+0.01%)`	⬆️
src/rdb.c	`77.17% <100.00%> (-0.39%)`	⬇️
src/rdb.h	`100.00% <ø> (ø)`
src/server.c	`88.39% <100.00%> (-0.04%)`	⬇️

... and 101 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

ranshid · 2025-08-06T13:00:00Z

@arthurkiller can you also signoff the commits?

arthurkiller · 2025-08-06T13:12:11Z

@arthurkiller can you also signoff the commits?

@ranshid done

@secwall I've fixed this. Cause repl_buffer is created when we get a replica attached. We'd create this on data load. I'll add a tcl case for this later.

Not so sure if we use this replid from the previous RDB, will it make any side effects

arthurkiller · 2025-08-06T13:14:57Z

Here is how I get an error:

git clone https://github.com/arthurkiller/valkey.git
cd valkey
git checkout fix/load-replid-from-preamble-base-aof
make -j
./runtest-cluster

It fails on failover loop test:

*** Crash report found in valkey_6/log.txt ***
=== VALKEY BUG REPORT START: Cut & paste starting from here ===
1555848:M 28 Jul 2025 11:02:33.690 # === ASSERTION FAILED ===
1555848:M 28 Jul 2025 11:02:33.690 # ==> aof.c:1488 'server.repl_backlog' is not true

------ STACK TRACE ------

1555848 valkey-server *
../../../src/valkey-server *:30006 [cluster](loadSingleAppendOnlyFile+0xf39)[0x5d6ce14d5af9]
../../../src/valkey-server *:30006 [cluster](loadAppendOnlyFiles+0x112)[0x5d6ce14d5cd2]
../../../src/valkey-server *:30006 [cluster](loadDataFromDisk+0x114)[0x5d6ce1443434]
../../../src/valkey-server *:30006 [cluster](main+0x378)[0x5d6ce1415778]
/lib/x86_64-linux-gnu/libc.so.6(+0x2a1ca)[0x73b7bf42a1ca]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0x8b)[0x73b7bf42a28b]
../../../src/valkey-server *:30006 [cluster](_start+0x25)[0x5d6ce14171a5]

1555849 bio_close_file
/lib/x86_64-linux-gnu/libc.so.6(+0x98d71)[0x73b7bf498d71]
/lib/x86_64-linux-gnu/libc.so.6(pthread_cond_wait+0x20d)[0x73b7bf49b7ed]
../../../src/valkey-server *:30006 [cluster](bioProcessBackgroundJobs+0x1af)[0x5d6ce1516a9f]
/lib/x86_64-linux-gnu/libc.so.6(+0x9caa4)[0x73b7bf49caa4]
/lib/x86_64-linux-gnu/libc.so.6(+0x129c3c)[0x73b7bf529c3c]

1555850 bio_aof
/lib/x86_64-linux-gnu/libc.so.6(+0x98d71)[0x73b7bf498d71]
/lib/x86_64-linux-gnu/libc.so.6(pthread_cond_wait+0x20d)[0x73b7bf49b7ed]
../../../src/valkey-server *:30006 [cluster](bioProcessBackgroundJobs+0x1af)[0x5d6ce1516a9f]
/lib/x86_64-linux-gnu/libc.so.6(+0x9caa4)[0x73b7bf49caa4]
/lib/x86_64-linux-gnu/libc.so.6(+0x129c3c)[0x73b7bf529c3c]

1555851 bio_lazy_free
/lib/x86_64-linux-gnu/libc.so.6(+0x98d71)[0x73b7bf498d71]
/lib/x86_64-linux-gnu/libc.so.6(pthread_cond_wait+0x20d)[0x73b7bf49b7ed]
../../../src/valkey-server *:30006 [cluster](bioProcessBackgroundJobs+0x1af)[0x5d6ce1516a9f]
/lib/x86_64-linux-gnu/libc.so.6(+0x9caa4)[0x73b7bf49caa4]
/lib/x86_64-linux-gnu/libc.so.6(+0x129c3c)[0x73b7bf529c3c]

4/4 expected stacktraces.

------ STACK TRACE DONE ------

Currently, our cluster cases can not cover this situation

I still failed get this by run cluster test

21:10:51> Wait cluster to be stable: OK
21:10:51> Master #0 still should have its replicas: OK
21:10:51> Each master should have at least two replicas attached: OK
Testing unit: 28-cluster-shards.tcl
21:10:51> (init) Restart killed instances: OK
21:10:51> Cluster nodes are reachable: OK
21:10:51> Cluster nodes hard reset: OK
21:10:51> Cluster Join and auto-discovery test: OK
21:10:55> Before slots allocation, all nodes report cluster failure: OK
21:10:55> Create a 8 nodes cluster with 4 shards: OK
21:10:55> Cluster should start ok: OK
21:10:59> Set cluster hostnames and verify they are propagated: OK
21:11:01> Verify information about the shards: OK
21:11:01> Verify no slot shard: OK
21:11:01> Kill a node and tell the replica to immediately takeover: OK
21:11:02> Verify health as fail for killed node: OK
21:11:06> Restarting primary node: OK
21:11:06> Instance #0 gets converted into a replica: OK
21:11:06> Test the replica reports a loading state while it's loading: OK
21:11:09> Regression test for a crash when calling SHARDS during handshake: OK
21:11:09> Cluster is up: OK
21:11:12> Shard ids are unique: OK
21:11:12> CLUSTER MYSHARDID reports same id for both primary and replica: OK
21:11:12> New replica receives primary's shard id: OK
21:11:12> CLUSTER MYSHARDID reports same shard id after shard restart: OK
21:11:14> CLUSTER MYSHARDID reports same shard id after cluster restart: OK
Cleaning up...
killing stale instance 2625352
killing stale instance 2625364
killing stale instance 2625395
killing stale instance 2625405
killing stale instance 2625416
killing stale instance 2625432
killing stale instance 2625443
killing stale instance 2625452
killing stale instance 2625462
killing stale instance 2625473
killing stale instance 2626003
killing stale instance 2626009
killing stale instance 2628745
killing stale instance 2628751
killing stale instance 2628757
killing stale instance 2628763
killing stale instance 2628769
killing stale instance 2628775
killing stale instance 2628782
killing stale instance 2628789
GOOD! No errors.

zuiderkwast · 2025-09-22T13:43:43Z

Great finding @arthurkiller! One of the main points of AOF is to be able to avoid full sync after a restart, so it's pretty sad that it doesn't work and isn't tested. Maybe this is why our documentation about prsistence says this:

There are many users using AOF alone, but we discourage it since to have an RDB snapshot from time to time is a great idea for doing database backups, for faster restarts, and in the event of bugs in the AOF engine.

It's too late to get this in 9.0 but I hope we can have it ready for 9.1. Will you add a test of the scenario (AOF rewrite, restart, psync)?

arthurkiller · 2025-09-22T13:48:46Z

Great finding @arthurkiller! One of the main points of AOF is to be able to avoid full sync after a restart, so it's pretty sad that it doesn't work and isn't tested. Maybe this is why our documentation about prsistence says this:

There are many users using AOF alone, but we discourage it since to have an RDB snapshot from time to time is a great idea for doing database backups, for faster restarts, and in the event of bugs in the AOF engine.

It's too late to get this in 9.0 but I hope we can have it ready for 9.1. Will you add a test of the scenario (AOF rewrite, restart, psync)?

@zuiderkwast Thank you for your attention. I almost forgot about this Pull Request :) .

I'll try to add more comprehensive tests as soon as possible. Hectic these days.

zuiderkwast · 2025-10-07T15:08:54Z

When you have time, will you add these two in some file under tests/unit/cluster/:

Replica restart with AOF load, then PSYNC.
Primary loads AOF, then replica doing partial sync from the primary.

We should use an AOF file with preamble and multiple AOF commands after the preamble to check that the offset is calculated correctly.

Signed-off-by: arthur.lee <[email protected]>

zuiderkwast

Some hints for the failed CI jobs.

We can ignore this one because it's a known flaky test: https://github.com/valkey-io/valkey/actions/runs/18647217373/job/53157000675?pr=2366#step:4:8645

#2699

tests/integration/replication-4.tcl

arthurkiller · 2025-10-21T08:14:42Z

Some hints for the failed CI jobs.

We can ignore this one because it's a known flaky test: https://github.com/valkey-io/valkey/actions/runs/18647217373/job/53157000675?pr=2366#step:4:8645

[TEST-FAILURE] Primaries will not time out then they are elected in the same epoch #2699

I have fixed all the test cases, and now it can run stably.
But the implementation of the tcl cases is not perfect yet. I will try to find a way to remove the after in the case later.

arthurkiller · 2025-10-21T08:17:47Z

wait_for_ofs_sync and wait_for_sync did not work stably in these cases, and wait_for_log_messages is hard to use when I need to restart the server.

zuiderkwast · 2025-10-21T15:06:16Z

wait_for_log_messages is hard to use when I need to restart the server.

Not sure what the problem is, but maybe this helps.

Before you restart, you can get the number of lines in the log file using wc -l, and after restart, start searching from that line:

set logfile [srv 0 stdout]
set num_lines [lindex [exec wc -l $logfile] 0]
# ...
restart_server
# ...
wait_for_log_messages 0 [list {*Pattern*}] $num_lines 100 100

Another way is to count the matching lines before and after restart using count_message_lines $logfile $pattern.

Signed-off-by: arthur.lee <[email protected]>

arthurkiller · 2025-10-24T11:27:22Z

@zuiderkwast I think we are almost done, wc -l really helps.
The real problem here is restart server should be called with now. This will avoid additional writes, which will do harm to the replication offset in cases

src/aof.c

…eInfo Signed-off-by: arthur.lee <[email protected]>

src/rdb.c

Co-authored-by: Viktor Söderqvist <[email protected]> Signed-off-by: Arthur Lee <[email protected]> Signed-off-by: arthur.lee <[email protected]>

zuiderkwast

Very good, thank you for your patience!

I have only very minor comments about comments. (I can apply them before merging, if you don't.)

src/rdb.c

src/aof.c

Co-authored-by: Viktor Söderqvist <[email protected]> Signed-off-by: Arthur Lee <[email protected]>

arthurkiller · 2025-11-07T03:11:42Z

Thanks a lot :)

The AOF preamble mechanism replaces the traditional AOF base file with an RDB snapshot during rewrite operations, which reduces I/O overhead and improves loading performance. However, when valkey loads the RDB-formatted preamble from the base AOF file, it does not process the replication ID (replid) information within the RDB AUX fields. This omission has two limitations: * On a primary, it prevents the primary from accepting PSYNC continue requests after restarting with a preamble-enabled AOF file. * On a replica, it prevents the replica from successfully performing partial sync requests (avoiding full sync) after restarting with a preamble-enabled AOF file. To resolve this, this commit aligns the AOF preamble handling with the logic used for standalone RDB files, by storing the replication ID and replication offset in the AOF preamble and restoring them when loading the AOF file. Resolves valkey-io#2677 --------- Signed-off-by: arthur.lee <[email protected]> Signed-off-by: Arthur Lee <[email protected]> Co-authored-by: Viktor Söderqvist <[email protected]>

arthurkiller force-pushed the fix/load-replid-from-preamble-base-aof branch from e594366 to 462e1f5 Compare July 17, 2025 14:50

arthurkiller changed the title ~~feat: add replid for base aof file with preamble~~ [Q] add replid for base aof file with preamble Jul 17, 2025

arthurkiller force-pushed the fix/load-replid-from-preamble-base-aof branch from a52ec12 to 98ad37b Compare August 6, 2025 13:03

zuiderkwast added this to Valkey 9.1 Sep 22, 2025

zuiderkwast mentioned this pull request Oct 2, 2025

Stop discouraging AOF valkey-io/valkey-doc#360

Open

zuiderkwast moved this to Todo in Valkey 9.1 Oct 2, 2025

zuiderkwast mentioned this pull request Oct 4, 2025

[NEW] Support partial sync of a replica after a restart when aof is enabled #2677

Closed

arthurkiller force-pushed the fix/load-replid-from-preamble-base-aof branch 2 times, most recently from 3e9a4fd to bf34cd1 Compare October 18, 2025 08:16

arthurkiller changed the title ~~[Q] add replid for base aof file with preamble~~ [Feat] add replid for base aof file with preamble Oct 18, 2025

arthurkiller force-pushed the fix/load-replid-from-preamble-base-aof branch 5 times, most recently from b1d8058 to 1644ee2 Compare October 20, 2025 08:55

arthurkiller added 4 commits October 20, 2025 16:56

feat: add replid for base aof file with preamble

3bd50f4

Signed-off-by: arthur.lee <[email protected]>

fix: create replbuffer before we start feed

b05bb46

Signed-off-by: arthur.lee <[email protected]>

fix: replication feed log and debug loadaof check cached primary first

75008d4

Signed-off-by: arthur.lee <[email protected]>

fix: only load and change cached master on start time

0e33b42

Signed-off-by: arthur.lee <[email protected]>

fix: update test cases

2a53e11

Signed-off-by: arthur.lee <[email protected]>

arthurkiller force-pushed the fix/load-replid-from-preamble-base-aof branch from 1644ee2 to 2a53e11 Compare October 20, 2025 08:56

zuiderkwast reviewed Oct 20, 2025

View reviewed changes

tests/integration/replication-4.tcl Outdated Show resolved Hide resolved

tests/integration/replication-4.tcl Outdated Show resolved Hide resolved

tests/integration/replication-4.tcl Outdated Show resolved Hide resolved

arthurkiller force-pushed the fix/load-replid-from-preamble-base-aof branch from be68807 to caaa70a Compare October 21, 2025 08:07

fix: figure out tcl cases

41d02f9

Signed-off-by: arthur.lee <[email protected]>

arthurkiller force-pushed the fix/load-replid-from-preamble-base-aof branch from caaa70a to 41d02f9 Compare October 24, 2025 11:24

zuiderkwast reviewed Oct 29, 2025

View reviewed changes

src/aof.c Outdated Show resolved Hide resolved

arthurkiller commented Oct 30, 2025

View reviewed changes

src/aof.c Outdated Show resolved Hide resolved

misc: de-duplicate the code by adding replicationRestoreOffsetFromSav…

abbc6a2

…eInfo Signed-off-by: arthur.lee <[email protected]>

arthurkiller force-pushed the fix/load-replid-from-preamble-base-aof branch from 9a31911 to abbc6a2 Compare November 5, 2025 10:02

zuiderkwast reviewed Nov 5, 2025

View reviewed changes

src/rdb.c Outdated Show resolved Hide resolved

src/rdb.c Outdated Show resolved Hide resolved

Update src/rdb.c

4842aa7

Co-authored-by: Viktor Söderqvist <[email protected]> Signed-off-by: Arthur Lee <[email protected]> Signed-off-by: arthur.lee <[email protected]>

arthurkiller force-pushed the fix/load-replid-from-preamble-base-aof branch from 6ff5646 to 4842aa7 Compare November 6, 2025 12:57

zuiderkwast approved these changes Nov 6, 2025

View reviewed changes

src/rdb.c Outdated Show resolved Hide resolved

src/aof.c Show resolved Hide resolved

zuiderkwast added the release-notes This issue should get a line item in the release notes label Nov 6, 2025

arthurkiller and others added 2 commits November 7, 2025 11:10

Update src/rdb.c

e3bf363

Co-authored-by: Viktor Söderqvist <[email protected]> Signed-off-by: Arthur Lee <[email protected]>

Update src/aof.c

1d03246

Co-authored-by: Viktor Söderqvist <[email protected]> Signed-off-by: Arthur Lee <[email protected]>

zuiderkwast changed the title ~~[Feat] add replid for base aof file with preamble~~ Allow partial sync after loading AOF with preamble Nov 11, 2025

zuiderkwast merged commit 2da21d9 into valkey-io:unstable Nov 11, 2025
55 checks passed

github-project-automation bot moved this from Todo to Done in Valkey 9.1 Nov 11, 2025

Allow partial sync after loading AOF with preamble #2366

Allow partial sync after loading AOF with preamble #2366

Conversation

arthurkiller commented Jul 17, 2025 • edited by zuiderkwast Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

secwall commented Jul 25, 2025

Uh oh!

arthurkiller commented Jul 26, 2025

Uh oh!

arthurkiller commented Jul 28, 2025

Uh oh!

secwall commented Jul 28, 2025

Uh oh!

codecov bot commented Aug 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ranshid commented Aug 6, 2025

Uh oh!

arthurkiller commented Aug 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arthurkiller commented Aug 6, 2025

Uh oh!

zuiderkwast commented Sep 22, 2025

Uh oh!

arthurkiller commented Sep 22, 2025

Uh oh!

zuiderkwast commented Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zuiderkwast left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

arthurkiller commented Oct 21, 2025

Uh oh!

arthurkiller commented Oct 21, 2025

Uh oh!

zuiderkwast commented Oct 21, 2025

Uh oh!

arthurkiller commented Oct 24, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zuiderkwast left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

arthurkiller commented Nov 7, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

arthurkiller commented Jul 17, 2025 •

edited by zuiderkwast

Loading

codecov bot commented Aug 6, 2025 •

edited

Loading

arthurkiller commented Aug 6, 2025 •

edited

Loading

zuiderkwast commented Oct 7, 2025 •

edited

Loading