ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Nikita Amelchev <nsamelc...@gmail.com>
Subject Re: IGNITE-2894 - Binary object inside of Externalizable still serialized with OptimizedMarshaller
Date Thu, 07 Sep 2017 09:57:24 GMT
I also agree that we should try to use Externalizable without
deserialization on servers. I will do it in a way suggested in the review.
The Externalizable will marshal through type GridBinaryMarshaller.OBJ, in
addition, I’ll add a flag in BinaryConfiguration which will be used to turn
on the old way with OptimizedMarshaller for compatibility with the current
data format.

2017-09-06 4:21 GMT+03:00 Dmitriy Setrakyan <dsetrakyan@apache.org>:

> 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
> > >
> >
>



-- 
Best wishes,
Amelchev Nikita

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