A delayed lock refresh could send a signal on the `refreshed` channel
while the `monitorLockRefresh` goroutine waits for a reply to its
`refreshLockRequest`. As the channels are unbuffered, this resulted in a
deadlock.
A stale lock may be refreshed if it continues to exist until after a
replacement lock has been created. This ensures that a repository was
not unlocked in the meantime.
Conceptually the backend configuration should be validated when creating
or opening the backend, but not when filling in information from
environment variables into the configuration.
This unified construction removes most backend-specific code from
global.go. The backend registry will also enable integration tests to
use custom backends if necessary.
In order to change the backend initialization in `global.go` to be able
to generically call cfg.ApplyEnvironment() for supported backends, the
`interface{}` returned by `ParseConfig` must contain a pointer to the
configuration.
An alternative would be to use reflection to convert the type from
`interface{}(Config)` to `interface{}(*Config)` (from value to pointer
type). However, this would just complicate the type mess further.
The index used by restic consumes a major part of the total memory
usage. This leads to an unnecessarily large amount of memory that
contains ephemeral objects that are only used for a short time.
Modifies format module to add options for human readable storage size formatting, using size parsing already in ui/format.
Cmd flag --human-readable added to ls and find commands.
Additional option added to formatNode to support printing size in regular or new human readable format
The tests are now split into individual files for each command. The
separation isn't perfect as many tests make use of multiple commands. In
particular `init`, `backup`, `check` and `list` are used by a larger
number of test cases.
Most tests now reside in files name cmd_<name>_integration_test.go. This
provides a certain indication which commands have significant test
coverage.
Use the logging methods from testing.TB to make use of tb.Helper(). This
allows the tests to log the filename and line number in which the test
helper was called. Previously the test helper was logged which is rarely
useful.
As the `Fatal` error type only includes a string, it becomes impossible
to inspect the contained error. This is for a example a problem for the
fuse implementation, which must be able to detect context.Canceled
errors.
Co-authored-by: greatroar <61184462+greatroar@users.noreply.github.com>
The previous approach of rewriting all snapshots first, then flushing
the repository data and finally removing old snapshots has the downside
that an interrupted command execution leaves behind broken snapshots as
not all new data is already flushed.
This adds support for caching already rewritten trees, handling of load
errors and disabling the check that the serialization doesn't lead to
data loss.
The more generic RewriteNode callback replaces the SelectByName and
PrintExclude functions. The main part of this change is a preparation to
allow using the TreeRewriter for the `repair snapshots` command.
The files in a tree must be sorted in lexical order. However, this
cannot be guaranteed when appending a filename suffix. For two files
file, file.rep
where "file" is broken, this would result in
file.repaired, file.rep
which is no longer sorted.
In addition, adding a filename suffix is also prone to filename
collisions which would require a rather complex search for a
collision-free name in order to work reliably.
Simplify CLI options:
* Rename "DeleteSnapshots" to "Forget"
* Replace "AddTag" and "Append" with hardcoded values
Change output and snapshot modifications to be more in line with the
"rewrite" command.
The builtin mechanism to capture a stacktrace in Go is to send a SIGQUIT
to the running process. However, this mechanism is not avaiable on
Windows. Thus, tweak the SIGINT handler to dump a stacktrace if the
environment variable `RESTIC_DEBUG_STACKTRACE_SIGINT` is set.
When saving files to the local backend, in some cases the used fsync
calls are slow enough to cause the tests to time out. Thus, increase the
test timeouts as a stopgap measure until we can use the mem backend for
these tests.
The SemaphoreBackend now uniformly enforces the limit of concurrent
backend operations. In addition, it unifies the parameter validation.
The List() methods no longer uses a semaphore. Restic already never runs
multiple list operations in parallel.
By managing the semaphore in a wrapper backend, the sections that hold a
semaphore token grow slightly. However, the main bottleneck is IO, so
this shouldn't make much of a difference.
The key insight that enables the SemaphoreBackend is that all of the
complex semaphore handling in `openReader()` still happens within the
original call to `Load()`. Thus, getting and releasing the semaphore
tokens can be refactored to happen directly in `Load()`. This eliminates
the need for wrapping the reader in `openReader()` to release the token.
This turns snapshotFilterOptions from cmd into a restic.SnapshotFilter
type and makes restic.FindFilteredSnapshot and FindFilteredSnapshots
methods on that type. This fixes#4211 by ensuring that hosts and paths
are named struct fields instead of unnamed function arguments in long
lists of such.
Timestamp limits are also included in the new type. To avoid too much
pointer handling, the convention is that time zero means no limit.
That's January 1st, year 1, 00:00 UTC, which is so unlikely a date that
we can sacrifice it for simpler code.
The output is now
```
-v, --verbose be verbose (specify multiple times or a level using --verbose=n, max level/times is 2)
```
instead of
```
-v, --verbose n be verbose (specify multiple times or a level using --verbose=n, max level/times is 2)
```
Fixes restic#719
If the option is passed, restic will wait the specified duration of time
and retry locking the repo every 10 seconds (or more often if the total
timeout is relatively small).
- Play nice with json output
- Reduce wait time in lock tests
- Rework timeout last attempt
- Reduce test wait time to 0.1s
- Use exponential back off for the retry lock
- Don't pass gopts to lockRepo functions
- Use global variable for retry sleep setup
- Exit retry lock on cancel
- Better wording for flag help
- Reorder debug statement
- Refactor tests
- Lower max sleep time to 1m
- Test that we cancel/timeout in time
- Use non blocking sleep function
- Refactor into minDuration func
Co-authored-by: Julian Brost <julian@0x4a42.net>
This turns snapshotFilterOptions from cmd into a restic.SnapshotFilter
type and makes restic.FindFilteredSnapshot and FindFilteredSnapshots
methods on that type. This fixes#4211 by ensuring that hosts and paths
are named struct fields instead of unnamed function arguments in long
lists of such.
Timestamp limits are also included in the new type. To avoid too much
pointer handling, the convention is that time zero means no limit.
That's January 1st, year 1, 00:00 UTC, which is so unlikely a date that
we can sacrifice it for simpler code.
The output is now
```
-v, --verbose be verbose (specify multiple times or a level using --verbose=n, max level/times is 2)
```
instead of
```
-v, --verbose n be verbose (specify multiple times or a level using --verbose=n, max level/times is 2)
```
This method had a buffer argument, but that was nil at all call sites.
That's removed, and instead LoadUnpacked now reuses whatever it
allocates inside its retry loop.
The StdioWrapper is not used at all by the ProgressPrinters. It is
called a bit earlier than previously. However, as the password prompt
directly accessed stdin/stdout this doesn't cause problems.
The maximum for `--verbose=n` is n=2. Internally it is translated into a
scale from 0 to 3. However, the default (without verbose) is 1, thus the
verbosity level can only be increased two times.
Only the repacking of *un*compressed packs reduces the amount of
uncompressed data. Previously the counter even overflowed for fully
compressed repositories.
Commands should use the normal shutdown path. In addition, the Exitf
function was only used by `dump` and `restore` but not any other command
which introduces the risk of inconsistent behavior.
When reporting an error for a tree, the output message can overlap with
the progress bar output, e.g. `error for tree e91ef6fb:napshots`.
The fix only applies for this specific message and does not work on
Windows.
Revert what seems to be a typo introduced as part of the fix for #2041
in 2018 7d0f2eaf24.
`xbuild` does not look like a go build/tag keyword to me, I failed to
find documentation for it and using `go install -tags '!selfupdate' ...`
has no effect, i.e. self-update code is still compiled.
`+build` however works; updating the OpenBSD port/binary package
security/restic to apply this PR works as expected:
```
$ restic help | grep self
$ restic self-update
unknown command "self-update" for "restic"
```
(Using `go:build` now as per restic's style and gofmt.)
Previously, using `restic-0.14.0p1` on OpenBSD/amd64 7.2-current would
check for a newer version and probably attempt replacing the system wide
root-owned executable (on a read-only filesystem) as unprivileged user:
```
$ restic version
restic 0.14.0 compiled with go1.19.2 on openbsd/amd64
$ restic help | grep self
self-update Update the restic binary
$ restic self-update
writing restic to /usr/local/bin/restic
find latest release of restic at GitHub
restic is up to date
```
(It never tried to actually write besaid path; doing so would fail, so
the current message can be considered misleading.)
The scanner process has only cosmetic effect for the progress printer,
and can be disabled without impacting functionality when the user does
not need an estimate of completion.
In many cases the scanner process can provide beneficial priming of
the file system cache, so as general advice it should not be disabled.
However, tests have shown that backup of NFS and fuse based filesystems,
where stat(2) is relatively expensive, can be significantly faster
without the scanner.
The Original field is meant to remember the original snapshot id if e.g.
changing its tags. It was only set by the `rewrite` command if it was
not set previously. However, a rewritten snapshot is potentially rather
different from the original snapshot. Thus just always set the Original
field. This also makes it easier to later on detect and potentially
remove the original snapshots.
The ioutil functions are deprecated since Go 1.17 and only wrap another
library function. Thus directly call the underlying function.
This commit only mechanically replaces the function calls.
The old check did not consider files containing case insensitive
excludes. The check is now implemented as a function of the
excludePatternOptions struct to improve cohesion.
The test did not wait for the mount command to fully shutdown all
running goroutines. This caused the go race detector to report a data
race related to lock refreshes.
==================
WARNING: DATA RACE
Write at 0x0000021bdfdb by goroutine 667:
github.com/restic/restic/internal/backend/retry.TestFastRetries()
/restic/restic/internal/backend/retry/testing.go:7 +0x18f
github.com/restic/restic/cmd/restic.withTestEnvironment()
/restic/restic/cmd/restic/integration_helpers_test.go:175 +0x183
github.com/restic/restic/cmd/restic.TestMountSameTimestamps()
/restic/restic/cmd/restic/integration_fuse_test.go:202 +0xac
testing.tRunner()
/usr/lib/go/src/testing/testing.go:1446 +0x216
testing.(*T).Run.func1()
/usr/lib/go/src/testing/testing.go:1493 +0x47
Previous read at 0x0000021bdfdb by goroutine 609:
github.com/restic/restic/internal/backend/retry.(*Backend).retry()
/restic/restic/internal/backend/retry/backend_retry.go:72 +0x9e
github.com/restic/restic/internal/backend/retry.(*Backend).Remove()
/restic/restic/internal/backend/retry/backend_retry.go:149 +0x17d
github.com/restic/restic/internal/cache.(*Backend).Remove()
/restic/restic/internal/cache/backend.go:38 +0x11d
github.com/restic/restic/internal/restic.(*Lock).Unlock()
/restic/restic/internal/restic/lock.go:190 +0x249
github.com/restic/restic/cmd/restic.refreshLocks.func1()
/restic/restic/cmd/restic/lock.go:86 +0xae
runtime.deferreturn()
/usr/lib/go/src/runtime/panic.go:476 +0x32
github.com/restic/restic/cmd/restic.lockRepository.func2()
/restic/restic/cmd/restic/lock.go:61 +0x71
[...]
Goroutine 609 (finished) created at:
github.com/restic/restic/cmd/restic.lockRepository()
/restic/restic/cmd/restic/lock.go:61 +0x488
github.com/restic/restic/cmd/restic.lockRepo()
/restic/restic/cmd/restic/lock.go:25 +0x219
github.com/restic/restic/cmd/restic.runMount()
/restic/restic/cmd/restic/cmd_mount.go:126 +0x1f8
github.com/restic/restic/cmd/restic.testRunMount()
/restic/restic/cmd/restic/integration_fuse_test.go:61 +0x1ce
github.com/restic/restic/cmd/restic.checkSnapshots.func1()
/restic/restic/cmd/restic/integration_fuse_test.go:90 +0x124
==================
Counting the first occurrence of a duplicate blob as used and counting
all other as duplicates, independent of which instance of the blob is
kept, is only accurate if all copies of the blob have the same size. This
is no longer the case for a repository containing both compressed and
uncompressed blobs.
Thus for duplicated blobs first count all instances as duplicates and
then subtract the actually used instance later on.
As long as only a small fraction of the data in a repository is
rewritten, the keepBlobs set will be rather small after cleaning it up.
As golang maps do not shrink their memory usage, just copy the contents
over to a new map. However, only copy the map if the cleanup removed at
least half the entries.
The set covers necessary, existing and duplicate blobs. This removes the
duplicate sets used to track whether all necessary blobs also exist.
This reduces the memory usage of prune by about 20-30%.
The RetryBackend tests depend on the mock backend. When the Backend
interface is eventually split from the restic package, this will lead to
a dependency cycle between backend and backend/mock. Thus split the
RetryBackend into a separate package to avoid this problem.
The comparison of the current time and the last lock refresh were using
seconds represented as integers. As the test only waits for up to one
second, the associated number truncation can cause the test to take
longer than once second and thus to fail.
Switch to nanoseconds to avoid this problem. This also slightly speeds
up the test.
