`registry/storage/driver/inmemory/driver_test.go` times out after ~10min. The slow test is `testsuites.go:TestWriteReadLargeStreams()` which writes a 5GB file.
Root cause is inefficient slice reallocation algorithm. The slice holding file bytes grows only 32K on each allocation. To fix it, this PR grows slice exponentially.
Signed-off-by: Wei Meng <wemeng@microsoft.com>
Go 1.18 and up now provides a strings.Cut() which is better suited for
splitting key/value pairs (and similar constructs), and performs better:
```go
func BenchmarkSplit(b *testing.B) {
b.ReportAllocs()
data := []string{"12hello=world", "12hello=", "12=hello", "12hello"}
for i := 0; i < b.N; i++ {
for _, s := range data {
_ = strings.SplitN(s, "=", 2)[0]
}
}
}
func BenchmarkCut(b *testing.B) {
b.ReportAllocs()
data := []string{"12hello=world", "12hello=", "12=hello", "12hello"}
for i := 0; i < b.N; i++ {
for _, s := range data {
_, _, _ = strings.Cut(s, "=")
}
}
}
```
BenchmarkSplit
BenchmarkSplit-10 8244206 128.0 ns/op 128 B/op 4 allocs/op
BenchmarkCut
BenchmarkCut-10 54411998 21.80 ns/op 0 B/op 0 allocs/op
While looking at occurrences of `strings.Split()`, I also updated some for alternatives,
or added some constraints;
- for cases where an specific number of items is expected, I used `strings.SplitN()`
with a suitable limit. This prevents (theoretical) unlimited splits.
- in some cases it we were using `strings.Split()`, but _actually_ were trying to match
a prefix; for those I replaced the code to just match (and/or strip) the prefix.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- use strings.HasPrefix() to check for the prefix we're interested in instead
of doing a strings.Split() without limits. This makes the code both easier
to read, and prevents potential situations where we end up with a long slice.
- use consts for defaults; these should never be modified, so better to use
consts for them to indicate they're fixed values.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This package was only used for the deprecated "shortid" syntax. Now that
support for this syntax was removed, we can also remove this package.
This patch deprecates and removes the package, adding temporary aliases pointing
to the new location to ease migration from docker/distribution to the new
distribution/distribution/v3. We should remove those aliases in a future update.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This upstreams `Sort()` as originally implemented in containerd in
https://github.com/containerd/containerd/0886ceaea2470edc7339dfc5ebe0e3257ae84d06
From that commit:
> Fix reference ordering in CRI image store
>
> Currently image references end up being stored in a
> random order due to the way maps are iterated through
> in Go. This leads to inconsistent identifiers being
> resolved when a single reference is needed to identify
> an image and the ordering of the references is used for
> the selection.
>
> Sort references in a consistent and ranked manner,
> from higher information formats to lower.
>
> Note: A `name + tag` reference is considered higher
> information than a `name + digest` reference since a
> registry may be used to resolve the digest from a
> `name + tag` reference.
Co-Authored-by: Derek McGowan <derek@mcg.dev>
Co-Authored-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Both of these were deprecated in 55f675811a,
but the format of the GoDoc comments didn't follow the correct format, which
caused them not being picked up by tools as "deprecated".
This patch updates uses in the codebase to use the alternatives.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Go requires "deprecated" comments to have an empty line before them,
and to not be all-caps.
This updates to the comment so that it's correctly picked up as deprecated.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The "shortid" syntax was added in d26a3b37a6,
and allowed for matching an image on its ID prefix (this is before images were
content-addressable). With the introduction of content-addressable references,
this syntax became problematic, and Docker deprecated this syntax in 2016
(Docker v1.13.0) through commit; 5fc71599a0
> The `repository:shortid` syntax for referencing images is very little used,
> collides with tag references, and can be confused with digest references.
Support for this syntax was removed in 2017 (Docker 17.12) through commit:
a942c92dd7
containerd uses a fork of the reference package with this syntax removed, and
does not support this syntax:
901bcb2231
This patch removes the deprecated syntax, the ParseAnyReferenceWithSet function,
and the ShortIdentifierRegexp regex.
As there are no external consumers for this function, nor the regexp, I'm
skipping a deprecation cycle for this;
- https://grep.app/search?q=.ShortIdentifierRegexp
- https://grep.app/search?q=.ParseAnyReferenceWithSet%28
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
gofumpt (https://github.com/mvdan/gofumpt) provides a supserset of `gofmt` / `go fmt`,
and addresses various formatting issues that linters may be checking for.
We can consider enabling the `gofumpt` linter to verify the formatting in CI, although
not every developer may have it installed, so for now this runs it once to get formatting
in shape.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
We were using v1.0.0 of Cobra as newer versions added spf13/viper as dependency,
which came with many indirect dependencies. Cobra v1.6.0 and up no longer depend
on viper, so we can now safely upgrade to the latest version.
full diff: https://github.com/spf13/cobra/compare/v1.0.0...v1.6.1
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Instead of first collecting all keys and then batch deleting them,
we will do the incremental delete _online_ per max allowed batch.
Doing this prevents frequent allocations for large S3 keyspaces
and OOM-kills that might happen as a result of those.
This commit introduces storagedriver.Errors type that allows to return
multierrors as a single error from any storage driver implementation.
Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
1, return the right upload offset for client when asks.
2, do not call ResumeBlobUpload on getting status.
3, return 416 rather than 404 on failed to patch chunk blob.
4, add the missing upload close
Signed-off-by: Wang Yan <wangyan@vmware.com>
Instead of letting the cache grow without bound, use a LRU to impose a
size limit.
The limit is configurable through a new `blobdescriptorsize` config key.
Signed-off-by: Aaron Lehmann <alehmann@netflix.com>
