Skip to content

Conversation

@jayshrivastava
Copy link
Collaborator

@jayshrivastava jayshrivastava commented Jan 26, 2026

Dupe of #281 with minor fixes

  • Update test assertions to allow empty metrics for PartitionIsolatorExec nodes
  • Fix assert_metrics_present_in_plan test helper to handle PartitionIsolatorExec wrapped in MetricsWrapperExec
  • Fix count_plan_nodes_up_to_network_boundary test helper now that network boundary nodes have metrics

Fixes issue where PartitionIsolatorExec nodes were missing from displayed plans after metrics rewriting. All metrics collection tests now pass.

#[ignore] // https://github.com/datafusion-contrib/datafusion-distributed/issues/260
#[ignore]
async fn test_metrics_collection_e2e_4() {
run_metrics_collection_e2e_test("SELECT distinct company from table2").await;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

FYI @marc-pydantic since you added this test.

- Fix stage_metrics_rewriter to correctly match metrics to nodes in pre-order traversal
- Update test assertions to allow empty metrics for PartitionIsolatorExec nodes
- Fix assert_metrics_present_in_plan to handle PartitionIsolatorExec wrapped in MetricsWrapperExec

Fixes issue where PartitionIsolatorExec nodes were missing from displayed plans
after metrics rewriting. All metrics collection tests now pass.
@gabotechs gabotechs merged commit 971dfab into main Jan 27, 2026
7 checks passed
@gabotechs gabotechs deleted the js/fix-metrics-bug branch January 27, 2026 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants