lib/versioner: Clean the versions dir of symlinks, not the full folder #4289
Conversation
|
Why can't we do a one-off cleanup as part of config migrations? If someone sets up a symlink explicitly perhaps we should not nuke it? |
|
Regarding recovery, we could check time of |
|
Sry, pocket actions. |
|
This thing should be removed in the long run anyway. I'm not super keen on trusting this to a migration; there might be folders that are offline or whatnot. |
|
Offline? |
|
So what we could do for recovery is list our "have" index, then look for any other older entries in the index for deleted symlinks where the delete is recent. I'm not super hopeful that many places will have those entries intact but it's worth a try I guess. "Offline" as is on a removable disk for example. I'd remove this when we have a small-digit number of <0.14.34 clients out there. |
|
In my phone so can't check, but I don't think we can classify deletion as recent as we don't update the mtime? |
|
If we knew they are recent, we could check mtime of syncthing.old binary or time when release hit the shelves as the cutoff period, and time when the release was pulled as the other cutoff |
|
We set the mtime to the delete time. I'm just less hopeful that we find an old index entry. But I'm hacking it currently. |
|
OK, here's another attempt that's cleaner. It does the cleanup in a migration after all. Later, as part of the same migration, it attempts a recovery sweep. |
|
@st-review merge |
|
|
||
| // Try to find an older index entry. | ||
| for deviceID := range m.cfg.Devices() { | ||
| olderFI, ok := fs.Get(deviceID, fi.Name) |
There was a problem hiding this comment.
Do we unset target on delete?
|
@AudriusButkevicius I merged it because I need to drop something within the hour, but since you're here I'll give you a few minutes to find serious problems with this as well. Not minor nits though, at this point. |
|
Lgtm |
|
Oh. We also don't actually set the modified time like I though we do, so the recovery step is broken. |
|
I don't think there's anything reliable we can do do undo that then unfortunately |
|
Fine, drop the extra code in that case, and make an enhancement for the future? |
|
Yes |
GitHub-Pull-Request: syncthing#4289
st-review
added
the
pr-merged
st-review
added
the
frozen-due-to-age
Prevents the damage, while not recovering from it.