This was a nasty bug. Users reported that restic aborts with panic:
panic: store new item in finalized index
The code calling panic() is in the Store() method of an index and guards
the failure case that an index is to be modified while it has already
been saved in the repo.
What happens here (at least that's what I suspect): PackerManager calls
Current() on a MasterIndex, which yields one index A. Concurrently,
another goroutine calls Repository.SaveFullIndex(), which in turn calls
MasterIndex.FullIndexes(), which (among others) yields the index A. Then
all indexes are marked as final. Then the other goroutine is executed
which adds an entry to the index A, which is now marked as final. Then
the panic occurs.
The commit solves this by removing MasterIndex.Current() and adding a
Store() method that stores the entry in one non-finalized index. This
method uses the same RWMutex as the other methods (e.g. FullIndexes()),
thereby ensuring that the full indexes can only be processed before or
after Store() is called.
Closes#367
Target directories from the from-files argument get added to the command
line args, after which all command line args were appended to the same
variable again causing duplicates. Split the used variables to avoid
this.
Signed-off-by: Sjoerd Simons <sjoerd@luon.net>
Previously such files (typically log files) wouldn't be backed up at
all!
The proper behaviour is to backup what we can, and warn the operator
that file is possibly not complete. But it is a warning, not an error.
Closes#689
Since client.BucketExists was changed to return a separate 'found' value, instead of reporting an error when the bucket doesn't exist, the error code path does no longer imply a call to client.MakeBucket. So the second part of the debug message, "...trying to create the bucket" doesn't apply any more.
Also, changed the name of the return value from 'ok' to 'found', matching the API documentation at https://docs.minio.io/docs/golang-client-api-reference#BucketExists.
* When a directory already exists, CreateDirAt returns an error stating so
* This means that the restoreMetadata step is skipped, so for directories which already exist no file permissions, owners, groups, etc will be restored on them
* Not returning the error if it's a "directory exists" error means the metadata will get restored
* It also removes the superfluous "error for ...: mkdir ...: file exists" messages
* This makes the behaviour of directories consistent with that of files (which always have their content & metadata restored, regardless of whether they existed or not)
This is subtle. A combination od fast client disk (read: SSD) with lots
of files and fast network connection to restic-server would suddenly
start getting lots of "dial tcp: connect: cannot assign requested
address" errors during backup stage. Further inspection revealed that
client machine was plagued with TCP sockets in TIME_WAIT state. When
ephemeral port range was finally exhausted, no more sockets could be
opened, so restic would freak out.
To understand the magnitude of this problem, with ~18k ports and default
timeout of 60 seconds, it means more than 300 HTTP connections per
seconds were created and teared down. Yeah, restic-server is that
fast. :)
As it turns out, this behavior was product of 2 subtle issues:
1) The body of HTTP response wasn't read completely with io.ReadFull()
at the end of the Load() function. This deactivated HTTP keepalive,
so already open connections were not reused, but closed instead, and
new ones opened for every new request. io.Copy(ioutil.Discard,
resp.Body) before resp.Body.Close() remedies this.
2) Even with the above fix, somehow having MaxIdleConnsPerHost at its
default value of 2 wasn't enough to stop reconnecting. It is hard to
understand why this would be so detrimental, it could even be some
subtle Go runtime bug. Anyhow, setting this value to match the
connection limit, as set by connLimit global variable, finally nails
this ugly bug.
I fixed several other places where the response body wasn't read in
full (or at all). For example, json.NewDecoder() is also known not to
read the whole body of response.
Unfortunately, this is not over yet. :( The check command is firing up
to 40 simultaneous connections to the restic-server. Then, once again,
MaxIdleConnsPerHost is too low to support keepalive, and sockets in the
TIME_WAIT state pile up. But, as this kind of concurrency absolutely
kill the poor disk on the server side, this is a completely different
bug then.
$ bin/restic forget /d 7 /w 4 /m 12
argument "/d" is not a snapshot ID, ignoring
argument "7" is not a snapshot ID, ignoring
argument "/w" is not a snapshot ID, ignoring
argument "4" is not a snapshot ID, ignoring
argument "/m" is not a snapshot ID, ignoring
cound not find a snapshot for ID "12", ignoring
Matthew Holt
Alexander Neumann
Zlatko Čalušić
Sjoerd Simons
Jan Niehusmann
Jan Niehusmann
JP Mens
Seb Patane
Alexander Thaller
Justin Clift
David
Daniel Örn
Christian Kemper