ref(migrations): ddl timeout 5min -> 20min - #8415
Open
MeredithAnya wants to merge 2 commits into
Open
Conversation
Comment on lines
65
to
68
| "distributed_ddl_task_timeout": 600, # 10 minute ON CLUSTER DDL timeout | ||
| # Quiet ON CLUSTER DDL (DROP MV waiting on a hot-table lock) writes | ||
| # no bytes until it finishes. Pin a 15s header so http_send_timeout | ||
| # / idle do not IncompleteRead the HTTP body. |
Contributor
There was a problem hiding this comment.
Bug: The server-side DDL timeout (10 min) is longer than the client-side HTTP timeout (5 min), creating a risk of premature client timeouts during long migrations.
Severity: MEDIUM
Suggested Fix
To ensure consistency and prevent potential timeout failures, increase the client-side send_receive_timeout in the MIGRATE profile to match the server-side distributed_ddl_task_timeout. Set it to at least 600000 milliseconds (10 minutes).
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: snuba/clusters/cluster.py#L65-L68
Potential issue: The pull request increases the server-side
`distributed_ddl_task_timeout` to 600 seconds (10 minutes) but leaves the corresponding
client-side HTTP `send_receive_timeout` for the `MIGRATE` profile at 300 seconds (5
minutes). While the use of `send_progress_in_http_headers` is intended to keep the
connection alive, this timeout mismatch creates a latent bug. If for any reason (e.g., a
bug in ClickHouse, a network issue, or a misconfiguration) the progress headers are not
sent for a 5-minute interval, a long-running migration (between 5 and 10 minutes) will
fail due to a client-side timeout, even though the server is configured to allow it to
complete.
Did we get this right? 👍 / 👎 to inform future reviews.
Contributor
|
I think I'd rather do #8416 than just increase the timeout. |
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.
No description provided.