Skip to content

Conversation

@arpitbbhayani
Copy link

The AOF rewrite tests were failing because bloom filter data was not being properly restored after server restart. The root cause was twofold:

  1. BF.LOAD command was replicating during AOF loading, causing duplicate entries
  2. Tests were not properly configuring AOF persistence for server restart

Changes:

  • Modified BF.LOAD handler to skip replication when loading from AOF by checking must_obey_client() context
  • Updated AOF tests to enable appendonly before adding data
  • Added wait for initial AOF rewrite to prevent concurrent rewrites
  • Set appendonly in server startup args for proper AOF loading on restart

Also includes minor fixes:

  • Changed build.sh shebang to bash for better compatibility
  • Updated pytest requirement to 7.4.3
  • Fixed indentation in utils.rs documentation

Fixes #74

@enjoy-binbin
Copy link
Member

@arpitbbhayani can you fix the DCO?

@arpitbbhayani arpitbbhayani force-pushed the 1.0 branch 2 times, most recently from 07d7fef to 07488e2 Compare December 4, 2025 06:25
The AOF rewrite tests were failing because bloom filter data was not
being properly restored after server restart. The root cause was
twofold:

Root Cause: Tests were not properly configuring AOF persistence for server
   restart

Changes:
- Updated AOF tests to enable appendonly before adding data
- Added wait for initial AOF rewrite to prevent concurrent rewrites
- Set appendonly in server startup args for proper AOF loading on
  restart

Also includes minor fixes:
- Changed build.sh shebang to bash for better compatibility
- Updated pytest requirement to 7.4.3
- Fixed indentation in utils.rs documentation

Fixes valkey-io#74

Signed-off-by: Arpit Bhayani <[email protected]>
@arpitbbhayani
Copy link
Author

Thank you so much for reviewing the code. Apologies that it took some time for me to come back.
I have addressed all the concerns and made the changes.

Now, the PR only fixes the AOF tests that were failing and other changes have been reverted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants