Centralize HTTP client into a shared global instance by shnayak-msft · Pull Request #3436 · Azure/azure-storage-azcopy

shnayak-msft · 2026-04-09T23:30:42Z

Introduce common.GetGlobalHTTPClient() to create a single process-wide HTTP client, replacing per-job client creation in jobMgr and the separate front-end client in traverser/credentialUtil.go.

Add common/azHttpClient.go with GetGlobalHTTPClient, MaxIdleConnsPerHost tuning logic, and optional connection-reuse tracing via httptrace
Replace NewAzcopyHTTPClient in ste/mgr-JobMgr.go with the global client
Update traverser/credentialUtil.go to use the global client, removing the local frontEndMaxIdleConnectionsPerHost constant
Add NewTracingTransport wrapper for per-label connection stats logging (GotConn reuse/idle metrics, PutIdleConn errors, periodic dump)

Current http client configuration sets the connection reuse to 2 instead of a value based on concurrency. High concurrency leads to a high number of new TCP connections which causes socket errors and high syscall latency impacting performance. This change addresses this issue.

Description

Feature / Bug Fix: (Brief description of the feature or issue being addressed)
Related Links:
Issues
Team thread
Documents
[Email Subject]

Type of Change

How Has This Been Tested?

Pipeline run - https://dev.azure.com/azstorage/AzCopy-NextGen/_build/results?buildId=30434&view=results

Thank you for your contribution to AzCopy!

…tracing Introduce common.GetGlobalHTTPClient() to create a single process-wide HTTP client, replacing per-job client creation in jobMgr and the separate front-end client in traverser/credentialUtil.go. - Add common/azHttpClient.go with GetGlobalHTTPClient, MaxIdleConnsPerHost tuning logic, and optional connection-reuse tracing via httptrace - Replace NewAzcopyHTTPClient in ste/mgr-JobMgr.go with the global client - Update traverser/credentialUtil.go to use the global client, removing the local frontEndMaxIdleConnectionsPerHost constant - Add NewTracingTransport wrapper for per-label connection stats logging (GotConn reuse/idle metrics, PutIdleConn errors, periodic dump)

Copilot

Pull request overview

This PR centralizes HTTP client creation by introducing a single process-wide HTTP client in common, and switching both the STE job manager and traverser client-options creation to reuse that shared client (with optional httptrace-based connection reuse instrumentation).

Changes:

Add common.GetGlobalHTTPClient() and related MaxIdleConnsPerHost tuning + optional connection tracing helpers.
Update STE job manager to use the global HTTP client instead of creating a per-job client.
Update traverser credential/client options creation to use the global HTTP client.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

File	Description
common/azHttpClient.go	Introduces the global HTTP client and connection-pool tuning / tracing helpers.
ste/mgr-JobMgr.go	Switches job manager HTTP client initialization to the shared global client.
traverser/credentialUtil.go	Switches traverser client options to reuse the shared global client.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-09T23:34:28Z

+	autoTune := true
+	envVar := EEnvironmentVariable.ConcurrencyValue()
+	envValue := GetEnvironmentVariable(envVar)
+	concurrencyValue := maxIdleConnsPerHost_MaxValue
+
+	if envValue != "AUTO" && envValue != "" {
+		concurrencyVal, err := strconv.ParseInt(envValue, 10, 0)
+		if err == nil {
+			concurrencyValue = min(int(concurrencyVal), concurrencyValue)
+			autoTune = false


GetMaxIdleConnsPerHost defaults autoTune to true when the env var is unset. That differs from ste/getMainPoolSize (auto-tune only when AZCOPY_CONCURRENCY_VALUE == "AUTO") and causes the function to take the auto-tune path in normal runs.

Suggested change

autoTune := true

envVar := EEnvironmentVariable.ConcurrencyValue()

envValue := GetEnvironmentVariable(envVar)

concurrencyValue := maxIdleConnsPerHost_MaxValue

if envValue != "AUTO" && envValue != "" {

concurrencyVal, err := strconv.ParseInt(envValue, 10, 0)

if err == nil {

concurrencyValue = min(int(concurrencyVal), concurrencyValue)

autoTune = false

autoTune := false

envVar := EEnvironmentVariable.ConcurrencyValue()

envValue := GetEnvironmentVariable(envVar)

concurrencyValue := maxIdleConnsPerHost_MaxValue

if envValue == "AUTO" {

autoTune = true

} else if envValue != "" {

concurrencyVal, err := strconv.ParseInt(envValue, 10, 0)

if err == nil {

concurrencyValue = min(int(concurrencyVal), concurrencyValue)

Copilot · 2026-04-09T23:34:28Z

+		if autoTune {
+			computedDefaultVal = 4 // deliberately start with a small initial value if we are auto-tuning.  If it's not small enough, then the auto tuning becomes
+			// sluggish since, every time it needs to tune downwards, it needs to let a lot of data (num connections * block size) get transmitted,
+			// and that is slow over very small links, e.g. 10 Mbps, and produces noticeable time lag when downsizing the connection count.
+			// So we start small. (The alternatives, of using small chunk sizes or small file sizes just for the first 200 MB or so, were too hard to orchestrate within the existing app architecture)
+		} else if numOfCPUs <= 4 {


The inner if autoTune { ... } else if ... block is unreachable in the else if numOfCPUs <= 4 { ... }/CPU-based branches because the outer if autoTune { already guarantees autoTune is true. As written, computedDefaultVal will always be set to 4, making MaxIdleConnsPerHost far too low.

Suggested change

if autoTune {

computedDefaultVal = 4 // deliberately start with a small initial value if we are auto-tuning. If it's not small enough, then the auto tuning becomes

// sluggish since, every time it needs to tune downwards, it needs to let a lot of data (num connections * block size) get transmitted,

// and that is slow over very small links, e.g. 10 Mbps, and produces noticeable time lag when downsizing the connection count.

// So we start small. (The alternatives, of using small chunk sizes or small file sizes just for the first 200 MB or so, were too hard to orchestrate within the existing app architecture)

} else if numOfCPUs <= 4 {

if numOfCPUs <= 4 {

Copilot · 2026-04-09T23:34:28Z

+// GlobalHTTPClient is the process-wide HTTP client used by AzCopy when initialized via InitGlobalHTTPClient.
+var (
+	GlobalHTTPClient     *http.Client
+	globalHTTPClientOnce sync.Once
+)


Comment mentions initialization via InitGlobalHTTPClient, but no such function exists in this package/file. This is misleading—either rename the reference to GetGlobalHTTPClient or add the missing initializer if intended.

Copilot · 2026-04-09T23:34:29Z

+// Usage: replace the Transport field of an *http.Client with the result of this function.
+// Use common.NewTracingTransport(client, "createClientOptions", logger) for http.Trace


NewTracingTransport’s doc says to “replace the Transport field of an *http.Client”, but it returns policy.Transporter (not http.RoundTripper) and is intended to be passed to azcore.ClientOptions.Transport (or to wrap an existing policy.Transporter). Please adjust the comment to match the actual usage/type expectations.

Suggested change

// Usage: replace the Transport field of an *http.Client with the result of this function.

// Use common.NewTracingTransport(client, "createClientOptions", logger) for http.Trace

// Usage: wrap an existing policy.Transporter with the result of this function, or assign the result

// to azcore.ClientOptions.Transport. For example: common.NewTracingTransport(inner, "createClientOptions", logger).

Copilot · 2026-04-09T23:34:29Z


 	jm := jobMgr{jobID: jobID, jobPartMgrs: newJobPartToJobPartMgr(),
-		httpClient:           NewAzcopyHTTPClient(concurrency.MaxIdleConnections),
+		httpClient:           common.GetGlobalHTTPClient(jobLogger),


Switching from NewAzcopyHTTPClient(concurrency.MaxIdleConnections) to the global client means ConcurrencySettings.MaxIdleConnections is no longer used anywhere. If MaxIdleConnections is still meant to tune connection pooling, consider initializing the global client with this value (or remove/update the setting to avoid dead configuration).

Suggested change

httpClient: common.GetGlobalHTTPClient(jobLogger),

httpClient: common.NewAzcopyHTTPClient(concurrency.MaxIdleConnections),

Copilot · 2026-04-09T23:34:29Z

-		ApplicationID: common.AddUserAgentPrefix(common.UserAgent),
-	}, ste.NewAzcopyHTTPClient(frontEndMaxIdleConnectionsPerHost), logOptions, srcCred, reauthCred)
-}
+	// Job-level/global client if available so we reuse connections and transports.


The comment says “Job-level/global client if available”, but GetGlobalHTTPClient always returns a process-global client (not job-scoped and not conditional). Consider rewording to avoid implying per-job behavior.

Suggested change

// Job-level/global client if available so we reuse connections and transports.

// Use the process-global HTTP client so we reuse connections and transports.

adreed-msft · 2026-04-10T20:25:07Z

+
+// GlobalHTTPClient is the process-wide HTTP client used by AzCopy when initialized via InitGlobalHTTPClient.
+var (
+	GlobalHTTPClient     *http.Client


Singleton pattern, but this is exported? I don't see this used anywhere else. I'd recommend un-exporting it to avoid improper use.

adreed-msft · 2026-04-10T22:50:36Z

+
+// This is a code duplication of GetMainPoolSize in ste/concurrency.go
+// Ideally we should just move concurrency.go to common package. That is a todo for future.
+func GetMaxIdleConnsPerHost() int {


This doesn't seem like it ever updates, akin to GetMainPoolSize? Could that prove to eventually be an issue?

shnayak-msft requested review from adreed-msft, dphulkar-msft, gapra-msft, seanmcc-msft, souravgupta-msft and tanyasethi-msft as code owners April 9, 2026 23:30

Copilot AI review requested due to automatic review settings April 9, 2026 23:30

shnayak-msft requested a review from wonwuakpa-msft as a code owner April 9, 2026 23:30

Copilot started reviewing on behalf of shnayak-msft April 9, 2026 23:31 View session

Copilot AI reviewed Apr 9, 2026

View reviewed changes

adreed-msft approved these changes Apr 10, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Centralize HTTP client into a shared global instance#3436

Centralize HTTP client into a shared global instance#3436
shnayak-msft wants to merge 1 commit into
mainfrom
users/shnayak/common-http-client-changes

shnayak-msft commented Apr 9, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

adreed-msft Apr 10, 2026

Uh oh!

adreed-msft Apr 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		// Usage: replace the Transport field of an *http.Client with the result of this function.
		// Use common.NewTracingTransport(client, "createClientOptions", logger) for http.Trace

	httpClient: common.GetGlobalHTTPClient(jobLogger),
	httpClient: common.NewAzcopyHTTPClient(concurrency.MaxIdleConnections),

	// Job-level/global client if available so we reuse connections and transports.
	// Use the process-global HTTP client so we reuse connections and transports.

Conversation

shnayak-msft commented Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of Change

How Has This Been Tested?

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

adreed-msft Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

adreed-msft Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

shnayak-msft commented Apr 9, 2026 •

edited

Loading