FolderSync retries with the same key shouldn't happen regularly when
there are changes. When the same synckey is retried over and over, it is
an indication that a device is stuck.

Re-sending a FolderSync without new changes does not trigger the
sync-key resent code-path, and thus doesn't trigger the counting.

Diff Detail

Repository

rS syncroton

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

mollekopf requested review of this revision.Jun 30 2025, 3:10 PM

mollekopf created this revision.

Harbormaster completed remote builds in B54357: Diff 15543.Jun 30 2025, 3:10 PM

This will turn a read-only operation into an UPDATE query, and the getting the abstraction lasagna to execute the update query is fairly messy, but I suppose it's still worth it unless there's a better approach.

machniak requested changes to this revision.Jun 30 2025, 4:22 PM

machniak subscribed.

machniak added inline comments.

lib/ext/Syncroton/Command/FolderSync.php
120	Maybe this should not be a different level than warning?
132	So, the UPDATE will be done in first 10 requests, right? I don't get why this `counterNext` dance here. I don't see it used anywhere.

This revision now requires changes to proceed.Jun 30 2025, 4:22 PM

mollekopf added inline comments.Jun 30 2025, 6:34 PM

lib/ext/Syncroton/Command/FolderSync.php
120	Probably a good idea now that we warn later on.
132	The unsetting is the same than what we do in kolab_sync_backend_state::create, because counterNext does not actually exist in the database and the update method will complain if we don't unset. counterNext is set in the kolab_sync_backend_state::validate. I agree it's obscure, and I often wonder how much clearer the code could be without all the abstractions, but here we are...

mollekopf added inline comments.Jun 30 2025, 6:37 PM

lib/ext/Syncroton/Command/FolderSync.php
132	So, the UPDATE will be done in first 10 requests, right? The update will be done in the first 10 requests indeed. A cleaner way of the above counterNext dance would be to override update and do that there, so I'll improve this.