[FLINK-13938][yarn] Enable configure shared libraries on YARN #10187

tisonkun · 2019-11-14T06:06:35Z

What is the purpose of the change

Enable configure shared libraries on YARN. See also https://issues.apache.org/jira/browse/FLINK-13938

Brief change log

Add command-line option -ysl --yarnsharedLibs and config option yarn.shared-libraries
respect options

Verifying this change

I am looking for adding a test case but it seems hard without a real HDFS env. Maybe e2e tests required. But one can still verify it manually.

Does this pull request potentially affect one of the following parts:

Dependencies (does it add or upgrade a dependency): (no)
The public API, i.e., is any changed class annotated with @Public(Evolving): (no)
The serializers: (no)
The runtime per-record code paths (performance sensitive): (no)
Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (yes, enable configure shared libs on YARN)
The S3 file system connector: (no)

Documentation

Does this pull request introduce a new feature? (yes)
If yes, how is the feature documented? (docs)

cc @wangyang0918

flinkbot · 2019-11-14T06:09:18Z

Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
to review your pull request. We will use this comment to track the progress of the review.

Automated Checks

Last check on commit e1c1980 (Thu Nov 14 06:09:18 UTC 2019)

Warnings:

No documentation files were touched! Remember to keep the Flink docs up to date!

_{Mention the bot in a comment to re-run the automated checks.}

Review Progress

❓ 1. The [description] looks good.
❓ 2. There is [consensus] that the contribution should go into to Flink.
❓ 3. Needs [attention] from.
❓ 4. The change fits into the overall [architecture].
❓ 5. Overall code [quality] is good.

Please see the Pull Request Review Guide for a full explanation of the review process.

Details

The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required

Bot commands

The @flinkbot bot supports the following commands:

@flinkbot approve description to approve one or more aspects (aspects: description, consensus, architecture and quality)
@flinkbot approve all to approve all aspects
@flinkbot approve-until architecture to approve everything until architecture
@flinkbot attention @username1 [@username2 ..] to require somebody's attention
@flinkbot disapprove architecture to remove an approval you gave earlier

flinkbot · 2019-11-14T06:34:17Z

CI report:

e1c1980 : CANCELED Build
5363eb5 : FAILURE Build
6ee5b27 : SUCCESS Build

Bot commands

The @flinkbot bot supports the following commands:

@flinkbot run travis re-run the last Travis build

tisonkun · 2019-11-14T09:40:51Z

cc @yanghua @MalcolmSanders

yanghua

Hi @tisonkun Thanks for implementing this feature. I think it would be helpful for the users who use Flink on Yarn. I left some tiny comments you can consider. Do not know whether this (partial) feature can be tested with a unit test or not. If we can not, did you tested it offline?

yanghua · 2019-11-15T03:02:53Z

docs/_includes/generated/yarn_config_configuration.html

+        <tr>
+            <td><h5>yarn.shared-libraries</h5></td>
+            <td style="word-wrap: break-word;">(none)</td>
+            <td>specify the path to use public visibility feature of YARN NodeManager localizing resources.</td>


Shall we capitalize the first letter? Like other config option description.

yanghua · 2019-11-15T03:16:42Z

flink-yarn/src/main/java/org/apache/flink/yarn/YarnClusterDescriptor.java

 	/** Lazily initialized list of files to ship. */
 	private final List<File> shipFiles = new LinkedList<>();

+	private final List<Path> sharedLibs = new LinkedList<>();


If we want to test this feature like ship files, we may expose getter/setter methods of this field.

yanghua · 2019-11-15T03:18:11Z

flink-yarn/src/main/java/org/apache/flink/yarn/cli/FlinkYarnSessionCli.java

 		applicationId = new Option(shortPrefix + "id", longPrefix + "applicationId", true, "Attach to running YARN session");
 		queue = new Option(shortPrefix + "qu", longPrefix + "queue", true, "Specify YARN queue.");
 		shipPath = new Option(shortPrefix + "t", longPrefix + "ship", true, "Ship files in the specified directory (t for transfer)");
+		sharedLibs = new Option(shortPrefix + "sl", longPrefix + "sharedLibs", true, "Upload a copy of flink libs beforehand and specify the path to use public visibility feature of YARN NodeManager localizing resources.");


Can we add a test case about this config option or the whole feature like ship files?

yanghua · 2019-11-15T03:18:57Z

