This version (2017/05/27 13:44) is a draft.
Approvals: 0/1

[09:43:44] <purplefox_> temporal_: cescoffier pmlopes morning folks

[09:43:55] <cescoffier> good morning purplefox_

[09:44:04] <pmlopes> good morning

[09:44:17] <purplefox_> how are you both?

[09:44:55] <cescoffier> good, back from Munich

[09:51:05] <purplefox_> ok great. we are due a meeting in 10 minutes, not sure if temporal_ is here yet though

[09:51:21] <purplefox_> i can see he was working late last night

[09:55:19] <cescoffier> yes, his computer is off (which is surprising)

[09:55:55] <cescoffier> should I ping him ?

[09:56:49] <purplefox_> yes please

[10:01:39] <cescoffier> just sent him a text message

[10:01:50] <cescoffier> you both have the meeting link right ?

[10:01:57] <pmlopes> no

[10:03:06] <pmlopes> i don't have the meeting link

[10:03:56] <cescoffier> should be in your mailbox

[10:04:05] <cescoffier> (just sent it again)

[10:06:11] <purplefox_> cescoffier: that's weird - it seems the V3 website has reverted to an earlier version - did you accidentally push an old branch?

[10:06:29] <cescoffier> purplefox_let me check

[10:07:28] <cescoffier> yep, my fault

[10:08:31] <purplefox_> let's hold the meeting until julien's here, as we need discuss a few issues with him

[10:09:03] <purplefox_> cescoffier: ha, i was about to get angry with the eclipse guys as I assumed they had changed the main page back to “Eclipse Vert.x” ;)

[10:09:23] <cescoffier> two branches named master and not enough coffee

[10:10:18] <cescoffier> purplefox_: are you joining the meeting ?

[10:10:36] <aesteve> hi everyone, good to know the hazlcast bug is fixed and the release is soon !

[10:10:50] <purplefox_> cescoffier: can we wait until julien is here?

[10:10:57] <cescoffier> no problem

[10:11:15] <temporal_> purplefox_ I'm here

[10:11:17] <temporal_> sorry a bit late

[10:11:23] <purplefox_> np

[10:12:40] <purplefox_> cescoffier: pmlopes temporal_ joining now

[11:26:58] <purplefox_> cescoffier: how did you push to the web-site - it seems to have got pretty screwed up

[11:27:37] <cescoffier> mvn site-deploy ?

[11:27:43] <cescoffier> just did it a couple of seconds ago

[11:28:33] <purplefox_> ah ok, maybe you were doing it at the same time as me, therefore mine failed ;)

[11:28:40] <purplefox_> if it's ok now, that's great

[11:29:45] <cescoffier> yes, there is no transactions ;-)

[11:29:53] <cescoffier> let me know if you see something wrong

[11:31:38] <cescoffier> temporal_ or purplefox_ can you add me as committer to the vertx-awesome repo ?

[11:33:18] <purplefox_> cescoffier: done

[11:33:30] <cescoffier> thanks !

[12:08:35] <cescoffier> purplefox_: the content of the wiki page is now on:

[12:10:13] <purplefox_> awesome! ;)

[12:10:30] <aesteve> do you think this : could be in the “awesome” repo ? (and maybe qbit, too), or ?

[12:10:37] <aesteve> let me know, I can submit a PR

[12:11:22] <temporal_> purplefox_ here is a reproducer for the IsolatingClassLoader issue

[12:12:04] <purplefox_> temporal_: thanks. i am looking at the redeploy issue right now, but will look at this next :)

[12:12:32] <temporal_> I'll do that Http tests after lunch

[12:38:29] <purplefox_> temporal_: thanks for the reproducer, but what i don't understand still is how that relates to the groovy case

[12:43:31] <cescoffier> aesteve: it should definitely be on the vertx-awesome page, just submit a PR

[13:09:57] <temporal_> purplefox_ in Groovy, the code instantiating the GroovyVerticle (GroovyVerticleFactory) is executed by Vertx core. The instantiated verticle extends GroovyVerticle. The GroovyVerticleFactory has a different definition of the GroovyVerticle than the verticle has.

[13:11:08] <temporal_> the thing is that the GroovyVerticleFactory is loaded from the ServiceLoader mechanism that finds it in the parent classloader. It is more or less the same with metrics that uses a ServiceLoader mechanism

[13:12:18] <temporal_> a fix for languages would be to find the service factory from the isolating classloader, but that would not fix the case for metrics used within a verticle, since metrics are loaded by Vert.x core .

[13:25:47] <purplefox_> temporal_: isn't a simple fix just to add io.vertx.ext.* to the list of system classes? then they will always be loaded by parent classloader

[13:26:00] <temporal_> yes that was my suggestion more or less

[13:26:08] <temporal_> either to use io.vertx.* or something like that

[13:26:28] <temporal_> but this change anyway needs to be reproduced and discussed :-)

[13:27:15] <temporal_> I can try this locally and see how it goes with groovy and metrics cases to be sure

[13:31:58] <temporal_> purplefox_ here is the reproducers for http response exception handlers

[13:32:07] <temporal_> s/is/are

[13:35:15] <temporal_> I'm gonna look at the isolating classloader and fix it this way and see how it works for both cases

[13:36:38] <aesteve> thanks cescoffier, was away for lunch. I'll submit a PR after 0.1 is released for both projects :)

[13:36:51] <cescoffier> aesteve: cool :-)

[13:37:09] <cescoffier> aesteve: I'm announcing the vert.x awesome project on the mailing list

[13:37:33] <aesteve> ok, nice, see you there then :p

[13:38:18] <temporal_> purplefox_ so actually it should be really “io.vertx.” prefix because the groovy classes are “io.vertx.groovy”, “io.vertx.lang” , is that ok ?

[13:41:47] <purplefox_> hmmm, but what if someone writes their own verticle factory which is not io.vertx?

[13:45:58] <temporal_> ok so

[13:46:17] <temporal_> we can address his in two ways maybe

[13:46:33] <temporal_> for verticle factory : use the classloader of the isolating classloader

[13:46:58] <temporal_> for metrics : consider that io.vertx.ext is a Vertx class as metrics uses the package io.vertx.ext

[13:47:44] <temporal_> and later we can rethink this with more use cases

[13:47:56] <temporal_> (i.e the feedback after 3.0)

[13:50:58] <temporal_> that being said there is a third case to consider : 1+2 : i.e using metrics from a groovy verticle :)

[13:53:27] <temporal_> I will come up with a test for a verticle factory in vert.x core reproducing this at least first

[13:54:12] <temporal_> this change : at least fixes the metrics case

[13:54:14] <temporal_> (tested)

[13:54:28] <temporal_> but does not fix the verticle factory case for which I will make a different test

[13:59:01] <temporal_> ah nice now we have : registerVerticleFactory

[14:44:23] <purplefox_> temporal_: can you explain how the metrics case is different from the verticle factory case?

[14:45:04] <temporal_> purplefox_ it's not much different, it is the same thing but from the verticle perspective

[14:45:53] <temporal_> purplefox_ the metrics uses the MetricsService class that more or less cast a metrics object obtained from Vertx to an metric object loaded from the verticle classloader

[14:46:22] <purplefox_> I'm thinking maybe we can have a static method IsolatingClassloader.registerSystemClass(String className) which basically adds the class to the list of system classes

[14:46:40] <purplefox_> and you could call that in VerticleFactory or metrics

[14:48:13] <temporal_> that would not work because classes would already be loaded I htink

[14:48:32] <temporal_> I mean to have this working you need first to define the class via that classloader

[14:48:55] <temporal_> ah you mean from the service factory primary instantiation

[14:49:02] <temporal_> I mean service loader of course

[14:49:27] <temporal_> right now I do have a test case for custom verticle factory

[14:49:32] <temporal_> and a fix for this case too

[14:49:55] <temporal_> that uses verticle factories loaded from the isolating classloader

[14:50:08] <temporal_> in your case it seems it would be simpler

[14:50:32] <temporal_> I hope there wouldn't be timing issues though

[14:50:51] <temporal_> (race conditions with registrations)

[14:51:43] <temporal_> you can have a look at these commits :

[15:03:08] <temporal_> I'm gonna POC your change and see how it goes

[15:03:14] <temporal_> with these tests

[15:17:47] <temporal_> purplefox_ so this change works for metrics but for Groovy it gives me an ugly LinkageError :-)

[15:18:05] <purplefox_> do you mean the registerSystemClass suggestion?

[15:18:07] <temporal_> yes

[15:18:20] <temporal_> related to Groovy mop

[15:18:37] <temporal_> it's related to groovy internal class cache

[15:18:52] <purplefox_> did you use a wildcard?

[15:19:07] <temporal_> so I'm using indeed a prefix

[15:19:10] <purplefox_> i guess you want all org.groovy.* classes to be registerde

[15:19:12] <temporal_>

[15:19:22] <temporal_> let's try that

[15:20:00] <temporal_> still failing

[15:20:13] <purplefox_> and the glassloader you pass to the groovyclassloader?

[15:20:30] <purplefox_> that should not be the isolating classloader I guess

[15:20:48] <temporal_> works if I also add “groovy.lang.”

[15:21:12] <purplefox_> what were you adding before?

[15:21:21] <temporal_> first this

[15:21:22] <temporal_> IsolatingClassLoader.registerSystemClass(“io.vertx.lang.groovy.”)

[15:21:22] <temporal_> IsolatingClassLoader.registerSystemClass(“io.vertx.groovy.”)

[15:21:27] <temporal_> then you suggested

[15:21:27] <temporal_> IsolatingClassLoader.registerSystemClass(“org.codehaus.groovy.”)

[15:21:31] <temporal_> and I added

[15:21:31] <temporal_> IsolatingClassLoader.registerSystemClass(“groovy.lang.”)

[15:21:47] <temporal_> not sure about IsolatingClassLoader.registerSystemClass(“io.vertx.groovy.”) though

[15:22:15] <temporal_> but then I said also that I suspect issues with metrics in groovy

[15:22:42] <temporal_> because metrics will register “io.vertx.ext.metrics.” as prefix

[15:22:53] <temporal_> but the same case with “io.vertx.groovy.ext.metrics.” may happen

[15:25:52] <purplefox_> another solution would just be to change the isolatingclassloader behaviour so it uses a more standard parent first approach

[15:26:28] <purplefox_> by default

[15:27:36] <purplefox_> of course it won't be isolating then

[15:27:50] <purplefox_> but not sure if most users care about that

[15:28:18] <temporal_> a minority may

[15:28:35] <temporal_> I agree with you that sometimes it's better to fix the cause than the symptoms

[15:28:46] <purplefox_> yeah

[15:29:10] <purplefox_> the isolating is only really needed if you deploy two or more verticles using maven verticlel factory that reference different versions of the same classes

[15:29:42] <purplefox_> so maybe isolation shouldn't be the default

[15:29:53] <purplefox_> and that should fix all this problems straight away

[15:30:01] <temporal_> so this use case we are hitting is not Maven service facotry

[15:30:05] <temporal_> use case

[15:30:14] <temporal_> it's the redeploy use case for which we use the IsolatingClassLoader

[15:30:54] <purplefox_> ah ok. for redeploy it certainly doesn't need to be isolated i think

[15:31:04] <rajith> purplefox_: hey Tim, back from the customer visit …anything else I need to do apart from the doc updates you mentioned?

[15:31:17] <purplefox_> could just use any url classloader i guess

[15:32:13] <temporal_> purplefox_ we should isolate on the class created by the verticle factories

[15:33:01] <temporal_> also maybe on the resources, right ?

[15:33:12] <purplefox_> which class do you mean?

[15:33:40] <temporal_> the java class or groovy classe specified in the Vertx.deployVerticle

[15:34:35] <purplefox_> not sure i understand what you mean

[15:35:28] <temporal_> so if you do

[15:35:34] <temporal_> vertx.deployVerticle(“foo.groovy”)

[15:35:50] <temporal_> foo.groovy would somehow needs to be isolated

[15:35:59] <temporal_> perhaps the use case is more for .java

[15:36:21] <temporal_> where the IDE could have loaded a copy of the .java class before

[15:41:06] <purplefox_> this is making my head hurt

[15:43:10] <temporal_> without the benefit of the beers before

[15:43:43] <purplefox_> seriously i am having trouble thinking straight today anyway :(

[15:45:46] <temporal_> I can try to remove the IsolatingClassLoader and replace it with an URLClassLoader to see how it goes

[15:47:14] <purplefox_> on second thoughts i think it is necessary for redeploy as the old classes need to be thrown away

[15:47:53] <purplefox_> so everything has to be loaded by the icl

[15:48:07] <purplefox_> but that won't work with groovy is the class cache is loaded by the vert.x classloader

[15:48:38] <purplefox_> it was exactly this kind of shit I was hoping to avoid by having a flatclassloader model in V3 ;)

[15:49:17] <purplefox_> brb

[15:55:55] <temporal_> I think the GroovyClassLoader is not static and is created when a verticle is created, so that should not be an issue

[16:02:02] <purplefox_> temporal_: cescoffier pmlopes interview just been published on InfoQ:

[16:02:50] <temporal_> nice :-)

[16:02:55] <purplefox_> this was supposed to be timed with the release but is one day early now

[16:02:59] <purplefox_> but not a big deal

[16:03:25] <temporal_> :-)

[16:04:03] <pmlopes> +1

[16:05:58] <temporal_> quoting purplefox_ “We've also simplified the classloader model and have a simple flat model now by default (i.e. no extra classloaders).”

[16:06:12] <purplefox_> lol

[16:06:24] <purplefox_> well.. it is flat by default ;)

[16:06:56] <purplefox_> going back to the cl issue, a part of me is thinking we should consider disabling the redploy feature

[16:07:10] <purplefox_> tbh it was going to be hidden anyway

[16:07:28] <purplefox_> and then we could just use a normal url cl for the maven factory

[16:08:02] <purplefox_> one lesson i should have learned from vert.x 2 is that reversing the cl delegation order tends to fuck things up ;)

[16:10:30] <temporal_> any .war file user could tell the same, you should have written more java ee web apps

[16:11:05] <cescoffier> wanna play with classloader, look at Apache Felix ;-)

[16:11:54] <cescoffier> in osgi we have the hierarchy and siblings

[16:12:00] <cescoffier> that makes things interesting

[16:12:14] <temporal_> I like when you say “we” when you talk about OSGi :-)

[16:12:35] <cescoffier> :-)

[16:12:46] <cescoffier> my last 10 years ;-)

[16:12:48] <temporal_> I think you need to come up with a microservice OSGi based framework :-)

[16:13:05] <temporal_> that basically loads an OSGi service from a java main :-)

[16:13:06] <cescoffier> come on, we have nano-services

[16:13:28] <cescoffier> the osgi services are just slightly smaller than the micro services ;-)

[16:14:48] <purplefox_> well i hope java modules will solve all this shit ;)

[16:16:55] <temporal_> the benefit of java modules will be to be designed by the guys doing the JVM runtime and the modules fixes will come along with the JDK :-)

[16:17:50] <purplefox_> yes exactly, it means THEY can deal with the complexity not us :)

[17:00:14] <purplefox_> temporal_: just a thought…

[17:01:28] <purplefox_> temporal_: in the groovy verticle factory - if you move the GroovyVerticleFactory stuff into another class called “InnerGroovyVerticleFactory”, and then the actual GroovyVerticleFactory loads the InnerGroovyVerticleFactory then it should work I guess:

[17:01:51] <purplefox_> then in the GroovyVerticleFactory.createVerticle do something like:

[17:01:52] <purplefox_> @Override

[17:01:52] <purplefox_> public Verticle createVerticle(String verticleName, ClassLoader classLoader) throws Exception {

[17:01:52] <purplefox_> Class<?> vfClass = classLoader.loadClass(“io.vertx.lang.groovy.InnerGroovyVerticleFactory”);

[17:01:52] <purplefox_> VerticleFactory vf = (VerticleFactory)vfClass.newInstance();

[17:01:53] <purplefox_> Method init = vfClass.getMethod(“init”, Vertx.class);

[17:01:54] <purplefox_> init.invoke(vf, vertx);

[17:01:56] <purplefox_> Verticle verticle = vf.createVerticle(verticleName, classLoader);

[17:01:58] <purplefox_> return verticle;

[17:02:00] <purplefox_> }

[17:02:19] <purplefox_> this should ensure the groovy classes are always loaded in the isolating cl

[17:03:30] <temporal_> actually today the groo yverticle works

[17:03:35] <temporal_> with a similar work around

[17:03:38] <temporal_> using reflection

[17:03:46] <temporal_> what does not work is the Metrics case

[17:04:11] <purplefox_> why does the metrics case not work?

[17:04:13] <temporal_> purplefox_

[17:04:34] <temporal_> what does not work is to obtain MetricsService inside a -redeploy Verticle

[17:04:39] <temporal_> but it works outside

[17:04:58] <temporal_> see

[17:05:06] <temporal_> System.out.println(“metrics json in verticle:” + metrics);

[17:05:08] <temporal_> will print null

[17:05:13] <temporal_> instead of the metrics

[17:06:21] <temporal_> purplefox_ so actually the change for -redeploy of IsolatingClassLoader is in DeploymentManager I think

[17:06:51] <temporal_> the change I've seen is a different case I think and it's for -cp starter issue

[17:07:23] <purplefox_> not sure I follow what you mean

[17:08:10] <purplefox_> just going back to the metrics issue

[17:08:18] <purplefox_> can you explain why this test does not work?

[17:09:06] <purplefox_> are you saying the MetricsService is loadeed twice?

[17:10:27] <temporal_> yes

[17:10:47] <temporal_> inside the Verticle and in the main classloader (since it's on the classpath anyway)

[17:11:21] <temporal_> at some point the code does

[17:11:22] <temporal_> public static AbstractMetrics unwrap(Measured measured) {

[17:11:22] <temporal_> MetricsProvider provider = (MetricsProvider) measured;

[17:11:22] <temporal_> Metrics baseMetrics = provider.getMetrics();

[17:11:23] <temporal_> if (baseMetrics instanceof AbstractMetrics) {

[17:11:23] <temporal_> return (AbstractMetrics) baseMetrics;

[17:11:23] <temporal_> }

[17:11:24] <temporal_> return null;

[17:11:24] <temporal_> }

[17:11:30] <temporal_> and the instanceof does not work

[17:11:45] <temporal_> because AbstractMetrics is loaded from the verticle IsolatingClassLoader

[17:12:05] <temporal_> and baseMetrics comes from Vert.x itself that instantiated the metrics SPI

[17:13:52] <purplefox_> Can't these classes be marked as system classes?

[17:14:27] <temporal_> io.vertx.ext ?

[17:14:41] <temporal_> please have a look at the 4 commits

[17:14:42] <temporal_>

[17:14:45] <temporal_> there are 2 fixes

[17:14:51] <temporal_> one for io.vert.ext

[17:14:51] <purplefox_> well. if we have a method IsolatingClassloader.registerSystemClassNames() then anyone can call that

[17:15:23] <temporal_> one for DeploymentManager verticle factory that is different

[17:16:21] <temporal_> for io.vertx.ext it is similar to registring system class

[17:16:43] <purplefox_> I gues I don't understand why the deploymentfactories fix is necessary

[17:16:51] <temporal_> for the verticle factory the idea is to load the verticle factory from the isolating classloader

[17:16:55] <purplefox_> yes

[17:17:12] <temporal_> it is necessary if we don't fix wit the GroovyLangFactory case

[17:17:13] <purplefox_> but why is that necessary if we register the right system class names

[17:17:17] <temporal_> otherwise it's not

[17:17:42] <temporal_> I think registring system classes creates potential classloader issues with verticle factories

[17:17:44] <purplefox_> why not the groovy factory>

[17:19:01] <temporal_> thing is that registring system classes can be a problem because we don't know all the prefixes

[17:19:15] <temporal_> for instance in the current impl we register com.sun, java. etc…

[17:19:27] <temporal_> we don't register for instance the XML API classes in the JDK

[17:19:31] <temporal_> I know you don't like XML

[17:19:35] <temporal_> still it should be added

[17:19:58] <temporal_> if we do something like that in Groovy, I fear we will find issues with more complex case than the easy ones we test

[17:20:34] <purplefox_> hmm

[17:20:37] <purplefox_> what a pita!

[17:20:59] <purplefox_> it's always nice to have something like this the day before a major release ;)

[17:21:18] <temporal_> yeah

[17:21:35] <temporal_> perhaps the best option is to disable -redeploy for now

[17:37:30] <purplefox_> yes maybe, but we would also have to disable isolation groups too

[17:37:39] <purplefox_> here's a thought

[17:37:42] <purplefox_> :

[17:38:15] <purplefox_> in the isolating classloader, before it tries to load the class itself perhaps it could check if the class has already been loaded by the parent classloader?

[17:38:25] <purplefox_> but how to do this is tricky as findLooadClass is protected

[17:38:33] <purplefox_> but maybe we could use reflection to call it

[17:38:49] <purplefox_> then this should prevent classes being loaded more than once

[17:43:23] <temporal_> purplefox_ I think one purpose of the IsolatingCL is to override classes loaded by the parent, like a different Log4J class used in a maven deployment

[17:44:23] <purplefox_> i don't think anythingn in vert.x core will load log4j classes ?

[17:44:31] <temporal_> it's a stupid example :-)

[17:44:43] <temporal_> but right

[17:44:51] <temporal_> probably that we should not a child first policy

[17:45:01] <temporal_> and we use the deployment groups with parent first

[17:45:05] <purplefox_> i don't think the purpose of the icl is to override classes loaded by the parent, it's so that two different icl can load different application classes

[17:45:21] <temporal_> and it's up to the user to make sure that there is nothing wrong in this classloader

[17:45:38] <temporal_> purplefox_ agreed it's saner

[17:45:44] <purplefox_> yes ideally we would use a standard delegation order but then the isolation semantics change

[17:46:04] <temporal_> if there is an issue with the class in the “vertx” classloader, then the user should tweak his runtime / setup to remove it

[17:46:37] <temporal_> in reality I think are talking of the main/junit classloader here

[17:46:58] <temporal_> the possible issue I think is the vertx “CLI” version that loads a bunch of stuff in this classloader

[17:47:06] <temporal_> (/lib)

[17:47:11] <purplefox_> not any more

[17:47:21] <purplefox_> i just changed that to use a standard urlc

[17:47:39] <purplefox_> but.. if we use parent first then we will lose isolation

[17:48:10] <purplefox_> unless the user doesn't include their application classes on the main classpath, and they use extra classpath on the deployment options

[17:48:15] <purplefox_> but that's a pita

[17:48:48] <temporal_> so the use case is the user deploy a service via maven and one class loaded by this conflicts with a user class

[17:49:44] <purplefox_> maven is a different case, and actually we don't need isolating cl there at all

[17:49:51] <purplefox_> we can just use a normal urlc

[17:50:11] <purplefox_> as the maven cp won't alredy be on the main classpath anyway

[17:50:18] <purplefox_> the issue is with:

[17:50:21] <purplefox_> 1. redeploy

[17:50:24] <purplefox_> 2. isolation groups

[17:50:40] <purplefox_> both 1 and 2 need isolating classloader to work

[17:52:35] <purplefox_> but isolating classloader has an inherent problem which can result in classes being loaded more than once by parent or child loader

[18:05:21] <purplefox_> temporal_: ok… i can make your isolating reproducer test with a fairly simple change:

[18:07:03] <purplefox_> temporal_:

[18:07:47] <temporal_> ok, let me try with the 2 concretes cases groovy and metrics

[18:07:49] <purplefox_> so basically you'll get a warning if the class is already loaded but it won't load it again with a different cl so things should still work

[18:08:49] <purplefox_> oops one mistake in that class, one sec

[18:09:33] <temporal_> ok

[18:10:05] <purplefox_> ok fixed now

[18:10:39] <purplefox_>

[18:10:47] <temporal_> ok

[18:10:50] <temporal_> testing

[18:15:43] <cescoffier> guys, something have slightly changed in openshift. I've only one port opened (well actually two because web sockets have their own port). So, can't get Hazelcast clustering to work again. I've asked the question to the openshift forum and wait for an answer

[18:15:58] <cescoffier> If I don't get any, I will remove the hazelcast part of the documentation until it's fixed

[18:16:39] <temporal_> purplefox_ I get ugly groovy compilation failures

[18:16:49] <temporal_> due to AST transformation classloading issues

[18:16:52] <temporal_> Transform Class org.codehaus.groovy.ast.builder.AstBuilderTransformation specified at jar:file:/Users/julien/.m2/repository/org/codehaus/groovy/groovy-all/2.3.10/groovy-all-2.3.10.jar!/META-INF/services/org.codehaus.groovy.transform.ASTTransformation is not an ASTTransformation.

[18:17:12] <purplefox_> hmm, maybe the groovy code makes some assumptions about the classloaders that load the classes

[18:17:36] <purplefox_> do you get any warnings when you run it?

[18:17:40] <temporal_> yes

[18:17:48] <temporal_> indeed

[18:17:49] <temporal_> Not isolated! org.codehaus.groovy.transform.GroovyASTTransformation will not be isolated as it's already been loaded by a parent class

[18:18:03] <temporal_> that's an internal compiler failure

[18:27:11] <purplefox_> temporal_: ok, so to recap:

[18:27:59] <purplefox_> do we have a solution that's going to work for the groovy case?

[18:28:16] <temporal_> currently it works with a work around

[18:28:32] <purplefox_> you mean the reflection stuff?

[18:28:35] <temporal_> yes

[18:28:41] <temporal_> there is another solution with the deployment manager

[18:28:56] <temporal_> that uses serviceloaders loaded by the IsolatingClassLoader

[18:29:00] <purplefox_> ok so maybe this is a general principal - verticle factories MUST NOT load the classes in the main classloader first

[18:29:18] <purplefox_> we can check this in the isolatingclassloader and fail the deployment if this happens

[18:29:37] <temporal_> what do you mean exactly ?

[18:29:41] <purplefox_> so instead of this:

[18:29:42] <purplefox_> Class<?> res = (Class<?>)findLoadedClassMethod.invoke(getParent(), name);

[18:29:42] <purplefox_> if (res != null) {

[18:29:42] <purplefox_> log.warn(“Not isolated! ” + name + “ will not be isolated as it's already been loaded by a parent class”);

[18:29:42] <purplefox_> return res;

[18:29:45] <purplefox_> }

[18:29:48] <purplefox_> we throw an exception

[18:29:57] <temporal_> the issue I think with that

[18:30:17] <temporal_> in the jar of the verticle facotry

[18:30:41] <temporal_> is that in a verticle factory there are classes loaded / instantiated by Vert.x : the VerticleFactory itself

[18:30:59] <purplefox_> right, that's what i'm saying, it's not allowed

[18:31:25] <temporal_> Vert.x instanitates the VerticleFactory, unless we use the patch I did for deployment manager

[18:31:36] <cescoffier> nevermind, openshift seems to reinject the values….

[18:31:40] <purplefox_> VerticleFactory is a system class

[18:31:44] <purplefox_> so it won't fail on that

[18:31:51] <temporal_> ah I mean the GroovyVerticleFactory

[18:32:01] <temporal_> VerticleFactory in general :-)

[18:32:08] <purplefox_> i'm not sure I follow

[18:32:17] <purplefox_> can you give a concrete example?

[18:32:20] <temporal_> yes

[18:32:42] <temporal_> the groovy lang integration has GroovyVerticle and GroovyVerticleFactory class

[18:33:07] <temporal_> the GroovyVerticleFactory loads a GroovyVerticle

[18:33:23] <temporal_> and it uses it actually since it references it

[18:33:28] <purplefox_> right, what i'm saying is that it needs to do that by reflection

[18:33:36] <temporal_> it means that the GroovyVerticleFactory should never uses the GroovyVerticle

[18:33:39] <temporal_> and use reflection

[18:33:39] <purplefox_> yes

[18:33:42] <temporal_> ok

[18:33:57] <purplefox_> and that could be a general rule for any verticle factory

[18:34:17] <temporal_> not sure

[18:34:26] <temporal_> in rx-java it is different

[18:34:34] <temporal_> there is no classloading per se

[18:34:47] <temporal_> but there is no verticle factory in rx-java

[18:34:55] <temporal_> so I'm saying stupid things

[18:34:57] <temporal_> :-)

[18:35:11] <temporal_> let me double check

[18:35:34] <temporal_> indeed so it would be fine with rxjava too

[18:35:42] <purplefox_> the other option is to make our lives much simpler and just use standard parent first delegation. BUT this means we lose the redeployment feature

[18:36:01] <purplefox_> coz I don't know how to do redeployment without isolating cl

[18:37:55] <temporal_> my patch for DeploymentManager, loads the verticle factories SPI with the IsolatingClassLoader and seems to address this case though

[18:38:14] <temporal_> the issue is that we end up with a part of the classes loaded by Vert.x and a port loaded by the IsolatingCL

[18:39:10] <temporal_> the other way as you said is to use reflection from the GroovyVerticleFactory

[18:39:19] <purplefox_> yeah that's another possibility - although i think we neeed to maintain one deploymentfactorues per isolating classloader not per deployment

[18:39:31] <temporal_> the third one is to disable the redeploy feature

[18:39:51] <purplefox_> what does spring boot do for redeploy?

[18:40:02] <temporal_> I don't know how it works for spring boot at all

[18:40:13] <temporal_> does it have redeploy faeture ?

[18:40:23] <purplefox_> i'm just wondering if our competitors have a similar geature

[18:40:35] <temporal_> I know spring has a special feature for redeplooyment

