You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iggy.apache.org by "hubcio (via GitHub)" <gi...@apache.org> on 2026/09/12 11:53:39 UTC
[PR] fix(connectors): validate sink writes and startup readiness [iggy]
hubcio opened a new pull request, #4154:
URL: https://github.com/apache/iggy/pull/4154
Sink startup can accept failed probes or race backend readiness,
numeric conversion can panic or overflow, and authentication or
delivery metrics can report incorrect outcomes.
Validate ClickHouse values and PostgreSQL batch sizes, authenticate
backend checks, correct write and loss accounting, and wait for
Quickwit ingestion readiness. Keep backend regression tests and
configuration guidance with these fixes.
MongoDB duplicate policy and the S3 probe-key contract remain separate decisions.
Validation: formatting, manifest sorting, Clippy and build passed; selected sink crate suites passed 555 tests. The integration run stopped after 25 passes and one failure: elasticsearch_sink_handles_bulk_messages timed out waiting for connector startup. The cause is unconfirmed. The remaining integration cases and final standalone Markdown/TOML checks are left to CI; normal commit hooks passed.
CI owns the remaining validation.
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(connectors): validate sink writes and startup readiness [iggy]
Posted by "spetz (via GitHub)" <gi...@apache.org>.
spetz commented on code in PR #4154:
URL: https://github.com/apache/iggy/pull/4154#discussion_r3999143044
##########
core/connectors/sinks/elasticsearch_sink/src/lib.rs:
##########
@@ -231,12 +232,13 @@ impl ElasticsearchSink {
}
}
+ documents_indexed = items.len() - errors;
let mut state = self.state.lock().await;
state.errors_count += errors;
- state.documents_indexed += items.len() - errors;
+ state.documents_indexed += documents_indexed;
}
- Ok(())
+ Ok(documents_indexed)
Review Comment:
`bulk_index_documents` now knows how many documents Elasticsearch rejected but still returns `Ok`, so `consume` at line 362 returns `Ok(())` and the runtime Acks the batch and counts every message as processed. A bulk response of HTTP 200 where every item carries an `error` therefore reports full success to the runtime. The only external signal is one `warn!` per rejected document plus `state.errors_count`, which is surfaced once in the close log at line 369 and never reaches the runtime's `errors_total` or `/stats`. The new integration test `given_rejected_document_when_indexing_should_report_zero_indexed` locks this in by sending `[1,2]` and asserting only that the log says zero. Worth deciding whether a batch with `documents_indexed == 0` and a non-empty input should return a nonzero FFI status now that PR 4152 makes the runtime act on one. As a smaller point on the same line, the operator-facing message at line 357 reads "Successfully indexed 0 documents".
##########
core/connectors/sinks/doris_sink/config.toml:
##########
@@ -21,7 +21,6 @@ enabled = true
version = 0
name = "Doris sink"
path = "../../target/release/libiggy_connector_doris_sink"
-plugin_config_format = "toml"
Review Comment:
Drops `plugin_config_format = "toml"`. That is a no-op, since `ConfigFormat` defaults to `Toml` at `core/connectors/runtime/src/configs/connectors.rs:47`, and it matches fourteen of the sixteen sink configs. `surrealdb_sink` still declares it.
##########
core/connectors/sinks/quickwit_sink/src/lib.rs:
##########
@@ -172,6 +173,40 @@ impl QuickwitSink {
}
}
+ async fn wait_for_index(
Review Comment:
`wait_for_index` probes readiness by sending POST with an empty body to the real `?commit=auto` ingest endpoint rather than a metadata endpoint. Zero documents should be a no-op, and the test asserts the probe bodies are empty, but it does put a live ingest call on every `open()`.
##########
core/connectors/sinks/s3_sink/config.toml:
##########
@@ -20,7 +20,7 @@ key = "s3"
enabled = true
version = 0
name = "S3 sink"
-path = "../../target/release/libiggy_connector_s3_sink"
+path = "target/release/libiggy_connector_s3_sink"
Review Comment:
Same in `core/connectors/sinks/surrealdb_sink/config.toml:23` change `path` from `../../target/release/...` to `target/release/...`. Both forms work: `resolve_plugin_path` in `core/connectors/runtime/src/main.rs:349` tries the literal path relative to the CWD first, then falls back to matching the file name against the exe directory, which is where `../../target/...` gets rescued from. The change is correct, but it leaves the tree at three sinks on the new form (clickhouse was already there) and thirteen on the old one, against the connectors-overview rule that plugin configs stay uniform. Either sweep the rest or leave these two alone.
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(connectors): validate sink writes and startup readiness [iggy]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #4154:
URL: https://github.com/apache/iggy/pull/4154#issuecomment-5645723941
## [Codecov](https://app.codecov.io/gh/apache/iggy/pull/4154?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 86.00%. Comparing base ([`dc2b382`](https://app.codecov.io/gh/apache/iggy/commit/dc2b382097ed7fb6cfeb8a31fed0fbc74af4433e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`b662af3`](https://app.codecov.io/gh/apache/iggy/commit/b662af325fe2485988eb0628aed163f5e9807028?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 2 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #4154 +/- ##
============================================
- Coverage 86.38% 86.00% -0.39%
Complexity 1455 1455
============================================
Files 1259 1257 -2
Lines 205992 205695 -297
Branches 171193 171100 -93
============================================
- Hits 177955 176913 -1042
- Misses 23597 24337 +740
- Partials 4440 4445 +5
```
| [Components](https://app.codecov.io/gh/apache/iggy/pull/4154/components?src=pr&el=components&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [Rust Core](https://app.codecov.io/gh/apache/iggy/pull/4154/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `87.33% <ø> (-0.01%)` | :arrow_down: |
| [Java SDK](https://app.codecov.io/gh/apache/iggy/pull/4154/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `67.58% <ø> (ø)` | |
| [C# SDK](https://app.codecov.io/gh/apache/iggy/pull/4154/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `76.97% <ø> (ø)` | |
| [Python SDK](https://app.codecov.io/gh/apache/iggy/pull/4154/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `91.34% <ø> (ø)` | |
| [PHP SDK](https://app.codecov.io/gh/apache/iggy/pull/4154/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `85.65% <ø> (ø)` | |
| [Node SDK](https://app.codecov.io/gh/apache/iggy/pull/4154/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `89.97% <ø> (-6.21%)` | :arrow_down: |
| [Go SDK](https://app.codecov.io/gh/apache/iggy/pull/4154/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `69.40% <ø> (ø)` | |
| [Files with missing lines](https://app.codecov.io/gh/apache/iggy/pull/4154?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...ore/connectors/sinks/clickhouse\_sink/src/binary.rs](https://app.codecov.io/gh/apache/iggy/pull/4154?src=pr&el=tree&filepath=core%2Fconnectors%2Fsinks%2Fclickhouse_sink%2Fsrc%2Fbinary.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9jb25uZWN0b3JzL3NpbmtzL2NsaWNraG91c2Vfc2luay9zcmMvYmluYXJ5LnJz) | `83.64% <ø> (-0.06%)` | :arrow_down: |
| [core/connectors/sinks/doris\_sink/src/lib.rs](https://app.codecov.io/gh/apache/iggy/pull/4154?src=pr&el=tree&filepath=core%2Fconnectors%2Fsinks%2Fdoris_sink%2Fsrc%2Flib.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9jb25uZWN0b3JzL3NpbmtzL2RvcmlzX3Npbmsvc3JjL2xpYi5ycw==) | `95.11% <ø> (ø)` | |
| [...ore/connectors/sinks/elasticsearch\_sink/src/lib.rs](https://app.codecov.io/gh/apache/iggy/pull/4154?src=pr&el=tree&filepath=core%2Fconnectors%2Fsinks%2Felasticsearch_sink%2Fsrc%2Flib.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9jb25uZWN0b3JzL3NpbmtzL2VsYXN0aWNzZWFyY2hfc2luay9zcmMvbGliLnJz) | `67.39% <ø> (-0.22%)` | :arrow_down: |
| [core/connectors/sinks/influxdb\_sink/src/lib.rs](https://app.codecov.io/gh/apache/iggy/pull/4154?src=pr&el=tree&filepath=core%2Fconnectors%2Fsinks%2Finfluxdb_sink%2Fsrc%2Flib.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9jb25uZWN0b3JzL3NpbmtzL2luZmx1eGRiX3Npbmsvc3JjL2xpYi5ycw==) | `96.03% <ø> (-0.01%)` | :arrow_down: |
| [...ore/connectors/sinks/influxdb\_sink/src/protocol.rs](https://app.codecov.io/gh/apache/iggy/pull/4154?src=pr&el=tree&filepath=core%2Fconnectors%2Fsinks%2Finfluxdb_sink%2Fsrc%2Fprotocol.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9jb25uZWN0b3JzL3NpbmtzL2luZmx1eGRiX3Npbmsvc3JjL3Byb3RvY29sLnJz) | `100.00% <ø> (ø)` | |
| [core/connectors/sinks/mongodb\_sink/src/lib.rs](https://app.codecov.io/gh/apache/iggy/pull/4154?src=pr&el=tree&filepath=core%2Fconnectors%2Fsinks%2Fmongodb_sink%2Fsrc%2Flib.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9jb25uZWN0b3JzL3NpbmtzL21vbmdvZGJfc2luay9zcmMvbGliLnJz) | `87.05% <ø> (ø)` | |
| [core/connectors/sinks/postgres\_sink/src/lib.rs](https://app.codecov.io/gh/apache/iggy/pull/4154?src=pr&el=tree&filepath=core%2Fconnectors%2Fsinks%2Fpostgres_sink%2Fsrc%2Flib.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9jb25uZWN0b3JzL3NpbmtzL3Bvc3RncmVzX3Npbmsvc3JjL2xpYi5ycw==) | `87.27% <ø> (-0.03%)` | :arrow_down: |
| [core/connectors/sinks/quickwit\_sink/src/lib.rs](https://app.codecov.io/gh/apache/iggy/pull/4154?src=pr&el=tree&filepath=core%2Fconnectors%2Fsinks%2Fquickwit_sink%2Fsrc%2Flib.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9jb25uZWN0b3JzL3NpbmtzL3F1aWNrd2l0X3Npbmsvc3JjL2xpYi5ycw==) | `93.01% <ø> (-0.29%)` | :arrow_down: |
| [core/connectors/sinks/s3\_sink/src/client.rs](https://app.codecov.io/gh/apache/iggy/pull/4154?src=pr&el=tree&filepath=core%2Fconnectors%2Fsinks%2Fs3_sink%2Fsrc%2Fclient.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9jb25uZWN0b3JzL3NpbmtzL3MzX3Npbmsvc3JjL2NsaWVudC5ycw==) | `89.62% <ø> (+0.83%)` | :arrow_up: |
| [core/connectors/sinks/s3\_sink/src/sink.rs](https://app.codecov.io/gh/apache/iggy/pull/4154?src=pr&el=tree&filepath=core%2Fconnectors%2Fsinks%2Fs3_sink%2Fsrc%2Fsink.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9jb25uZWN0b3JzL3NpbmtzL3MzX3Npbmsvc3JjL3NpbmsucnM=) | `83.09% <ø> (+0.42%)` | :arrow_up: |
| ... and [1 more](https://app.codecov.io/gh/apache/iggy/pull/4154?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | |
... and [54 files with indirect coverage changes](https://app.codecov.io/gh/apache/iggy/pull/4154/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(connectors): validate sink writes and startup readiness [iggy]
Posted by "hubcio (via GitHub)" <gi...@apache.org>.
hubcio merged PR #4154:
URL: https://github.com/apache/iggy/pull/4154
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org