fix(connectors): Harden Elasticsearch sink test readiness by ryerraguntla · Pull Request #3729 · apache/iggy

ryerraguntla · 2026-07-22T01:57:38Z

Summary

Fixes flaky Elasticsearch sink integration test where iggy-connectors
never becomes healthy because sink open() can hang on Elasticsearch HTTP
with no client timeout, while the connectors HTTP API only binds after
plugin open.
Hardens the Elasticsearch Testcontainers fixture: use 127.0.0.1, always
re-check /_cluster/health on reuse attach, self-heal a wedged
iggy-test-elasticsearch container only, and sweep stale harness indices
older than 30 minutes.
Improves harness readiness: poll connectors /health and fail fast if the
child process dies during retries; refresh the index before document-count
polls in the sink fixture.

Test plan

cargo fmt --all
cargo sort --no-format --workspace
cargo clippy -p iggy_connector_elasticsearch_sink -p integration --all-features --all-targets -- -D warnings
cargo test -p integration -- connectors::elasticsearch::elasticsearch_sink::elasticsearch_sink_stores_json_messages
Confirm docker rm -f recovery targets only iggy-test-elasticsearch
(no other Elasticsearch containers)
Optional: leave a reused iggy-test-elasticsearch running and re-run
the sink test to verify reuse + readiness path

Notes

Container force-remove uses the fixed harness name
iggy-test-elasticsearch only.
Stale index sweep is best-effort and age-gated so concurrent harness
tests sharing the reused container are not disrupted.

Add a timeout to the Elasticsearch sink transport, make connectors-runtime readiness check `/health`, and strengthen the Elasticsearch test fixture startup path with retry, cluster-health polling, and stale-index cleanup. Also refresh the sink index before counting documents so test assertions see newly indexed data sooner.

Use short-timeout probes without retries for cluster health and stale index sweep, and cap docker rm so a wedged daemon cannot stall setup.

github-actions · 2026-07-22T01:57:48Z

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.

codecov · 2026-07-22T02:09:06Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 39.73%. Comparing base (f67f038) to head (7d568fb).

Additional details and impacted files

@@              Coverage Diff              @@
##             master    #3729       +/-   ##
=============================================
- Coverage     75.10%   39.73%   -35.37%     
  Complexity      969      969               
=============================================
  Files          1307     1305        -2     
  Lines        152751   131680    -21071     
  Branches     128185   107116    -21069     
=============================================
- Hits         114726    52329    -62397     
- Misses        34494    76581    +42087     
+ Partials       3531     2770      -761

Components	Coverage Δ
Rust Core	`31.64% <100.00%> (-43.88%)`	⬇️
Java SDK	`62.71% <ø> (ø)`
C# SDK	`72.29% <ø> (+0.03%)`	⬆️
Python SDK	`92.27% <ø> (ø)`
PHP SDK	`84.52% <ø> (ø)`
Node SDK	`92.24% <ø> (+0.12%)`	⬆️
Go SDK	`43.08% <ø> (ø)`

Files with missing lines	Coverage Δ
...ore/connectors/sinks/elasticsearch_sink/src/lib.rs	`67.60% <100.00%> (+1.42%)`	⬆️

... and 423 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.

ryerraguntla · 2026-07-22T17:26:01Z

/request-review @user-or-team

slbotbm

