ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dmitriy Setrakyan <dsetrak...@apache.org>
Subject Re: OSGi migration may require a special marshaller
Date Thu, 12 Nov 2015 01:13:24 GMT
Raul,


I think I see your point and agree. However, in Ignite 1.5 the
OptimizedMarshaller will be deprecated in favor of internal cross-platform
Binary format. I think we should wait till 1.5 is released and then
implement your fix in 1.6 within the scope of Ignite binary protocol.

Just one ask, let’s not use abbreviations in the method names (counter to
coding guidelines). How about setTransmitOsgiPackageVersion(…)?

D.

On Wed, Nov 11, 2015 at 5:31 AM, Raul Kripalani <raulk@apache.org> wrote:

> On Wed, Nov 11, 2015 at 1:48 AM, Dmitriy Setrakyan <dsetrakyan@apache.org>
> wrote:
>
> > 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.
> >
>
> Why do we need to add the package name? The OptimizedObjectOutputStream
> already encodes the classname, which is then read on line 310 of
> OptimizedObjectInputStream (ignite-1.5 branch). Therefore, we already have
> the package name. All we need to encode is the version number which will be
> an additional String which can be further compressed:
>
> * For "1.0.0" we transmit "1".
> * For "1.1.0", we transmit "1.1".
> * etc.
>
> And this will only occur on a conditional basis when we detect we're
> running in an OSGi environment, although we need to cater for hybrid grid
> scenarios (comprising OSGi containers + non-OSGi containers). For all other
> cases, it would be skipped.
>
> If you'd still like to avoid these additional bytes, then a mode switch in
> OptimizedMarshaller: setTransmitOsgiPkgVer(boolean) should suffice.
>
> As I said, I don't see enough justification for pluggability in this
> context. I could see it if we were creating a mechanism for transmitting
> generic serialisation context. But everybody seems fixated on tackling
> classloader complexity only.
>
> I could also see some level of pluggability if this was applicable to any
> other marshallers. But how exactly do you see this being applicable to
> Kryo, Protobuf, or other potential ones (which we don't offer now)? How
> would you "encode the classloader" and where in the OutputStream? At the
> header? Before delegating the serialisation to the selected framework?
>
> 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.
>
>
> It's not about sentiment, it's about objectivity. The first step is to move
> away from the ClassLoaderCodec name, it's inaccurate. If anything I would
> propose SerializationContextCodec. But it implies extending its role to
> what I proposed: to provide and interpret information that assists the
> serialisation/deserialisation context.
>
> 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.
>
>
> No one questions that. The additional package version transmission would
> only kick in if we're in an OSGi environment.
>
> Having said that, I would be against artificially forcing this change into
> > 1.5, especially given the amount of controversy it generated.
>
>
> I don't think it's impossible to get it into 1.5 if that's the timeline.
>
> 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
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message