-
Notifications
You must be signed in to change notification settings - Fork 20
Enhanced Server Stability and Configuration Control #150
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
… query timeout - Fixed parameter type inference for arithmetic operations with untyped parameters (defaults to TEXT instead of throwing fatal errors) - Fixed Arrow schema mismatches by unifying all UTF8 variants to use TEXT type consistently - Added configurable query timeout with ServerOptions.with_query_timeout_secs(0) for no timeout - Added configurable max_connections with ServerOptions.with_max_connections(n) - Added connection limiting with semaphore to prevent resource exhaustion under load - Simplified API with single constructor that takes timeout parameter directly - Added comprehensive unit tests for timeout and connection configuration functionality
- Fixed compiler warnings and clippy suggestions - Updated test files to use new DfSessionService constructor signature - Applied cargo fmt formatting across the codebase - All tests now pass successfully
@sunng87 please take a look. |
Thank you.
I'm also running into this issue with #149 |
pub fn new( | ||
session_context: Arc<SessionContext>, | ||
auth_manager: Arc<AuthManager>, | ||
query_timeout: Option<std::time::Duration>, |
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.
I suggest you to put this into a new pull request. Also it should be configured at per-session level using SET statement_timeout
port: 5432, | ||
tls_cert_path: None, | ||
tls_key_path: None, | ||
max_connections: 1000, |
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.
default to 0, which means no restriction on this.
I'd look into that and have it fixed |
Hi @iPeluwa , my suggestion is to split this into 3 different PR:
|
Fixed Parameter Type Issues: Resolved crashes from untyped parameters (e.g., $1 + $2) by setting them to default to TEXT, allowing DataFusion to handle type coercion.
Resolved Schema Consistency Problems: Unified VARCHAR and TEXT types to TEXT, ensuring compatibility with most Postgres tools and eliminating schema mismatches.
Improved Connection Stability: Implemented connection limiting to prevent server crashes from excessive concurrent connections, avoiding resource exhaustion.
Added Configurable Query Timeout: Introduced query timeout settings via ServerOptions::new().with_query_timeout_secs(30) for a 30-second timeout, or 0 for no timeout, to prevent runaway queries.
Enhanced Connection Control: Added .with_max_connections(500) to allow limiting connections, enabling the server to reject excess connections gracefully.