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.

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

Implement test for qualification tool sql metric aggregates #2591

Merged
merged 11 commits into from
Jun 5, 2021

Conversation

andygrove
Copy link
Contributor

@andygrove andygrove commented Jun 4, 2021

Adds tests to ensure that we are correctly aggregating SQL metrics in the qualification tool.

Signed-off-by: Andy Grove <andygrove@nvidia.com>
Signed-off-by: Andy Grove <andygrove@nvidia.com>
Signed-off-by: Andy Grove <andygrove@nvidia.com>
Signed-off-by: Andy Grove <andygrove@nvidia.com>
Signed-off-by: Andy Grove <andygrove@nvidia.com>
Signed-off-by: Andy Grove <andygrove@nvidia.com>
@andygrove andygrove self-assigned this Jun 4, 2021
@andygrove andygrove added the test Only impacts tests label Jun 4, 2021
@andygrove andygrove added this to the May 24 - Jun 4 milestone Jun 4, 2021
Signed-off-by: Andy Grove <andygrove@nvidia.com>
@andygrove andygrove changed the title WIP: Implement test for qualification tool sql metric aggregates Implement test for qualification tool sql metric aggregates Jun 4, 2021
@andygrove andygrove marked this pull request as ready for review June 4, 2021 19:55
Signed-off-by: Andy Grove <andygrove@nvidia.com>
@@ -97,4 +99,73 @@ class QualificationSuite extends FunSuite with Logging {
val logFiles = Array(s"$logDir/nds_q86_test")
runQualificationTest(logFiles, "nds_q86_test_expectation.csv")
}

test("sql metric agg") {
Copy link
Collaborator

Choose a reason for hiding this comment

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

we could do this separately and if you don't have time this week just let me know, but it would be nice to also validate the sql time aggregation across multiple SQL jobs. Add in one with dataset for bonus.

tgravescs
tgravescs previously approved these changes Jun 4, 2021
@tgravescs
Copy link
Collaborator

build


def generateEventLog(eventLogDir: File, appName: String)
(fun: SparkSession => DataFrame): String = {
val spark = SparkSession
Copy link
Collaborator

Choose a reason for hiding this comment

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

make this lazy

nartal1
nartal1 previously approved these changes Jun 4, 2021
@nartal1
Copy link
Collaborator

nartal1 commented Jun 4, 2021

@andygrove Test seems to be failing.

@andygrove andygrove dismissed stale reviews from nartal1 and tgravescs via a6cf909 June 4, 2021 21:42
Signed-off-by: Andy Grove <andygrove@nvidia.com>
@andygrove
Copy link
Contributor Author

build

tgravescs
tgravescs previously approved these changes Jun 4, 2021
nartal1
nartal1 previously approved these changes Jun 4, 2021
@andygrove andygrove dismissed stale reviews from nartal1 and tgravescs via 6221047 June 4, 2021 23:34
Signed-off-by: Andy Grove <andygrove@nvidia.com>
@andygrove
Copy link
Contributor Author

build

@tgravescs tgravescs merged commit 35f910d into NVIDIA:branch-21.06 Jun 5, 2021
nartal1 pushed a commit to nartal1/spark-rapids that referenced this pull request Jun 9, 2021
)

* Fix package name

Signed-off-by: Andy Grove <andygrove@nvidia.com>

* Rename to ToolTestUtils

Signed-off-by: Andy Grove <andygrove@nvidia.com>

* Implement basic structure for test

Signed-off-by: Andy Grove <andygrove@nvidia.com>

* Test checks that number of tasks is correct

Signed-off-by: Andy Grove <andygrove@nvidia.com>

* Add more tests

Signed-off-by: Andy Grove <andygrove@nvidia.com>

* Add cpuTime test, commented out

Signed-off-by: Andy Grove <andygrove@nvidia.com>

* Change executorCpuTime logic to match qualification tool

Signed-off-by: Andy Grove <andygrove@nvidia.com>

* use dedicated spark session in test

* create new SparkSession before each test

Signed-off-by: Andy Grove <andygrove@nvidia.com>
nartal1 pushed a commit to nartal1/spark-rapids that referenced this pull request Jun 9, 2021
)

* Fix package name

Signed-off-by: Andy Grove <andygrove@nvidia.com>

* Rename to ToolTestUtils

Signed-off-by: Andy Grove <andygrove@nvidia.com>

* Implement basic structure for test

Signed-off-by: Andy Grove <andygrove@nvidia.com>

* Test checks that number of tasks is correct

Signed-off-by: Andy Grove <andygrove@nvidia.com>

* Add more tests

Signed-off-by: Andy Grove <andygrove@nvidia.com>

* Add cpuTime test, commented out

Signed-off-by: Andy Grove <andygrove@nvidia.com>

* Change executorCpuTime logic to match qualification tool

Signed-off-by: Andy Grove <andygrove@nvidia.com>

* use dedicated spark session in test

* create new SparkSession before each test

Signed-off-by: Andy Grove <andygrove@nvidia.com>
@andygrove andygrove deleted the sql-metric-agg-test branch June 23, 2021 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Only impacts tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants