Skip to content

[Push] Append sync when already syncing #1480

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 21, 2025
Merged

Conversation

sunkup
Copy link
Member

@sunkup sunkup commented May 21, 2025

Purpose

If a push message triggers a sync for an account that already has an ongoing sync, there's a risk of losing remote changes made near the end of the existing sync. To address this, we can always append an additional sync worker when triggered by a push request, but restrict it to at most one pending/blocked worker.

Short description

When sync request comes from push:

  • use synchronized block to avoid race conditions for the check, that ...
  • ... there is not already blocked or enqueued work for same unique work (given pair of account + data type)
  • if there is not, then enqueue with ExistingWorkPolicy.APPEND_OR_REPLACE
  • access enqueue operation result object (listenable future), to force waiting until work is enqueued

Checklist

  • The PR has a proper title, description and label.
  • I have self-reviewed the PR.
  • I have added documentation to complex functions and functions that can be used by other modules.
  • I have added reasonable tests or consciously decided to not add tests.

@sunkup sunkup changed the title [Push] Append sync work when already syncing [Push] Append sync when already syncing May 21, 2025
@rfc2822 rfc2822 linked an issue May 21, 2025 that may be closed by this pull request
Signed-off-by: Sunik Kupfer <kupfer@bitfire.at>
@sunkup sunkup added the bug Something isn't working label May 21, 2025
@sunkup
Copy link
Member Author

sunkup commented May 21, 2025

I think we might want to make the method suspending at some point.

@sunkup sunkup requested a review from rfc2822 May 21, 2025 09:50
@sunkup sunkup marked this pull request as ready for review May 21, 2025 09:52
Copy link
Member

@rfc2822 rfc2822 left a comment

Choose a reason for hiding this comment

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

🍀

@rfc2822 rfc2822 merged commit b306219 into main-ose May 21, 2025
16 checks passed
@rfc2822 rfc2822 deleted the push-sync-enqueue-append branch May 21, 2025 09:54
@rfc2822 rfc2822 added the push related to WebDAV-Push label May 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working push related to WebDAV-Push
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Push] Notification doesn't disappear on its own
2 participants