fix: add timeout for database query in workflow estimation to prevent blocking. #15116
+12
−1
Conversation
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
shuangkun
force-pushed
the
fix/estimation-db-query-timeout
branch
4 times, most recently
from
c1938dc to
bd2c203
Compare
MasonM
reviewed
Dec 8, 2025
MasonM
reviewed
Dec 8, 2025
Joibel
requested changes
Dec 11, 2025
… blocking Add timeout control for GetWorkflowForEstimator database query to prevent workflow execution from being blocked when database is slow or locked. - Add default 5 second timeout for database queries - Configurable via WORKFLOW_ESTIMATION_DB_QUERY_TIMEOUT environment variable - Return default estimator on timeout/error to ensure workflow continues - Add warning logs for timeout and error cases - Add documentation for new environment variable Signed-off-by: shuangkun <[email protected]>
Use WithContext(ctx) when building SQL selector to ensure the timeout context from estimator_factory.go is properly propagated to the database query execution. This allows the query timeout to be respected and prevents blocking workflow execution when database is slow or locked. Signed-off-by: shuangkun <[email protected]>
Move database query timeout handling from estimator_factory.go into GetWorkflowForEstimator method in workflow_archive.go. This improves code organization by: - Encapsulating timeout logic at the database layer where it belongs - Simplifying caller code (estimator_factory.go) by removing timeout setup and error handling complexity - Making timeout protection available to all callers automatically - Maintaining backward compatibility: respects existing context deadlines if set by caller, otherwise applies default timeout from environment variable or 5 second default The timeout is configurable via WORKFLOW_ESTIMATION_DB_QUERY_TIMEOUT environment variable and defaults to 5 seconds if not set. Signed-off-by: shuangkun <[email protected]>
Signed-off-by: shuangkun <[email protected]>
Signed-off-by: shuangkun <[email protected]>
Signed-off-by: shuangkun <[email protected]>
Co-authored-by: Mason Malone <[email protected]> Signed-off-by: shuangkun tian <[email protected]>
Signed-off-by: shuangkun <[email protected]>
When ctx is nil, explicitly create context.Background() first, then add logger to it. This makes the code more readable and clear about what happens in each step, while avoiding panic in env.LookupEnvDurationOr which requires a logger in the context. Signed-off-by: shuangkun <[email protected]>
Add nolint comment for contextcheck linter. When ctx is nil, we need to create a new context with logger, which is a valid use case. The context is immediately used to derive a new context with timeout, maintaining proper context inheritance. Signed-off-by: shuangkun <[email protected]>
… DB query timeout - Change from time.Duration to int (seconds) for timeout configuration - Rename env var from WORKFLOW_ESTIMATION_DB_QUERY_TIMEOUT to WORKFLOW_ESTIMATION_DB_QUERY_TIMEOUT_SECONDS - Remove nil context handling, require valid ctx parameter - Use ctx.WithTimeout directly instead of creating background context Signed-off-by: shuangkun <[email protected]>
shuangkun
force-pushed
the
fix/estimation-db-query-timeout
branch
from
dd28cf6 to
a9fec7d
Compare
Member
Author
|
/retest |
MasonM
approved these changes
Dec 15, 2025
Joibel
approved these changes
Dec 16, 2025
|
❌ Cherry-pick failed for 3.7. Please check the workflow logs for details. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
I found that when the database was locked, it blocked the execution of a large number of our workflows.
Add timeout control for GetWorkflowForEstimator database query to prevent workflow execution from being blocked when database is slow or locked.
Motivation
Modifications
Verification
Documentation