Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[BUG] Profiler ignoring some reused subqueries #718

Closed
rongou opened this issue Jan 8, 2024 · 2 comments · Fixed by #741
Closed

[BUG] Profiler ignoring some reused subqueries #718

rongou opened this issue Jan 8, 2024 · 2 comments · Fixed by #741
Assignees
@amahussein
Labels
bug Something isn't working core_tools Scope the core module (scala)

Comments

@rongou
Copy link

rongou commented Jan 8, 2024

Describe the bug
In the profiler when we construct a SparkPlanGraph object from spakPlanInfo, there are two problems:

  • For CPU, it's currently ignoring that SubqueryBroadcast can also be reused as a subquery.
  • For GPU, it's ignoring both GpuSubquery and GpuSubqueryBroadcast as reusable.
    This results in reused subqueries counted in all the metrics, perhaps a source of qualification tool inaccuracy.

Steps/Code to reproduce bug
Profile any cpu query with a ReusedSubquery that is a SubqueryBroadcast, the reused subquery is shown in the metrics. Any gpu query with reused subqueries also show up.

Expected behavior
Reused subqueries should be excluded from the metrics.
@rongou rongou added ? - Needs Triage bug Something isn't working labels Jan 8, 2024
@rongou
Copy link
Author

rongou commented Jan 8, 2024 •
edited
Loading

#693 is an attempt at fixing this.

@amahussein amahussein added the core_tools Scope the core module (scala) label Jan 10, 2024
@amahussein
Copy link
Collaborator

I remember that I handled the "ReusedExchange" in the Qualification tool before 2e25ce9
My fix was to traverse the graph marking the duplicate nodes; then skip collecting their speedup-factors.

@amahussein amahussein assigned amahussein and unassigned rongou Jan 12, 2024
amahussein added a commit to amahussein/spark-rapids-tools that referenced this issue Jan 25, 2024
@amahussein
Remove ReusedSubquery from SparkPlanGraph construction
bee74bb 
Signed-off-by: Ahmed Hussein (amahussein) <[email protected]>

Fixes NVIDIA#718

- Reused subqueries should be excluded from the metrics.
- Added a common graph builder to be used for both CPU/GPU logs
- Added the unit test for GPU eventlogs
This was referenced Jan 25, 2024
Merged
Open
amahussein added a commit that referenced this issue Jan 26, 2024
@amahussein
Remove ReusedSubquery from SparkPlanGraph construction (#741)
5bed34c 
* Remove ReusedSubquery from SparkPlanGraph construction

Fixes #718

- Reused subqueries should be excluded from the metrics.
- Added a common graph builder to be used for both CPU/GPU logs
- Added the unit test for GPU eventlogs

---------

Signed-off-by: Ahmed Hussein (amahussein) <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@amahussein amahussein

Labels
bug Something isn't working core_tools Scope the core module (scala)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remove ReusedSubquery from SparkPlanGraph construction amahussein/spark-rapids-tools
2 participants
@rongou @amahussein