Add nginx timeouts by hweawer · Pull Request #413 · uber/kraken

hweawer · 2025-07-16T09:52:59Z

Comprehensive Nginx Timeout Implementation

Robust timeout handling across all Kraken services
Service-specific timeout configurations tailored to each component's needs
Improved request reliability with appropriate timeout values
Better error handling for long-running operations

Enhanced System Reliability

Prevents hanging requests with appropriate timeout boundaries
Better resource management through controlled request lifecycles
Improved user experience with predictable response times
🛠 Technical Implementation
Service-wide timeout policies implemented across all nginx configurations
Consistent timeout patterns for different operation types
Graceful degradation when timeouts are reached
Proper error reporting for timeout scenarios
🎁 Benefits
Improved system stability through proper timeout management
Better operational visibility with predictable request boundaries
Enhanced debugging capabilities for timeout-related issues
Cleaner codebase with unnecessary configuration files removed

gkeesh7

Please address the review comments and explain in detail what each config change does

gkeesh7 · 2025-07-23T14:00:11Z

config/agent/base.yaml


+agentserver:
+  # Timeout configurations (also used by nginx)
+  download_timeout: 5m             # nginx proxy_read_timeout for downloads


Let's keep it to 15 minutes

gkeesh7 · 2025-07-23T14:00:45Z

config/origin/base.yaml

    addr: /tmp/kraken-origin.sock
+
+  # Timeout configurations (also used by nginx)
+  download_timeout: 5m       # nginx proxy_read_timeout for downloads


15 minutes here as well

gkeesh7 · 2025-07-23T14:11:51Z

nginx/config/origin.go

+  proxy_send_timeout {{.upload_timeout}};
+  proxy_read_timeout {{.download_timeout}};
+
+  # Keepalive settings


Can you describe what is meant by these keep alive setting you are adding and why are they being added ?

Added comments

gkeesh7 · 2025-07-23T14:14:13Z

nginx/config/origin.go

+    proxy_set_header X-Forwarded-Proto $scheme;
+  }
+
+  # Special handling for upload operations with longer timeout


How will these addtional settings help ?

As I understand the upload will happen ubuild -> kraken -> GCS, whereas replication will happen in between origin instances in out network, so I thought that it might have sense for splitting it, but if you think that there is not much sense in this I can put a single timeout.

gkeesh7 · 2025-07-23T14:18:57Z

nginx/nginx.go

+		return fmt.Sprintf("%ds", seconds)
+	}
+	// Fallback to milliseconds for very short durations
+	return fmt.Sprintf("%dms", bufferedDuration.Milliseconds())


In line 256 we are already adding 30seconds Will we ever reach this codepath ?

Made it convert to seconds only

gkeesh7 · 2025-07-23T14:19:38Z

nginx/config/tracker.go

@@ -29,10 +29,36 @@ server {
  access_log {{.access_log_path}};
  error_log {{.error_log_path}};



Same here as well please describe in more detail

Added comments

gkeesh7 · 2025-07-23T14:21:27Z

nginx/nginx.go

 	return addr
 }
+
+func FormatDurationForNginx(d time.Duration) string {


Please explain the purpose o fthis function

Added documentation

gkeesh7 · 2025-08-21T11:17:08Z

@hweawer can you rebase the changes and run against the updated CI ?

hweawer self-assigned this Jul 16, 2025

hweawer force-pushed the improve-kraken-origin-logging branch from 4c3f228 to 58bfdd6 Compare July 16, 2025 12:36

hweawer force-pushed the nginx-timeouts-large-blobs branch from 3d729cb to 100f8d0 Compare July 16, 2025 12:36

hweawer added 3 commits July 17, 2025 10:12

Add nginx timeouts

e140796

Remove max_request_body_size

b63b785

Remove .golangci.yml from repository and add to .gitignore

0441c6d

hweawer force-pushed the improve-kraken-origin-logging branch from 58bfdd6 to 28f4b6f Compare July 17, 2025 08:14

hweawer force-pushed the nginx-timeouts-large-blobs branch from dc6a30d to 0441c6d Compare July 17, 2025 08:15

hweawer requested review from Anton-Kalpakchiev and gkeesh7 July 17, 2025 08:15

gkeesh7 reviewed Jul 23, 2025

View reviewed changes

hweawer added 2 commits July 25, 2025 16:31

Adjust timeouts

dfd94ab

Update comments

85640f0

hweawer requested a review from gkeesh7 July 29, 2025 11:05

		@@ -29,10 +29,36 @@ server {
		access_log {{.access_log_path}};
		error_log {{.error_log_path}};

Conversation

hweawer commented Jul 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gkeesh7 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gkeesh7 commented Aug 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

hweawer commented Jul 16, 2025 •

edited

Loading