Skip to content

Commit 03ed46c

Browse files
committed
fixup(connectors): address Doris sink review findings
Addresses review feedback on the Doris sink connector before merge. Correctness: - Label format now appends an 8-hex blake3 of the *raw* stream/topic names, so streams that sanitize identically (e.g. `events.v1` vs `events_v1`) can no longer collide and silently dedupe against each other in Doris. Each variable-length segment is also truncated; total label is bounded under Doris's 128-char cap regardless of input length. - `build_label` is now a pure `pub` free function. The integration test's manual label construction (used to verify server-side dedupe) now calls it directly, so the test cannot drift from the production format. - `consume` tracks the *most severe* error across chunks via `record_error`: permanent shadows transient. The previous first-error strategy let a transient error from chunk N hide a permanent error from chunk M and caused the runtime to retry forever instead of routing to DLQ. - HTTP 408 (Request Timeout) and 429 (Too Many Requests) classified as `CannotStoreData` (transient). They are 4xx but recoverable; the old code lumped them with all 4xx and DLQ'd retryable conditions. - Parse failures on the response body now return `PermanentHttpError`. An unparseable 200-OK is almost always a Doris bug or proxy interference — retrying the same bytes won't help. Security: - `open()` rejects `database`/`table` values outside `[A-Za-z0-9_]+`. Doris would reject them server-side anyway, but rejecting at config-load also prevents path traversal in the `/api/{db}/{table}/_stream_load` URL. - `open()` emits a `warn!` when `fe_url` is `http://` and the host is not loopback. README's new "Security notes" section spells out the trust boundary the manual-redirect-following implies (a compromised FE could exfiltrate credentials via a hostile `Location` header). - Response body truncated to 4 KB at a UTF-8 boundary before being formatted into errors or logs, so a misbehaving proxy that returns a giant body cannot OOM the connector or flood logs. Robustness: - Explicit `connect_timeout` (5 s) so an unreachable FE fails fast instead of consuming the full request timeout on the handshake alone. - `send_stream_load` takes `bytes::Bytes`; clones inside the redirect loop are now refcount bumps instead of full `Vec<u8>` copies. Observability: - `warn!` when Doris reports `number_filtered_rows > 0` — schema drift in upstream messages was previously logged at `info!` and easy to miss. - Per-batch success log demoted from `info!` to `debug!`. - README documents `Expect: 100-continue`, `label_keep_max_second` guidance, and the filtered-row alert. Tests: 21 unit tests pass (was 13, added 8 covering hash-suffix label collision resistance, label length cap, severity ordering, identifier validation, and log truncation). All 6 testcontainer integration tests pass against a real Doris all-in-one image.
1 parent e338bc3 commit 03ed46c

6 files changed

Lines changed: 330 additions & 71 deletions

File tree

Cargo.lock

Lines changed: 4 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

core/connectors/sinks/doris_sink/Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ crate-type = ["cdylib", "lib"]
3434
[dependencies]
3535
async-trait = { workspace = true }
3636
base64 = { workspace = true }
37+
blake3 = { workspace = true }
38+
bytes = { workspace = true }
3739
iggy_connector_sdk = { workspace = true }
3840
reqwest = { workspace = true }
3941
secrecy = { workspace = true }

core/connectors/sinks/doris_sink/README.md

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,14 @@ The Doris sink connector consumes JSON messages from Iggy streams and writes the
1111
## How it works
1212

1313
1. For each batch of messages, the connector serializes the JSON payloads into a JSON array.
14-
2. It computes a deterministic Stream Load `label` of the form `{label_prefix}-{stream}-{topic}-{partition}-{first_offset}-{last_offset}`. Doris dedupes loads by label inside its `label_keep_max_second` window, so a replayed batch (after restart, retry, etc.) is silently absorbed instead of producing duplicates.
15-
3. It `PUT`s the batch to `{fe_url}/api/{database}/{table}/_stream_load` with HTTP Basic auth and the headers `format: json`, `strip_outer_array: true`, `label: <label>`.
14+
2. It computes a deterministic Stream Load `label` of the form `{label_prefix}-{stream}_{hash8}-{topic}_{hash8}-{partition}-{first_offset}-{last_offset}`. Each variable-length segment carries a 32-bit blake3 hash of the raw name so that names which sanitize to the same string (e.g. `events.v1` vs `events_v1`) cannot collide. The total label is bounded under Doris's 128-char cap regardless of input length. Doris dedupes loads by label inside its `label_keep_max_second` window, so a replayed batch (after restart, retry, etc.) is silently absorbed instead of producing duplicates.
15+
3. It `PUT`s the batch to `{fe_url}/api/{database}/{table}/_stream_load` with HTTP Basic auth and the headers `Expect: 100-continue`, `format: json`, `strip_outer_array: true`, `label: <label>`. (`Expect: 100-continue` lets Doris reject auth/4xx failures before the connector uploads the whole body — important for large batches and required if a reverse proxy sits in front of Doris.)
1616
4. The Doris frontend (FE) responds with a `307 Temporary Redirect` to a backend (BE). The connector follows the redirect manually so that the `Authorization` header is preserved across the hop (`reqwest`'s default policy strips it on cross-host redirects).
1717
5. The HTTP body is parsed as JSON and the `Status` field decides the outcome:
1818
- `Success` → batch accepted.
1919
- `Label Already Exists` → idempotent replay, treated as success.
20-
- `Publish Timeout` or HTTP `5xx` → transient error (`Error::CannotStoreData`); the runtime can retry.
21-
- `Fail` or HTTP `4xx` → permanent error (`Error::PermanentHttpError`); retrying is not useful.
20+
- `Publish Timeout` or HTTP `5xx`/`408`/`429` → transient error (`Error::CannotStoreData`); the runtime can retry.
21+
- `Fail`, any other `4xx`, or an unparseable response body → permanent error (`Error::PermanentHttpError`); retrying is not useful.
2222

2323
## Configuration
2424

@@ -66,6 +66,17 @@ batch_size = 1000
6666
timeout_secs = 30
6767
```
6868

69+
## Security notes
70+
71+
- **Use `https://` in production.** The connector accepts `http://` URLs and logs a `warn!` when `fe_url` points at a non-loopback host over plain HTTP, but it does not refuse. Over `http://`, the HTTP Basic credentials travel in cleartext.
72+
- **Trust boundary on the FE.** The connector intentionally preserves the `Authorization` header across the FE → BE 307 redirect (reqwest would otherwise strip it on cross-host redirects). A compromised or MITM'd FE could exfiltrate credentials by responding with `Location: http://attacker/`. The `http://` warning above is the primary mitigation; deploy Doris over TLS in hostile networks.
73+
- **`columns` and `where` are SQL-expression pass-throughs.** Whatever you put in those config fields is forwarded verbatim to Doris's Stream Load and evaluated as a SQL expression. Keep this config trusted.
74+
75+
## Operational guidance
76+
77+
- **`label_keep_max_second`.** Idempotent replay relies on Doris retaining each label for at least as long as it could take the Iggy runtime to redrive a failed batch. The Doris default is 3 days, which is conservative. If you set this lower on the Doris side, make sure your runtime retry budget fits inside the window — once a label expires, a replay re-loads instead of deduping, producing duplicate rows.
78+
- **Filtered-row alerts.** When Doris reports `number_filtered_rows > 0`, the connector emits a `warn!`. This is your signal that upstream message shapes have drifted from the table schema; alert on it.
79+
6980
## Limitations (v1)
7081

7182
- JSON payload only. CSV and raw-text payloads are not supported yet.

0 commit comments

Comments
 (0)