diff --git a/babushka-core/src/client/mod.rs b/babushka-core/src/client/mod.rs index f54c11a8c4..9e8965a226 100644 --- a/babushka-core/src/client/mod.rs +++ b/babushka-core/src/client/mod.rs @@ -237,8 +237,6 @@ fn sanitized_request_string(request: &ConnectionRequest) -> String { let tls_mode = request.tls_mode.enum_value_or_default(); let cluster_mode = request.cluster_mode_enabled; let request_timeout = format_non_zero_value("response timeout", request.request_timeout); - let client_creation_timeout = - format_non_zero_value("client creation timeout", request.client_creation_timeout); let database_id = format_non_zero_value("database ID", request.database_id); let rfr_strategy = request.read_from.enum_value_or_default(); let connection_retry_strategy = match &request.connection_retry_strategy.0 { @@ -250,24 +248,20 @@ fn sanitized_request_string(request: &ConnectionRequest) -> String { }; format!( - "\naddresses: {addresses}\nTLS mode: {tls_mode:?}\ncluster mode: {cluster_mode}{request_timeout}{client_creation_timeout}\nRead from replica strategy: {rfr_strategy:?}{database_id}{connection_retry_strategy}", + "\naddresses: {addresses}\nTLS mode: {tls_mode:?}\ncluster mode: {cluster_mode}{request_timeout}\nRead from replica strategy: {rfr_strategy:?}{database_id}{connection_retry_strategy}", ) } impl Client { pub async fn new(request: ConnectionRequest) -> Result { - const DEFAULT_CLIENT_CREATION_TIMEOUT: Duration = Duration::from_millis(2500); + const DEFAULT_CLIENT_CREATION_TIMEOUT: Duration = Duration::from_secs(10); log_info( "Connection configuration", sanitized_request_string(&request), ); let request_timeout = to_duration(request.request_timeout, DEFAULT_RESPONSE_TIMEOUT); - let total_connection_timeout = to_duration( - request.client_creation_timeout, - DEFAULT_CLIENT_CREATION_TIMEOUT, - ); - tokio::time::timeout(total_connection_timeout, async move { + tokio::time::timeout(DEFAULT_CLIENT_CREATION_TIMEOUT, async move { let internal_client = if request.cluster_mode_enabled { let client = create_cluster_client(request) .await diff --git a/babushka-core/src/protobuf/connection_request.proto b/babushka-core/src/protobuf/connection_request.proto index 07d4192b25..86912a9535 100644 --- a/babushka-core/src/protobuf/connection_request.proto +++ b/babushka-core/src/protobuf/connection_request.proto @@ -30,11 +30,10 @@ message ConnectionRequest { TlsMode tls_mode = 2; bool cluster_mode_enabled = 3; uint32 request_timeout = 4; - uint32 client_creation_timeout = 5; - ReadFrom read_from = 6; - ConnectionRetryStrategy connection_retry_strategy = 7; - AuthenticationInfo authentication_info = 8; - uint32 database_id = 9; + ReadFrom read_from = 5; + ConnectionRetryStrategy connection_retry_strategy = 6; + AuthenticationInfo authentication_info = 7; + uint32 database_id = 8; } message ConnectionRetryStrategy { diff --git a/babushka-core/tests/test_client.rs b/babushka-core/tests/test_client.rs index db60ab80a4..4310953a1b 100644 --- a/babushka-core/tests/test_client.rs +++ b/babushka-core/tests/test_client.rs @@ -190,49 +190,4 @@ mod shared_client_tests { assert!(err.is_timeout(), "{err}"); }); } - - #[rstest] - #[timeout(SHORT_CLUSTER_TEST_TIMEOUT)] - fn test_connection_timeout(#[values(false, true)] use_cluster: bool) { - let use_tls = true; - async fn expect_timeout_on_client_creation( - addresses: &[redis::ConnectionAddr], - cluster_mode: ClusterMode, - use_tls: bool, - ) { - let mut configuration = TestConfiguration { - cluster_mode, - use_tls, - ..Default::default() - }; - - configuration.connection_timeout = Some(1); - let err = Client::new(create_connection_request(addresses, &configuration)) - .await - .map(|_| ()) - .unwrap_err(); - assert!(matches!(err, babushka::client::ConnectionError::Timeout)); - } - - block_on_all(async { - if use_cluster { - let cluster = RedisCluster::new(use_tls, &None, Some(3), Some(0)); - expect_timeout_on_client_creation( - &cluster.get_server_addresses(), - ClusterMode::Enabled, - use_tls, - ) - .await; - } else { - let server = RedisServer::new(ServerType::Tcp { tls: use_tls }); - wait_for_server_to_become_ready(&server.get_client_addr()).await; - expect_timeout_on_client_creation( - &[server.get_client_addr()], - ClusterMode::Disabled, - use_tls, - ) - .await; - } - }); - } } diff --git a/babushka-core/tests/utilities/cluster.rs b/babushka-core/tests/utilities/cluster.rs index 6de52f7bdf..941f6d85c6 100644 --- a/babushka-core/tests/utilities/cluster.rs +++ b/babushka-core/tests/utilities/cluster.rs @@ -249,7 +249,6 @@ pub async fn setup_test_basics_internal(mut configuration: TestConfiguration) -> } configuration.cluster_mode = ClusterMode::Enabled; configuration.request_timeout = configuration.request_timeout.or(Some(10000)); - configuration.connection_timeout = configuration.connection_timeout.or(Some(10000)); let connection_request = create_connection_request(&addresses, &configuration); let client = Client::new(connection_request).await.unwrap(); diff --git a/babushka-core/tests/utilities/mod.rs b/babushka-core/tests/utilities/mod.rs index 0a1c33ab59..5b7ecac84e 100644 --- a/babushka-core/tests/utilities/mod.rs +++ b/babushka-core/tests/utilities/mod.rs @@ -566,9 +566,6 @@ pub fn create_connection_request( if let Some(request_timeout) = configuration.request_timeout { connection_request.request_timeout = request_timeout; } - if let Some(connection_timeout) = configuration.connection_timeout { - connection_request.client_creation_timeout = connection_timeout; - } if let Some(strategy) = configuration.read_from { connection_request.read_from = strategy.into() } @@ -589,7 +586,6 @@ pub struct TestConfiguration { pub connection_info: Option, pub cluster_mode: ClusterMode, pub request_timeout: Option, - pub connection_timeout: Option, pub shared_server: bool, pub read_from: Option, pub database_id: u32, diff --git a/node/src/BaseClient.ts b/node/src/BaseClient.ts index 67ee12c96c..f09cab4376 100644 --- a/node/src/BaseClient.ts +++ b/node/src/BaseClient.ts @@ -120,14 +120,6 @@ export type BaseClientConfiguration = { * Value must be an integer. */ requestTimeout?: number; - /** - * The duration in milliseconds that the client should wait its initialization, - * encompassing tasks such as connection establishment and topology mapping. - * If the client fails to complete its initialization within this specified time, an error will be returned. - * If not set, a default value will be used. - * Value must be an integer. - */ - clientCreationTimeout?: number; /** * Represents the client's read from strategy. * If not set, `Primary` will be used. @@ -876,7 +868,6 @@ export class BaseClient { : connection_request.TlsMode.NoTls, requestTimeout: options.requestTimeout, clusterModeEnabled: false, - clientCreationTimeout: options.clientCreationTimeout, readFrom, authenticationInfo, }; diff --git a/python/python/pybushka/config.py b/python/python/pybushka/config.py index 2b9042cf21..9decdd7510 100644 --- a/python/python/pybushka/config.py +++ b/python/python/pybushka/config.py @@ -81,7 +81,6 @@ def __init__( use_tls: bool = False, credentials: Optional[RedisCredentials] = None, read_from: ReadFrom = ReadFrom.PRIMARY, - client_creation_timeout: Optional[int] = None, request_timeout: Optional[int] = None, ): """ @@ -103,10 +102,6 @@ def __init__( credentials (RedisCredentials): Credentials for authentication process. If none are set, the client will not authenticate itself with the server. read_from (ReadFrom): If not set, `PRIMARY` will be used. - client_creation_timeout (Optional[int]): The duration in milliseconds that the client should wait its initialization, - encompassing tasks such as connection establishment and topology mapping. - If the client fails to complete its initialization within this specified time, an error will be returned. - If not set, a default value will be used. request_timeout (Optional[int]): The duration in milliseconds that the client should wait for a request to complete. This duration encompasses sending the request, awaiting for a response from the server, and any required reconnections or retries. If the specified timeout is exceeded for a pending request, it will result in a timeout error. If not set, a default value will be used. @@ -115,7 +110,6 @@ def __init__( self.use_tls = use_tls self.credentials = credentials self.read_from = read_from - self.client_creation_timeout = client_creation_timeout self.request_timeout = request_timeout def _create_a_protobuf_conn_request( @@ -139,8 +133,6 @@ def _create_a_protobuf_conn_request( request.read_from = self.read_from.value if self.request_timeout: request.request_timeout = self.request_timeout - if self.client_creation_timeout: - request.client_creation_timeout = self.client_creation_timeout request.cluster_mode_enabled = True if cluster_mode else False if self.credentials: if self.credentials.username: @@ -167,10 +159,6 @@ class RedisClientConfiguration(BaseClientConfiguration): credentials (RedisCredentials): Credentials for authentication process. If none are set, the client will not authenticate itself with the server. read_from (ReadFrom): If not set, `PRIMARY` will be used. - client_creation_timeout (Optional[int]): The duration in milliseconds that the client should wait its initialization, - encompassing tasks such as connection establishment and topology mapping. - If the client fails to complete its initialization within this specified time, an error will be returned. - If not set, a default value will be used. request_timeout (Optional[int]): The duration in milliseconds that the client should wait for a request to complete. This duration encompasses sending the request, awaiting for a response from the server, and any required reconnections or retries. If the specified timeout is exceeded for a pending request, it will result in a timeout error. @@ -187,7 +175,6 @@ def __init__( use_tls: bool = False, credentials: Optional[RedisCredentials] = None, read_from: ReadFrom = ReadFrom.PRIMARY, - client_creation_timeout: Optional[int] = None, request_timeout: Optional[int] = None, reconnect_strategy: Optional[BackoffStrategy] = None, database_id: Optional[int] = None, @@ -197,7 +184,6 @@ def __init__( use_tls=use_tls, credentials=credentials, read_from=read_from, - client_creation_timeout=client_creation_timeout, request_timeout=request_timeout, ) self.reconnect_strategy = reconnect_strategy @@ -238,10 +224,6 @@ class ClusterClientConfiguration(BaseClientConfiguration): credentials (RedisCredentials): Credentials for authentication process. If none are set, the client will not authenticate itself with the server. read_from (ReadFrom): If not set, `PRIMARY` will be used. - client_creation_timeout (Optional[int]): The duration in milliseconds that the client should wait its initialization, - encompassing tasks such as connection establishment and topology mapping. - If the client fails to complete its initialization within this specified time, an error will be returned. - If not set, a default value will be used. request_timeout (Optional[int]): The duration in milliseconds that the client should wait for a request to complete. This duration encompasses sending the request, awaiting for a response from the server, and any required reconnections or retries. If the specified timeout is exceeded for a pending request, it will result in a timeout error. If not set, a default value will be used. @@ -257,7 +239,6 @@ def __init__( use_tls: bool = False, credentials: Optional[RedisCredentials] = None, read_from: ReadFrom = ReadFrom.PRIMARY, - client_creation_timeout: Optional[int] = None, request_timeout: Optional[int] = None, ): super().__init__( @@ -265,6 +246,5 @@ def __init__( use_tls=use_tls, credentials=credentials, read_from=read_from, - client_creation_timeout=client_creation_timeout, request_timeout=request_timeout, ) diff --git a/python/python/pybushka/pybushka.pyi b/python/python/pybushka/pybushka.pyi index 4966644a05..72eafde3dc 100644 --- a/python/python/pybushka/pybushka.pyi +++ b/python/python/pybushka/pybushka.pyi @@ -4,6 +4,8 @@ from typing import Optional from pybushka.constants import TResult +DEFAULT_TIMEOUT_IN_MILLISECONDS: int = ... + class Level(Enum): Error = 0 Warn = 1 diff --git a/python/python/pybushka/redis_client.py b/python/python/pybushka/redis_client.py index a92dd23b4a..e3ab5aa82b 100644 --- a/python/python/pybushka/redis_client.py +++ b/python/python/pybushka/redis_client.py @@ -24,7 +24,11 @@ from pybushka.routes import Route, set_protobuf_route from typing_extensions import Self -from .pybushka import start_socket_listener_external, value_from_pointer +from .pybushka import ( + DEFAULT_TIMEOUT_IN_MILLISECONDS, + start_socket_listener_external, + value_from_pointer, +) def get_request_error_class( @@ -100,7 +104,7 @@ def init_callback(socket_path: Optional[str], err: Optional[str]): async def _create_uds_connection(self) -> None: try: # Open an UDS connection - async with async_timeout.timeout(self.config.client_creation_timeout): + async with async_timeout.timeout(DEFAULT_TIMEOUT_IN_MILLISECONDS): reader, writer = await asyncio.open_unix_connection( path=self.socket_path ) diff --git a/python/src/lib.rs b/python/src/lib.rs index ffb9c260f3..8b489e2d8a 100644 --- a/python/src/lib.rs +++ b/python/src/lib.rs @@ -6,6 +6,9 @@ use pyo3::Python; use redis::Value; +pub const DEFAULT_TIMEOUT_IN_MILLISECONDS: u32 = + babushka::client::DEFAULT_RESPONSE_TIMEOUT.as_millis() as u32; + #[pyclass] #[derive(PartialEq, Eq, PartialOrd, Clone)] pub enum Level { @@ -28,6 +31,10 @@ impl Level { #[pymodule] fn pybushka(_py: Python, m: &PyModule) -> PyResult<()> { m.add_class::()?; + m.add( + "DEFAULT_TIMEOUT_IN_MILLISECONDS", + DEFAULT_TIMEOUT_IN_MILLISECONDS, + )?; #[pyfn(m)] fn py_log(log_level: Level, log_identifier: String, message: String) {