Skip to content
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

Changing a folder label should not trigger a rescan #6891

Open
tomasz1986 opened this issue Aug 13, 2020 · 5 comments
Open

Changing a folder label should not trigger a rescan #6891

tomasz1986 opened this issue Aug 13, 2020 · 5 comments
Labels
enhancement New features or improvements of some kind, as opposed to a problem (bug)

Comments

@tomasz1986
Copy link
Contributor

Right now, changing a folder label immediately triggers a rescan of that folder. While usually not a big deal, this can be annoying when dealing with huge folders and/or slow hardware. I think that it would be nice not to trigger a rescan if the change does not affect the actual synchronisation.

@tomasz1986 tomasz1986 added enhancement New features or improvements of some kind, as opposed to a problem (bug) needs-triage New issues needed to be validated labels Aug 13, 2020
@calmh calmh removed the needs-triage New issues needed to be validated label Aug 13, 2020
@rjpruitt16
Copy link
Contributor

Is this something a noob like me could try?

@imsodin
Copy link
Member

imsodin commented Aug 15, 2020

It's not super easy, as it involves model-folder interactions and handling config changes. As in currently folders are indiscriminately restarted when their config changes, which would have to be changed to some kind of mechanism to let folders know about changed config and then decide whether they can adjust on the fly or need to restart (maybe restart should never be necessary). However it would definitely be an opportunity to learn about Syncthing internals :)

@AudriusButkevicius
Copy link
Member

AudriusButkevicius commented Aug 15, 2020

I think there is already code that should handle this, as label is marked as restart=false in the config, so folders should not be restarted in the model. That doesn't work for some reason.

I'd start debugging here:

https://github.com/syncthing/syncthing/blob/main/lib/model/model.go#L2493

@imsodin
Copy link
Member

imsodin commented Aug 15, 2020

I thought the restart annotations are only for full Syncthing restarts. If it is meant to apply to folder restarts, that's not only not working, but also not fully implemented: There's no way the folder gets config updates for non-restart config changes (e.g. the label is relevant for the folder).

@AudriusButkevicius
Copy link
Member

Restart annotations are what we make of it, there is simply a util function that returns a copy of the struct with fields that have restart:"true" or that do not have restart:"false". This is exactly what RequiresRestartOnly() on a folder does, returns a copy of the struct only with the fields that require restart, so the reflect.DeepEquals is what checks that. This is checked in the model (which I guess is somewhat wrong), so any folder change does run the check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New features or improvements of some kind, as opposed to a problem (bug)
Projects
None yet
Development

No branches or pull requests

5 participants