Gentlemen, I have several comments below…
On Tue, Nov 10, 2015 at 11:52 AM, Raul Kripalani <[email protected]> wrote: > Andrey, I already specified some points of my disagreement in the Wiki. In > my previous email I also said: "If we create an SPI it should be for more > generic hinting applicable in other circumstances during serialisation with > other frameworks and what not." > We should not use the SPI acronym liberally, as SPIs have a totally different meaning in Ignite. The ClassLoaderCodec should just be a setter on IgniteConfiguration, not a full-blown SPI. > What I don't understand is the interest in an SPI for something that > appears to have been solved by just transmitting the package version and > using the correct logic on the receiving side to locate the appropriate > Bundle and its classloader, as I explained in the wiki. That's a fully > deterministic way of solving this problem. Raul, we cannot be adding package name and version in all cases, as it will have a negative impact on performance, especially when there is an *optimistic* approach that requires no over-the-wire overhead whatsoever. As I already mentioned before, the default implementation *must* be the optimistic one, since Ignite is measured based on its performance first and foremost. This is the main reason why we actually need an API like ClassLoaderCodec, so it can have more than one implementation. > To me, creating an SPI targeted to OSGi class loading doesn't solve any > problem. We don’t need that much flexibility. A different story would be to > provide a mechanism to compute generic serialisation/deserialisation > context data, but you guys already rejected this need in the Wiki. > I don’t see how a ClassLoaderCodec is specific to OSGI at all. It does not have OSGI in its name and should be useful in other cases where custom class-loading is required. I would say, however, that ClassLoaderCodec should be specific to class-loader detection only, and we should not overburden it with other semantics solved elsewhere in Ignite. For example, custom serialization/deserialization is handled in Binaryzable interface, much in the same way as Java solves it with Externalizable interface. Raul, if you disagree, can you please provide your reasoning here? > With regards to the naming, sorry to be blunt, but ClassLoaderCodec is an > awful name. It implies that we are serialising a class loader. Wikipedia > definition for Codec: "A codec is a device or computer program capable of > encoding or decoding a digital data stream or signal." ClassLoaderCodec > would mean "a coder/decoder for a ClassLoader". And we would have not been > doing that; we would have just calculated and transmitted context data to > aid deserialisation, i.e. "hints", not a classloader. > Naming discussions are always passionate :) Raul, personally I understand your sentiments, but to be honest I dislike the names you are proposing even more. I still consider ClassLoaderCodec to be the most elegant name here, considering all the other options I have seen. It is concise and symmetric. I am open to changing it, but it must be for the better, not for the worse. > If you don't mind, could we please pause this discussion until I get a > proof of concept working these days? And then we can discuss over an actual > implementation to get consensus. I'd rather invest my time in that manner. > I see your point, but I still felt compelled to reply, especially because of possible performance impact of the proposed design. Let’s make sure that we follow the 80/20 rule and keep the default implementation suitable for 80% of use cases with zero performance overhead. > When is Ignite 1.5 due? > Given what I am seeing in the 1.5 release thread, I think we are realistically looking at the end of next week. Having said that, I would be against artificially forcing this change into 1.5, especially given the amount of controversy it generated. Let’s postpone it for 1.6. > > *Raúl Kripalani* > PMC & Committer @ Apache Ignite, Apache Camel | Integration, Big Data and > Messaging Engineer > http://about.me/raulkripalani | http://www.linkedin.com/in/raulkripalani > http://blog.raulkr.net | twitter: @raulvk > > On Tue, Nov 10, 2015 at 7:23 PM, Andrey Kornev <[email protected]> > wrote: > > > Raul, > > > > Could you please be a bit more specific about the nature of your > > disagreement? Is the proposed SPI not generic enough? Or, is it just the > > naming? > > > > As per Romain's suggestion, could you please just make sure the SPI is > > hooked into the new marshalling implementation and a no-op codec > > implementation is available in time for the 1.5 release? This would be > the > > first step toward full OSGi support in 1.6. > > > > Thanks > > Andrey > > > > > From: [email protected] > > > Date: Tue, 10 Nov 2015 18:22:59 +0000 > > > Subject: Re: OSGi migration may require a special marshaller > > > To: [email protected] > > > > > > Hi Romain, > > > > > > The implementation I have in mind won't be costly. I'm working on it > this > > > week. > > > > > > I still don't agree with a ClassLoaderCodec SPI as-is. If we create an > > SPI > > > it should be for more generic hinting applicable in other circumstances > > > during serialisation with other frameworks and what not. > > > > > > Otherwise, we should not create a pluggable SPI at all and simply > > implement > > > an option. As a matter of fact, with the last solution I proposed I > don't > > > think there will be any edge cases at all where users would need to > > create > > > their own "ClassLoaderCodec" at all. > > > > > > Regards, > > > > > > *Raúl Kripalani* > > > PMC & Committer @ Apache Ignite, Apache Camel | Integration, Big Data > and > > > Messaging Engineer > > > http://about.me/raulkripalani | > http://www.linkedin.com/in/raulkripalani > > > http://blog.raulkr.net | twitter: @raulvk > > > > > > On Tue, Nov 10, 2015 at 3:06 PM, Romain Gilles < > [email protected]> > > > wrote: > > > > > > > To be honest Raoul, I'm more interesting in the interface (SPI) > > declaration > > > > than the implementation. If you can do it for the 1.5 I'm ok. > > Otherwise I > > > > ready to start it by introducing the interface and implementing the > > dummy > > > > version for non OSGi environment. > > > > > > > > It will free me to implement a temporary OSGi version by the time you > > are > > > > done with the overall IGNITE-1270 tasks. > > > > > > > > Thanks, > > > > > > > > Romain > > > > > > > > Le mar. 10 nov. 2015 à 15:42, Raul Kripalani <[email protected]> a > > écrit : > > > > > > > > > Hi Romain, > > > > > > > > > > If you don't mind, I'm working on the entire OSGi integration > > including > > > > the > > > > > serialisation technique. > > > > > > > > > > I'll ping you if I need help. > > > > > > > > > > Thanks for your collaboration, > > > > > > > > > > *Raúl Kripalani* > > > > > PMC & Committer @ Apache Ignite, Apache Camel | Integration, Big > > Data and > > > > > Messaging Engineer > > > > > http://about.me/raulkripalani | > > http://www.linkedin.com/in/raulkripalani > > > > > http://blog.raulkr.net | twitter: @raulvk > > > > > > > > > > On Tue, Nov 10, 2015 at 1:50 PM, Romain Gilles < > > [email protected]> > > > > > wrote: > > > > > > > > > > > Hi, > > > > > > I have put my comments. Hope they will make the things progress > :) > > > > > > Should I start to implement this ticket or should I wait and see > if > > > > Raoul > > > > > > want to take it? > > > > > > > > > > > > Romain > > > > > > > > > > > > Le mar. 10 nov. 2015 à 02:58, Dmitriy Setrakyan < > > [email protected] > > > > > > > > > > a > > > > > > écrit : > > > > > > > > > > > > > Thanks Raul, great points! I have created a ticket for the > > > > > > > class-loader-detection design, described on wiki: > > > > > > > > > > > > > > https://issues.apache.org/jira/browse/IGNITE-1877 > > > > > > > > > > > > > > D. > > > > > > > > > > > > > > On Mon, Nov 9, 2015 at 3:42 PM, Raul Kripalani < > [email protected] > > > > > > > > wrote: > > > > > > > > > > > > > > > Hey Romain, > > > > > > > > > > > > > > > > I've updated the design proposal in the Wiki with my input. > > Could > > > > you > > > > > > > > please have a look and let me know what you think? > > > > > > > > > > > > > > > > I'll implement a proof of concept of my proposed > > > > > > > marshalling/unmarshalling > > > > > > > > strategy and push it to Git so you can take it for a spin. > > > > > > > > > > > > > > > > Regards, > > > > > > > > > > > > > > > > *Raúl Kripalani* > > > > > > > > PMC & Committer @ Apache Ignite, Apache Camel | Integration, > > Big > > > > Data > > > > > > and > > > > > > > > Messaging Engineer > > > > > > > > http://about.me/raulkripalani | > > > > > > http://www.linkedin.com/in/raulkripalani > > > > > > > > http://blog.raulkr.net | twitter: @raulvk > > > > > > > > > > > > > > > > On Fri, Nov 6, 2015 at 4:05 PM, Romain Gilles < > > > > > [email protected] > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > Hi, > > > > > > > > > I will put some sample code this WE. I'm exhausted sorry > for > > the > > > > > > > delay... > > > > > > > > > Romain > > > > > > > > > > > > > > > > > > Le ven. 6 nov. 2015 à 00:45, Andrey Kornev < > > > > > [email protected] > > > > > > > > > > > > > > a > > > > > > > > > écrit : > > > > > > > > > > > > > > > > > > > Hello, > > > > > > > > > > > > > > > > > > > > We've made an attempt to formalize the approach to > Ignite's > > > > OSGi > > > > > > > > > > enablement: > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/IGNITE/OSGI+Compatibility. > > > > > > > > > > Please have a look and feel free to provide your positive > > > > > feedback > > > > > > > :))) > > > > > > > > > > > > > > > > > > > > Thanks > > > > > > > > > > Andrey > > > > > > > > > > > > > > > > > > > > > From: [email protected] > > > > > > > > > > > Date: Wed, 4 Nov 2015 09:26:17 -0800 > > > > > > > > > > > Subject: Re: OSGi migration may require a special > > marshaller > > > > > > > > > > > To: [email protected] > > > > > > > > > > > > > > > > > > > > > > On Wed, Nov 4, 2015 at 9:07 AM, Romain Gilles < > > > > > > > > [email protected] > > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > Hi Dmitriy, > > > > > > > > > > > > I found this post that explain how to find a bundle > > based > > > > on > > > > > > its > > > > > > > > > bundle > > > > > > > > > > > > name and version. > > > > > > > > > > > > This post explain for the past to now how to do it in > > the > > > > > > > standard > > > > > > > > > way > > > > > > > > > > with > > > > > > > > > > > > "pull" approach: > > > > > > > > https://www.eclipse.org/forums/index.php/t/205719/ > > > > > > > > > > > > Regarding the version of OSGi you want to support > then > > some > > > > > > > > solutions > > > > > > > > > > will > > > > > > > > > > > > works some others will not. > > > > > > > > > > > > There is an other way to do this stuff without using > > those > > > > > > "pull" > > > > > > > > > style > > > > > > > > > > > > approach based on BundleTracker. If you want I can > > give you > > > > > the > > > > > > > > code > > > > > > > > > > to do > > > > > > > > > > > > it with BundlTracker. I think with this solution you > > will > > > > > > > support a > > > > > > > > > > wider > > > > > > > > > > > > range of OSGi version. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Romain, if you can provide a generic code sample to > look > > up a > > > > > > > > > ClassLoader > > > > > > > > > > > in OSGI based on manifest properties, would be great. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
