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] Qual tool should convert time units at stage/job/sql level #1511

Merged
merged 3 commits into from
Jan 27, 2025
Merged

[BUG] Qual tool should convert time units at stage/job/sql level #1511

merged 3 commits into from
Jan 27, 2025

Conversation

cindyyuanjiang
Copy link
Collaborator

Fixes #1481

Changes

  • Change qual tool to convert time units (ms to ns) when aggregating at stage/job/sql level instead of at task level
  • Update unit tests results due to the above change

Tests

  • Existing unit tests

@cindyyuanjiang cindyyuanjiang self-assigned this Jan 23, 2025
@cindyyuanjiang cindyyuanjiang added bug Something isn't working core_tools Scope the core module (scala) labels Jan 23, 2025
parthosa
parthosa previously approved these changes Jan 24, 2025
View reviewed changes
Copy link
Collaborator

@parthosa parthosa left a comment

Choose a reason for hiding this comment

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

Thanks @cindyyuanjiang for fixing this.

Minor comment - Can we add comments for these two fields in class SQLStageSummary? Or rename these variables to store the unit name (e.g. execCPUTimeNs)

@cindyyuanjiang
Copy link
Collaborator Author

Thanks @cindyyuanjiang for fixing this.

Minor comment - Can we add comments for these two fields in class SQLStageSummary? Or rename these variables to store the unit name (e.g. execCPUTimeNs)

@parthosa thanks! I added comments to the two fields in class SQLStageSummary.

amahussein
amahussein approved these changes Jan 27, 2025
View reviewed changes
Copy link
Collaborator

@amahussein amahussein left a comment

Choose a reason for hiding this comment

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

Thanks @cindyyuanjiang !
LGTME

parthosa
parthosa approved these changes Jan 27, 2025
View reviewed changes
Copy link
Collaborator

@parthosa parthosa left a comment

Choose a reason for hiding this comment

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

Thanks @cindyyuanjiang. LGTME.

@cindyyuanjiang cindyyuanjiang merged commit d15a999 into NVIDIA:dev Jan 27, 2025
13 checks passed
@cindyyuanjiang cindyyuanjiang deleted the spark-rapids-tools-1481 branch January 27, 2025 21:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@parthosa parthosa parthosa approved these changes

@amahussein amahussein amahussein approved these changes

Assignees

@cindyyuanjiang cindyyuanjiang

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

Successfully merging this pull request may close these issues.

[BUG] Qualification tool converts time units at task level
3 participants
@cindyyuanjiang @parthosa @amahussein