fix(connectors): bring quickwit_sink up to convention

[mfyuce]

Summary

• Defer index_id YAML parse to open() — return InvalidConfigValue instead of panicking via expect()
• Use ClientWithMiddleware (build_retry_client) for ingest retries with exponential backoff on 5xx / connection errors
• check_connectivity_with_retry in open() against /health/livez
• Map 4xx responses (except 429) to PermanentHttpError so the circuit breaker is not tripped by bad payloads
• Add verbose_logging / max_retries / retry_delay / max_retry_delay / max_open_retries / open_retry_max_delay to QuickwitSinkConfig (all optional, forward-compatible)
• Downgrade per-batch consume() log to debug! (upgraded to info! when verbose_logging = true)
• Set Content-Type: application/x-ndjson on ingest requests
• Drop unused dashmap / once_cell deps; add reqwest-middleware
• 5 unit tests covering verbose flag, client init state, and index_id init state

Test plan

• cargo clippy -p iggy_connector_quickwit_sink --all-targets -- -D warnings — clean
• cargo test -p iggy_connector_quickwit_sink — 5 tests pass
• Integration tests: cargo test -p integration -- connectors::quickwit

🤖 Generated with Claude Code

[github-actions]

Thanks for the PR. It is labeled S-waiting-on-review and queued for review.

Slash commands (own line, regular comment) move it around the queue:

• /ready - back to S-waiting-on-review after addressing feedback
• /author - flip to S-waiting-on-author while you finish changes
• /request-review @user-or-team - request a reviewer

See CONTRIBUTING.md for details.

[ryerraguntla]

lib.rs:ingest() — create_index() treats 409 Conflict as InitError; concurrent open() calls (multi-instance, restart race) both see has_index()=false, both POST, second gets 4xx → InitError → connector
never opens. Fix: absorb 409 (and 400 "already exists") as Ok(()) in create_index(). Retry middleware also retries a 5xx create that succeeded server-side; on retry, server returns 409 → same path. Same fix
covers both.

[mfyuce]

Sorry for the late response -- I was running a local benchmark to make sure the setup is working end-to-end.

Fixed in 512b71f04: create_index now handles 409 CONFLICT as a success case -- another instance beat us to it, but the index exists, which is what we wanted. Only other 4xx errors propagate as InitError.

[ryerraguntla]

reqwest::Client::new() has no timeout; health probe and has_index()/create_index() can hang indefinitely under network partition or slow-starting Quickwit. Fix: reqwest::Client::builder().timeout(...) with configurable or sensible default (e.g. 30s)

[mfyuce]

Sorry for the late response -- I was running a local benchmark to make sure the setup is working end-to-end.

Fixed in 512b71f04: added request_timeout: Option<String> to QuickwitSinkConfig (default "30s") and wired it into reqwest::Client::builder().timeout(request_timeout).build() in open(). The example config TOML has the field commented in as a reference.

[ryerraguntla]

@mfyuce - all the above are minors. I am not sure of quickwit s production data set distribution to mention about the need for circuit breakers (if there are huge number of records/documents for the same cursor key for a given batch size) . Please make a judgement call about the need for circuit breaker implementation . otherwise it is all set for second reviewer's comments before merging.

[ryerraguntla]

/author

[mfyuce]

Thanks for the thorough review @ryerraguntla! Both issues addressed in the latest push:

409 / "already exists" on create_index() — create_index() now absorbs 409 CONFLICT and 400 BAD_REQUEST whose body contains "already exists" as Ok(()) with an info! log. Covers the concurrent-open race and the retry-after-succeeded-5xx path.

No timeout on reqwest::Client — Added request_timeout: Option<String> to QuickwitSinkConfig (default 30s, configurable via TOML). The raw client is now built with Client::builder().timeout(request_timeout), bounding health probes, has_index(), and create_index() under network partition.

/ready

[mfyuce]

@ryerraguntla — judgment call on the circuit breaker:

The existing build_retry_client wraps a HttpRetryMiddleware that already retries 429 and 5xx with exponential backoff, and the PermanentHttpError mapping cuts retries for permanent 4xx errors (bad data won't hammer the backend). Ingest throughput is also naturally rate-bounded by batch_length and poll_interval.

QuickWit is typically an internal service in the same cluster, so prolonged partition is rare, and the runtime already isolates failures per connector. A full half-open / trip-threshold circuit breaker on top of this would add complexity without clear benefit for this specific sink. Happy to revisit if the second reviewer sees a concrete failure mode that the existing retry policy doesn't cover.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.40%. Comparing base (d8903dd) to head (a92de65).

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #3523      +/-   ##
============================================
- Coverage     74.46%   74.40%   -0.06%     
  Complexity      937      937              
============================================
  Files          1243     1243              
  Lines        125937   125520     -417     
  Branches     101857   101484     -373     
============================================
- Hits          93775    93396     -379     
+ Misses        29152    29071      -81     
- Partials       3010     3053      +43     

Components	Coverage Δ
Rust Core	75.16% <ø> (-0.03%)	⬇️
Java SDK	62.44% <ø> (ø)
C# SDK	71.40% <ø> (-0.71%)	⬇️
Python SDK	88.88% <ø> (ø)
PHP SDK	84.29% <ø> (ø)
Node SDK	91.35% <ø> (ø)
Go SDK	40.36% <ø> (ø)

Files with missing lines	Coverage Δ
core/connectors/sinks/quickwit_sink/src/lib.rs	77.08% <ø> (+9.26%)	⬆️

... and 29 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[kriti-sc]

Can you please add a brief rationale/problem statement/why behind the change? I am finding the PR hard to review without clarity on the goal being achieved.
@mfyuce

[mfyuce]

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.

[kriti-sc]

instead of 30s, define a const like the DEFAULT_* defined at the top of this file, and use that here