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

[ISSUE #367] Optimize plugin load #519

Merged
merged 4 commits into from
Oct 3, 2021

Conversation

ruanwenjun
Copy link
Member

@ruanwenjun ruanwenjun commented Sep 10, 2021

  1. Optimize plugin load, support load plugin by plugin instance name, these means we needn't load all plugin
  2. Optimize install plugin, suppory install plugin by ./gradlew jar dist
  3. Make UrlClassLoader siglenton

@codecov-commenter
Copy link

codecov-commenter commented Sep 10, 2021

Codecov Report

Merging #519 (25903df) into develop (4b2fef9) will increase coverage by 0.06%.
The diff coverage is 45.94%.

❗ Current head 25903df differs from pull request most recent head b91c48a. Consider uploading reports for the commit b91c48a to get more accurate results
Impacted file tree graph

@@              Coverage Diff              @@
##             develop     #519      +/-   ##
=============================================
+ Coverage      10.94%   11.01%   +0.06%     
- Complexity       376      378       +2     
=============================================
  Files            258      260       +2     
  Lines          12247    12264      +17     
  Branches        1039     1038       -1     
=============================================
+ Hits            1341     1351      +10     
- Misses         10791    10798       +7     
  Partials         115      115              
Impacted Files Coverage Δ
...nector/rocketmq/consumer/RocketMQConsumerImpl.java 0.00% <0.00%> (ø)
...nector/rocketmq/producer/RocketMQProducerImpl.java 0.00% <0.00%> (ø)
.../eventmesh/spi/loader/EventMeshUrlClassLoader.java 0.00% <0.00%> (ø)
.../eventmesh/spi/loader/JarExtensionClassLoader.java 10.52% <18.18%> (+2.97%) ⬆️
...g/apache/eventmesh/spi/EventMeshExtensionType.java 88.88% <88.88%> (ø)
...pache/eventmesh/spi/EventMeshExtensionFactory.java 55.00% <100.00%> (ø)
...ntmesh/spi/loader/MetaInfExtensionClassLoader.java 77.14% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4b2fef9...b91c48a. Read the comment docs.

@ruanwenjun ruanwenjun force-pushed the dev_wenjun_optimizeSPI branch 4 times, most recently from 870258a to f8402b2 Compare September 22, 2021 15:04
task copyConnectorPlugin(dependsOn: ['jar']) {
doFirst {
new File(projectDir, '../eventmesh-connector-plugin/dist/apps').mkdir()
new File(projectDir, '../dist/plugin/connector').mkdirs()
Copy link
Contributor

Choose a reason for hiding this comment

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

may not need dist directory, just $HOME/plugin/connector

Copy link
Member Author

Choose a reason for hiding this comment

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

Now this task has been removed, we can use the dist task to insatll all plugin, the plugin directory will look like
image

@@ -24,14 +24,14 @@ dependencies {
api 'io.prometheus:simpleclient_httpserver'

implementation project(":eventmesh-connector-plugin:eventmesh-connector-api")
implementation project(":eventmesh-connector-plugin:eventmesh-connector-standalone")
implementation project(":eventmesh-connector-plugin:eventmesh-connector-rocketmq")
Copy link
Contributor

Choose a reason for hiding this comment

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

Set default connector as standalone is ok in my opinion.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agree with you, done.

task copyAclPlugin(dependsOn: ['jar']) {
doFirst {
new File(projectDir, '../eventmesh-security-plugin/dist/apps').mkdir()
new File(projectDir, '../dist/plugin/security').mkdirs()
Copy link
Contributor

Choose a reason for hiding this comment

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

also, do not need dist directory.

private static final String EVENTMESH_EXTENSION_PLUGIN_DIR = System.getProperty("eventMeshPluginDir",
"./dist/plugin");
Paths.get(".", "dist", "plugin").toString());
Copy link
Contributor

Choose a reason for hiding this comment

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

also, do not need dist 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.

Yes, has removed the dist.

2. Optimize install plugin, suppory install plugin by ./gradlew jar dist
3. Make UrlClassLoader siglenton
@qqeasonchen qqeasonchen merged commit 6b7280a into apache:develop Oct 3, 2021
@qqeasonchen
Copy link
Contributor

@ruanwenjun please help to fix it, when execute build.

FAILURE: Build failed with an exception.

  • Where:
    Build file '/Users/easonchen/git/incubator-eventmesh/build.gradle' line: 215

  • What went wrong:
    Could not compile build file '/Users/easonchen/git/incubator-eventmesh/build.gradle'.

startup failed:
build file '/Users/easonchen/git/incubator-eventmesh/build.gradle': 215: expecting ')', found ',' @ line 215, column 46.
ginTypeMap.forEach((pluginType, pluginIn
^

1 error

xwm1992 pushed a commit to xwm1992/EventMesh that referenced this pull request Dec 27, 2021
* 1. Optimize plugin load, support load plugin by plugin instance name
2. Optimize install plugin, suppory install plugin by ./gradlew jar dist
3. Make UrlClassLoader siglenton

* add license header

* optimize path

* resolve confilct
xwm1992 pushed a commit that referenced this pull request Aug 4, 2022
* 1. Optimize plugin load, support load plugin by plugin instance name
2. Optimize install plugin, suppory install plugin by ./gradlew jar dist
3. Make UrlClassLoader siglenton

* add license header

* optimize path

* resolve confilct
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants