Skip to content

Conversation

@Subhra264
Copy link
Contributor

@Subhra264 Subhra264 commented Oct 28, 2025

A timeout is added to the ingestion_service::ingest function which is used for internal ingestion. Default value of the new env variable ZO_GRPC_INGEST_TIMEOUT which is used for the above timeout, is 600 seconds

@Subhra264 Subhra264 requested a review from hengfeiyang October 28, 2025 08:49
@github-actions github-actions bot added ☢️ Bug Something isn't working ✏️ Feature labels Oct 28, 2025
@github-actions
Copy link
Contributor

Failed to generate code suggestions for PR

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Overview

Greptile Summary

Added timeout configuration to internal ingestion gRPC calls to prevent indefinite hangs. The default timeout is set to 600 seconds, matching the query timeout setting.

Key Changes:

  • Updated ZO_QUERY_INGESTER_TIMEOUT default from 0 (no timeout) to 600 seconds in config.rs:1295
  • Applied timeout to gRPC requests in ingestion_service.rs:40-43 using request.set_timeout()
  • Follows existing pattern used in other gRPC clients like make_grpc_search_client in infra/src/client/grpc.rs:115

Impact:

  • Affects internal ingestion calls from alerts, self-reporting, and cloud events services
  • Prevents indefinite blocking on slow or unresponsive ingester nodes
  • Improves system resilience and observability

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The changes follow established patterns in the codebase, add critical timeout protection to prevent indefinite hangs, and use a reasonable default value that matches the query timeout. The implementation is simple, well-scoped, and addresses a real operational concern.
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
src/config/src/config.rs 5/5 Changed default value of ZO_QUERY_INGESTER_TIMEOUT from 0 to 600 seconds to match query_timeout
src/service/ingestion/ingestion_service.rs 5/5 Added timeout configuration to internal ingestion gRPC calls using query_ingester_timeout setting

Sequence Diagram

sequenceDiagram
    participant Caller as Calling Service<br/>(Alerts/SelfReporting)
    participant IS as ingestion_service::ingest
    participant Config as Configuration
    participant Client as IngestClient<br/>(gRPC)
    participant Ingester as Remote Ingester Node

    Caller->>IS: ingest(IngestionRequest)
    IS->>Config: get_config()
    Config-->>IS: query_ingester_timeout: 600s
    IS->>IS: Create tonic::Request
    IS->>IS: Set timeout to 600s
    IS->>Client: Configure compression & message size
    Client->>Ingester: ingest(request) with 600s timeout
    alt Success
        Ingester-->>Client: IngestionResponse
        Client-->>IS: Ok(response)
        IS-->>Caller: Ok(IngestionResponse)
    else Timeout or Error
        Ingester-->>Client: Error/Timeout
        Client-->>IS: Err(e)
        IS->>IS: Log error
        IS-->>Caller: Err(IngestionError)
    end
Loading

2 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@testdino-playwright-reporter
Copy link

⚠️ Test Run Unstable


Author: Subhra264 | Branch: sc/ingest_timeout | Commit: 7ee1113

Testdino Test Results

Status Total Passed Failed Skipped Flaky Pass Rate Duration
All tests passed 365 342 0 19 4 94% 4m 40s

View Detailed Results

@testdino-playwright-reporter
Copy link

⚠️ Test Run Unstable


Author: Subhra264 | Branch: sc/ingest_timeout | Commit: e1fb123

Testdino Test Results

Status Total Passed Failed Skipped Flaky Pass Rate Duration
All tests passed 365 343 0 19 3 94% 4m 42s

View Detailed Results

@testdino-playwright-reporter
Copy link

⚠️ Test Run Unstable


Author: Subhra264 | Branch: sc/ingest_timeout | Commit: e1fb123

Testdino Test Results

Status Total Passed Failed Skipped Flaky Pass Rate Duration
All tests passed 365 344 0 19 2 94% 4m 40s

View Detailed Results

@testdino-playwright-reporter
Copy link

⚠️ Test Run Unstable


Author: Subhra264 | Branch: sc/ingest_timeout | Commit: b1d33f4

Testdino Test Results

Status Total Passed Failed Skipped Flaky Pass Rate Duration
All tests passed 366 346 0 19 1 95% 4m 39s

View Detailed Results

@testdino-playwright-reporter
Copy link

⚠️ Test Run Unstable


Author: Subhra264 | Branch: sc/ingest_timeout | Commit: 7ba50b8

Testdino Test Results

Status Total Passed Failed Skipped Flaky Pass Rate Duration
All tests passed 366 343 0 19 4 94% 4m 38s

View Detailed Results

@hengfeiyang hengfeiyang merged commit ea245c1 into main Oct 29, 2025
32 checks passed
@hengfeiyang hengfeiyang deleted the sc/ingest_timeout branch October 29, 2025 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

☢️ Bug Something isn't working ✏️ Feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants