[pull] main from containerd:main#391
Merged
Merged
Conversation
dmsetup.Status() returns a nil DeviceStatus when it fails. The mkfs failure path records that error, but still unconditionally dereferences status when logging the pool status. Avoid the panic by logging a fallback status string when dmsetup.Status() does not return a status. Found by Linux Verification Center (linuxtesting.org) with SVACE. Signed-off-by: Mikhail Dmitrichenko <m.dmitrichenko222@gmail.com>
The image_pulling_throughput histogram divided the full image size (all layers plus config) by wall-clock pull duration. Layers already present in the content store were counted in the numerator, so the reported MB/s came out way higher than what was actually fetched. Fully-cached pulls were the worst case: they "pulled" in milliseconds but reported the full image size, showing up as huge outliers in the histogram. Both pull paths (local client.Pull and the transfer service) already maintain a totalBytesRead counter in pullRequestReporter for progress and timeout checks. Cached blobs never trigger a fetch, so the counter naturally excludes them. Plumb that value out of both helpers and use it in place of image.Size(ctx). Fully-cached pulls (bytesPulled == 0) are not observed, so they don't produce near-infinite samples. Also updates the metric's Help text to say the denominator is the end-to-end pull duration including layer extraction and snapshotter unpack, not just network transfer. Addresses #13244 Signed-off-by: Ahmet Alp Balkan <ahmet@linkedin.com>
…hroughput Address review feedback on the previous commit: - Use MiB/s (binary, 1024^2) consistently. The divisor was already 1024*1024; the constant name (mbToByte) and the Help/comment text mislabeled the unit. - Keep image_pulling_throughput for backwards compatibility but mark it Deprecated. Its prom.DefBuckets top out at 10 MiB/s, which saturates almost immediately on modern hardware. - Add image_pulling_throughput_mibps with buckets covering 0.1 MiB/s through 4000 MiB/s (~31 Gbps), enough for 10G+ NICs. Above that, disk write throughput becomes the bottleneck rather than the network, so finer buckets aren't worth the cardinality. - Both metrics observe the same value (fetched bytes / pull duration, cached layers excluded; fully-cached pulls skipped). Only the buckets and deprecation status differ. Signed-off-by: Ahmet Alp Balkan <ahmet@linkedin.com>
This test verifies the maximum file-size constraints that were added in [containerd7b05ec4]. However, github.com/moby/sys@v0.4.1 adds similar constraints, including a constraint on line-length (1M): [moby/sys@2c56c3d] that may hit before the file-size limit is reached if the data does not contain newlines. This patch updates the test to use data that includes newlines to make sure it's testing the file-size constraints, not line-limit constraints. [containerd7b05ec4]: 7b05ec4 [moby/sys@2c56c3d]: moby/sys@2c56c3d Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Go 1.26 adds support for passing Windows file flags via os.OpenFile, eliminating the need to call windows.CreateFile directly. github.com/moby/sys/sequential v0.7.0 uses this functionality when compiled with go1.26, but provides fallbacks for older Go versions. Given that containerd has go1.26 as a minimum requirement, we can remove github.com/moby/sys/sequential as an intermediate, and implement the code locally. ref: - moby/sys@9d2fc63 - https://go-review.googlesource.com/c/go/+/699415 - https://go-review.googlesource.com/c/go/+/724621 Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
…ched-layers cri: exclude cached layer bytes from image_pulling_throughput_mibps
…eads pkg/oci: update TestOpenUserFileCapsReads to use newlined data
pkg/archive: remove redundant github.com/moby/sys/sequential dependency
snapshots/devmapper: avoid nil status deref after mkfs failure
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )