-
Notifications
You must be signed in to change notification settings - Fork 368
fix(connectors): Harden Elasticsearch sink test readiness #3729
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
2d221c1
72fd3b7
ada822d
dd2c04c
d185f6c
62933fc
d0edc3b
dbf3ef7
7d568fb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,11 +31,14 @@ use secrecy::{ExposeSecret, SecretString}; | |
| use serde::{Deserialize, Serialize}; | ||
| use serde_json::json; | ||
| use simd_json::{OwnedValue, prelude::*}; | ||
| use std::time::Duration; | ||
| use tokio::sync::Mutex; | ||
| use tracing::{info, warn}; | ||
|
|
||
| sink_connector!(ElasticsearchSink); | ||
|
|
||
| const DEFAULT_TIMEOUT_SECONDS: u64 = 30; | ||
|
|
||
| #[derive(Debug)] | ||
| struct State { | ||
| invocations_count: usize, | ||
|
|
@@ -51,6 +54,11 @@ pub struct ElasticsearchSinkConfig { | |
| #[serde(serialize_with = "iggy_common::serde_secret::serialize_optional_secret")] | ||
| pub password: Option<SecretString>, | ||
| pub batch_size: Option<usize>, | ||
| /// Client-wide HTTP timeout for open() and bulk consume(). Values of `0` | ||
| /// are clamped to 1s (a zero duration would fail every request immediately). | ||
| /// Raise this for slow bulk workloads; until runtime ack/retry (#2927/#2928), | ||
| /// a timeout on consume drops the batch after the poll offset is already | ||
| /// committed. | ||
| pub timeout_seconds: Option<u64>, | ||
| pub create_index_if_not_exists: Option<bool>, | ||
| pub index_mapping: Option<serde_json::Value>, | ||
|
|
@@ -83,7 +91,17 @@ impl ElasticsearchSink { | |
| .map_err(|error| Error::Connection(format!("Invalid Elasticsearch URL: {error}")))?; | ||
|
|
||
| let conn_pool = elasticsearch::http::transport::SingleNodeConnectionPool::new(url); | ||
| let mut transport_builder = TransportBuilder::new(conn_pool); | ||
| // elasticsearch-rs defaults to no timeout. This client-global timeout is | ||
| // an infinite-hang backstop for open() and for bulk consume() — not the | ||
| // primary #3728 flake fix (that is the harness readiness gate, which | ||
| // expires sooner than the 30s default). Values of 0 clamp to 1s. | ||
| let timeout_seconds = self | ||
| .config | ||
| .timeout_seconds | ||
| .unwrap_or(DEFAULT_TIMEOUT_SECONDS) | ||
| .max(1); | ||
| let mut transport_builder = | ||
| TransportBuilder::new(conn_pool).timeout(Duration::from_secs(timeout_seconds)); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this timeout is client-global in elasticsearch-rs, so it bounds bulk requests in 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. |
||
|
|
||
| if let (Some(username), Some(password)) = (&self.config.username, &self.config.password) { | ||
| let credentials = | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.