Skip to content

FullSync Initiate Check when at least one sync session is still active.#1193

Merged
vazois merged 2 commits intomicrosoft:mainfrom
vazois:vazois/fsync-fix
May 8, 2025
Merged

FullSync Initiate Check when at least one sync session is still active.#1193
vazois merged 2 commits intomicrosoft:mainfrom
vazois:vazois/fsync-fix

Conversation

@vazois
Copy link
Copy Markdown
Contributor

@vazois vazois commented May 8, 2025

This PR ensures that streaming snapshot is initiated only one at least one sync session requires fully synchronization.

Copilot AI review requested due to automatic review settings May 8, 2025 17:27
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR ensures that a full streaming snapshot is initiated only when at least one sync session requires a full synchronization. The changes include:

  • Updating the test case to wait for the replica to catch up after validating replica data.
  • Modifying the DisklessReplication manager to have PrepareForSync return a boolean indicating whether a full sync is required.
  • Introducing a boolean flag within PrepareForSync to conditionally trigger the streaming checkpoint.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
test/Garnet.test.cluster/ReplicationTests/ClusterReplicationDisklessSyncTests.cs Added wait functionality for replica AOF synchronization in the test.
libs/cluster/Server/Replication/PrimaryOps/DisklessReplication/ReplicationSyncManager.cs Changed PrepareForSync to return a boolean and conditionally trigger the streaming checkpoint based on active sync sessions.
Comments suppressed due to low confidence (1)

libs/cluster/Server/Replication/PrimaryOps/DisklessReplication/ReplicationSyncManager.cs:219

  • [nitpick] Consider renaming the method 'PrepareForSync' to better reflect its boolean return value (e.g., 'ShouldPerformFullSync') to improve code clarity.
async Task<bool> PrepareForSync()

@vazois vazois merged commit e117649 into microsoft:main May 8, 2025
26 checks passed
@vazois vazois deleted the vazois/fsync-fix branch June 26, 2025 16:54
@github-actions github-actions bot locked and limited conversation to collaborators Aug 27, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants