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

[ZEPPELIN-5897] Remove launcher properties and only use the current context properties #4602

Merged
merged 1 commit into from May 26, 2023

Conversation

Reamer
Copy link
Contributor

@Reamer Reamer commented May 10, 2023

What is this PR for?

This PR removes the double holding of properties.
The main problem: The InterpreterLauncher class is a singleton, because of your PluginManager Cache Implementation.

https://github.com/apache/zeppelin/blob/22ae6cdcd1f526b66808c2b994b53f808f390cfe/zeppelin-zengine/src/main/java/org/apache/zeppelin/plugin/PluginManager.java#L104C23-L126

As a result, every InterpreterProcess started by an InterpreterLauncher has the same properties, because we have the following lines of code in almost all launchers.


The problem that arises here is when multiple jobs are started in parallel via CronJobs using one launcher. In the end, the "last" process overwrites the properties of the previous ones. This makes problems, if the individual InterpreterProcesses adjust the properties, as for example in the K8sInterpreterProcess.

private void addSparkK8sProperties() {
properties.setProperty("spark.master", "k8s://https://kubernetes.default.svc");
properties.setProperty("spark.submit.deployMode", "client");
properties.setProperty("spark.kubernetes.namespace", getInterpreterNamespace());
properties.setProperty("spark.kubernetes.driver.pod.name", getPodName());
properties.setProperty("spark.kubernetes.container.image", properties.containsKey(SPARK_CONTAINER_IMAGE) ?
properties.getProperty(SPARK_CONTAINER_IMAGE) : sparkImage);
properties.setProperty("spark.driver.bindAddress", "0.0.0.0");
properties.setProperty("spark.driver.host", getInterpreterPodDnsName());
properties.setProperty("spark.driver.port", String.valueOf(getSparkDriverPort()));
properties.setProperty("spark.blockManager.port", String.valueOf(getSparkBlockManagerPort()));
}

For this reason, this PR removes the properties in the launcher object and all launchers are forced to access the properties in the current context.

What type of PR is it?

Bug Fix
Improvement
Refactoring

What is the Jira issue?

How should this be tested?

  • CI

Questions:

  • Does the license files need to update? No
  • Is there breaking changes for older versions? No
  • Does this needs documentation? No

@jongyoul
Copy link
Member

BTW,
image

Keep failing again?

@Reamer
Copy link
Contributor Author

Reamer commented May 16, 2023

Keep failing again?

Yes the selenium test is very unstable.

@jongyoul
Copy link
Member

Keep failing again?

Yes the selenium test is very unstable.

Let me check it sooner than later 😢

@Reamer
Copy link
Contributor Author

Reamer commented May 17, 2023

I will observe this feature a bit more in my productive environment. So far, everything looks very good.

@Reamer Reamer merged commit f2eb337 into apache:master May 26, 2023
22 of 23 checks passed
@Reamer Reamer deleted the properties_share branch May 26, 2023 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants