ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dmitriy Setrakyan <dsetrak...@apache.org>
Subject Re: IGNITE-2894 - Binary object inside of Externalizable still serialized with OptimizedMarshaller
Date Wed, 06 Sep 2017 01:21:09 GMT
Vova, I agree. Let's stay loyal to our binary serialization protocol.

Do you know if we will be loosing any functionality available in
Externalizable, but not present in our binary protocol?

D.

On Mon, Sep 4, 2017 at 11:38 PM, Vladimir Ozerov <vozerov@gridgain.com>
wrote:

> Folks,
>
> Let's discuss how this should be handled properly. I proposed to use the
> same format as regular binary object, with all data being written in "raw"
> form. This would give us one critical advantage - we will be able to work
> with such objects without deserialization on the server. Hence, no classes
> will be needed on the server side. Current implementation (see PR in the
> ticket) defines separate format which require deserialization, I am not OK
> with it.
>
> Thoughts?
>
> On Wed, Aug 23, 2017 at 6:11 PM, Nikita Amelchev <nsamelchev@gmail.com>
> wrote:
>
> > Hello, Igniters!
> >
> > I've developed Externalizable interface support using BinaryMarshaller
> [1].
> >
> > I have a misunderstanding with defining BinaryWriteMode in
> > BinaryUtils.mode(cls): there is AffinityKey class which implements
> > Externalizable and registered with ReflectiveSerialize, BinaryMarshaller
> > defines it as BinaryWriteMode.OBJ and uses reflection according to
> current
> > logic. I want to say that AffinityKey must be defined as
> > BinaryWriteMode.OBJ although the class implements the Externalizable
> > interface.
> > I have to add a new one more parameter in BinaryUtils.mode(cls) to define
> > BinaryWriteMode in a proper way.
> > Could you please review and comment my solution [2]?
> >
> > BTW, I have benchmarked my solution by GridMarshallerPerformanceTest and
> it
> > becomes faster up to 2 times (GridMarshaller).My JMH tests show that
> > marshal faster up to 50% and unmarshal faster up to 100% on an
> > Externalizable object.
> >
> > Also, I've filed an issue for Serializable interface support using
> > BinaryMarshaller [3] as it has been described earlier.
> >
> > [1] https://issues.apache.org/jira/browse/IGNITE-2894
> > [2] https://reviews.ignite.apache.org/ignite/review/IGNT-CR-278
> > [3] https://issues.apache.org/jira/browse/IGNITE-6172
> >
> > 2017-08-21 20:43 GMT+03:00 Valentin Kulichenko <
> > valentin.kulichenko@gmail.com>:
> >
> > > Nikita,
> > >
> > > I think anything binary related should have higher priority than
> > > Externalizable. I.e. if user explicitly implemented Binarylizable or
> > > provided a BinarySerializer, then BinaryMarshaller should of course use
> > > that and ignore Externalizable.
> > >
> > > -Val
> > >
> > > On Mon, Aug 21, 2017 at 9:29 AM, Nikita Amelchev <nsamelchev@gmail.com
> >
> > > wrote:
> > >
> > > > Hello, Igniters.
> > > >
> > > > I am developing Externalizable interface support by BinaryMarshaller
> > > > through new type constant. BinaryMarshaller allows using
> > BinarySerializer
> > > > to manage serialization. I need to define BinaryWriteMode in the
> > > > BinaryClassDescriptor constructor. In case of the Binarylizable
> > > interface -
> > > > serializer is ignored and BinaryWriteMode is BINARY. Can I do the
> same
> > > with
> > > > the Externalizable interface?
> > > >
> > > > In this case, I have issues with AffinityKey: some tests have failed
> > > > because of they except serialization logic of defined the serializer
> > > > instead of Externalizable logic. What is the priority between
> > predefined
> > > > BinarySerializer for class and implementation of Externalizable
> > > interface?
> > > >
> > > > 2017-08-01 13:09 GMT+03:00 Vladimir Ozerov <vozerov@gridgain.com>:
> > > >
> > > > > Valya,
> > > > > It makes sense to have both Externalizable and Binarylizable, as
> user
> > > > might
> > > > > want to serialize object for different systems. E.g. deserialize
> > binary
> > > > > stream from Kafka in Externalizable mode, and then put it to Ignite
> > > with
> > > > > Binarylizable to allow for field access without deserialization.
> > > > >
> > > > > Nikita,
> > > > > I think that Externalizable should be written in the same way as
we
> > > write
> > > > > fields in "raw" mode. So may be it will be enough to simply
> implement
> > > our
> > > > > own ObjectOutput interface on top of existing BinaryWriterExImpl.
> > Makes
> > > > > sense?
> > > > >
> > > > > Vladimir.
> > > > >
> > > > > On Thu, Jun 22, 2017 at 1:30 AM, Valentin Kulichenko <
> > > > > valentin.kulichenko@gmail.com> wrote:
> > > > >
> > > > > > Hi Nikita,
> > > > > >
> > > > > > 1. Makes sense to me.
> > > > > >
> > > > > > 2. Externalizable object should not be written as binary with
> flag
> > > 103,
> > > > > it
> > > > > > should be written in the same way it's written now. I don't
see
> any
> > > > > reason
> > > > > > to change the protocol. Purpose of this task it to move the
logic
> > to
> > > > > binary
> > > > > > marshaller instead of depending on optimized marshaller, and
also
> > > fully
> > > > > > support handles for these objects and objects included in them.
> > > > Currently
> > > > > > binary marshaller and optimized marshaller use different set
of
> > > > handles -
> > > > > > this is the main downside of current implementation.
> > > > > >
> > > > > > 3. I think this order is correct, but does it even make sense
to
> > > > > implement
> > > > > > both Binarylizable and Externalizable?
> > > > > >
> > > > > > -Val
> > > > > >
> > > > > > On Mon, Jun 19, 2017 at 8:58 AM, Nikita Amelchev <
> > > nsamelchev@gmail.com
> > > > >
> > > > > > wrote:
> > > > > >
> > > > > > > Hello everebody.
> > > > > > >
> > > > > > > I would like to clarify about some moments in marshaller
about
> > > custom
> > > > > > > serialization.
> > > > > > >
> > > > > > > 1. I suggest to divide the issue into two tasks: support
the
> > > > > > Externalizable
> > > > > > > and support the Serializable. The second task is to do
as a
> > > separate
> > > > > > issue.
> > > > > > >
> > > > > > > 2. In case the Optimized marshaller when object is the
> > > Extenalizable
> > > > > > > BinaryUtils.unmarshal() return deserialize value. But if
we
> will
> > > not
> > > > > use
> > > > > > > Optimized marshaller and write the Extenalizable as the
> > Object(103)
> > > > it
> > > > > > > return the BinaryObjectExImpl. It break
> > testBuilderExternalizable.
> > > > (If
> > > > > we
> > > > > > > replace Externalizable to Binarilylizable it also dont
work).
> > Fix -
> > > > > check
> > > > > > > that object is the Extenalizable and deserialize
> > > > > > > manual(BinaryUtils.java:1833 in PR). We will use this fix
or
> > return
> > > > > > > BinaryObjectExImpl?
> > > > > > >
> > > > > > > 3. What are priority if was implemented several interfaces:
> > > > > Binarylizable
> > > > > > > -> Externalizable -> Serializable ?
> > > > > > >
> > > > > > > Also can you pre review this issue?
> > > > > > > PR: https://github.com/apache/ignite/pull/2160
> > > > > > >
> > > > > > > 2017-04-18 17:41 GMT+03:00 Valentin Kulichenko <
> > > > > > > valentin.kulichenko@gmail.com>:
> > > > > > >
> > > > > > > > Nikita,
> > > > > > > >
> > > > > > > > For Externalizable option 1 is the correct one.
> Externalizable
> > > > > objects
> > > > > > > > should not be treated as binary objects.
> > > > > > > >
> > > > > > > > For read/writeObject, you indeed have to extend
> > > ObjectOutputStream.
> > > > > > > > writeObject() is final because you should extend
> > > > > writeObjectOverride()
> > > > > > > > instead. Take a look at ObjectOutputStream's JavaDoc
and on
> how
> > > > this
> > > > > is
> > > > > > > > done in OptimizedObjectOutputStream. Note that ideally
we
> need
> > to
> > > > > > > implement
> > > > > > > > everything that is included in Java serialization
spec,
> > including
> > > > > some
> > > > > > > > non-trivial stuff like PutField. I would check if
it's
> possible
> > > to
> > > > > > > somehow
> > > > > > > > reuse the code that already exists in optimized marshaller
as
> > > much
> > > > as
> > > > > > > > possible.
> > > > > > > >
> > > > > > > > -Val
> > > > > > > >
> > > > > > > > On Tue, Apr 18, 2017 at 1:36 PM, Nikita Amelchev <
> > > > > nsamelchev@gmail.com
> > > > > > >
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > > > I see two ways to support the Externalizable
in the BM:
> > > > > > > > > 1. Add a new type constant to the GridBinaryMarshaller
> class
> > > etc
> > > > > and
> > > > > > > > > read/writeExternal in the BinaryClassDescriptor.
> > > > > > > > > 2. Make read/writeExternal through the BINARY
type without
> > > > updating
> > > > > > > > > metadata.
> > > > > > > > > I don't know how to make a support read/writeObject
of the
> > > > > > Serializable
> > > > > > > > > without delegating to the OM. Because read/writeObject
> > methods
> > > > need
> > > > > > the
> > > > > > > > > Objectoutputstream class argument. One way is
to delegate
> it
> > to
> > > > the
> > > > > > > > > OptimizedObjectOutputStream. Second way is to
extend the
> > > > > > > > Objectoutputstream
> > > > > > > > > in the BinaryWriterExImpl. But it is wrong way
because the
> > > > > > writeObject
> > > > > > > is
> > > > > > > > > final.
> > > > > > > > >
> > > > > > > > > 2017-01-19 20:46 GMT+03:00 Valentin Kulichenko
<
> > > > > > > > > valentin.kulichenko@gmail.com>:
> > > > > > > > >
> > > > > > > > > > Nikita,
> > > > > > > > > >
> > > > > > > > > > In my view we just need to support Externalizable
and
> > > > > > > > > > writeObject/readObject in BinaryMarshaller
and get rid of
> > > > > > delegation
> > > > > > > to
> > > > > > > > > > optimized marshaller. Once such classes
also go through
> > > > > > > > BinaryMarshaller
> > > > > > > > > > streams, they will be aware of binary configuration
and
> > will
> > > > > share
> > > > > > > the
> > > > > > > > > same
> > > > > > > > > > set of handles as well. This should take
care of all the
> > > issues
> > > > > we
> > > > > > > have
> > > > > > > > > > here.
> > > > > > > > > >
> > > > > > > > > > -Val
> > > > > > > > > >
> > > > > > > > > > On Thu, Jan 19, 2017 at 7:26 AM, Nikita
Amelchev <
> > > > > > > nsamelchev@gmail.com
> > > > > > > > >
> > > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > > I have some questions about single
Marshaller.
> > > > > > > > > > > It seems not easy to merge OptimizedMarshaller
with
> > > > > > > BinaryMarshaller
> > > > > > > > > and
> > > > > > > > > > is
> > > > > > > > > > > there any sense in it?
> > > > > > > > > > > When Binary object inside Externalizable
serialized
> with
> > > > > > optimized
> > > > > > > it
> > > > > > > > > > > losing all benefits.
> > > > > > > > > > > Will OptimizedMarshaller be supported
at 2.0 version?
> Or
> > to
> > > > > merge
> > > > > > > > they
> > > > > > > > > is
> > > > > > > > > > > better?
> > > > > > > > > > > What do you think about it?
> > > > > > > > > > >
> > > > > > > > > > > In addition, Vladimir Ozerov, I would
like to hear your
> > > > > opinion.
> > > > > > > > > > >
> > > > > > > > > > > 2017-01-17 23:32 GMT+03:00 Denis Magda
<
> > dmagda@apache.org
> > > >:
> > > > > > > > > > >
> > > > > > > > > > > > Someone else added you to the
contributors list in
> > JIRA.
> > > > This
> > > > > > is
> > > > > > > > why
> > > > > > > > > I
> > > > > > > > > > > > couldn’t add you for the second
time. Ignite
> > committers,
> > > > > please
> > > > > > > > reply
> > > > > > > > > > on
> > > > > > > > > > > > the dev list if you add someone
to the list.
> > > > > > > > > > > >
> > > > > > > > > > > > Nikita, yes, this ticket is still
relevant. Go ahead
> > and
> > > > > assign
> > > > > > > it
> > > > > > > > on
> > > > > > > > > > > > yourself.
> > > > > > > > > > > >
> > > > > > > > > > > > Also please you may want to help
with approaching 2.0
> > > > release
> > > > > > and
> > > > > > > > > take
> > > > > > > > > > > > care of one of the sub-tasks that
must be included in
> > > 2.0:
> > > > > > > > > > > > https://issues.apache.org/jira/browse/IGNITE-4547
<
> > > > > > > > > > > > https://issues.apache.org/jira/browse/IGNITE-4547>
> > > > > > > > > > > >
> > > > > > > > > > > > —
> > > > > > > > > > > > Denis
> > > > > > > > > > > >
> > > > > > > > > > > > > On Jan 15, 2017, at 9:02
PM, Nikita Amelchev <
> > > > > > > > nsamelchev@gmail.com
> > > > > > > > > >
> > > > > > > > > > > > wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > This issue was created long
ago. Is still relevant?
> > > > > > > > > > > > >
> > > > > > > > > > > > > JIRA account:
> > > > > > > > > > > > > Username: NSAmelchev
> > > > > > > > > > > > > Full Name: Amelchev Nikita
> > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > > > 2017-01-14 1:52 GMT+03:00
Denis Magda <
> > > dmagda@apache.org
> > > > >:
> > > > > > > > > > > > >
> > > > > > > > > > > > >> Hi Nikita,
> > > > > > > > > > > > >>
> > > > > > > > > > > > >> I can’t find provided
account in Ignite JIRA
> > > > > > > > > > > > >> https://issues.apache.org/jira/browse/IGNITE
<
> > > > > > > > > > > > https://issues.apache.org/
> > > > > > > > > > > > >> jira/browse/IGNITE>
> > > > > > > > > > > > >>
> > > > > > > > > > > > >> Please create an account
there and share with me.
> > > > > > > > > > > > >>
> > > > > > > > > > > > >> This information might
be useful for you as well.
> > > > > > > > > > > > >>
> > > > > > > > > > > > >> Subscribe to both dev
and user lists:
> > > > > > > > > > > > >> https://ignite.apache.org/
> > > > community/resources.html#mail-
> > > > > > lists
> > > > > > > > > > > > >>
> > > > > > > > > > > > >> Get familiar with Ignite
development process
> > described
> > > > > here:
> > > > > > > > > > > > >> https://cwiki.apache.org/
> confluence/display/IGNITE/
> > > > > > > > > > > Development+Process
> > > > > > > > > > > > >>
> > > > > > > > > > > > >> Instructions on how to
contribute can be found
> here:
> > > > > > > > > > > > >> https://cwiki.apache.org/
> > > confluence/display/IGNITE/How+
> > > > > > > > > > to+Contribute
> > > > > > > > > > > > >>
> > > > > > > > > > > > >> Project setup in Intellij
IDEAL
> > > > > > > > > > > > >> https://cwiki.apache.org/
> confluence/display/IGNITE/
> > > > > > > > Project+Setup
> > > > > > > > > > > > >>
> > > > > > > > > > > > >> Regards,
> > > > > > > > > > > > >> Denis
> > > > > > > > > > > > >>
> > > > > > > > > > > > >>> On Jan 13, 2017,
at 1:37 AM, Nikita Amelchev <
> > > > > > > > > nsamelchev@gmail.com
> > > > > > > > > > >
> > > > > > > > > > > > >> wrote:
> > > > > > > > > > > > >>>
> > > > > > > > > > > > >>> Hello everyone.
> > > > > > > > > > > > >>>
> > > > > > > > > > > > >>> I'd like to take
IGNITE-2894. Can you assign to
> me?
> > > > > > > > > > > > >>>
> > > > > > > > > > > > >>> Username: NSAmelchev
> > > > > > > > > > > > >>>
> > > > > > > > > > > > >>> --
> > > > > > > > > > > > >>> Best wishes,
> > > > > > > > > > > > >>> Amelchev Nikita
> > > > > > > > > > > > >>
> > > > > > > > > > > > >>
> > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > > > --
> > > > > > > > > > > > > Best wishes,
> > > > > > > > > > > > > Amelchev Nikita
> > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > --
> > > > > > > > > > > Best wishes,
> > > > > > > > > > > Amelchev Nikita
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > --
> > > > > > > > > Best wishes,
> > > > > > > > > Amelchev Nikita
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > --
> > > > > > > Best wishes,
> > > > > > > Amelchev Nikita
> > > > > > >
> > > > > >
> > > > >
> > > >
> > > >
> > > >
> > > > --
> > > > Best wishes,
> > > > Amelchev Nikita
> > > >
> > >
> >
> >
> >
> > --
> > Best wishes,
> > Amelchev Nikita
> >
>

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