`

slbotbm · 2026-07-22T18:48:07Z

start() force-removes the shared container after any try_start() error, not only after confirming it is wedged. Transient health, port-inspection, or Docker errors can therefore remove an Elasticsearch container still used by another process. The fixed name limits which container is removed but does not provide ownership or concurrency safety. Could this recovery path verify the container is unhealthy and coordinate removal with a cross-process lock before running docker rm -f?

Let me add the logic to remove when required.

Fixed in dd2c04c

Only docker rm the shared reuse container after inspect shows a removable state, and serialize recovery with a cross-process lock so transient start flakes cannot yank a healthy instance from a peer nextest worker.

ryerraguntla · 2026-07-23T00:47:21Z

/ready

hubcio

not in this diff, same class of problem:

elasticsearch source create_client() (core/connectors/sources/elasticsearch_source/src/lib.rs:304) still builds its transport with no timeout - same open()-hang failure mode this PR fixes for the sink, so source tests keep the #3728 flake class. worth a follow-up.
mcp.rs wait_ready() still uses the pattern this PR fixed for connectors: GET /, any Ok(_) counts as ready, no process-crash fast-fail. consistency follow-up.

hubcio · 2026-07-23T09:00:41Z

+            .unwrap_or(DEFAULT_TIMEOUT_SECONDS)
+            .max(1);
+        let mut transport_builder =
+            TransportBuilder::new(conn_pool).timeout(Duration::from_secs(timeout_seconds));


this timeout is client-global in elasticsearch-rs, so it bounds bulk requests in consume() too, not just open(). that changes the failure mode: before, a bulk on degraded ES stalled until it eventually succeeded; now anything over timeout_seconds returns Err, which the runtime discards (#2927) with the offset already committed at poll (#2928) - the batch is silently dropped. mechanism is pre-existing and tracked there, but this PR is what arms it for slow-ES bulk. worth an upgrade note in the PR description: raise timeout_seconds for slow bulk workloads, and note the tradeoff until #2927/#2928 land.

separately, the comment slightly oversells the timeout as the flake fix - with the 30s default the harness readiness budget (~20s) expires first, so what fixes #3728 is the fixture readiness gate; this is the infinite-hang backstop. maybe say that instead.

short-timeout health clients, wait_for_status , cluster readiness, async try_lock recovery, and timeout docs clarifying client-global clamp vs apache#3728 flake fix.

ryerraguntla · 2026-07-26T19:09:21Z

/ready

numinnex · 2026-07-31T13:38:46Z

+    matches!(
+        status,
+        "exited" | "dead" | "created" | "paused" | "restarting"
+    ) || health == "unhealthy"


This predicate cannot fire for the wedge state the recovery path was written for.

docker.io/library/elasticsearch:9.3.0 ships no HEALTHCHECK: docker image inspect docker.io/library/elasticsearch:9.3.0 --format '{{json .Config.Healthcheck}}' returns null, and GenericImage does not add one. So .State.Health is always nil, the {{if .State.Health}} branch in inspect_format yields the empty string, and health == "unhealthy" is unreachable. The predicate reduces to the status list alone.

That list covers container-level death (an OOM-killed JVM exits, so exited catches it), but not the case wait_until_ready was added to detect: ES process up, cluster red or HTTP wedged from a corrupted data dir, which is exactly what the doc comment above names as motivation. Docker reports running, this returns false, and every Elasticsearch test then fails at fixture setup until someone runs docker rm -f iggy-test-elasticsearch by hand. That manual step is what the recovery path was meant to remove.

Two ways out:

Treat running as removable when readiness already exhausted all CLUSTER_READY_ATTEMPTS. Pass that signal in so it stays distinct from a transient start or port flake, and keep the removal under RecoveryLock.

If leaving a running container alone is the deliberate call, drop the unreachable health leg and put the remediation command in the warn! at the caller. It currently says "leaving it in place" with no next step, so whoever hits it has to work out docker rm -f iggy-test-elasticsearch themselves.

ryerraguntla added 2 commits July 21, 2026 14:44

fix(connectors): avoid ES fixture readiness hang

72fd3b7

Use short-timeout probes without retries for cluster health and stale index sweep, and cap docker rm so a wedged daemon cannot stall setup.

github-actions Bot added the S-waiting-on-review PR is waiting on a reviewer label Jul 22, 2026

ryerraguntla changed the title ~~fix(connectors): harden Elasticsearch sink test readiness~~ fix(connectors): Harden Elasticsearch sink test readiness Jul 22, 2026

ci: retrigger pre-merge

ada822d

slbotbm requested changes Jul 22, 2026

View reviewed changes

github-actions Bot added S-waiting-on-author PR is waiting on author response and removed S-waiting-on-review PR is waiting on a reviewer labels Jul 22, 2026

ryerraguntla added 2 commits July 22, 2026 15:38

fix(connectors): gate ES fixture force-remove on wedged state

dd2c04c

Only docker rm the shared reuse container after inspect shows a removable state, and serialize recovery with a cross-process lock so transient start flakes cannot yank a healthy instance from a peer nextest worker.

fix(connectors): silence unused RecoveryLock field for clippy

d185f6c

github-actions Bot added S-waiting-on-review PR is waiting on a reviewer and removed S-waiting-on-author PR is waiting on author response labels Jul 23, 2026

hubcio requested changes Jul 23, 2026

View reviewed changes

github-actions Bot added S-waiting-on-author PR is waiting on author response and removed S-waiting-on-review PR is waiting on a reviewer labels Jul 23, 2026

ryerraguntla added 3 commits July 24, 2026 13:46

fix(connectors): bound ES harness probes against hangs

62933fc

short-timeout health clients, wait_for_status , cluster readiness, async try_lock recovery, and timeout docs clarifying client-global clamp vs apache#3728 flake fix.

Fixing formating error

d0edc3b

To reTrigger CI

dbf3ef7

github-actions Bot added S-waiting-on-review PR is waiting on a reviewer and removed S-waiting-on-author PR is waiting on author response labels Jul 26, 2026

Merge branch 'master' into fix/elasticsearch-sink-test-flakiness

7d568fb

numinnex reviewed Jul 31, 2026

View reviewed changes

Uh oh!

Conversation

ryerraguntla commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Notes

Uh oh!

github-actions Bot commented Jul 22, 2026

Uh oh!

codecov Bot commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ryerraguntla commented Jul 22, 2026

Uh oh!

slbotbm left a comment

Choose a reason for hiding this comment

Uh oh!

slbotbm Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

ryerraguntla Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ryerraguntla Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ryerraguntla commented Jul 23, 2026

Uh oh!

hubcio left a comment

Choose a reason for hiding this comment

Uh oh!

hubcio Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ryerraguntla commented Jul 26, 2026

Uh oh!

numinnex Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ryerraguntla commented Jul 22, 2026 •

edited

Loading

codecov Bot commented Jul 22, 2026 •

edited

Loading

ryerraguntla Jul 22, 2026 •

edited

Loading

ryerraguntla Jul 22, 2026 •

edited

Loading