Different ignores between devices causes the completion percentage to never reach 100% and thus stay seemingly syncing forever. This is nothing new, in fact it’s almost ancient in syncthing-time.
An approach mentioned by @calmh and as far as I remember already once existing in a prototype was to base the status on syncing activity instead of what they have according to index. That came up a long(ish) time ago as well. I can’t find the detailed discussion, but issue #623 mentions it.
Another course of action I would like to hear opinions about:
Register every ignored file in the index for the ignoring device when it is encountered on scanning or pulling. Today that is already done if a file is already in the index but becomes ignored: Then it is marked invalid. The same could be done when in index is received/while pulling: Every file that is in the received index but ignored locally could be added to the index for the local device as well (as invalid or a new property specific for ignoring). Then the ambiguity in the completion based on the index is removed: If you don’t count entries that are invalid/ignored on the device in question you will get the real completion of that device.
I did a quick prototype, which to my surprise already seems to do what it is supposed to. It is just a proof of concept, needing several database lookups when calculation completion or pulling is hardly the way to go.
I was worrying this would blow the database set up. I did the tests on my production system (yes I know, but it so nicely had the problem to be solved already…) and the database size even decrease (841->839MB) which is probably due to non-related changes, but it shows that it doesn’t increase the size significantly.
I misunderstood what you were saying when reading the description above (thinking you meant to scan ignored files), but when looking at the code it’s actually quite elegant. I think it deserves cleaning and merging.
What I don’t like, or more accurately am unsure about whether it needs significantly more resources, are the frequent database calls: Before only db.WithNeed was needed, which is light as it only queries the global VersionList and then files that are actually needed. Now an additional file needs to be loaded via explicitly calling FileSet.Get. As pulling happens every 10s (see another PR ) that happens very often so unless this call is very cheap it should probably be optimized.
I was thinking of extending FileVersion by an invalid field to be able to exclude invalid files from db.WithNeed and thus not need to the additional FileSet.Get calls.
I think it makes sense from an architecture point of view that the puller makes the note that “yep, I’ve seen this file, and I’m not going to pull it ever”. I’m not keen on the extra database lookup, but I think that can be avoided indeed.
After some debugging (my database was luckily quite deranged, so I found corner cases) I have a new version that does not need additional database lookups, on the other side I had to change more code. I will post a PR once https://github.com/syncthing/syncthing/pull/4426 is reviewed (not connected changes that came up while hacking).