-
Notifications
You must be signed in to change notification settings - Fork 28.3k
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
[SPARK-25875][k8s] Merge code to set up driver command into a single step. #22897
Closed
Commits on Oct 30, 2018
-
[SPARK-25875][k8s] Merge code to set up driver command into a single …
…step. Right now there are 3 different classes dealing with building the driver command to run inside the pod, one for each "binding" supported by Spark. This has two main shortcomings: - the code in the 3 classes is very similar; changing things in one place would probably mean making a similar change in the others. - it gives the false impression that the step implementation is the only place where binding-specific logic is needed. That is not true; there was code in KubernetesConf that was binding-specific, and there's also code in the executor-specific config step. So the 3 classes weren't really working as a language-specific abstraction. On top of that, the current code was propagating command line parameters in a different way depending on the binding. That doesn't seem necessary, and in fact using environment variables for command line parameters is in general a really bad idea, since you can't handle special characters (e.g. spaces) that way. This change merges the 3 different code paths for Java, Python and R into a single step, and also merges the 3 code paths to start the Spark driver in the k8s entry point script. This increases the amount of shared code, and also moves more feature logic into the step itself, so it doesn't live in KubernetesConf. Note that not all logic related to setting up the driver lives in that step. For example, the memory overhead calculation still lives separately, except it now happens in the driver config step instead of outside the step hierarchy altogether. Some of the noise in the diff is because of changes to KubernetesConf, which will be addressed in a separate change. Tested with new and updated unit tests + integration tests.
Marcelo Vanzin committedOct 30, 2018 Configuration menu - View commit details
-
Copy full SHA for 8148f49 - Browse repository at this point
Copy the full SHA 8148f49View commit details
Commits on Oct 31, 2018
-
Merge branch 'master' into SPARK-25875
Marcelo Vanzin committedOct 31, 2018 Configuration menu - View commit details
-
Copy full SHA for b118b91 - Browse repository at this point
Copy the full SHA b118b91View commit details -
Marcelo Vanzin committed
Oct 31, 2018 Configuration menu - View commit details
-
Copy full SHA for 91a4741 - Browse repository at this point
Copy the full SHA 91a4741View commit details
Commits on Nov 1, 2018
-
Marcelo Vanzin committed
Nov 1, 2018 Configuration menu - View commit details
-
Copy full SHA for 1a61f72 - Browse repository at this point
Copy the full SHA 1a61f72View commit details -
Make mainAppResource not optional.
Marcelo Vanzin committedNov 1, 2018 Configuration menu - View commit details
-
Copy full SHA for 91765ab - Browse repository at this point
Copy the full SHA 91765abView commit details -
Marcelo Vanzin committed
Nov 1, 2018 Configuration menu - View commit details
-
Copy full SHA for 5601d16 - Browse repository at this point
Copy the full SHA 5601d16View commit details
Commits on Nov 2, 2018
-
Temporary debug change to see what's failing.
Marcelo Vanzin committedNov 2, 2018 Configuration menu - View commit details
-
Copy full SHA for 4152c76 - Browse repository at this point
Copy the full SHA 4152c76View commit details -
Merge branch 'master' into SPARK-25875
Marcelo Vanzin committedNov 2, 2018 Configuration menu - View commit details
-
Copy full SHA for da4856e - Browse repository at this point
Copy the full SHA da4856eView commit details -
Add constants for resource types.
Marcelo Vanzin committedNov 2, 2018 Configuration menu - View commit details
-
Copy full SHA for 0d755d6 - Browse repository at this point
Copy the full SHA 0d755d6View commit details
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.