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

[FEA] Use conventional jar layout in dist jar if there is only one input shim #3682

Closed
gerashegalov opened this issue Sep 27, 2021 · 5 comments · Fixed by #5528
Closed

[FEA] Use conventional jar layout in dist jar if there is only one input shim #3682

gerashegalov opened this issue Sep 27, 2021 · 5 comments · Fixed by #5528
Assignees
Labels
build Related to CI / CD or cleanly building feature request New feature or request

Comments

@gerashegalov
Copy link
Collaborator

gerashegalov commented Sep 27, 2021

Is your feature request related to a problem? Please describe.
In a targeted cloud / vendor environment there is typically no need to have multi-spark-version jar artifact allowing to switch between Spark versions back and forth.

We expect that the dist jar for such a targeted environment is likely to be built using the individual profile. In this case it's unnecessary to use non-standard Parallel World jar layout

dest="${project.build.directory}/parallel-world/${spark.version.classifier}"

Describe the solution you'd like
We need an option for dist jar based on a single individual buildver classified output to use the standard jar layout without /spark3* directories at the root.

Describe alternatives you've considered
keep using PW layout even for single shim builds which elevates the risk of classloading issues

Additional context
#3381

@gerashegalov gerashegalov added feature request New feature or request ? - Needs Triage Need team to review and classify build Related to CI / CD or cleanly building labels Sep 27, 2021
@revans2
Copy link
Collaborator

revans2 commented Sep 28, 2021

I personally disagree with this for exactly the same reasons pointed out here.

keep using PW layout even for single shim builds which elevates the risk of classloading issues

If what we ship and expect most people to use is the uber/parallel world jar, then I want everyone to be testing that situation. I personally build and test regularly for a single version because I would rather wait 3 mins to build a single version than 10 mins for buildall to finish. But if I am testing things in a very different way I don't want that. If someone has a targeted cloud environment where they are building, testing, and deploying for a single version of Spark, then they should make any changes needed to the build scripts. The common code should stay as is.

@gerashegalov
Copy link
Collaborator Author

I think the intent of my issue and the wording diverged in that it sounds generally applicable to individual profile. Instead it should read Parallel World layout should be opt-out. Note that this should not incur source code modification, just a packaging option.

I too advocate that we should definitely do most of our testing with the muti-shim jar, preferably minimumFeatureVersionMix profile.

What I argue for vendor environments has little to do with the multi-shim build overhead, What we have there is an environment we have no control over, whereas we are pushing a jar that provides a solution that this type of environment most often doesn't have. E.g., if we deploy to Databricks for a specific runtime version known apriori there is absolutely no need for a complex mult-shim paralllel-world layout.

Therefore, I think having an option to turn off non-standard jar layout creation at build time when needed is reasonable. Again, I agree that it should not be the default behavior.

@revans2
Copy link
Collaborator

revans2 commented Sep 28, 2021

Can we wait on this until we have an actual customer that asks us for this? I don't really want to have to maintain/test a feature that someone somewhere might use.

@gerashegalov
Copy link
Collaborator Author

yes we can wait, i didn't put any release label on it. But if you will I am the "customer" asking for it to reduce our issues overhead with the vendors when possible.

@Salonijain27 Salonijain27 removed the ? - Needs Triage Need team to review and classify label Sep 28, 2021
gerashegalov added a commit that referenced this issue May 20, 2022
This PR closes #4649, #3682.

- add a doc for JDK9+ property with potential cross-compilation to lower Java major versions
- `allowConventionalDistJar` for conventional jar creation when only one shim is included
- automatically detect conventional jar and suppress warnings about classloader issues 
- remove manual configuration of javac args for scala-maven-plugin.  This plugin processes `maven .compiler.*` and adds the right set of javac opts automatically. 3.4.x we use for db doesn't handle maven.compiler.release, however, we don't need it at the moment and it's currently not used.
- disambiguate calls to buf.position() generating errors on JDK11

Note this PR only builds skipping running tests `-DskipTests` because we still need to address #3851 (comment) 
  
Signed-off-by: Gera Shegalov <gera@apache.org>
@gerashegalov
Copy link
Collaborator Author

This option was implemented in #5528

@gerashegalov gerashegalov self-assigned this May 20, 2022
@gerashegalov gerashegalov added this to the May 2 - May 20 milestone May 20, 2022
@gerashegalov gerashegalov linked a pull request May 20, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Related to CI / CD or cleanly building feature request New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants