Skip to content

Multihost tests#5711

Merged
blp merged 5 commits intomainfrom
multihost-tests
Feb 28, 2026
Merged

Multihost tests#5711
blp merged 5 commits intomainfrom
multihost-tests

Conversation

@blp
Copy link
Member

@blp blp commented Feb 27, 2026

These fixes make more of the Python integration tests pass on multihost.

@blp blp requested a review from ryzhyk February 27, 2026 00:31
@blp blp self-assigned this Feb 27, 2026
@blp blp added connectors Issues related to the adapters/connectors crate rust Pull requests that update Rust code python Pull requests that update python code metrics Metrics about feldera pipelines multihost Related to multihost or distributed pipelines labels Feb 27, 2026
r_stats = get(api_url(f"/pipelines/{pipeline_name}/stats"))
assert r_stats.status_code == HTTPStatus.OK, (r_stats.status_code, r_stats.text)
stats = r_stats.json()
print(stats)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this intended?

Copy link
Member Author

Choose a reason for hiding this comment

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

It was useful for debugging and I don't think it can hurt in a test.

StepAction::Step if request.step >= step => Some(Action::Step),
StepAction::Trigger if request.step >= step => None,
StepAction::Trigger if request.step >= step => {
if self.controller.status.unset_step_requested() {
Copy link
Contributor

Choose a reason for hiding this comment

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

This I defer to @ryzhyk

Copy link
Collaborator

@mythical-fred mythical-fred left a comment

Choose a reason for hiding this comment

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

One test bug that needs fixing before this can merge.

out = outputs[0]
assert out.get("config", {}).get("stream") == "v1"
assert out.get("metrics", {}).get("total_processed_input_records") == 5
assert out.get("metrics", {}).get("total_processed_input_steps") == steps
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wrong field name: total_processed_input_steps does not exist. The field added to OutputEndpointMetrics is total_processed_steps. This assertion always gets None and will fail once steps > 0.

Copy link
Member Author

Choose a reason for hiding this comment

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

This was buggy.

I pushed a new version that adds a type-safe API for these properties and uses it in the test.

@blp blp enabled auto-merge February 27, 2026 17:56
@blp blp disabled auto-merge February 27, 2026 17:56
blp added 2 commits February 27, 2026 09:56
Signed-off-by: Ben Pfaff <blp@feldera.com>
This doesn't test JSON metrics, which haven't been implemented in
multihost and maybe will never need to be.

Signed-off-by: Ben Pfaff <blp@feldera.com>
@blp blp enabled auto-merge February 27, 2026 17:56
@blp blp added this pull request to the merge queue Feb 27, 2026
blp added 2 commits February 27, 2026 11:18
This has been available inside the pipeline for a while, but we forgot to
expose it through the API.  This fixes that.

This also fixes up and enables the corresponding Python integration test.

Signed-off-by: Ben Pfaff <blp@feldera.com>
The clock connector requests steps instead of queueing data, but we were
ignoring that in multihost.

Also, connect the clock only on the first host in a multihost cluster.

This fixes a Python integration test for now in multihost, so enable that
test.

Signed-off-by: Ben Pfaff <blp@feldera.com>
@blp blp removed this pull request from the merge queue due to a manual request Feb 27, 2026
Signed-off-by: feldera-bot <feldera-bot@feldera.com>
Copy link
Collaborator

@mythical-fred mythical-fred left a comment

Choose a reason for hiding this comment

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

Fix looks good — total_processed_steps is now correctly exposed in the API and the test uses it properly. LGTM.

@blp blp added this pull request to the merge queue Feb 27, 2026
Merged via the queue into main with commit 97fd261 Feb 28, 2026
2 checks passed
@blp blp deleted the multihost-tests branch February 28, 2026 01:46
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 metrics Metrics about feldera pipelines multihost Related to multihost or distributed pipelines python Pull requests that update python code rust Pull requests that update Rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants