-
Notifications
You must be signed in to change notification settings - Fork 246
Track request lifetimes, add it as a scaling parameter in the concurrency tuner, default to always tuning. #3074
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a mechanism to track individual HTTP request lifetimes and feeds that metric into the concurrency auto-tuner, while also making auto-tuning the default behavior.
- Added
RequestLifetimeTracker
with sliding-window bucket aggregation, exposed as an HTTP pipeline policy. - Enhanced
autoConcurrencyTuner
to back off when unusually long request lifetimes are detected. - Switched CLI defaults to always enable auto-tuning and removed special-case file transfer overrides.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
ste/xferLifetimeTracker.go | New tracker for request durations and policy API. |
ste/mgr-JobPartMgr.go | Injected lifetime tracker into per-retry pipeline. |
ste/concurrencyTuner.go | Added lifetime-based backoff and updated tuner API. |
jobsAdmin/JobsAdmin.go | Adjusted tuner creation call; removed old setter. |
cmd/root.go | Defaulted CLI to always use auto-tuning. |
cmd/copy.go | Removed legacy file-type concurrency override. |
common/linkedList.go | Introduced generic LinkedList utility. |
Comments suppressed due to low confidence (3)
ste/mgr-JobPartMgr.go:108
- Appending
NewDestReauthPolicy
toperRetryPolicies
instead ofperCallPolicies
is likely a copy-paste error. It should beappend(perCallPolicies, ...)
to maintain intended ordering.
perCallPolicies = append(perRetryPolicies, NewDestReauthPolicy(dstCred))
ste/concurrencyTuner.go:150
- [nitpick] The constant name
minMulitplier
is misspelled. Rename it tominMultiplier
for clarity and consistency.
const minMulitplier = 1.19 // really this is 1.2, but use a little less to make the floating point comparisons robust
ste/xferLifetimeTracker.go:14
- New
RequestLifetimeTracker
functionality lacks unit tests for bucket rotation and policy behavior. Adding tests will help ensure the tracking and backoff logic remains correct.
/*
Description
This half-fixes a bug that's evoked in situations where the number of goroutines we initialize far out-scales the available network bandwidth. It can be evoked by setting
AZCOPY_CONCURRENCY_VALUE
super high and--cap-mbps
super low.The janky fix present in this PR is to default the auto-scaler to on. Because it's seeking the optimal output, and trying to not over-scale, scaling past the network's capabilities will cause it to scale down as throughput didn't increase as expected. This does not fix the problem, but it does work around it for the grand and wide majority of users. More PRs are to come of this.
Type of Change
How Has This Been Tested?
Manual testing, set benchmark with a 100gb upload and 5mbps cap-mbps. Request lifetime stabilizes out to around a minute max after the concurrency tuner settles. This still isn't great, it's a really severe issue, but it does mean the job works, which is way better than it not working.
A user manually specifying both could still trigger the underlying behavior.