Separate init and runtime timeout budgets, add per-request timeouts#1953
Open
moguchev wants to merge 1 commit into
Open
Separate init and runtime timeout budgets, add per-request timeouts#1953moguchev wants to merge 1 commit into
moguchev wants to merge 1 commit into
Conversation
Splits previously conflated timeout budgets so a low user-supplied Timeout no longer breaks session establishment, and adds dedicated client-side budgets for system queries and per-request overrides. Also fixes a potential controlConn reconnect deadlock. * ClusterConfig.ReadTimeout: socket-level read deadline, defaults to 11s, falls back to ConnectTimeout if unset. Decoupled from Timeout, which is now purely a per-request client-side budget. * ClusterConfig.Metadata.SystemRequestTimeout (default 60s): per-request budget for system/system_schema queries and other runtime control-conn frames. Applied via Conn.querySystem, controlConn.query and controlConn.writeFrame. * Query/Batch.WithRequestTimeout / SetRequestTimeout / GetRequestTimeout: per-request override of ClusterConfig.Timeout, enforced via callReq.timer in Conn.exec independently of the socket deadline. 0 disables the client-side timer. * Conn.finalizeConnection: switches an initialized connection from ConnectTimeout-bound init mode to operational ReadTimeout / WriteTimeout / Metadata.SystemRequestTimeout. Called by hostConnPool.connect, controlConn.connect and controlConn.attemptReconnectToAnyOfHosts. Until then, all socket reads/writes and internal init-phase requests (STARTUP/AUTH/OPTIONS/system.local/USE keyspace) are pinned to ConnectTimeout. Fixes "no connections were made when creating the session" when Timeout is low. * controlConn.HandleError now launches reconnect in a goroutine to avoid a potential deadlock during a connection storm where attemptReconnect holds the controlConn mutex while doing DNS resolution / TCP dial. * connReader.timeout, deadlineContextWriter.timeout and writeCoalescer.timeout are now atomics to allow finalizeConnection to update them concurrently with reader/writer goroutines. * Regression tests: TestNewConnectWithLowTimeout, TestInitSchemaQueryRespectsConnectTimeout, TestControlConnQueryRespectsSystemRequestTimeout, TestQueryRequestTimeout, TestControlConn_HandleError_LaunchesReconnectInGoroutine. * README: new "Connection Timeouts" section documenting the layers and picking guidance.
Contributor
|
We want to prioritize a fix for a quick patch release so we need a PR that focuses on removing the connection read deadline and therefore making After that we can work on a more comprehensive improvement of the existing timeout configuration fields such as this PR. cc @worryg0d |
Author
Of course, this PR is more for inspiration for the future works with timeouts. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Related Issue: #1919
Splits previously conflated timeout budgets so a low user-supplied Timeout no longer breaks session establishment, and adds dedicated client-side budgets for system queries and per-request overrides. Also fixes a potential controlConn reconnect deadlock.
ClusterConfig.ReadTimeout: socket-level read deadline, defaults to11s, falls back toConnectTimeoutif unset. Decoupled fromTimeout, which is now purely a per-request client-side budget.ClusterConfig.Metadata.SystemRequestTimeout(default60s): per-request budget forsystem/system_schemaqueries and other runtime control-conn frames. Applied viaConn.querySystem,controlConn.queryandcontrolConn.writeFrame.Query/Batch.WithRequestTimeout/SetRequestTimeout/GetRequestTimeout: per-request override ofClusterConfig.Timeout, enforced viacallReq.timerinConn.execindependently of the socket deadline. 0 disables the client-side timer.Conn.finalizeConnection: switches an initialized connection from ConnectTimeout-bound init mode to operationalReadTimeout/WriteTimeout/Metadata.SystemRequestTimeout. Called byhostConnPool.connect,controlConn.connectandcontrolConn.attemptReconnectToAnyOfHosts. Until then, all socket reads/writes and internal init-phase requests (STARTUP/AUTH/OPTIONS/system.local/USE keyspace) are pinned toConnectTimeout. Fixes "no connections were made when creating the session" when Timeout is low.controlConn.HandleErrornow launches reconnect in a goroutine to avoid a potential deadlock during a connection storm where attemptReconnect holds the controlConn mutex while doing DNS resolution / TCP dial.connReader.timeout,deadlineContextWriter.timeoutandwriteCoalescer.timeoutare now atomics to allowfinalizeConnectionto update them concurrently with reader/writer goroutines.TestNewConnectWithLowTimeout,TestInitSchemaQueryRespectsConnectTimeout,TestControlConnQueryRespectsSystemRequestTimeout,TestQueryRequestTimeout,TestControlConn_HandleError_LaunchesReconnectInGoroutine.README: new "Connection Timeouts" section documenting the layers and picking guidance.