[18:40:40] <purplefox_> i know some people can just use jrebel

[18:40:45] <temporal_> that is tuned to make Spring injection works well

[18:41:05] <temporal_>

[18:41:36] <temporal_>

[18:42:26] <purplefox_> so maybe we should just drop redeploy and say its someone elses problem?

[18:42:33] <purplefox_> it's a can of worms

[18:42:55] <purplefox_> and tbh i wish I hadn't made it public, as it was sitting there privately for some time because I had concerns….

[18:43:07] <temporal_> well at some point I thought that redeployment could be itself a VerticleFactory

[18:43:11] <temporal_> like you would do

[18:43:19] <temporal_> redeploy:(something here)

[18:43:38] <temporal_> and the VF would take encapsulate that outside of vertx core

[18:45:15] <temporal_> in my former OSS life I did a classloader that loaded only the changed classes by comparing the classes

[18:45:18] <temporal_> to have hot java reload

[18:45:57] <purplefox_> doing redeploy for the entire process is a simpler problem

[18:46:01] <temporal_>

[18:46:11] <purplefox_> the trouble is we support redeploy on a finer grained - per deployment

[18:49:24] <purplefox_> does your patch fix the metrics issue too?

[18:50:03] <purplefox_> why do we need deploymentfactories patch AND changes in groovy verticle factory? Isn't the deploymentfactories patch sufficient?

[18:54:02] <temporal_> it does not solve the metrics case

[18:54:37] <temporal_> metrics case is solve by adding “io.vertx.ext” to system classes

[18:54:50] <temporal_> purplefox_ yes if we have the deployment facotry patch then we don't need anymore the current GroovyVerticleFactory reflective code

[18:55:29] <temporal_> io.vertx.ext or using a addToSystemClass(prefix) dynamic way

[18:56:50] <purplefox_> a part of me is saying we just drop redeploy as it would make everything so much simpler…

[18:56:51] <purplefox_> wdyt?

[18:58:52] <temporal_> I think the fixes I did are ok for the current status but they don't fix issues we are not yet aware of

[18:59:00] <temporal_> dropping redeploy would avoid these problems

[18:59:51] <temporal_> so I would rather remove it for now

[19:02:48] <mark_> Is there a fat-jar example in the vertx-examples project? [19:03:58] <purplefox_> mark_: yep, top of the README, maven simplest/gradle simplest

[19:04:09] <mark_> Thanks [19:04:11] <purplefox_> it's the first example that's discussed [19:04:40] <purplefox_> i know people don't read READMEs ;) Maybe if we named them DONTREADME then people would read them.. [19:04:52] <mark_> I was looking at the top one core-examples.

[19:05:07] <mark_> And I was going to go through them one by one till I found it. ;) [19:05:18] <purplefox_> core examples is not the top README [19:05:42] <purplefox_> start here [19:05:59] <purplefox_> click on the link for helloworld in the first paragrpah [19:06:23] <purplefox_> or click on the link that says main README [19:06:44] <purplefox_> and the very first examples are fatjars :) [19:07:45] <mark_> hmm. I don't see a blob directory under the root of vertx-examples. Wish I could post screen shots.

[19:08:02] <purplefox_> did you click on the link above?

[19:08:56] <purplefox_> there is no blob directory that's a url

[19:09:30] <mark_> I get it. THanks. Also is there a page in the docs with all the deploy options you can set. The names like instances, etc? [19:09:37] <purplefox_> yes [19:09:45] <mark_> I'll find it. Thanks

[19:14:03] <mark_> Found the DeploymentOptions, which seems nicer because you don't have to know them by heart. ;) [19:14:35] <aesteve> nice interview purplefox_ loved the Scala part [19:14:50] <mark_> And I love being able to chain method calls.

[19:17:09] <purplefox_> mark_: cool, glad you like it :) [19:17:28] <purplefox_> sorry, just a bit stressed/grumpy today due to the major release tomorrow and lack of sleep [19:18:24] <mark_> I hear you. Past few days I think I might have gotten 4 hours each night. Kept waking up. But really excited to upgrade our app to 3.x We do have a lot of work to do it, get to untangle all the hacks we did. ;)

[19:19:00] <aesteve> oh didn't notice it was Rick interviewing you. btw QBit should definitely be in the vertx-awesome repo, it inspired me so much building my own little framework

[19:19:11] <purplefox_> mark_: you're mark spritzler? [19:19:16] <mark_> I am keeping a log of the changes we have to make, then I will make a blog post somewhere with that list. Some of the things are a little bit specific to our app, but most for every one.

[19:19:20] <mark_> Of course, it is me. [19:19:29] <purplefox_> oh hi mark :) [19:19:33] <mark_> hehehe

[19:19:35] <purplefox_> sorry didn't realise

[19:19:51] <purplefox_> i was expecting a bytor

