Bugfix/resolving dependency of some libraries by installed services.

[shishir-lab]

Issue #141: Bug related to dependency resolving.

io.vertx classes are only loaded by main classloader not isolated classloader i.e. the verticles that are deployed dynamically using Maven verticle factory cannot find io.vertx classes.

Ideally, this module loads all required io.vertx dependencies to edge-bios module main classpath (or server-bios if required).

Alternative is to use compile "io.vertx:vertx-dependencies". However, there are many unnecessary dependencies.

[zero88]

No, it is totally wrong fix and wrong direction.
This bug doesn't relate anything to classpath isolate or classpath sharing.
It is just simple because declare dependencies order, and I guess vertx-maven-service-factory is constructing a classpath is failed resolving this case.

Check:
https://github.com/NubeIO/iot-engine/blob/master/core/httpserver/build.gradle#L11-L12
https://github.com/NubeIO/iot-engine/blob/master/edge/module/gateway/build.gradle#L2-L3

Resolve by switch 2 lines in edge:module:gateway:build.gradle to

compile project(':core:httpserver')
compile project(':core:micro')

if not, try to update core:httpserver:build.gradle to

implementation project(':core:micro')
implementation "io.vertx:vertx-service-discovery:$project.versions.vertx"

Read https://docs.gradle.org/current/userguide/java_library_plugin.html#sec:java_library_configurations_graph

Never do it, add countless unnecessary dependencies like that. Ask @RaiBnod to know the reason

[shishir-lab]

@Zero-88 What do you mean by

"This bug doesn't relate anything to classpath isolate or classpath sharing.
It is just simple because declare dependencies order, and I guess vertx-maven-service-factory is constructing a classpath is failed resolving this case."

I think you have misunderstood the issue. I have done a lot of research on this issue from the time this project was started and I know it is because of classpath isolation feature provided by vertx.

I will explain the big picture here.

1. We will have a nube-bios that has a module nube-installer (using maven verticle factory).
2. The module nube-installer is responsible to install/uninstall other jars (services) containing any dependency.
3. We need to treat nube-bios and nube-installer as our PLATFORM where we have our control whereas the jars installed can be built independently by our clients and they can include any library they want as dependency.

The problem:
All other dependencies in independently built jars are resolved except for the dependencies with package starting with "io.vertx" and not present in the classpath of nube-bios (i.e. main classpath).

This problem is not due to our project but it is a corner case of vertx itself. So, your suggested changes will not solve the problem. In fact, edge:module:gateway:build.gradle and core:httpserver:build.gradle should be independent with our bios.

The solution:
The main classpath (i.e. nube-bios) should load all the classes starting with "io.vertx" that are used by other install-able modules OR THAT WILL BE USED BY CLIENTS IN FUTURE.
The solution is just encapsulating all the "io.vertx" classes and putting them in the main classpath.

I will be happy to answer if you have any other questions but I must suggest you to understand the issue before deciding or guessing if the approach is wrong or right :/ .

[zero88]

Hi there,
You may don't know this ticket is requested by me and the temporary fixed is from me also. @RaiBnod did it on behalf of me. So I think it is funny if you think I don't understand this ticket and your solution. Anyway, skip this attitude. Back to this ticket.

Just simple question for your (or you think) talent: Do you realized just simple disable :core:mirco in :edge:module:gateway then bios install successfully http server verticle. It absolute includes io.vertx.ext.web.*. So it should not work as your theory about package starting with "io.vertx", huh?

Your link corner case of vertx does not make any sense. Because we doesn't have any build context of this thread guy. And believe it is a huge mistake. Let see this Vertx DeploymentManager and Vertx IsolatingClassLoader, dude. Btw, I guess you also don't understand why not load these packages in isolate classpath.

Remember

Truth can only be found in one place: the code.

This is one valuable in your mention is isolation group. It is from here

By default, Vert.x has a flat classpath. I.e, when Vert.x deploys verticles it does so with the current classloader - it doesn’t create a new one.

However, MavenVerticleFactory is using isolation group. This docs

1. uses the Aether client try and locate the artifact com.mycompany:my-artifact:1.2 and all its dependencies in the configured Maven repositories and download and install it locally if it’s not already installed.
2. constructs a classpath including all those artifacts and creates a classloader with that classpath
3. service using the vert.x service factory.

So follow the code base both vertx and ours, it seems correct. However, only failure with :core:mirco. Somehow, vertx deployer in bios doesn't switch to correct isolate group to able load vertx-service-factory and vertx-circuit-breaker in runtime although they are included in isolate group classpath and of course not in main classpath
It is still ambiguous reason even for me. That's why I guessed a reason is due to order and compile only dependencies. however, it seems not correct because I realized :edge:module:installer depends directly on :core:mirco

It is obviously very difficult issue, so don't treat as you are only guy in here understand it.

This is quick git patch for debug, If you are interesting about what thing about isolate group, take it to understand more. Also I disable :core:mirco in :edge:module:gateway, also add :edge:module:kafka to prove why only :core:micro module is issue
issue-141.txt

The last words
I and no one in this team never merge current solution with "spaghetti dependencies" like that. It is most silly doing way. PLATFORM or ENGINE must be minimum stable version to accept any coming install request with any dependencies though instead of narrow to support a short list as iotvertx did (and of course it is totally redundant and wrong way).

[shishir-lab]

Seems like we are now going towards the same page.
From:

"This bug doesn't relate anything to classpath isolate or classpath sharing."

To:

"Somehow, vertx deployer in bios doesn't switch to correct isolate group to able load vertx-service-factory and vertx-circuit-breaker in runtime although they are included in isolate group classpath and of course not in main classpath"

However, I believe this issue is not only limited to :core:micro because we had faced similar issue way back in the proof of concept development phase where these new modules were not implemented. And the issue then was almost similar to vertx corner case issue that I linked above. Unfortunately at that time this thread was not present and we didn't ask the question as well.

My comment as well as the proposed problem/solution is based on the answer provided by Julien Viet, the top contributor and vertx lead. In the thread he answers by saying:

"I think you are running into a corner case because the io.vertx.* classes are never loaded from the isolating classloader"

I am not sure why you believe "it is a huge mistake" to pursue this lead.

Anyway, I agree that this issue is difficult and mysterious as well. Yes, there are the cases where classes starting with "io.vertx" are resolved as well. I am still investigating these cases by identifying the path to these classes and also trying to come up with a demo where this issue is seen in other place except core:micro.

I understand agile development process and I have started this pull request so that this issue will be discussed with @RaiBnod and I don't expect my code should be merged right away. If you see the readme file, I have mentioned the limitation and alternative approach as well. The approach I took is just a work around for the corner case and I welcome better approaches and new insights.

[zero88]

However, I believe this issue is not only limited to :core:micro because we had faced similar issue way back in the proof of concept development phase where these new modules were not implemented.

We did go a long path after you did MVP. Many things were changed and your classpath concern is not matched in this time

the answer provided by Julien Viet

I know who he is. But why I said big mistake because actual code is not matched with his comment. It is normal because leader only remember key not detail. The actual code is in my previous comment. And they just narrow io.vertx.core not io.vertx.*. It is totally different meaning. Because all verticle absolutely implemented based on io.vertx.core and this package is shared to reuse anywhere. Same with systemClass term. If Vertx do share io.vertx.*, I will raise issue to him directly because it is actually crazy point.

private boolean isVertxOrSystemClass(String name) {
    return
        name.startsWith("java.") ||
        name.startsWith("javax.") ||
        name.startsWith("sun.*") ||
        name.startsWith("com.sun.") ||
        name.startsWith("io.vertx.core") ||
        name.startsWith("io.netty.") ||
        name.startsWith("com.fasterxml.jackson");
}

My clue is might be somehow conflict classpath between classloader and jar file itself (it is defined in META-INF/MANIFEST in jar) but it is not explained why it occurs only for service-discovery and circuit-breaker. Need to look detail of implementation of these libs to see any special.

