spark-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Marcelo Vanzin (Jira)" <>
Subject [jira] [Commented] (SPARK-25874) Simplify abstractions in the K8S backend
Date Thu, 29 Aug 2019 17:49:00 GMT


Marcelo Vanzin commented on SPARK-25874:

I'm going to close this one; the only remaining task is for adding (internal developer) documentation,
which is minor.

> Simplify abstractions in the K8S backend
> ----------------------------------------
>                 Key: SPARK-25874
>                 URL:
>             Project: Spark
>          Issue Type: Umbrella
>          Components: Kubernetes
>    Affects Versions: 2.4.0
>            Reporter: Marcelo Vanzin
>            Priority: Major
> I spent some time recently re-familiarizing myself with the k8s backend, and I think
there is room for improvement. In the past, SPARK-22839 was added which improved things a
lot, but it is still hard to follow the code, and it is still more complicated than it should
be to add a new feature.
> I've worked on the main things that were bothering me and came up with these changes:
> Now that patch (first commit of the branch) is a little large, which makes it hard to
review and to properly assess what it is doing. So I plan to break it down into a few steps
that I will file as sub-tasks and send for review independently.
> The commit message for that patch has a lot of the background of what I changed and why.
Since I plan to delete that branch after the work is done, I'll paste it here:
> {noformat}
> There are two main changes happening here.
> (1) Simplify the KubernetesConf abstraction.
> The current code around KubernetesConf has a few drawbacks:
> - it uses composition (with a type parameter) for role-specific configuration
> - it breaks encapsulation of the user configuration, held in SparkConf, by
>   requiring that all the k8s-specific info is extracted from SparkConf before
>   the KubernetesConf object is created.
> - the above is usually done by parsing the SparkConf info into k8s-backend
>   types, which are then transformed into k8s requests.
> This ends up requiring a whole lot of code that is just not necessary.
> The type parameters make parameter and class declarations full of needless
> noise; the breakage of encapsulation makes the code that processes SparkConf
> and the code that builds the k8s descriptors live in different places, and
> the intermediate representation isn't adding much value.
> By using inheritance instead of the current model, role-specific
> specialization of certain config properties works simply by implementing some
> abstract methods of the base class (instead of breaking encapsulation), and
> there's no need anymore for parameterized types.
> By moving config processing to the code that actually transforms the config
> into k8s descriptors, a lot of intermediate boilerplate can be removed.
> This leads to...
> (2) Make all feature logic part of the feature step itself.
> Currently there's code in a lot of places to decide whether a feature
> should be enabled. There's code when parsing the configuration, building
> the custom intermediate representation in a way that is later used by
> different code in a builder class, which then decides whether feature A
> or feature B should be used.
> Instead, it's much cleaner to let a feature decide things for itself.
> If the config to enable feature A exists, it proceses the config and
> sets up the necessary k8s descriptors. If it doesn't, the feature is
> a no-op.
> This simplifies the shared code that calls into the existing features
> a lot. And does not make the existing features any more complicated.
> As part of this I merged the different language binding feature steps
> into a single step. They are sort of related, in the sense that if
> one is applied the others shouldn't, and merging them makes the logic
> to implement that cleaner.
> The driver and executor builders are now also much simpler, since they
> have no logic about what steps to apply or not. The tests were removed
> because of that, and some new tests were added to the suites for
> specific features, to verify what the old builder suites were testing.
> On top of the above I made a few minor changes (in comparison):
> - KubernetesVolumeUtils was modified to just throw exceptions. The old
>   code tried to avoid throwing exceptions by collecting results in `Try`
>   objects. That was not achieving anything since all the callers would
>   just call `get` on those objects, and the first one with a failure
>   would just throw the exception. The new code achieves the same
>   behavior and is simpler.
> - A bunch of small things, mainly to bring the code in line with the
>   usual Spark code style. I also removed unnecessary mocking in tests,
>   unused imports, and unused configs and constants.
> - Added some basic tests for KerberosConfDriverFeatureStep.
> Note that there may still be leftover intermediate types that could
> potentially be removed. But at this point the change is already pretty
> large as it is.
> {noformat}

This message was sent by Atlassian Jira

To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message