Implement way for python integration tests to validate Exec is in GPU plan #1668

tgravescs · 2021-02-04T15:56:39Z

I've been wanting this for a while. It allows us to verify that certain GPU execs are actually in the plan for the tests we run. Spark has so many optimizations and such that sometimes you think your testing one thing but it gets changed to something else. So this allows us to make sure the gpu execs are in the plan.
It is intended for testing purposes only and is pretty basic. We could easily make it smarter later if needed. Like if you need to verify it was in plan multiple times, etc.

You simple tag the test with:
@validate_execs_in_gpu_plan('HostColumnarToGpu')
It allows multiple like:
@validate_execs_in_gpu_plan('HostColumnarToGpu', "fooExec")
If the execs not found it throws an exception and fails the test. The failure looks like:

e = IllegalArgumentException('Plan GpuColumnarToRow false\n+- GpuSort [col1#2 ASC NULLS FIRST], true, RequireSingleBatch, ...:79)\n\tat py4j.GatewayConnection.run(GatewayConnection.java:238)\n\tat java.lang.Thread.run(Thread.java:748)\n', None)

>   ???
E   pyspark.sql.utils.IllegalArgumentException: Plan GpuColumnarToRow false
E   +- GpuSort [col1#2 ASC NULLS FIRST], true, RequireSingleBatch, 0
E      +- GpuCoalesceBatches RequireSingleBatch
E         +- GpuShuffleCoalesce 2147483647
E            +- GpuColumnarExchange gpurangepartitioning(col1#2 ASC NULLS FIRST, 200, StructField(col1,IntegerType,true)), true, [id=#32]
E               +- HostColumnarToGpu TargetSize(2147483647)
E                  +- BatchScan[col1#2] class com.nvidia.spark.rapids.tests.datasourcev2.parquet.ArrowColumnarDataSourceV2$MyScanBuilder
E    does not contain the following execs: Set(fooExec)

Signed-off-by: Thomas Graves <tgraves@nvidia.com>

tests

Signed-off-by: Thomas Graves <tgraves@apache.org>

Signed-off-by: Thomas Graves <tgraves@nvidia.com>

…tained

Signed-off-by: Thomas Graves <tgraves@nvidia.com>

tgravescs · 2021-02-04T15:56:59Z

build

sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuTransitionOverrides.scala

integration_tests/src/main/python/spark_session.py

tests/src/test/scala/com/nvidia/spark/rapids/BroadcastHashJoinSuite.scala

tests/src/test/scala/com/nvidia/spark/rapids/BroadcastNestedLoopJoinSuite.scala

sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuTransitionOverrides.scala

tests/src/test/scala/com/nvidia/spark/rapids/TestUtils.scala

tgravescs · 2021-02-04T16:47:11Z

build

tgravescs · 2021-02-04T18:23:26Z

looking at failures

tgravescs · 2021-02-04T18:44:47Z

build

gerashegalov · 2021-02-04T18:19:47Z

sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuTransitionOverrides.scala

+      if (execsNotFound.nonEmpty) {
+        throw new IllegalArgumentException(
+          s"Plan ${plan.toString()} does not contain the following execs: $execsNotFound")
+      }


nit: we can use the syntax sugar require to produce this IllegalArgumentException

Suggested change

if (execsNotFound.nonEmpty) {

throw new IllegalArgumentException(

s"Plan ${plan.toString()} does not contain the following execs: $execsNotFound")

}

require(execsNotFound.nonEmpty, s"Plan ${plan.toString()} does not contain the following execs: $execsNotFound")

sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuTransitionOverrides.scala

tgravescs · 2021-02-04T19:12:18Z

build failing due to changes in 3.0.1 for ShuffleLike, working on changes

between spark versions Signed-off-by: Thomas Graves <tgraves@nvidia.com>

Signed-off-by: Thomas Graves <tgraves@nvidia.com>

tgravescs · 2021-02-04T20:06:17Z

build

gerashegalov

LGTM, 🚀

… plan (NVIDIA#1668) * Add Data source v2 test classes Signed-off-by: Thomas Graves <tgraves@nvidia.com> * update v2 source testing * fix batch num rows and logging * update the numberin batch * Fix issue with reading booleans from ArrowColumnVectors and add more tests * move test file so pytest regex pick it up * add comments * Validate that the plan actually contains the exec Signed-off-by: Thomas Graves <tgraves@apache.org> * fix getting class name Signed-off-by: Thomas Graves <tgraves@nvidia.com> * change to use findOperators * fix calls to findOperators * turn on for all tests * add a mark for validating execs in gpu plan * fix name * cleanup * fix line length * update comment Signed-off-by: Thomas Graves <tgraves@nvidia.com> * Update copyrights to have 2021 * Move findOperators into shim layer due to changes in ShuffleExchangeExec between spark versions Signed-off-by: Thomas Graves <tgraves@nvidia.com> * add comments to Shim and change to use require Signed-off-by: Thomas Graves <tgraves@nvidia.com>

tgravescs and others added 19 commits February 3, 2021 09:01

Add Data source v2 test classes

11454b2

Signed-off-by: Thomas Graves <tgraves@nvidia.com>

update v2 source testing

5f54cee

fix batch num rows and logging

8141f3e

update the numberin batch

ab4aebd

Fix issue with reading booleans from ArrowColumnVectors and add more

3f96cf7

tests

move test file so pytest regex pick it up

742f4a5

add comments

9316c73

Merge remote-tracking branch 'origin/branch-0.4' into arrowTesting

f5898ae

Validate that the plan actually contains the exec

828d00c

Signed-off-by: Thomas Graves <tgraves@apache.org>

fix getting class name

858818a

Signed-off-by: Thomas Graves <tgraves@nvidia.com>

change to use findOperators

eb84eba

fix calls to findOperators

dcdeaca

turn on for all tests

daa055a

add a mark for validating execs in gpu plan

6708306

fix name

e3f0fed

Merge remote-tracking branch 'origin/branch-0.4' into validateExecCon…

c368284

…tained

cleanup

edadb85

fix line length

0eba227

update comment

cf705b5

Signed-off-by: Thomas Graves <tgraves@nvidia.com>

tgravescs added the test Only impacts tests label Feb 4, 2021

tgravescs added this to the Feb 1 - Feb 12 milestone Feb 4, 2021

tgravescs self-assigned this Feb 4, 2021

tgravescs commented Feb 4, 2021

View reviewed changes

sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuTransitionOverrides.scala Outdated Show resolved Hide resolved

jlowe requested changes Feb 4, 2021

View reviewed changes

Update copyrights to have 2021

dc7dfca

jlowe previously approved these changes Feb 4, 2021

View reviewed changes

gerashegalov reviewed Feb 4, 2021

View reviewed changes

tgravescs added 2 commits February 4, 2021 13:15

Move findOperators into shim layer due to changes in ShuffleExchangeExec

9f8d02b

between spark versions Signed-off-by: Thomas Graves <tgraves@nvidia.com>

add comments to Shim and change to use require

9528103

Signed-off-by: Thomas Graves <tgraves@nvidia.com>

tgravescs dismissed jlowe’s stale review via 9528103 February 4, 2021 20:05

jlowe approved these changes Feb 4, 2021

View reviewed changes

gerashegalov approved these changes Feb 4, 2021

View reviewed changes

tgravescs merged commit 8985d85 into NVIDIA:branch-0.4 Feb 5, 2021

tgravescs deleted the validateExecContained branch February 5, 2021 00:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement way for python integration tests to validate Exec is in GPU plan #1668

Implement way for python integration tests to validate Exec is in GPU plan #1668

tgravescs commented Feb 4, 2021

tgravescs commented Feb 4, 2021

tgravescs commented Feb 4, 2021

tgravescs commented Feb 4, 2021

tgravescs commented Feb 4, 2021

gerashegalov Feb 4, 2021

tgravescs Feb 4, 2021

tgravescs commented Feb 4, 2021

tgravescs commented Feb 4, 2021

gerashegalov left a comment

Implement way for python integration tests to validate Exec is in GPU plan #1668

Implement way for python integration tests to validate Exec is in GPU plan #1668

Conversation

tgravescs commented Feb 4, 2021

tgravescs commented Feb 4, 2021

tgravescs commented Feb 4, 2021

tgravescs commented Feb 4, 2021

tgravescs commented Feb 4, 2021

gerashegalov Feb 4, 2021

Choose a reason for hiding this comment

tgravescs Feb 4, 2021

Choose a reason for hiding this comment

tgravescs commented Feb 4, 2021

tgravescs commented Feb 4, 2021

gerashegalov left a comment

Choose a reason for hiding this comment