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

Update GpuDataSourceScanExec and GpuBroadcastExchangeExec to fix audit issues #1760

Merged
merged 3 commits into from
Feb 19, 2021

Conversation

nartal1
Copy link
Collaborator

@nartal1 nartal1 commented Feb 18, 2021

This fixes #1460 and fixes #1731.

Update to GpuBroadcastExchangeExec.scala file fixes #1731.

Rest of the files are updated to fix #1460

Signed-off-by: Niranjan Artal nartal@nvidia.com

…t issues

Signed-off-by: Niranjan Artal <nartal@nvidia.com>
@nartal1 nartal1 added feature request New feature or request audit_3.1.0 labels Feb 18, 2021
@nartal1 nartal1 added this to the Feb 16 - Feb 26 milestone Feb 18, 2021
@nartal1 nartal1 self-assigned this Feb 18, 2021
@@ -265,7 +265,10 @@ abstract class GpuBroadcastExchangeExecBase(
@transient
private val timeout: Long = SQLConf.get.broadcastTimeout

val _runId: UUID = UUID.randomUUID()
// Cancelling a SQL statement from Spark ThriftServer needs to cancel
Copy link
Collaborator

@gerashegalov gerashegalov Feb 19, 2021

Choose a reason for hiding this comment

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

how does it relate to #1760?

Oh it's for #1731

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

yeah generally speaking would be better to separate issues in separate prs.

Copy link
Collaborator

Choose a reason for hiding this comment

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

note its fine for now but something to keep in mind for future

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks @tgravescs and @gerashegalov. Will post separate PR's for separate issues in the future.

gerashegalov
gerashegalov previously approved these changes Feb 19, 2021
Copy link
Collaborator

@gerashegalov gerashegalov left a comment

Choose a reason for hiding this comment

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

LGTM, 🚢

@nartal1
Copy link
Collaborator Author

nartal1 commented Feb 19, 2021

build

@@ -138,6 +139,8 @@ trait SparkShims {
readFunction: (PartitionedFile) => Iterator[InternalRow],
filePartitions: Seq[FilePartition]): RDD[InternalRow]

def getFileSourceMaxMetadataValueLength(sqlContext: SQLConf): Int
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would rename sqlContext to be sqlConf here because you aren't passing the entire sqlContext in

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@nartal1
Copy link
Collaborator Author

nartal1 commented Feb 19, 2021

build

@nartal1 nartal1 merged commit b4d996b into branch-0.4 Feb 19, 2021
@nartal1 nartal1 deleted the gpuFileScan branch February 23, 2021 19:48
tgravescs added a commit to tgravescs/spark-rapids that referenced this pull request Mar 22, 2021
…fix audit issues (NVIDIA#1760)"

This reverts commit b4d996b.

Signed-off-by: Thomas Graves <tgraves@nvidia.com>
nartal1 added a commit to nartal1/spark-rapids that referenced this pull request Jun 9, 2021
…t issues (NVIDIA#1760)

Signed-off-by: Niranjan Artal <nartal@nvidia.com>
nartal1 added a commit to nartal1/spark-rapids that referenced this pull request Jun 9, 2021
…t issues (NVIDIA#1760)

Signed-off-by: Niranjan Artal <nartal@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
audit_3.1.0 feature request New feature or request
Projects
None yet
3 participants