Skip to content

Conversation

@abhizer
Copy link
Contributor

@abhizer abhizer commented Dec 26, 2025

Removes the previous faulty implementation where we tried to take a lock on the UUID of a checkpoint when syncing it.
The newer implementation mirrors the implementation of RunningCheckpoint to introduce RunningCheckpointSync.

Once the sync is done running, all other sync requests made for the same checkpoint will be removed and marked as completed.

Copilot AI review requested due to automatic review settings December 26, 2025 22:07
@abhizer abhizer added the connectors Issues related to the adapters/connectors crate label Dec 26, 2025
Copy link
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 refactors checkpoint synchronization to prevent multiple concurrent S3 push operations for the same checkpoint. The implementation replaces a faulty UUID-based locking mechanism with a RunningCheckpointSync state machine that mirrors the existing RunningCheckpoint pattern.

Key changes:

  • Introduces RunningCheckpointSync enum to track sync operation lifecycle (Error/Waiting/Done states)
  • Replaces single sync_checkpoint_request with a queue of sync_checkpoint_requests
  • Adds polling mechanism to process queued sync requests after active sync completes

Removes the previous faulty implementation where we tried to take a lock
on the UUID of a checkpoint when syncing it.
The newer implementation mirrors the implementation of
`RunningCheckpoint` to introduce `RunningCheckpointSync`.

Once the sync is done running, all other sync requests made for the same
checkpoint will be removed and marked as completed.

Signed-off-by: Abhinav Gyawali <[email protected]>
@abhizer abhizer force-pushed the once-sync-at-a-time branch from 350de7a to eefd011 Compare December 26, 2025 22:20
@abhizer abhizer requested a review from blp December 26, 2025 22:35
Copy link
Member

@blp blp left a comment

Choose a reason for hiding this comment

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

Thank you!

@abhizer abhizer added this pull request to the merge queue Dec 31, 2025
@abhizer abhizer removed this pull request from the merge queue due to a manual request Dec 31, 2025
@abhizer abhizer added this pull request to the merge queue Dec 31, 2025
Merged via the queue into main with commit 5c3c48e Dec 31, 2025
1 check passed
@abhizer abhizer deleted the once-sync-at-a-time branch December 31, 2025 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

connectors Issues related to the adapters/connectors crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants