Skip to content

Commit 5a1cf88

Browse files
committed
docs: consolidated timeout configuration documentation
## Summary This PR consolidates **2 documentation PRs** for timeout configuration. ### Included PRs: - #49: Add comprehensive timeout configuration documentation - #86: Add comprehensive timeout hierarchy documentation to HTTP client ### Key Changes: - Added detailed header documentation explaining the timeout hierarchy - Documented each timeout constant with its purpose and use case - Established naming conventions for constants vs config fields - Added timeout hierarchy table for quick reference ### Timeout Hierarchy | Use Case | Module | Constant | Value | Rationale | |-----------------------------|-----------------------------|-----------------------------|-------|-----------| | Health checks | cortex-common/http_client | HEALTH_CHECK_TIMEOUT | 5s | Quick validation | | Standard HTTP requests | cortex-common/http_client | DEFAULT_TIMEOUT | 30s | Normal API calls | | Connection pool idle | cortex-common/http_client | POOL_IDLE_TIMEOUT | 60s | DNS refresh | | LLM Request (non-streaming) | cortex-exec/runner | DEFAULT_REQUEST_TIMEOUT_SECS| 120s | Model inference | | LLM Streaming total | cortex-common/http_client | STREAMING_TIMEOUT | 300s | Long-running streams | | Server request lifecycle | cortex-app-server/config | request_timeout | 300s | Full request handling | | Per-chunk reads | cortex-app-server/config | read_timeout | 30s | Chunk timeout | | Graceful shutdown | cortex-app-server/config | shutdown_timeout | 30s | Time for cleanup | | Entire execution | cortex-exec/runner | DEFAULT_TIMEOUT_SECS | 600s | Headless exec limit | ### Files Modified: - src/cortex-common/src/http_client.rs - src/cortex-app-server/src/config.rs - src/cortex-exec/src/runner.rs Closes #49, #86
1 parent c398212 commit 5a1cf88

File tree

3 files changed

+97
-5
lines changed

3 files changed

+97
-5
lines changed

src/cortex-app-server/src/config.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,12 +49,18 @@ pub struct ServerConfig {
4949
pub max_body_size: usize,
5050

5151
/// Request timeout in seconds (applies to full request lifecycle).
52+
///
53+
/// See `cortex_common::http_client` module documentation for the complete
54+
/// timeout hierarchy across Cortex services.
5255
#[serde(default = "default_request_timeout")]
5356
pub request_timeout: u64,
5457

5558
/// Read timeout for individual chunks in seconds.
5659
/// Applies to chunked transfer encoding to prevent indefinite hangs
5760
/// when clients disconnect without sending the terminal chunk.
61+
///
62+
/// See `cortex_common::http_client` module documentation for the complete
63+
/// timeout hierarchy across Cortex services.
5864
#[serde(default = "default_read_timeout")]
5965
pub read_timeout: u64,
6066

@@ -71,12 +77,16 @@ pub struct ServerConfig {
7177
pub cors_origins: Vec<String>,
7278

7379
/// Graceful shutdown timeout in seconds.
80+
///
81+
/// See `cortex_common::http_client` module documentation for the complete
82+
/// timeout hierarchy across Cortex services.
7483
#[serde(default = "default_shutdown_timeout")]
7584
pub shutdown_timeout: u64,
7685
}
7786

7887
fn default_shutdown_timeout() -> u64 {
7988
30 // 30 seconds for graceful shutdown
89+
// See cortex_common::http_client for timeout hierarchy documentation
8090
}
8191

8292
fn default_listen_addr() -> String {

src/cortex-common/src/http_client.rs

Lines changed: 79 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,25 +9,99 @@
99
//!
1010
//! DNS caching is configured with reasonable TTL to allow failover and load
1111
//! balancer updates (#2177).
12+
//!
13+
//! # Timeout Configuration Guide
14+
//!
15+
//! This section documents the timeout hierarchy across the Cortex codebase. Use this
16+
//! as a reference when configuring timeouts for new features or debugging timeout issues.
17+
//!
18+
//! ## Timeout Hierarchy
19+
//!
20+
//! | Use Case | Timeout | Constant/Location | Rationale |
21+
//! |-----------------------------|---------|--------------------------------------------|-----------------------------------------|
22+
//! | Health checks | 5s | `HEALTH_CHECK_TIMEOUT` (this module) | Quick validation of service status |
23+
//! | Standard HTTP requests | 30s | `DEFAULT_TIMEOUT` (this module) | Normal API calls with reasonable margin |
24+
//! | Per-chunk read (streaming) | 30s | `read_timeout` (cortex-app-server/config) | Individual chunk timeout during stream |
25+
//! | Pool idle timeout | 60s | `POOL_IDLE_TIMEOUT` (this module) | DNS re-resolution for failover |
26+
//! | LLM Request (non-streaming) | 120s | `DEFAULT_REQUEST_TIMEOUT_SECS` (cortex-exec/runner) | Model inference takes time |
27+
//! | LLM Streaming total | 300s | `STREAMING_TIMEOUT` (this module) | Long-running streaming responses |
28+
//! | Server request lifecycle | 300s | `request_timeout` (cortex-app-server/config) | Full HTTP request/response cycle |
29+
//! | Entire exec session | 600s | `DEFAULT_TIMEOUT_SECS` (cortex-exec/runner) | Multi-turn conversation limit |
30+
//! | Graceful shutdown | 30s | `shutdown_timeout` (cortex-app-server/config) | Time for cleanup on shutdown |
31+
//!
32+
//! ## Module-Specific Timeouts
33+
//!
34+
//! ### cortex-common (this module)
35+
//! - `DEFAULT_TIMEOUT` (30s): Use for standard API calls.
36+
//! - `STREAMING_TIMEOUT` (300s): Use for LLM streaming endpoints.
37+
//! - `HEALTH_CHECK_TIMEOUT` (5s): Use for health/readiness checks.
38+
//! - `POOL_IDLE_TIMEOUT` (60s): Connection pool cleanup for DNS freshness.
39+
//!
40+
//! ### cortex-exec (runner.rs)
41+
//! - `DEFAULT_TIMEOUT_SECS` (600s): Maximum duration for entire exec session.
42+
//! - `DEFAULT_REQUEST_TIMEOUT_SECS` (120s): Single LLM request timeout.
43+
//!
44+
//! ### cortex-app-server (config.rs)
45+
//! - `request_timeout` (300s): Full request lifecycle timeout.
46+
//! - `read_timeout` (30s): Per-chunk timeout for streaming reads.
47+
//! - `shutdown_timeout` (30s): Graceful shutdown duration.
48+
//!
49+
//! ### cortex-engine (api_client.rs)
50+
//! - Re-exports constants from this module for consistency.
51+
//!
52+
//! ## Recommendations
53+
//!
54+
//! When adding new timeout configurations:
55+
//! 1. Use constants from this module when possible for consistency.
56+
//! 2. Document any new timeout constants with their rationale.
57+
//! 3. Consider the timeout hierarchy - inner timeouts should be shorter than outer ones.
58+
//! 4. For LLM operations, use longer timeouts (120s-300s) to accommodate model inference.
59+
//! 5. For health checks and quick validations, use short timeouts (5s-10s).
1260
1361
use reqwest::Client;
1462
use std::time::Duration;
1563

64+
// ============================================================
65+
// Timeout Configuration Constants
66+
// ============================================================
67+
//
68+
// Timeout Hierarchy Documentation
69+
// ===============================
70+
//
71+
// This module defines the authoritative timeout constants for HTTP operations.
72+
// Other modules should reference these constants or document deviations.
73+
//
74+
// Timeout Categories:
75+
// - Request timeouts: Total time for a complete request/response cycle
76+
// - Connection timeouts: Time to establish TCP connection
77+
// - Read timeouts: Time to receive response data
78+
// - Pool timeouts: How long idle connections stay in pool
79+
//
80+
// Recommended Naming Convention:
81+
// - Constants: SCREAMING_SNAKE_CASE with Duration type
82+
// - Config fields: snake_case with _secs suffix for u64 values
83+
//
84+
// ============================================================
85+
1686
/// User-Agent string for all HTTP requests
1787
pub const USER_AGENT: &str = concat!("cortex-cli/", env!("CARGO_PKG_VERSION"));
1888

19-
/// Default timeout for standard API requests (30 seconds)
89+
/// Default timeout for standard HTTP requests (30 seconds).
90+
/// Used for non-streaming API calls, health checks with retries, etc.
2091
pub const DEFAULT_TIMEOUT: Duration = Duration::from_secs(30);
2192

22-
/// Extended timeout for LLM streaming requests (5 minutes)
93+
/// Timeout for streaming HTTP requests (5 minutes).
94+
/// Longer duration to accommodate LLM inference time.
2395
pub const STREAMING_TIMEOUT: Duration = Duration::from_secs(300);
2496

25-
/// Short timeout for health checks (5 seconds)
97+
/// Timeout for health check requests (5 seconds).
98+
/// Short timeout since health endpoints should respond quickly.
2699
pub const HEALTH_CHECK_TIMEOUT: Duration = Duration::from_secs(5);
27100

28-
/// Connection pool idle timeout to ensure DNS is re-resolved periodically.
101+
/// Idle timeout for connection pool (60 seconds).
102+
/// Connections are closed after being idle for this duration
103+
/// to allow DNS re-resolution for services behind load balancers.
29104
/// This helps with failover scenarios where DNS records change (#2177).
30-
/// Set to 60 seconds to balance between performance and DNS freshness.
31105
pub const POOL_IDLE_TIMEOUT: Duration = Duration::from_secs(60);
32106

33107
/// Creates an HTTP client with default configuration (30s timeout).

src/cortex-exec/src/runner.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,17 @@ use cortex_protocol::ConversationId;
2727
use crate::output::{OutputFormat, OutputWriter};
2828

2929
/// Default timeout for the entire execution (10 minutes).
30+
///
31+
/// This is the maximum duration for a multi-turn exec session.
32+
/// See `cortex_common::http_client` module documentation for the complete
33+
/// timeout hierarchy across Cortex services.
3034
const DEFAULT_TIMEOUT_SECS: u64 = 600;
3135

3236
/// Default timeout for a single LLM request (2 minutes).
37+
///
38+
/// Allows sufficient time for model inference while preventing indefinite hangs.
39+
/// See `cortex_common::http_client` module documentation for the complete
40+
/// timeout hierarchy across Cortex services.
3341
const DEFAULT_REQUEST_TIMEOUT_SECS: u64 = 120;
3442

3543
/// Maximum retries for transient errors.

0 commit comments

Comments
 (0)