flink-yarn/src/main/java/org/apache/flink/yarn/configuration/YarnConfigOptions.java

+				.stringType()
+				.asList()
+				.noDefaultValue()
+				.withDescription("specify the path to use public visibility feature of " +


Also, shall we capitalize the first letter? Like other config option description.

Make sense. Will update.

tisonkun · 2019-11-15T03:32:53Z

I'd like to add a unit test but fail to have an idea how to start a mini HDFS. Let me think of how to introduce it within YarnTestBase...

tisonkun · 2019-11-15T03:34:20Z

I've manually verify the changes locally.

My rough thought about testing is using a test job running with YarnTestBase and verify the resource, maybe we have to verify no files uploaded by Flink also.

tisonkun · 2019-11-15T03:35:45Z

flink-yarn/src/main/java/org/apache/flink/yarn/YarnClusterDescriptor.java

+				while (i.hasNext()) {
+					LocatedFileStatus fileStatus = i.next();
+					if (fileStatus.isFile()) {
+						String classpath = setupSharedLib(fileStatus.getPath(), fs, remotePaths, localResources, envShipFileList);


Here we flatten all files in the root relative path of program. Maybe it is better we keep the directory layout. Locally changed and verifying...

tisonkun · 2019-11-15T05:13:34Z

closed for now in favor of another issue.

After discussion with @wangyang0918 I notice FLINK-13938 is not what proposed here.

walterddr · 2019-11-22T18:55:26Z

sorry for the late follow up @tisonkun . I come to understand that this PR is for something different from FLINK-13938. did we create a new JIRA ticket? I think the approach this PR implements is highly valuable especially for a production cluster setup :-) --Rong

tisonkun requested review from GJL, kl0u and walterddr November 14, 2019 06:06

tisonkun force-pushed the FLINK-13938 branch from 9f8bb18 to e1c1980 Compare November 14, 2019 06:06

rmetzger added the review=description? label Nov 14, 2019

tisonkun force-pushed the FLINK-13938 branch from e1c1980 to 5363eb5 Compare November 14, 2019 06:44

rmetzger added the component=Deployment/YARN label Nov 14, 2019

[FLINK-13938][yarn] Enable configure shared libraries on YARN

2386c06

tisonkun force-pushed the FLINK-13938 branch from 5363eb5 to 6ee5b27 Compare November 14, 2019 09:07

[FLINK-13938][yarn] Generate document

6ee5b27

yanghua reviewed Nov 15, 2019

View reviewed changes

tisonkun commented Nov 15, 2019

View reviewed changes

tisonkun closed this Nov 15, 2019

tisonkun deleted the FLINK-13938 branch November 15, 2019 05:13

wangyang0918 mentioned this pull request Nov 15, 2019

[FLINK-13938][yarn] Use pre-uploaded flink binary to accelerate flink submission #10193

Closed

flinkbot added the component=Client/JobSubmission label Mar 17, 2022

[FLINK-13938][yarn] Enable configure shared libraries on YARN #10187

[FLINK-13938][yarn] Enable configure shared libraries on YARN #10187

Uh oh!

Conversation

tisonkun commented Nov 14, 2019

What is the purpose of the change

Brief change log

Verifying this change

Does this pull request potentially affect one of the following parts:

Documentation

Uh oh!

flinkbot commented Nov 14, 2019

Automated Checks

Review Progress

Uh oh!

flinkbot commented Nov 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CI report:

Uh oh!

tisonkun commented Nov 14, 2019

Uh oh!

yanghua left a comment

Choose a reason for hiding this comment

Uh oh!

yanghua Nov 15, 2019

Choose a reason for hiding this comment

Uh oh!

yanghua Nov 15, 2019

Choose a reason for hiding this comment

Uh oh!

yanghua Nov 15, 2019

Choose a reason for hiding this comment

Uh oh!

yanghua Nov 15, 2019

Choose a reason for hiding this comment

Uh oh!

tisonkun Nov 15, 2019

Choose a reason for hiding this comment

Uh oh!

tisonkun commented Nov 15, 2019

Uh oh!

tisonkun commented Nov 15, 2019

Uh oh!

tisonkun Nov 15, 2019

Choose a reason for hiding this comment

Uh oh!

tisonkun commented Nov 15, 2019

Uh oh!

walterddr commented Nov 22, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

flinkbot commented Nov 14, 2019 •

edited

Loading