Skip to content

Conversation

@heyihong
Copy link
Contributor

@heyihong heyihong commented Jan 9, 2026

What changes were proposed in this pull request?

This PR optimizes the tryComplete method in ObservationManager by:

  1. Making allMetrics a lazy val to defer metric collection until needed
  2. Simplifying the conditional logic using getOrElse(c.name, Row.empty)

Why are the changes needed?

  • Performance: The current implementation eagerly collects observed metrics even when the logical plan doesn't contain CollectMetrics nodes. Using lazy val allows tryComplete to complete faster when metric collection is unnecessary.
  • Code simplification: The original code had separate branches for handling missing vs. present metrics. This consolidates them into a single, cleaner code path.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Existing unit tests for ObservationManager and observation functionality cover this change. The refactoring maintains identical behavior.

Was this patch authored or co-authored using generative AI tooling?

Yes

@github-actions
Copy link

github-actions bot commented Jan 9, 2026

JIRA Issue Information

=== Sub-task SPARK-54988 ===
Summary: Simplify the implementation of ObservationManager.tryComplete
Assignee: None
Status: Open
Affected: ["4.2.0"]


This comment was automatically generated by GitHub Actions

@github-actions github-actions bot added the SQL label Jan 9, 2026
@heyihong heyihong changed the title [SPARK-54988][SQL] Simplify the implementation of Observation.tryComplete [SPARK-54988][SQL] Simplify the implementation of ObservationManager.tryComplete Jan 9, 2026
@cloud-fan
Copy link
Contributor

thanks, merging to master!

@cloud-fan cloud-fan closed this in 70a4a1e Jan 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants