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

Move ProxyShuffleInternalManagerBase to api [databricks] #9506

Merged

Conversation

gerashegalov
Copy link
Collaborator

@gerashegalov gerashegalov commented Oct 21, 2023

This PR is taken from #9444 to separate the aspect of moving ProxyShuffleInternalManagerBase
to the sql-plugin-api module

  • moves ProxyShuffleInternalManagerBase to sql-plugin-api module along
    with the required dependencies including ShimLoader
  • Adds dbdeps profile to sql-plugin-api because we now have to depend
    on spark classes in the API module as well
  • Leave the parts of ShimLoader that cannot be moved now as ShimLoaderTemp

Signed-off-by: @gerashegalov

- moves ProxyShuffleInternalManagerBase to sql-plugin-api module along
  with the required dependencies including ShimLoader
- Leave the parts of ShimLoader that cannot moved now as ShimLoaderTemp

Signed-off-by: Gera Shegalov <gera@apache.org>
Signed-off-by: Gera Shegalov <gera@apache.org>
Signed-off-by: Gera Shegalov <gera@apache.org>
@gerashegalov gerashegalov changed the title Move ProxyShuffleInternalManagerBase to api Move ProxyShuffleInternalManagerBase to api [databricks] Oct 21, 2023
@gerashegalov
Copy link
Collaborator Author

build

1 similar comment
@gerashegalov
Copy link
Collaborator Author

build

jlowe
jlowe previously approved these changes Oct 23, 2023
Copy link
Member

@jlowe jlowe left a comment

Choose a reason for hiding this comment

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

Minor copyright nit, otherwise lgtm.

Signed-off-by: Gera Shegalov <gera@apache.org>
jlowe
jlowe previously approved these changes Oct 23, 2023
@jlowe
Copy link
Member

jlowe commented Oct 23, 2023

build

Signed-off-by: Gera Shegalov <gera@apache.org>
abellina
abellina previously approved these changes Oct 23, 2023
Copy link
Collaborator

@abellina abellina left a comment

Choose a reason for hiding this comment

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

LGTM

jlowe
jlowe previously approved these changes Oct 23, 2023
@gerashegalov gerashegalov dismissed stale reviews from jlowe and abellina via a7b5778 October 23, 2023 22:29
@gerashegalov
Copy link
Collaborator Author

had to resolve a conflict again

@gerashegalov
Copy link
Collaborator Author

build

@gerashegalov gerashegalov merged commit 93c03e8 into NVIDIA:branch-23.12 Oct 24, 2023
29 of 30 checks passed
@gerashegalov gerashegalov deleted the moveSomethingToSQLApi-2 branch October 24, 2023 13:56
gerashegalov added a commit that referenced this pull request Oct 24, 2023
…9444)

Closes #9456
Contributes to #6565 

- additionally remove unsued pom properties
- remove unnecessary unshimmed*txt entries missed in #9506 
- get shuffle manager definition to the point of an easy template, probably can be a shimplify feature

```scala
/*** spark-rapids-shim-json-lines
{"spark": "${buldver}"}
spark-rapids-shim-json-lines ***/
package com.nvidia.spark.rapids.spark${buldver}

import org.apache.spark.SparkConf
import org.apache.spark.shuffle.rapids.ProxyRapidsShuffleInternalManagerBase

/** A shuffle manager optimized for the RAPIDS Plugin for Apache Spark. */
sealed class RapidsShuffleManager(
    conf: SparkConf,
    isDriver: Boolean
) extends ProxyRapidsShuffleInternalManagerBase(conf, isDriver)
```
 
Signed-off-by: Gera Shegalov <gera@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants