Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a new envinfo package to log system environment details, including CPU, memory, disk usage, and cgroup limits, and adds an iometrics package to track and summarize IO throughput during push, pull, and fetch operations. The review identified several areas for improvement: tryV2 and tryV1 in cgroup_linux.go should be refactored to ensure memory limits are detected even if CPU limits are inaccessible; cpu.Percent usage in envinfo.go may return unhelpful data at startup; the isVirtualFS function appears to be dead code; and the formatBytes utility is duplicated and should be consolidated or replaced with the existing go-humanize dependency.
Add envinfo package that logs build version, runtime, CPU, memory, and disk usage information at startup. On Linux, cgroup CPU/memory limits are also logged to help diagnose container resource constraints. Signed-off-by: Zhao Chen <zhaochen.zju@gmail.com> Signed-off-by: Zhao Chen <winters.zc@antgroup.com>
Add pkg/iometrics package with a Tracker that wraps source io.Reader using atomic counters to measure bytes and cumulative Read() call duration. TrackTransfer() measures per-goroutine wall-clock time. After each operation completes, a summary is output to both terminal (stderr) and log file showing effective throughput, source read throughput, and readFraction (sourceReadTime / transferTime) to help identify whether the bottleneck is in source reading or sink writing. Signed-off-by: Zhao Chen <winters.zc@antgroup.com>
Replace generic "source/effective" labels with operation-aware labels (push: disk read / network write, pull: network read / disk write). Derive sink throughput from transferTime - sourceReadTime. Mark the slower side as bottleneck. Add overall effective throughput to output. Signed-off-by: Zhao Chen <winters.zc@antgroup.com>
- Fix cgroup v1/v2: detect CPU and memory limits independently so one missing limit doesn't prevent detecting the other - Remove cpu.Percent(0) from startup log — unreliable at process start - Remove unused isVirtualFS function (dead code after IO removal) - Replace duplicated formatBytes with humanize.IBytes from go-humanize Signed-off-by: Zhao Chen <winters.zc@antgroup.com>
- Read /proc/self/cgroup to find the process's actual cgroup path before reading limit files, so containers in k8s/Docker sub-cgroups get the correct CPU/memory limits instead of root cgroup values - Wrap manifest bytes.NewReader with tracker.WrapReader so push summaries include manifest bytes in totalBytes Signed-off-by: Zhao Chen <winters.zc@antgroup.com>
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Add diagnostic logging to help identify performance bottlenecks in push/pull/fetch operations:
Example Output
Push (100MB to registry, tested on Linux x86_64):
Pull (50MB from registry):
Log file (structured, for automated analysis):
How It Works
Wraps the source
io.Readerwith acountingReaderusingsync/atomiccounters. EachRead()call records bytes and duration.TrackTransfer()measures per-goroutine wall-clock time. After all goroutines complete:← bottleneckLabels are operation-aware: push shows
disk read/network write, pull/fetch showsnetwork read/disk write.Changes
New packages:
pkg/envinfo/— CPU, memory, disk usage, cgroup v1/v2 limits (Linux), virtual FS detectionpkg/iometrics/—Tracker,countingReader, throughput formattingModified files:
cmd/root.go— callenvinfo.LogEnvironment()at startupcmd/build.go,cmd/pull.go— log disk info for work directoriespkg/backend/push.go,pull.go,fetch.go— integrateTrackerTest Results
Unit tests (macOS, 17 tests):
Covers: concurrent aggregation, timing accuracy, bottleneck detection, error propagation, zero-value safety, data integrity, virtual FS detection.
Remote machine test (Linux x86_64, Intel Xeon E5-2640 v3):
Both correctly identified the bottleneck side.