[19:19:51] <mark_> No worries, You can still treat me like a Noob. ;) [19:19:57] <purplefox_> lol [19:20:21] <mark_> Typically yes it will be bytor. I just rarely use IRC, so I always forget the command to change username.

[19:22:05] <bytor99999> OK, so it was / nick. :D

[19:24:41] <purplefox_> bytor99999: the five 9s represents your uptime? No wonder you are tired ;)

[19:25:23] <purplefox_> aesteve: thanks

[19:25:38] <bytor99999> Yeah. Back in the old days of the internet I used to be able to get just bytor. Now other people use it, so I set myself apart with 5 9s. @ yahoo and @ gmail email account names.

[20:12:04] <purplefox_> temporal_: you there still?

[20:13:05] <temporal_> purplefox_ yes

[20:13:23] <temporal_> I will take care of my daughter soon

[20:13:31] <temporal_> but I'll be available later

[20:13:32] <purplefox_> temporal_: i was thinking about this some more and I think we can get things to work without dropping redeployment

[20:13:38] <purplefox_> do you have 5 mins now?

[20:13:57] <purplefox_> or probably less ;)

[20:13:58] <temporal_> yes

[20:14:01] <purplefox_> ok cool

[20:14:57] <purplefox_> so, basicall i think we can change the default behaviour of the icl to parent first, but the extra urls specified are not parent first

[20:15:09] <purplefox_> this will fix the groovy and metrics issue

[20:15:14] <purplefox_> and make things simpler

[20:15:30] <purplefox_> and fix the maven deployment isolation issue

[20:15:47] <purplefox_> then for redeployment, the user must specify which urls to watch

[20:16:01] <purplefox_> (right now it just watches everything which is actually wrong)

[20:16:02] <temporal_> are you sure it won't be an issue ?

[20:16:10] <temporal_> I see a potential problem

[20:16:13] <temporal_> at least for now

[20:16:44] <temporal_> for redeploy, we construct an IsolatingCL that uses as URLs the URL[] of the parent URLClassLoader

[20:16:51] <temporal_> so somehow that would be removed, right ?

[20:16:56] <purplefox_> yes i think that's wrong

[20:17:00] <temporal_> ok

[20:17:34] <purplefox_> so the urls should actually be the urls from the extra classpath

[20:17:45] <purplefox_> so basically it doesn't make sense to actually watch the system classes

[20:17:47] <temporal_> ah ok

[20:17:49] <temporal_> I see

[20:17:57] <purplefox_> the only thinks you want to redeploy are the app classes

[20:18:15] <purplefox_> right now we actually watch everything which is wrong

[20:18:35] <purplefox_> coz if you had 10 verticles on your cp, then change any of them and they'd all redeploy

[20:18:50] <purplefox_> so it's actually broken now anyway

[20:19:55] <purplefox_> i think i will try and hack something together in a branch, and get you to review it later or tomorrow. how does that sound?

[20:20:09] <temporal_> it sounds good to me

[20:20:13] <temporal_> later tonight is ok for reviewing

[20:20:25] <temporal_> and most importantly try it :-)

[20:20:41] <purplefox_> we need to sleep too. i am feeling pretty shit today anyway, so don't want to be too bad tomorrow

[20:22:27] <temporal_> makes sense

[20:22:37] <temporal_> that being said, I would sleep better if the issue is fixed

[20:34:18] <aesteve> good night folks and especially good luck with the final adjustments !

[20:49:53] <purplefox_> temporal_: ok branch is here if you wanna try it with groovy

[20:49:55] <purplefox_>

[20:50:15] <purplefox_> it should work for your use case although I've yet to fix tests (some will fail)

[20:59:12] <temporal_> ok

[21:05:04] <temporal_> purplefox_ it works for metrics and groovy cases

[21:05:38] <purplefox_> good

[21:07:12] <purplefox_> temporal_: now I just need to add an extra boolean switch to deployment options

[21:07:29] <purplefox_> to turn parent first classloading off, then that can be used by the maven verticle factory

[21:07:45] <purplefox_> which means users can share their routers and stuff between verticles when using that

[21:08:26] <temporal_> yes

[21:08:30] <purplefox_> i mean turn parent last classloading off ;)

[21:08:47] <purplefox_> so basically whether it's parent first or parent last becomes configurable

[21:09:03] <purplefox_> default is parent last to preserve isolation behaviour

[23:03:46] <bytor99999> Sorry, doc question again. I am looking for the doc that shows how to create the fatjar. I have one created, but when I go to run via java -jar I get a security error about the manifest file, and I recall reading somewhere in the docs about the main class and another property that needs to be set in the manifest file that I can't relocate.

[23:09:13] <bytor99999> Error: A JNI error has occurred, please check your installation and try again

[23:09:14] <bytor99999> Exception in thread “main” java.lang.SecurityException: Invalid signature file digest for Manifest main attributes

[23:09:23] <bytor99999> That is what I see when I try to run it.

[23:11:21] <bytor99999> Might have found a solution. Filter configuration in the shade plugin configuration.