The QUIC package is notorious for being incompatible with either too
old or too new Go releases. Currently it doesn't build with Go 1.15 RC
and I want to test the rest with Go 1.15. With this I can do `go run
build.go --tags noquic` to do that.
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>
Prompted by https://forum.syncthing.net/t/infinite-filesystem-recursion-detected/15285. In my opinion the filesystem shouldn't throw warnings but pass on errors for the caller to decide what's to be happening with it. Right now in this PR an infinite recursion is a normal scan error, i.e. folder is in failed state and displays failed items, but no warning. I think that's appropriate but if deemed appropriate an additional warning can be thrown in the scanner.
This fixes the change in #6674 where the weak hash became a deciding
factor. Now we again just use it to accept a block, but don't take a
negative as meaning the block is bad.
If the GC finds a key k that it wants to keep, it records that in a
Bloom filter. If a key k' can be removed but its hash collides with k,
it will be kept. Since the old Bloom filter code was completely
deterministic, the next run would encounter the same collision, assuming
k must still be kept.
A randomized hash function that uses all the SHA-256 bits solves this
problem: the second run has a non-zero probability of removing k', as
long as the Bloom filter is not completely full.
* 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 f74af2b067.
* Revert "Remove pq and sql dep from syncthing, split build targets"
This reverts commit 8fc295ad00.
* Revert "Split "all" build target, add package name as a tag"
This reverts commit f4dc889951.
* Normalise contract types
* Fix build add more logging
This is shorter, skips two allocations, makes the function inlineable
and is safer, since the compiler now check whether
DeviceIDLength == sha256.Size.
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...
crypto/rand output is cryptographically secure by the Go library
documentation's promise. That, rather than strength (= passes randomness
tests) is the property that Syncthing needs).
This makes Go 1.15 test/vet happy, avoiding "conversion from untyped int
to string yields a string of one rune" warning where we do
string(KeyTypeWhatever) in namespaced.go.
It also clarifies and enforces the currently allowed range of these
numbers so I think it's fine.
This matches the convention of the stdlib and avoids ambiguity: when
customErr{} and &customErr{} both implement error, client code needs to
check for both.
Memory use should remain the same, since storing a non-pointer type in
an interface value still copies the value to the heap.
This adds some env vars to the long version string as if they were build
tags. The purpose is to better understand what code was running or not
in the version output, usage reporting and crash reports. In order to
prevent possible privacy issues the actual value of the variable is not
reported, just the fact that it was set to something non-empty.
Example:
% ./bin/syncthing --version
syncthing v1.6.1+47-g1eb104f3-buildtags "Fermium Flea" (go1.14.3 darwin-amd64) jb@kvin.kastelo.net 2020-06-03 07:25:46 UTC [stnoupgrade, use_badger]
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>
This reduces the size of our write batches before we flush them. This
has two effects: reducing the amount of data lost if we crash when
updating the database, and reducing the amount of memory used when we do
large updates without checkpoint (e.g., deleting a folder).
I ran our SyncManyFiles benchmark as it is the one doing most
transactions, however there was no relevant change in any metric (it's
limited by our fsync I expect). This is good as any visible change would
just be a decrease in performance.
I don't have a benchmark on deleting a large folder, taking that part on
trust for now...
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.
Storing assets as []byte requires every compiled-in asset to be copied
into writable memory at program startup. That currently takes up 1.6MB
per syncthing process. Strings stay in the RODATA section and should be
shared between processes running the same binary.
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
This changes the build script to build all the things in one go
invocation, instead of one invocation per cmd. This is a lot faster
because it means more things get compiled concurrently. It's especially
a lot faster when things *don't* need to be rebuilt, possibly because it
only needs to build the dependency map and such once instead of once per
binary.
In order for this to work we need to be able to pass the same ldflags to
all the binaries. This means we can't set the program name with an
ldflag.
When it needs to rebuild everything (go clean -cache):
( ./old-build -gocmd go1.14.2 build all 2> /dev/null; ) 65.82s user 11.28s system 574% cpu 13.409 total
( ./new-build -gocmd go1.14.2 build all 2> /dev/null; ) 63.26s user 7.12s system 1220% cpu 5.766 total
On a subsequent run (nothing to build, just link the binaries):
( ./old-build -gocmd go1.14.2 build all 2> /dev/null; ) 26.58s user 7.53s system 582% cpu 5.853 total
( ./new-build -gocmd go1.14.2 build all 2> /dev/null; ) 18.66s user 2.45s system 1090% cpu 1.935 total
This makes the GC runner a service that will stop fairly quickly when
told to.
As a bonus, STTRACE=app will print the service tree on the way out,
including any errors they've flagged.
So, in a funny plot twist, it turns out that WriteFile in Go 1.13
doesn't actually set the read only bit on Windows when called with
permissions 0444 so my test was broken. With an improved test it turns
out that Rename does not, in fact, overwrite a read-only file on
Windows. This adds a fix for that.
(Rename might get improved in Go 1.15...)
This adds the functionality to run a user search with a filter for LDAP
authentication. The search is done after successful bind, as the binding
user. The typical use case is to limit authentication to users who are
member of a group or under a certain OU. For example, to only match
users in the "Syncthing" group in otherwise default Active Directory
set up for example.com:
<searchBaseDN>CN=Users,DC=example,DC=com</searchBaseDN>
<searchFilter>(&(sAMAccountName=%s)(memberOf=CN=Syncthing,CN=Users,DC=example,DC=com))</searchFilter>
The search filter is an "and" of two criteria (with the ampersand being
XML quoted),
- "(sAMAccountName=%s)" matches the user logging in
- "(memberOf=CN=Syncthing,CN=Users,DC=example,DC=com)" matches members
of the group in question.
Authentication will only proceed if the search filter matches precisely
one user.
The previous implementation was very generic; its tests didn't cover the
actual alphabet for device IDs.
Benchmark results on amd64:
name old time/op new time/op delta
Luhnify-8 1.00µs ± 1% 0.28µs ± 4% -72.38% (p=0.000 n=9+10)
Unluhnify-8 992ns ± 2% 274ns ± 1% -72.39% (p=0.000 n=10+9)
As of the latest database checker we are again putting files without
blocks. I'm not 100% convinced that's a great idea, but we also do it
for ignored files apparently so it looks like we probably should support
it. This adds an escape hatch that must be manually enabled...
- 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.
If we decide to recalculate the metadata we shouldn't start from
whatever we loaded from the database, as that data is wrong. We should
start from a clean slate.
I was working on indirecting version vectors, and that resulted in some
refactoring and improving the existing block indirection stuff. We may
or may not end up doing the version vector indirection, but I think
these changes are reasonable anyhow and will simplify the diff
significantly if we do go there. The main points are:
- A bunch of renaming to make the indirection and GC not about "blocks"
but about "indirection".
- Adding a cutoff so that we don't actually indirect for small block
lists. This gets us better performance when handling small files as it
cuts out the indirection for quite small loss in space efficiency.
- Being paranoid and always recalculating the hash on put. This costs
some CPU, but the consequences if a buggy or malicious implementation
silently substituted the block list by lying about the hash would be bad.
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 NAT discovery we block for 10s (NatTimeoutS) before returning.
This is mostly noticeable when Ctrl-C:ing a Syncthing directly after
startup as we wait for those ten seconds before shutting down. This
makes it check the context a little bit more frequently.
This adds metadata updates to the same write batch as the underlying
file change. The odds of a metadata update going missing is greatly
reduced.
Bonus change: actually commit the transaction in recalcMeta.
lib/db: Recover sequence number and metadata on startup (fixes#6335)
If we crashed after writing new file entries but before updating
metadata in the database the sequence number and metadata will be wrong.
This fixes that.
We could potentially get a snapshot and then fail to get a releaser,
leaking the snapshot. This takes the releaser first and makes sure to
release it on snapshot error.
The readWriteTransaction offered both commit() (the one to use) and
Commit() (via embedding) where the latter didn't close the read
transaction. This removes the lower cased variant in order to prevent
the mistake.
The only place where the mistake was made was the new gc runner, where
it would leave a read snapshot open forever.
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.
