Skip to content

Commit af14144

Browse files
Joeavaikathclaude
andcommitted
Optimize HTTP client creation with caching by timeout
Add sync.Map-based cache for HTTP clients indexed by timeout duration. This eliminates repeated client allocations during download operations. Changes: - httpClientWithTimeout() now caches clients by timeout value - Use sync.Map for thread-safe caching without lock contention - Add TestHTTPClientCaching to verify caching behavior Impact: Reduces allocations during describe --details operations (4 downloads per command) and all other download workflows. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent 35f80b8 commit af14144

2 files changed

Lines changed: 34 additions & 1 deletion

File tree

cmd/shared/download.go

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"net/http"
2626
"os"
2727
"strings"
28+
"sync"
2829
"time"
2930

3031
nacv1alpha1 "github.com/migtools/oadp-non-admin/api/v1alpha1"
@@ -33,6 +34,11 @@ import (
3334
kbclient "sigs.k8s.io/controller-runtime/pkg/client"
3435
)
3536

37+
var (
38+
// Cache HTTP clients by timeout duration
39+
httpClientCache sync.Map // map[time.Duration]*http.Client
40+
)
41+
3642
// DefaultHTTPTimeout is the default timeout for HTTP requests when downloading content from object storage.
3743
// This prevents the CLI from hanging indefinitely if the connection stalls.
3844
const DefaultHTTPTimeout = 10 * time.Minute
@@ -69,9 +75,19 @@ func GetHTTPTimeoutWithOverride(override time.Duration) time.Duration {
6975
// httpClientWithTimeout returns an HTTP client with a configured timeout.
7076
// Using a custom client instead of http.DefaultClient ensures downloads don't hang indefinitely.
7177
func httpClientWithTimeout(timeout time.Duration) *http.Client {
72-
return &http.Client{
78+
// Try to load from cache
79+
if cached, ok := httpClientCache.Load(timeout); ok {
80+
return cached.(*http.Client)
81+
}
82+
83+
// Create new client
84+
client := &http.Client{
7385
Timeout: timeout,
7486
}
87+
88+
// Store in cache (LoadOrStore handles race conditions)
89+
actual, _ := httpClientCache.LoadOrStore(timeout, client)
90+
return actual.(*http.Client)
7591
}
7692

7793
// DownloadRequestOptions holds configuration for creating and processing NonAdminDownloadRequests

cmd/shared/download_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,23 @@ func TestHttpClientWithTimeout(t *testing.T) {
147147
}
148148
}
149149

150+
// TestHTTPClientCaching verifies that HTTP clients are cached by timeout duration
151+
func TestHTTPClientCaching(t *testing.T) {
152+
timeout := 10 * time.Minute
153+
client1 := httpClientWithTimeout(timeout)
154+
client2 := httpClientWithTimeout(timeout)
155+
156+
if client1 != client2 {
157+
t.Error("Expected same client instance for same timeout")
158+
}
159+
160+
differentTimeout := 5 * time.Minute
161+
client3 := httpClientWithTimeout(differentTimeout)
162+
if client1 == client3 {
163+
t.Error("Expected different client for different timeout")
164+
}
165+
}
166+
150167
// TestDownloadContentWithTimeout tests downloading content with explicit timeout
151168
func TestDownloadContentWithTimeout(t *testing.T) {
152169
tests := []struct {

0 commit comments

Comments
 (0)