mfyuce commented on PR #3523: URL: https://github.com/apache/iggy/pull/3523#issuecomment-4779705703
Sorry for the late response -- was doing a local benchmark to make sure the setup is working. @kriti-sc good call, here is the rationale: The original `quickwit_sink` was missing several patterns that exist in every other production-grade connector in this repo. Without them, it would fail silently or hang in real deployments: **1. No request timeout** -- `reqwest::Client::new()` has no timeout. Under network partition, `has_index()`, `create_index()`, and `ingest()` block forever. This PR adds `request_timeout` (default 30 s). **2. No connectivity check on `open()`** -- the connector would report `Running` in the runtime's `/stats` endpoint even if QuickWit was unreachable, then fail silently on first ingest. This PR adds the same `check_connectivity_with_retry` probe that `postgres_sink`, `http_sink`, and `elasticsearch_sink` all use. **3. No retry middleware** -- transient 5xx or 429 responses from QuickWit caused immediate batch failure and offset advancement. This PR wires `HttpRetryMiddleware` with exponential backoff, matching the behavior of the other HTTP-based sinks. **4. 409 Conflict on `create_index()` crashed `open()`** -- in a multi-instance or restart race, two connectors can call `create_index()` simultaneously. The second one got a 409 and propagated it as an `InitError`, killing the connector. This PR absorbs 409 (and "already exists" 400) as `Ok(())`. The goal is to bring `quickwit_sink` to the same robustness level as `postgres_sink` before it sees production traffic. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
