Adjust network call timeout behaviors #614
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Owner
Author
There was a problem hiding this comment. I hate this package comment, needs human rewrite. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| // Package net provides common network configuration constants and utilities | ||
| // for the skyeye application. | ||
| // | ||
| // # Timeout Configuration | ||
| // | ||
| // The constants defined in this package represent recommended defaults that | ||
| // balance reliability and responsiveness for typical deployment scenarios. | ||
| // These defaults assume reasonably stable network conditions and may need | ||
| // adjustment based on your environment: | ||
| // | ||
| // - LAN deployments: Current defaults are suitable | ||
| // - WAN/Internet deployments: May need longer timeouts for connection establishment | ||
| // - High-latency links: Should increase timeout values proportionally | ||
| // - Unstable networks: May benefit from shorter timeouts with more aggressive retry logic | ||
| // | ||
| // # Usage | ||
| // | ||
| // Application code should use these constants as defaults for CLI flags, | ||
| // allowing operators to override based on their network conditions: | ||
| // | ||
| // flag.DurationVar(&timeout, "timeout", net.DefaultConnectionTimeout, "connection timeout") | ||
| // | ||
| // The package also provides helper functions for calculating related timeouts: | ||
| // | ||
| // - CalculateReadTimeout: Determines read timeout from connection timeout | ||
| // - CalculateDeadlineRefreshInterval: Determines how often to refresh deadlines | ||
| package net | ||
|
|
||
| import "time" | ||
|
|
||
| const ( | ||
| // DefaultConnectionTimeout is the default timeout for establishing network connections. | ||
| // This applies to initial connection attempts for TCP, UDP resolution, and similar operations. | ||
| DefaultConnectionTimeout = 30 * time.Second | ||
|
|
||
| // ReadTimeoutMultiplier defines how much longer read timeouts should be compared to | ||
| // connection timeouts for streaming data. A multiplier of 2 allows for transient | ||
| // network delays while still detecting truly dead connections. | ||
| // | ||
| // Rationale: A 2x multiplier provides a balance between responsiveness to network | ||
| // issues and tolerance for normal network latency/congestion. Lower values may cause | ||
| // false positives during temporary network slowdowns; higher values delay detection | ||
| // of truly dead connections. | ||
| ReadTimeoutMultiplier = 2 | ||
|
|
||
| // DeadlineRefreshDivisor determines how frequently read deadlines should be refreshed | ||
| // for long-lived streaming connections. The refresh interval is calculated as: | ||
| // readTimeout / DeadlineRefreshDivisor. A divisor of 2 means we refresh at the | ||
| // halfway point, ensuring the deadline never expires during active streaming. | ||
| // | ||
| // Rationale: Refreshing at the halfway point (divisor of 2) ensures the deadline is | ||
| // always fresh with minimal overhead. This prevents legitimate long-running streams | ||
| // from timing out while still allowing the full timeout period to detect dead connections. | ||
| DeadlineRefreshDivisor = 2 | ||
|
|
||
| // ReconnectDelay is the delay before retrying a failed connection attempt. | ||
| // This applies to automatic reconnection logic after connection failures. | ||
| ReconnectDelay = 10 * time.Second | ||
|
|
||
| // GRPCKeepaliveTime is the interval at which gRPC keepalive pings are sent. | ||
| // This matches the default connection timeout to detect dead connections quickly. | ||
| GRPCKeepaliveTime = 30 * time.Second | ||
|
|
||
| // GRPCKeepaliveTimeout is how long to wait for a keepalive ping acknowledgment | ||
| // before considering the connection dead. | ||
| GRPCKeepaliveTimeout = 10 * time.Second | ||
|
|
||
| // OpenAIHTTPTimeout is the timeout for HTTP requests to the OpenAI API. | ||
| // Audio transcription can be slow for large files, so this is set higher | ||
| // than typical HTTP timeouts. | ||
| OpenAIHTTPTimeout = 60 * time.Second | ||
|
Owner
Author
There was a problem hiding this comment. Should remove this and just use the standard 30s |
||
| ) | ||
|
|
||
| // CalculateReadTimeout returns the recommended read timeout based on a connection timeout. | ||
| // This is useful for streaming connections where reads may take longer than initial connection. | ||
| func CalculateReadTimeout(connectionTimeout time.Duration) time.Duration { | ||
| return connectionTimeout * ReadTimeoutMultiplier | ||
| } | ||
|
|
||
| // CalculateDeadlineRefreshInterval returns the interval at which read deadlines should | ||
| // be refreshed for long-lived streaming connections. | ||
| func CalculateDeadlineRefreshInterval(readTimeout time.Duration) time.Duration { | ||
| return readTimeout / DeadlineRefreshDivisor | ||
| } | ||
|
Owner
Author
There was a problem hiding this comment. These tests are not valuable and should be removed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,158 @@ | ||
| package net | ||
|
|
||
| import ( | ||
| "testing" | ||
| "time" | ||
| ) | ||
|
|
||
| func TestCalculateReadTimeout(t *testing.T) { | ||
| t.Parallel() | ||
| tests := []struct { | ||
| name string | ||
| connectionTimeout time.Duration | ||
| expected time.Duration | ||
| }{ | ||
| { | ||
| name: "default connection timeout", | ||
| connectionTimeout: DefaultConnectionTimeout, | ||
| expected: 60 * time.Second, | ||
| }, | ||
| { | ||
| name: "10 second connection timeout", | ||
| connectionTimeout: 10 * time.Second, | ||
| expected: 20 * time.Second, | ||
| }, | ||
| { | ||
| name: "1 minute connection timeout", | ||
| connectionTimeout: 1 * time.Minute, | ||
| expected: 2 * time.Minute, | ||
| }, | ||
| { | ||
| name: "zero timeout", | ||
| connectionTimeout: 0, | ||
| expected: 0, | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| t.Parallel() | ||
| actual := CalculateReadTimeout(tt.connectionTimeout) | ||
| if actual != tt.expected { | ||
| t.Errorf("CalculateReadTimeout(%v) = %v, expected %v", tt.connectionTimeout, actual, tt.expected) | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestCalculateDeadlineRefreshInterval(t *testing.T) { | ||
| t.Parallel() | ||
| tests := []struct { | ||
| name string | ||
| readTimeout time.Duration | ||
| expected time.Duration | ||
| }{ | ||
| { | ||
| name: "60 second read timeout", | ||
| readTimeout: 60 * time.Second, | ||
| expected: 30 * time.Second, | ||
| }, | ||
| { | ||
| name: "20 second read timeout", | ||
| readTimeout: 20 * time.Second, | ||
| expected: 10 * time.Second, | ||
| }, | ||
| { | ||
| name: "2 minute read timeout", | ||
| readTimeout: 2 * time.Minute, | ||
| expected: 1 * time.Minute, | ||
| }, | ||
| { | ||
| name: "zero timeout", | ||
| readTimeout: 0, | ||
| expected: 0, | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| t.Parallel() | ||
| actual := CalculateDeadlineRefreshInterval(tt.readTimeout) | ||
| if actual != tt.expected { | ||
| t.Errorf("CalculateDeadlineRefreshInterval(%v) = %v, expected %v", tt.readTimeout, actual, tt.expected) | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestCalculateReadTimeoutMultiplier(t *testing.T) { | ||
| t.Parallel() | ||
| // Verify that the multiplier constant is correctly applied | ||
| connectionTimeout := 15 * time.Second | ||
| readTimeout := CalculateReadTimeout(connectionTimeout) | ||
|
|
||
| expected := time.Duration(int64(connectionTimeout) * ReadTimeoutMultiplier) | ||
| if readTimeout != expected { | ||
| t.Errorf("ReadTimeoutMultiplier not correctly applied: got %v, expected %v", readTimeout, expected) | ||
| } | ||
| } | ||
|
|
||
| func TestCalculateDeadlineRefreshDivisor(t *testing.T) { | ||
| t.Parallel() | ||
| // Verify that the divisor constant is correctly applied | ||
| readTimeout := 40 * time.Second | ||
| refreshInterval := CalculateDeadlineRefreshInterval(readTimeout) | ||
|
|
||
| expected := time.Duration(int64(readTimeout) / DeadlineRefreshDivisor) | ||
| if refreshInterval != expected { | ||
| t.Errorf("DeadlineRefreshDivisor not correctly applied: got %v, expected %v", refreshInterval, expected) | ||
| } | ||
| } | ||
|
|
||
| func TestEndToEndTimeoutCalculation(t *testing.T) { | ||
| t.Parallel() | ||
| // Test the complete flow from connection timeout to refresh interval | ||
| connectionTimeout := DefaultConnectionTimeout // 30s | ||
|
|
||
| readTimeout := CalculateReadTimeout(connectionTimeout) | ||
| if readTimeout != 60*time.Second { | ||
| t.Errorf("Expected read timeout of 60s for default connection timeout, got %v", readTimeout) | ||
| } | ||
|
|
||
| refreshInterval := CalculateDeadlineRefreshInterval(readTimeout) | ||
| if refreshInterval != 30*time.Second { | ||
| t.Errorf("Expected refresh interval of 30s for 60s read timeout, got %v", refreshInterval) | ||
| } | ||
| } | ||
|
|
||
| func TestConstantValues(t *testing.T) { | ||
| t.Parallel() | ||
| // Verify the constant values match expected defaults | ||
| if DefaultConnectionTimeout != 30*time.Second { | ||
| t.Errorf("DefaultConnectionTimeout = %v, expected 30s", DefaultConnectionTimeout) | ||
| } | ||
|
|
||
| if ReadTimeoutMultiplier != 2 { | ||
| t.Errorf("ReadTimeoutMultiplier = %v, expected 2", ReadTimeoutMultiplier) | ||
| } | ||
|
|
||
| if DeadlineRefreshDivisor != 2 { | ||
| t.Errorf("DeadlineRefreshDivisor = %v, expected 2", DeadlineRefreshDivisor) | ||
| } | ||
|
|
||
| if ReconnectDelay != 10*time.Second { | ||
| t.Errorf("ReconnectDelay = %v, expected 10s", ReconnectDelay) | ||
| } | ||
|
|
||
| if GRPCKeepaliveTime != 30*time.Second { | ||
| t.Errorf("GRPCKeepaliveTime = %v, expected 30s", GRPCKeepaliveTime) | ||
| } | ||
|
|
||
| if GRPCKeepaliveTimeout != 10*time.Second { | ||
| t.Errorf("GRPCKeepaliveTimeout = %v, expected 10s", GRPCKeepaliveTimeout) | ||
| } | ||
|
|
||
| if OpenAIHTTPTimeout != 60*time.Second { | ||
| t.Errorf("OpenAIHTTPTimeout = %v, expected 60s", OpenAIHTTPTimeout) | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Too many comments, needs human rewrite.