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.

Sign up for GitHub

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

Jump to bottom

[DSIP-38] Add plugin directory in binary package #16613

Merged
merged 47 commits into from
Sep 23, 2024
Merged

[DSIP-38] Add plugin directory in binary package #16613

merged 47 commits into from
Sep 23, 2024

Conversation

SbloodyS
Copy link
Member

@SbloodyS SbloodyS commented Sep 13, 2024 •
edited
Loading

Purpose of the pull request

close #16096
close #16097

Brief change log

  • Shade the plugins and put them into target plugins.
  • Add plugins directory.
  • Move each server's libs to shared libs directory and create a symbolic link from shared libs to eash server's libs.
  • Add dependency dolphinscheduler-common provided from dolphinscheduler-spi and dolphinscheduler-plugin module.
  • Refactor dist package and docker build/push logic base on the new binary package.
  • Add API-Test-Result to mergeable check.

Then the new binary package will be like

- dolphinscheduler-bin
  - api-server
  - master-server
  - alert-server
  - worker-server
  - standalone-server
  - tools
  - libs 370M
  - plugins
    - task-plugins 494M
    - alert-plugins 32M
    - storage-plugins 72M
    - datasource-plugins 284M

binary package size

before: 1.2G
after(without plugins directory): 450M

Todo

  • Remove duplicated jars in shade from libs directory.
  • Create shade plugins to push apache nexus's repository github action and remove plugins during release package and add it to the upgrade docs.

Verify this pull request

This pull request is code cleanup without any test coverage.

(or)

This pull request is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(or)

Pull Request Notice

Pull Request Notice

If your pull request contain incompatible change, you should also add it to docs/docs/en/guide/upgrede/incompatible.md

@github-actions github-actions bot added UI ui and front end related backend test CI&CD e2e e2e test labels Sep 13, 2024
@github-actions github-actions bot removed the UI ui and front end related label Sep 13, 2024
github-advanced-security[bot]
github-advanced-security bot found potential problems Sep 13, 2024
View reviewed changes
...hinscheduler-spi/src/main/java/org/apache/dolphinscheduler/spi/config/IPropertyDelegate.java Fixed Show fixed Hide fixed
...hinscheduler-spi/src/main/java/org/apache/dolphinscheduler/spi/config/IPropertyDelegate.java Fixed Show fixed Hide fixed
...hinscheduler-spi/src/main/java/org/apache/dolphinscheduler/spi/config/IPropertyDelegate.java Fixed Show fixed Hide fixed
...hinscheduler-spi/src/main/java/org/apache/dolphinscheduler/spi/config/IPropertyDelegate.java Fixed Show fixed Hide fixed
...hinscheduler-spi/src/main/java/org/apache/dolphinscheduler/spi/config/IPropertyDelegate.java Fixed Show fixed Hide fixed
dolphinscheduler-spi/src/main/java/org/apache/dolphinscheduler/spi/utils/FileUtils.java Fixed Show fixed Hide fixed
dolphinscheduler-spi/src/main/java/org/apache/dolphinscheduler/spi/utils/FileUtils.java Fixed Show fixed Hide fixed
dolphinscheduler-spi/src/main/java/org/apache/dolphinscheduler/spi/utils/JSONUtils.java Fixed Show fixed Hide fixed
dolphinscheduler-spi/src/main/java/org/apache/dolphinscheduler/spi/utils/JSONUtils.java Fixed Show fixed Hide fixed
dolphinscheduler-spi/src/main/java/org/apache/dolphinscheduler/spi/utils/JSONUtils.java Fixed Show fixed Hide fixed
@SbloodyS SbloodyS added this to the 3.3.0 milestone Sep 13, 2024
@SbloodyS SbloodyS added the DSIP label Sep 13, 2024
@github-actions github-actions bot added UI ui and front end related and removed UI ui and front end related labels Sep 13, 2024
@SbloodyS SbloodyS marked this pull request as ready for review September 20, 2024 00:39
ruanwenjun
ruanwenjun reviewed Sep 20, 2024
View reviewed changes
...src/main/java/org/apache/dolphinscheduler/plugin/alert/prometheus/PrometheusAlertSender.java Outdated Show resolved Hide resolved
pom.xml
@@ -477,64 +507,6 @@
<groupId>org.codehaus.mojo</groupId>
<artifactId>exec-maven-plugin</artifactId>
<version>${exec-maven-plugin.version}</version>
<executions>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why remove this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move docker build logic to the dolphinscheduler-dist module since docker build logic refactored.

dolphinscheduler-dist/src/main/assembly/assembly-plugins.sh
Comment on lines +39 to +44
for plugin_path in ${PLUGINS_PATH[@]}
do
cd $BIN_DIR/plugins/$plugin_path
find ./* -name "*.jar" | xargs -I {} mv {} ./
ls -d */ | xargs -I {} rm -rf {}
done
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use maven assembly plugin to manage the plugin directory

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assemby packaged directory:

  • plugins
    • alert-plugins
      • dolphinscheduler-alert-dingtalk/taget/dolphinscheduler-alert-dingtalk-shade.jar

Directory processed with shell:

  • plugins
    • alert-plugins
      • dolphinscheduler-alert-dingtalk-shade.jar

Assemby fuzzy matching directory can't remove the file path, so it needs secondary processing with shell.

dolphinscheduler-alert/dolphinscheduler-alert-server/src/main/bin/start.sh Show resolved Hide resolved
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Sep 22, 2024

Please retry analysis of this Pull-Request directly on SonarCloud

ruanwenjun
ruanwenjun approved these changes Sep 23, 2024
View reviewed changes
Copy link
Member

@ruanwenjun ruanwenjun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@EricGao888
Copy link
Member

Seems there were some network issues when I clicked Approve

@SbloodyS SbloodyS merged commit d67ea80 into apache:dev Sep 23, 2024
67 of 69 checks passed
@SbloodyS SbloodyS deleted the improvement_16096 branch September 23, 2024 01:59
@SbloodyS SbloodyS mentioned this pull request Sep 23, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@EricGao888 EricGao888 EricGao888 approved these changes

@ruanwenjun ruanwenjun ruanwenjun approved these changes

@caishunfeng caishunfeng Awaiting requested review from caishunfeng caishunfeng is a code owner

@Gallardot Gallardot Awaiting requested review from Gallardot Gallardot is a code owner

Assignees

@SbloodyS SbloodyS

Labels
backend CI&CD document DSIP e2e e2e test test
Projects
None yet
Milestone
3.3.0
Development

Successfully merging this pull request may close these issues.

[Improvement] Move plugin to plugin directory [Improvement] Shade plugin jar
3 participants
@SbloodyS @EricGao888 @ruanwenjun