But in another view, I don't see benefit for isolating service for each deployment from MavenDeployer in context of we develop our apps itself and we control it in this time. If any service using same dependency with another version, just add it by itself and isolate it. In short, we will do implement CustomMavenDeployer as I did in my patch. This drawback is failure if we open source edge in future.

However, I'm intending to merge among bios, gateway and installer in one app so this issue should be gone if we identify service-discovery and circuit-breaker only problem. But this draw back is harder to upgrade itself.

[shishir-lab]

@Zero-88 Can you confirm if CustomMavenDeployer is included in the patch?

Also, what is the rationale behind merging bios and installer ? The original idea was to treat bios as Android and installer as a playstore. bios was supposed to have minimal functionality and once installed on edge devices would rarely be updated. However, we could update installer in multiple edge devices at once. I hope this feature will be preserved otherwise it will be nightmare to update edge devices after changing bios program.
Moreover, as you know the microservice architecture recommends to isolate services. I just want to know your thought process behind merging services.

[shishir-lab]

I was able to replicate the issue in a module that is independent of :core:micro. You can
use the patch demo-bug-141.txt and add following config on sandbox/engine/bios.json to replicate the issue.

{
          "metadata": {
            "group_id": "com.nubeiot.edge.module",
            "artifact_id": "jdbc",
            "version": "1.0.0-SNAPSHOT",
            "service_name": "edge-jdbc-demo"
          }
}

Following error is produced.

bios_1            | Caused by: java.lang.NoClassDefFoundError: com/mchange/v2/c3p0/ComboPooledDataSource
bios_1            | 	at io.vertx.ext.jdbc.spi.impl.C3P0DataSourceProvider.getDataSource(C3P0DataSourceProvider.java:52)
bios_1            | 	at io.vertx.ext.jdbc.impl.JDBCClientImpl$DataSourceHolder.ds(JDBCClientImpl.java:275)
bios_1            | 	at io.vertx.ext.jdbc.impl.JDBCClientImpl.<init>(JDBCClientImpl.java:98)
bios_1            | 	at io.vertx.ext.jdbc.JDBCClient.createShared(JDBCClient.java:79)
bios_1            | 	at com.nubeiot.edge.module.jdbc.JdbcVerticle.start(JdbcVerticle.java:26)
bios_1            | 	at com.nubeiot.core.component.ContainerVerticle.start(ContainerVerticle.java:66)
bios_1            | 	at io.vertx.core.impl.DeploymentManager.lambda$doDeploy$8(DeploymentManager.java:494)
bios_1            | 	... 8 common frames omitted
bios_1            | Caused by: java.lang.ClassNotFoundException: com.mchange.v2.c3p0.ComboPooledDataSource

The Problem:
Class com.mchange.v2.c3p0.ComboPooledDataSource is only present in isolated classpath (i.e. /data/repositories/java/....).
However, the class io.vertx.ext.jdbc.spi.impl.C3P0DataSourceProvider, that uses above class, is present both on main classpath (i.e. /app/lib/) and isolated classpath. By default this class will be loaded from main classpath and it cannot find the required class present in isolated classpath.

The solution:
If the class prefix io.vertx.ext.jdbc.* is set as isolatedClasses in the deployment config, this issue is resolved. This forces vertx to load both classes from isolated classpath and com.mchange.v2.c3p0.ComboPooledDataSource is found.

This solution works for :core:micro as well by adding io.vertx.reactivex.servicediscovery.* and io.vertx.reactivex.circuitbreaker.* to isolatedClasses during deployment of the module.

I will create a new pull request with necessary changes following this approach.
With this approach, the modules/services should pass additional appConfig if the module/service have issue related with class not found.

{
          "metadata": {
            "group_id": "com.nubeiot.edge.module",
            "artifact_id": "jdbc",
            "version": "1.0.0-SNAPSHOT",
            "service_name": "edge-jdbc-demo"
          },
          "appConfig": {
            "__deploy__": {
              "isolatedClasses": [
                "io.vertx.ext.jdbc.*"
              ]
            },
.... // other app configs
          }
 }

The suggested review would not work. New pull request created.

[shishir-lab]

The solution discussed above is implemented by #201