This truncates times meant for API consumption to second precision,
where fractions won't typically matter or add any value. Exception to
this is timestamps on logs and events, and of course I'm not touching
things like file metadata.
I'm not 100% certain this is an exhaustive change, but it's the things I
found by grepping and following the breadcrumbs from lib/api...
I also considered general-but-ugly solutions, like having the API
serializer itself do reflection magic or even regexps on returned
objects, but decided against it because aurgh...
An untrusted device will receive padded info for small blocks, and hence
sometimes request a larger block than actually exists on disk.
Previously we let this pass because we didn't have a hash to compare to
in that case and we ignored the EOF error based on that.
Now the untrusted device does pass an encrypted hash that we decrypt and
verify. This means we can't check for len(hash)==0 any more, but on the
other hand we do have a valid hash we can apply to the data we actually
read. If it matches then we don't need to worry about the read
supposedly being a bit short.
* lib/db: Add ExpirePendingFolders().
Use-case is to drop any no-longer-pending folders for a specific
device when parsing its ClusterConfig message where previously offered
folders are not mentioned any more.
The timestamp in ObservedFolder is stored with only second precision,
so round to seconds here as well. This allows calling the function
within the same second of adding or updating entries.
* lib/model: Weed out pending folders when receiving ClusterConfig.
Filter the entries by timestamp, which must be newer than or equal to
the reception time of the ClusterConfig. For just mentioned ones,
this assumption will hold as AddOrUpdatePendingFolder() updates the
timestamp.
* lib/model, gui: Notify when one or more pending folders expired.
Introduce new event type FolderOfferCancelled and use it to trigger a
complete refreshCluster() cycle. Listing individual entries would be
much more code and probably just as much work to answer the API
request.
* lib/model: Add comment and rename ExpirePendingFolders().
* lib/events: Rename FolderOfferCancelled to ClusterPendingChanged.
* lib/model: Reuse ClusterPendingChanged event for cleanPending()
Changing the config does not necessarily mean that the
/resut/cluster/pending endpoints need to be refreshed, but only if
something was actually removed. Detect this and indicate it through
the ClusterPendingChanged event, which is already hooked up to requery
respective endpoints within the GUI.
No more need for a separate refreshCluster() in reaction to
ConfigSaved event or calling refreshConfig().
* lib/model: Gofmt.
* lib/db: Warn instead of info log for failed removal.
* gui: Fix pending notifications not loading on GUI start.
* lib/db: Use short device ID in log message.
* lib/db: Return list of expired folder IDs after deleting them.
* lib/model: Refactor Pending...Changed events.
* lib/model: Adjust format of removed pending folders enumeration.
Use an array of objects with device / folder ID properties, matching
the other places where it's used.
* lib/db: Drop invalid entries in RemovePendingFoldersBeforeTime().
* lib/model: Gofmt.
My local gofmt did not complain here, strangely...
* gui: Handle PendingDevicesChanged event.
Even though it currently only holds one device at a time, wrap the
contents in an array under the "added" property name.
* lib/model: Fix null values in PendingFoldersChanged removed member.
* gui: Handle PendingFoldersChanged event.
* lib/model: Simplify construction of expiredPendingList.
* lib/model: Reduce code duplication in cleanPending().
Use goto and a label for the common parts of calling the DB removal
function and building the event data part.
* lib/events, gui: Mark ...Rejected events deprecated.
Extend comments explaining the conditions when the replacement event
types are emitted.
* lib/model: Wrap removed devices in array of objects as well.
* lib/db: Use iter.Value() instead of needless db.Get(iter.Key())
* lib/db: Add comment explaining RemovePendingFoldersBeforeTime().
* lib/model: Rename fields folderID and deviceID in event data.
* lib/db: Only list actually expired IDs as removed.
Skip entries where Delete() failed as well as invalid entries that got
removed automatically.
* lib/model: Gofmt
This splits the ignore getting to two methods, one that loads from disk
(the old one) and one that just returns whatever is already loaded (the
new one). The folder summary service which is just interested in stats
now uses the latter method. This means that it, and API calls that call
it, does not get blocked by folder I/O.
This adds two new configuration options:
// The number of connections at which we stop trying to connect to more
// devices, zero meaning no limit. Does not affect incoming connections.
ConnectionLimitEnough int
// The maximum number of connections which we will allow in total, zero
// meaning no limit. Affects incoming connections and prevents
// attempting outgoing connections.
ConnectionLimitMax int
These can be used to limit the number of concurrent connections in
various ways.
This adds a statistic to track the last connection duration per device.
It isn't used for much in this PR, but it's available for #7223 to use
in deciding how to order device connection attempts (deprioritizing
devices that just dropped our connection the last time).
Add specific errors for the failures, resulting in this rather than just
the generic "invalid filename":
[MRIW7] 08:50:50 INFO: Puller (folder default, item "NUL"): syncing: filename is invalid: name is reserved
[MRIW7] 08:50:50 INFO: Puller (folder default, item "fail."): syncing: filename is invalid: name ends with space or period
[MRIW7] 08:50:50 INFO: Puller (folder default, item "sup:yo"): syncing: filename is invalid: name contains reserved character
[MRIW7] 08:50:50 INFO: default: Failed to sync 3 items
We created a new fileset before stopping the folder during restart. When
we create that fileset it loads the current metadata and sequence
numbers from the database. But the folder may have time to update those
before stopping, leaving the new fileset with bad data.
This would cause wrong accounting (forgotten files) and potentially
sequence reuse causing files not sent to other devices.
This change reuses the fileset on restart, skipping the issue entirely.
It also moves the creation of the fileset back under the lock so there
should be no chance of concurrency issues here.
The FileSet.Drop operation in there needs to potentially update a whole lot of global lists, which can take a while (longer than the deadlock interval apparently)
With this change we emulate a case sensitive filesystem on top of
insensitive filesystems. This means we correctly pick up case-only renames
and throw a case conflict error when there would be multiple files differing
only in case.
This safety check has a small performance hit (about 20% more filesystem
operations when scanning for changes). The new advanced folder option
`caseSensitiveFS` can be used to disable the safety checks, retaining the
previous behavior on systems known to be fully case sensitive.
Co-authored-by: Jakob Borg <jakob@kastelo.net>
* Fix ui, hide report date
* Undo Goland madness
* UR now web scale
* Fix migration
* Fix marshaling, force tick on start
* Fix tests
* Darwin build
* Split "all" build target, add package name as a tag
* Remove pq and sql dep from syncthing, split build targets
* Empty line
* Revert "Empty line"
This reverts commit f74af2b067dadda8a343714123512bd545a643c3.
* Revert "Remove pq and sql dep from syncthing, split build targets"
This reverts commit 8fc295ad007c5bb7886c557f492dacf51be307ad.
* Revert "Split "all" build target, add package name as a tag"
This reverts commit f4dc88995106d2b06042f30bea781a0feb08e55f.
* Normalise contract types
* Fix build add more logging
This changes the error handling in loading ignores slightly:
- There is a new ParseError type that is returned as the error
(somewhere in the chain) when the problem was not an I/O error loading
the file, but some issue with the contents.
- If the file was read successfully but not parsed successfully we still
return the lines read (in addition to nil patterns and a ParseError).
- In the API, if the error IsParseError then we return a successful
HTTP response with the lines and the actual error included in the JSON
object.
- In the GUI, as long as the HTTP call to load the ignores was
successful we can edit the ignores. If there was an error we show this
as a validation error on the dialog.
Also some cleanup on the Javascript side as it for some reason used
jQuery instead of Angular for this editor...
Group the global list of files by version, instead of having one flat list for all devices. This removes lots of duplicate protocol.Vectors.
Co-authored-by: Jakob Borg <jakob@kastelo.net>
If we fail to take the rename shortcut we may crash on a later loop,
because we do trickiness with the indexes but the original buckets[key]
in "range buckets[key]" isn't re-evaluated so i exceeds the max index.
This adds indirection of large version vectors in the same manner as we
already to block lists. The effect is the same: less duplicated data in
some situations.
To mitigate the impact for when this indirection
wouldn't be needed I've added an indirection cutoff for both blocks and
the new version vector stuff: we don't do the indirection at all for
small block lists or small version vectors, instead storing it directly
like we used to do. This is faster for small files and small setups.
This makes version vector values clock based instead of just incremented
from zero. The effect is that a vector that is created from scratch
(after database reset) will have a higher value for the local device
than what it could have been previously, causing a conflict. That is, if
we are A and we had
{A: 42, B: 12}
in the old scheme, a reset and rescan would give us
{A: 1}
which is a strict ancestor of the older file (this might be wrong). With
the new scheme we would instead have
{A: someClockTime, b: otherClockTime}
and the new version after reset would become
{A: someClockTime+delta}
which is in conflict with the previous entry (better).
In case the clocks are wrong (current time is less than the value in the
vector) we fall back to just simple increment like today.
This scheme is ineffective if we suffer a database reset while at the
same time setting the clock back far into the past. It's however no
worse than what we already do.
This loses the ability to emit the "added" event, as we can't look for
the magic 1 entry any more. That event was however already broken
(#5541).
Another place where we infer meaning from the vector itself is in
receive only folders, but there the only criteria is that the vector is
one item long and includes just ourselves, which remains the case with
this change.
* wip
- In the few places where we wrap errors, use the new Go 1.13 "%w"
construction instead of %s or %v.
- Where we create errors with constant strings, consistently use
errors.New and not fmt.Errorf.
- Remove capitalization from errors in the few places where we had that.
One of the causes of "panic: database is closed" is that we try to send
summaries after it's been closed. Calculating summaries can take a long
time and if we have a lot of folders it's not unreasonable to think
that we might be stopped in this loop, so prepare to bail here.
* push
During some other work I discovered these tests weren't great, so I've
rewritten them to be a little better. The real changes here are:
- Don't play games with not starting the folder and such, and don't
construct a fake folder instance -- just use the one the model has. The
folder starts and scans but the folder contents are empty at this point
so that's fine.
- Use a fakefs instead of a temp dir.
- To support the above, implement a fakefs option `?content=true` to
make the fakefs actually retain written content. Use sparingly,
obviously, but it means the fakefs can usually be used instead of an
on disk real directory.
This adds a new config with the simple and concise name
maxConcurrentIncomingRequestKiB. This limits how many bytes we have "in
the air" in the form of response data being read and processed.
After some testing I think that not having this limiter is seldom a
great idea and thus I propose a default value of 256 MiB for this new
setting.
I also refactored the folder IO limiter to be a model/folder attribute
instead of a package global.
Adds a new folder state "Waiting to Sync" in the same vein as the
existing "Waiting to Scan". This vastly improves performances in the
rare cases when there are lots and lots of folders operating.
* lib/db: Deduplicate block lists in database (fixes#5898)
This moves the block list in the database out from being just a field on
the FileInfo to being an object of its own. When putting a FileInfo we
marshal the block list separately and store it keyed by the sha256 of
the marshalled block list. When getting, if we are not doing a
"truncated" get, we do an extra read and unmarshal for the block list.
Old block lists are cleared out by a periodic GC sweep. The alternative
would be to use refcounting, but:
- There is a larger risk of getting that wrong and either dropping a
block list in error or keeping them around forever.
- It's tricky with our current database, as we don't have dirty reads.
This means that if we update two FileInfos with identical block lists in
the same transaction we can't just do read/modify/write for the ref
counters as we wouldn't see our own first update. See above about
tracking this and risks about getting it wrong.
GC uses a bloom filter for keys to avoid heavy RAM usage. GC can't run
concurrently with FileInfo updates so there is a new lock around those
operation at the lowlevel.
The end result is a much more compact database, especially for setups
with many peers where files get duplicated many times.
This is per-key-class stats for a large database I'm currently working
with, under the current schema:
```
0x00: 9138161 items, 870876 KB keys + 7397482 KB data, 95 B + 809 B avg, 1637651 B max
0x01: 185656 items, 10388 KB keys + 1790909 KB data, 55 B + 9646 B avg, 924525 B max
0x02: 916890 items, 84795 KB keys + 3667 KB data, 92 B + 4 B avg, 192 B max
0x03: 384 items, 27 KB keys + 5 KB data, 72 B + 15 B avg, 87 B max
0x04: 1109 items, 17 KB keys + 17 KB data, 15 B + 15 B avg, 69 B max
0x06: 383 items, 3 KB keys + 0 KB data, 9 B + 2 B avg, 18 B max
0x07: 510 items, 4 KB keys + 12 KB data, 9 B + 24 B avg, 41 B max
0x08: 1349 items, 12 KB keys + 10 KB data, 9 B + 8 B avg, 17 B max
0x09: 194 items, 0 KB keys + 123 KB data, 5 B + 634 B avg, 11484 B max
0x0a: 3 items, 0 KB keys + 0 KB data, 14 B + 7 B avg, 30 B max
0x0b: 181836 items, 2363 KB keys + 10694 KB data, 13 B + 58 B avg, 173 B max
Total 10426475 items, 968490 KB keys + 9202925 KB data.
```
Note 7.4 GB of data in class 00, total size 9.2 GB. After running the
migration we get this instead:
```
0x00: 9138161 items, 870876 KB keys + 2611392 KB data, 95 B + 285 B avg, 4788 B max
0x01: 185656 items, 10388 KB keys + 1790909 KB data, 55 B + 9646 B avg, 924525 B max
0x02: 916890 items, 84795 KB keys + 3667 KB data, 92 B + 4 B avg, 192 B max
0x03: 384 items, 27 KB keys + 5 KB data, 72 B + 15 B avg, 87 B max
0x04: 1109 items, 17 KB keys + 17 KB data, 15 B + 15 B avg, 69 B max
0x06: 383 items, 3 KB keys + 0 KB data, 9 B + 2 B avg, 18 B max
0x07: 510 items, 4 KB keys + 12 KB data, 9 B + 24 B avg, 41 B max
0x09: 194 items, 0 KB keys + 123 KB data, 5 B + 634 B avg, 11484 B max
0x0a: 3 items, 0 KB keys + 0 KB data, 14 B + 17 B avg, 51 B max
0x0b: 181836 items, 2363 KB keys + 10694 KB data, 13 B + 58 B avg, 173 B max
0x0d: 44282 items, 1461 KB keys + 61081 KB data, 33 B + 1379 B avg, 1637399 B max
Total 10469408 items, 969939 KB keys + 4477905 KB data.
```
Class 00 is now down to 2.6 GB, with just 61 MB added in class 0d.
There will be some additional reads in some cases which theoretically
hurts performance, but this will be more than compensated for by smaller
writes and better compaction.
On my own home setup which just has three devices and a handful of
folders the difference is smaller in absolute numbers of course, but
still less than half the old size:
```
0x00: 297122 items, 20894 KB keys + 306860 KB data, 70 B + 1032 B avg, 103237 B max
0x01: 115299 items, 7738 KB keys + 17542 KB data, 67 B + 152 B avg, 419 B max
0x02: 1430537 items, 121223 KB keys + 5722 KB data, 84 B + 4 B avg, 253 B max
...
Total 1947412 items, 151268 KB keys + 337485 KB data.
```
to:
```
0x00: 297122 items, 20894 KB keys + 37038 KB data, 70 B + 124 B avg, 520 B max
0x01: 115299 items, 7738 KB keys + 17542 KB data, 67 B + 152 B avg, 419 B max
0x02: 1430537 items, 121223 KB keys + 5722 KB data, 84 B + 4 B avg, 253 B max
...
0x0d: 18041 items, 595 KB keys + 71964 KB data, 33 B + 3988 B avg, 101109 B max
Total 1965447 items, 151863 KB keys + 139628 KB data.
```
* wip
* wip
* wip
* wip
Makes the logic a bit clearer and safer. This also sneakily redefines
the 0 interval to also mean disabled, whereas it previously meant ...
sometimes default to 1s, sometimes just spin.
This adds error returns to model methods called by the protocol layer.
Returning an error will cause the connection to be torn down as the
message couldn't be handled. Using this to signal that a folder isn't
currently available will then cause a reconnection a few moments later,
when it'll hopefully work better.
Tested manually by running with STRECHECKDBEVERY=0 on a nontrivially
sized setup. This panics reliably before this patch, but just causes a
disconnect/reconnect now.
As foretold by the prophecy, "once the database refactor is merged, then
shall appear a request to propagate errors from the store known
throughout the land as the NamedspacedKV, and it shall be good".
This PR does two things, because one lead to the other:
- Move the leveldb specific stuff into a small "backend" package that
defines a backend interface and the leveldb implementation. This allows,
potentially, in the future, switching the db implementation so another
KV store should we wish to do so.
- Add proper error handling all along the way. The db and backend
packages are now errcheck clean. However, I drew the line at modifying
the FileSet API in order to keep this manageable and not continue
refactoring all of the rest of Syncthing. As such, the FileSet methods
still panic on database errors, except for the "database is closed"
error which is instead handled by silently returning as quickly as
possible, with the assumption that we're anyway "on the way out".
* lib/versioner: Reduce surface area
This is a refactor while I was anyway rooting around in the versioner.
Instead of exporting every possible implementation and the factory and
letting the caller do whatever, this now encapsulates all that and
exposes a New() that takes a config.VersioningConfiguration.
Given that and that we don't know (from the outside) how a versioner
works or what state it keeps, we now just construct it once per folder
and keep it around. Previously it was recreated for each restore
request.
* unparam
* wip
Simon Frei
Lars Lehtonen
Jakob Borg
deepsource-autofix[bot]
Audrius Butkevicius
André Colomb
greatroar
Marcus Legendre