armistace/blog_creator
0
0
Fork 0
Code Issues Pull Requests Actions Packages Projects 1 Releases Wiki Activity

bump all timeouts to 60 minutes for large context models #28

Merged
armistace merged 2 commits from bump-timeouts-60min into master 2026-07-28 11:07:44 +10:00
Conversation 2 Commits 2 Files Changed 2 +3 -3
hermes commented 2026-07-28 10:38:16 +10:00
Collaborator
Copy Link

Bumps the AI generation timeout from 30 to 60 minutes and the web search HTTP timeout from 30s to 60s to accommodate slower large-context models like minimax-m3 and glm-5.2.

Bumps the AI generation timeout from 30 to 60 minutes and the web search HTTP timeout from 30s to 60s to accommodate slower large-context models like minimax-m3 and glm-5.2.
hermes added 2 commits 2026-07-28 10:38:16 +10:00
armistace commented 2026-07-28 10:38:17 +10:00
Owner
Copy Link

PR received — starting review, sit tight 🫡

PR received — starting review, sit tight :saluting_face:
armistace commented 2026-07-28 10:40:06 +10:00
Owner
Copy Link

PR Review Results

Comprehensive Review Report: Bump Timeouts for Large Context Models

1. Executive Summary

The proposed changes aim to resolve timeout issues encountered when using large-context AI models (minimax-m3, glm-5.2) by increasing the AI generation timeout from 30 to 60 minutes and the web search HTTP timeout from 30 to 60 seconds.

While the change is functionally correct in isolation, all three review domains (Code, Security, and Infrastructure) have identified significant risks. The current approach of increasing synchronous timeouts to one hour is considered an anti-pattern that introduces a high risk of Denial of Service (DoS) via resource exhaustion, creates a mismatch with infrastructure proxy settings, and increases technical debt through hardcoded values.

The consensus across all reviewers is that a "quick fix" of bumping numbers is insufficient for a production environment. A transition to an asynchronous processing architecture is strongly recommended.

2. Prioritized List of Issues

🔴 Critical

  • Resource Exhaustion & DoS Risk: Holding worker threads/connections open for 60 minutes can lead to complete service outages (Worker Starvation). A small number of slow requests could occupy all available execution threads.
  • Infrastructure Mismatch: Application-level timeouts of 60 minutes will be superseded by Load Balancer/Ingress timeouts (typically 60s), resulting in 504 Gateway Timeouts and rendering the code change ineffective.

🟠 High

  • Synchronous Anti-Pattern: Utilizing synchronous HTTP requests for tasks that can take an hour violates distributed systems best practices.
  • Hardcoded Configuration: Timeout values are hardcoded in logic files, preventing SRE/DevOps from tuning the system in production without a full redeployment.

🟡 Medium

  • Lack of Granularity: A blanket 60-minute timeout applies to all models, even those that should fail fast to trigger fail-overs.
  • Availability Vulnerability: Lack of per-user concurrency limits on these long-running tasks opens the system to "Slowloris" style attacks.

🔵 Low

  • Monitoring Gap: Lack of specific logging/alerting for requests that exceed the previous 30-minute threshold to validate if 60 minutes is actually necessary.

3. Domain-Specific Recommendations

💻 Code Quality

  • Externalize Configuration: Move all timeout values to a .env file or config.yaml. Reference these via a centralized settings.py or config.py.
  • Implement Model-Specific Logic: Create a mapping of model_name -> timeout to ensure fast models remain responsive and only large-context models receive extended windows.
  • Improve Logging: Add telemetry to capture the actual duration of AI generations to data-drive future timeout adjustments.

🛡️ Security

  • Shift to Asynchronous Processing: Implement a task queue (e.g., Celery, RabbitMQ, or AWS SQS). Move AI generation to a background job; return a 202 Accepted status and provide a polling endpoint or webhook for the result.
  • Concurrency Quotas: Implement strict limits on the number of concurrent long-running requests allowed per user to mitigate DoS risks.
  • Fail-Fast Implementation: Re-evaluate the necessity of a 60-minute window; if a model takes an hour, it should almost certainly be handled as an asynchronous job.

🏗️ Infrastructure

  • Align Proxy Timeouts: If the synchronous approach is maintained (not recommended), update the Kubernetes Ingress or Load Balancer (e.g., proxy-read-timeout) to match the 3600s window.
  • Circuit Breaker Pattern: Implement a circuit breaker to automatically trip and fail requests if the AI provider's latency spikes, preventing a cascading failure of the entire application cluster.
  • OOM Monitoring: Increase monitoring for memory usage on pods handling these requests, as long-running large-context tasks are prone to Out-of-Memory kills.

4. Positive Aspects of the PR

  • Direct Problem Solving: The PR accurately identifies the bottleneck and provides the direct functional fix needed to stop the immediate timeouts.
  • Targeted Scope: The changes are localized to the specific tools affected (ollama_md_generator and ollama_web_search_tool), minimizing unintended side effects in unrelated modules.

5. Overall Assessment & Recommendation

Overall Rating: ❌ Significant Changes Needed

Final Recommendation:
This PR cannot be approved in its current state. While it solves the immediate symptom, it introduces critical stability and security vulnerabilities.

Required Actions for Approval:

  1. Architectural Shift: Move the 60-minute AI generation process from a synchronous request to an asynchronous background job.
  2. Configuration Management: Remove all hardcoded timeout integers and move them to environment variables.
  3. Infra Coordination: Provide a corresponding IaC update for Ingress/Load Balancer timeouts if any synchronous window exceeds 60 seconds.
## PR Review Results # Comprehensive Review Report: Bump Timeouts for Large Context Models ## 1. Executive Summary The proposed changes aim to resolve timeout issues encountered when using large-context AI models (`minimax-m3`, `glm-5.2`) by increasing the AI generation timeout from 30 to 60 minutes and the web search HTTP timeout from 30 to 60 seconds. While the change is functionally correct in isolation, all three review domains (Code, Security, and Infrastructure) have identified significant risks. The current approach of increasing synchronous timeouts to one hour is considered an anti-pattern that introduces a high risk of **Denial of Service (DoS) via resource exhaustion**, creates a **mismatch with infrastructure proxy settings**, and increases **technical debt** through hardcoded values. The consensus across all reviewers is that a "quick fix" of bumping numbers is insufficient for a production environment. A transition to an asynchronous processing architecture is strongly recommended. --- ## 2. Prioritized List of Issues ### 🔴 Critical * **Resource Exhaustion & DoS Risk:** Holding worker threads/connections open for 60 minutes can lead to complete service outages (Worker Starvation). A small number of slow requests could occupy all available execution threads. * **Infrastructure Mismatch:** Application-level timeouts of 60 minutes will be superseded by Load Balancer/Ingress timeouts (typically 60s), resulting in `504 Gateway Timeouts` and rendering the code change ineffective. ### 🟠 High * **Synchronous Anti-Pattern:** Utilizing synchronous HTTP requests for tasks that can take an hour violates distributed systems best practices. * **Hardcoded Configuration:** Timeout values are hardcoded in logic files, preventing SRE/DevOps from tuning the system in production without a full redeployment. ### 🟡 Medium * **Lack of Granularity:** A blanket 60-minute timeout applies to all models, even those that should fail fast to trigger fail-overs. * **Availability Vulnerability:** Lack of per-user concurrency limits on these long-running tasks opens the system to "Slowloris" style attacks. ### 🔵 Low * **Monitoring Gap:** Lack of specific logging/alerting for requests that exceed the previous 30-minute threshold to validate if 60 minutes is actually necessary. --- ## 3. Domain-Specific Recommendations ### 💻 Code Quality * **Externalize Configuration:** Move all timeout values to a `.env` file or `config.yaml`. Reference these via a centralized `settings.py` or `config.py`. * **Implement Model-Specific Logic:** Create a mapping of `model_name -> timeout` to ensure fast models remain responsive and only large-context models receive extended windows. * **Improve Logging:** Add telemetry to capture the actual duration of AI generations to data-drive future timeout adjustments. ### 🛡️ Security * **Shift to Asynchronous Processing:** Implement a task queue (e.g., Celery, RabbitMQ, or AWS SQS). Move AI generation to a background job; return a `202 Accepted` status and provide a polling endpoint or webhook for the result. * **Concurrency Quotas:** Implement strict limits on the number of concurrent long-running requests allowed per user to mitigate DoS risks. * **Fail-Fast Implementation:** Re-evaluate the necessity of a 60-minute window; if a model takes an hour, it should almost certainly be handled as an asynchronous job. ### 🏗️ Infrastructure * **Align Proxy Timeouts:** If the synchronous approach is maintained (not recommended), update the Kubernetes Ingress or Load Balancer (e.g., `proxy-read-timeout`) to match the 3600s window. * **Circuit Breaker Pattern:** Implement a circuit breaker to automatically trip and fail requests if the AI provider's latency spikes, preventing a cascading failure of the entire application cluster. * **OOM Monitoring:** Increase monitoring for memory usage on pods handling these requests, as long-running large-context tasks are prone to Out-of-Memory kills. --- ## 4. Positive Aspects of the PR * **Direct Problem Solving:** The PR accurately identifies the bottleneck and provides the direct functional fix needed to stop the immediate timeouts. * **Targeted Scope:** The changes are localized to the specific tools affected (`ollama_md_generator` and `ollama_web_search_tool`), minimizing unintended side effects in unrelated modules. --- ## 5. Overall Assessment & Recommendation **Overall Rating: ❌ Significant Changes Needed** **Final Recommendation:** This PR **cannot be approved** in its current state. While it solves the immediate symptom, it introduces critical stability and security vulnerabilities. **Required Actions for Approval:** 1. **Architectural Shift:** Move the 60-minute AI generation process from a synchronous request to an **asynchronous background job**. 2. **Configuration Management:** Remove all hardcoded timeout integers and move them to **environment variables**. 3. **Infra Coordination:** Provide a corresponding IaC update for Ingress/Load Balancer timeouts if any synchronous window exceeds 60 seconds.
armistace merged commit a1a534a86c into master 2026-07-28 11:07:44 +10:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
No items
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
hermes armistace
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: armistace/blog_creator#28
No description provided.
Delete Branch "bump-timeouts-60min"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?