ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Valentin Kulichenko <valentin.kuliche...@gmail.com>
Subject Re: IGNITE-2894 - Binary object inside of Externalizable still serialized with OptimizedMarshaller
Date Tue, 19 Sep 2017 17:10:02 GMT
Nikita,

It sounds like the test should be changed, no? In case I'm missing
something, can you please give more details about the scenario which
requires deserialization? Generally, this sounds weird - in cases when we
can get advantage of binary format and avoid deserialization, we definitely
should not deserialize.

-Val

On Tue, Sep 19, 2017 at 6:17 AM, Nikita Amelchev <nsamelchev@gmail.com>
wrote:

> I have some problem when we don't deserialize Externalizable. Some messages
> require deserializing in GridCacheIoManager.message0(). For example, tests
> like testResponseMessageOnUnmarshallingFailed where readExternal throws an
> exception. A message containing Externalizable is deserialized and
> processed as a failed message. If we do not deserialize here, we won't
> process this message as failed. What way to resolve it? I see we can try to
> deserialize after a check on Externalizable in a finishUnmarshall method,
> but it looks bad. What are your thoughts?
>
> 2017-09-07 12:57 GMT+03:00 Nikita Amelchev <nsamelchev@gmail.com>:
>
> > 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
> >
>
>
>
> --
> Best wishes,
> Amelchev Nikita
>

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