You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@iggy.apache.org by "mlevkov (via GitHub)" <gi...@apache.org> on 2026/07/17 07:10:29 UTC
Re: Proposal: HTTP Source Connector (Webhook Gateway)
GitHub user mlevkov added a comment to the discussion: Proposal: HTTP Source Connector (Webhook Gateway)
## HTTP Source: refreshed v1 plan (grounded against master, 2026-07-16)
Picking this back up after three months. I re-verified every item from @hubcio's review against current master before writing a line of code. Status first, then the v1 shape we're committing to.
### Prerequisite status
| Item from the April review | Status on master |
|---|---|
| Shutdown ordering (`cleanup_sender` before `iggy_source_close`) | **Fixed** in #3321. `stop_connector` now closes the FFI first, then drops the sender, then awaits task handles with a 5s timeout. Thank you, this was P0 for us. |
| `FileStateProvider` crash safety | **Fixed** in #3321 (write-tmp, fsync, rename). |
| Unbounded flume channel in `spawn_source_handler` | **Still open** (`runtime/src/source.rs`, `flume::unbounded()`). This is the missing link in the backpressure chain: without it, a slow Iggy means unbounded memory growth instead of HTTP 429. **I'll file the bounded-channel PR as the first deliverable**: capacity from `SourceConfig`, generous default so existing sources are unaffected. Taking you up on the review offer. |
| `Arc::try_unwrap` race in `SourceContainer::close` | Still open. Mitigated in the connector design (background tasks hold `Arc<SharedState>`, never `Arc<Source>`); happy to file the SDK fix as a companion PR. |
### v1 plan: option (c), as recommended
Adopting single-topic-per-instance with a shared listener:
- One connector instance per target topic, producing through the standard SDK pipeline. Zero SDK changes, zero ABI breaks, transforms work as for any source.
- Multi-topic routing = multiple instances of the same plugin sharing one axum listener. The runtime loads one container per dylib path and shares it across instances, so the first `open()` binds, later `open()`s register routes into a shared table (fail-closed if their `listen_addr` disagrees), each `close()` deregisters, and the last `close()` releases the port.
- Per-instance bounded `ArrayQueue` + `Notify` bridge into `poll()`; buffer full returns 429 with `Retry-After`.
- Delivery semantics documented at the top of the README as **best-effort** (adopting @kriti-sc's correction: weaker than at-most-once, since a 200 can still be followed by loss on crash or producer error). Loss windows are enumerated and instrumented, with `TODO(delivery)` markers in code pointing at the v2 path.
### Config and dynamic endpoints: your suggestion, plus one finding
Own-IggyClient config consumer is dropped, as you recommended. Static endpoints (named topic path, secret-path endpoints, auth) live in TOML through the normal `ConnectorsConfigProvider` flow, and changes apply via restart-with-new-config (#2781). Restarting one instance re-registers its routes without disturbing the shared server.
For real-time operations (revoking a compromised endpoint, provisioning endpoints programmatically) we're adopting your management-endpoints suggestion: `POST/PATCH/DELETE /admin/endpoints` on a **separate admin listener** (default loopback), guarded by a management token, disabled unless the token is configured.
The finding worth calling out: dynamic registrations can be made durable with **zero new runtime machinery** via `ConnectorState`. Verified on master:
1. A management mutation swaps the registry, sets a dirty flag, and notifies the poll task.
2. `poll()` returns the serialized registry as `state`, even on an empty batch. The forwarding loop has no empty-batch short-circuit and saves state whenever it's `Some` and the send succeeded.
3. #3321 made that save atomic, and the FFI `open` hands the persisted state back to the plugin factory on restart (same pattern as `random_source`).
So revocations survive restarts, nothing resurrects, and there's no second connection to go stale. The fail-closed-on-staleness concern from the original design doesn't apply: the registry is process-local and mutations are synchronous. The known limits are documented: persistence lands on the next successful poll cycle (a `persisted` flag is exposed per endpoint), state files contain HMAC secrets (same at-rest posture as TOML credentials), and cross-process fleets don't converge on dynamic endpoints in v1.
### Other updates baked into the plan
- Multi-module layout from the start: `lib.rs`, `server.rs`, `routes.rs`, `management.rs`, `state.rs`, `auth.rs`, `types.rs`. No 2,000-line lib.rs.
- Credential fields use `SecretString` + `iggy_common::serde_secret` per current convention.
- Forwarded HTTP header values are clamped to 255 bytes (`HeaderValue` limit; `User-Agent` routinely exceeds it).
- Gateway metrics (`/admin/metrics`, Prometheus text) cover accept-to-200 latency and per-status counts, which the runtime's stage histograms can't see. Prefixed `http_source_` to avoid collisions.
- Config conventions aligned with the OTLP gRPC source (#3516), the other push-based source in flight: single `listen_addr` field, bounded channel sizing. Good precedent that the push-to-pull bridge is workable.
### Sequencing
1. Bounded-channel prerequisite PR (small, all sources benefit).
2. Connector v1 per the above.
3. A separate proposal discussion for the `PushSource`/`DirectSource` trait you outlined, as the v2 at-least-once path, cross-linked with #3518 so source-side and sink-side delivery vocabulary stays unified.
Given #3580 (connectors repo split) lists "land the open connector PRs" as a pre-split step, I'd like to land 1 and 2 inside that window; the v2 trait work gets more expensive once `iggy_connector_sdk` is a published crate with a semver policy.
Two things to confirm before I start:
1. Any objection to the management API being in v1 scope, given the admin-listener isolation, off-by-default posture, and `ConnectorState` durability above?
2. For the bounded channel: is a `SourceConfig` field with a large default (e.g. 8192 batches) the shape you'd want, or would you rather it derive from something existing?
If no objections, I'll start with the prerequisite PR.
GitHub link: https://github.com/apache/iggy/discussions/3039#discussioncomment-17669879
----
This is an automatically sent email for dev@iggy.apache.org.
To unsubscribe, please send an email to: dev-unsubscribe@iggy.apache.org