Skip to content

[adapters] Two HTTP connector fixes.#5873

Merged
ryzhyk merged 4 commits intomainfrom
http-connector-resume
Mar 27, 2026
Merged

[adapters] Two HTTP connector fixes.#5873
ryzhyk merged 4 commits intomainfrom
http-connector-resume

Conversation

@ryzhyk
Copy link
Contributor

@ryzhyk ryzhyk commented Mar 19, 2026

See commit messages.

@ryzhyk ryzhyk requested a review from gz March 19, 2026 20:25
@ryzhyk ryzhyk added the connectors Issues related to the adapters/connectors crate label Mar 19, 2026
@@ -593,6 +593,8 @@ impl InputQueue<(), Box<dyn InputBuffer>> {
///
/// Use [`TransportInputEndpoint::open`] to obtain an [`InputReader`].

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The as_any method requires 14 implementors to each copy the same one-liner body. A helper supertrait with a blanket default (e.g. AsAnyArc) or a derive macro could eliminate that boilerplate. Not a blocker, just worth noting.

}

impl InputReader for DeltaTableInputReader {
fn as_any(self: Arc<Self>) -> Arc<dyn std::any::Any + Send + Sync> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

couldn't this be a default function in the InputReader trait?

@ryzhyk ryzhyk added this pull request to the merge queue Mar 24, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 24, 2026
@ryzhyk ryzhyk added this pull request to the merge queue Mar 25, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 25, 2026
ryzhyk added 4 commits March 26, 2026 13:01
Fix several issues in the management of HTTP input connectors:

1. We used to create one connector per actix runtime thread for each connector
   config, e.g., with 8 threads we'd eventually have 8 connectors after several
   HTTP requests. This could be confusing to the user. Instead, we now create one
   connector, but cache a reference to it in a thread-local variable for fast
   lookup.
2. After a stop/resume cycle, we'd create a fresh set of connectors, while
   the old connectors were still there as well. The number of connectors could
   grow indefinitely with the number of restarts. We fix this by making connector
   names stabe across restarts.
3. We drop all HTTP connectors when the pipeline was modified and required
   bootstrapping; we would then create fresh HTTP connectors after restart. This
   is ok, except that we lost connector statistics (number of ingested records,
   errors, etc.) We now preserve HTTP connectors as long as the associated table
   is not affected by bootstrapping.

Signed-off-by: Leonid Ryzhyk <ryzhyk@gmail.com>
Auto-generated HTTP connector names did not follow the naming scheme
expected by the API (`relation_name.connector_name`). This broke connector
status reporting.

Signed-off-by: Leonid Ryzhyk <ryzhyk@gmail.com>
The test doesn't need to call wait_for_completion to make sure an HTTP ingest
has completed. We now use completion token for this; in fact input_json waits
for a completion token when called with `wait=true` (the default).

Signed-off-by: Leonid Ryzhyk <ryzhyk@gmail.com>
We used to drop the token returned by push_to_pipeline on the floor.
Instead, we now return it to the user so they can wait for the token
asynchronously.

Signed-off-by: Leonid Ryzhyk <ryzhyk@gmail.com>
@ryzhyk ryzhyk force-pushed the http-connector-resume branch from de31347 to 4aa9536 Compare March 26, 2026 20:02
@ryzhyk ryzhyk enabled auto-merge March 26, 2026 20:02
@ryzhyk ryzhyk added this pull request to the merge queue Mar 26, 2026
Merged via the queue into main with commit 4de8e17 Mar 27, 2026
1 check passed
@ryzhyk ryzhyk deleted the http-connector-resume branch March 27, 2026 02:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

connectors Issues related to the adapters/connectors crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants