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 <[email protected]>: > 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 <[email protected]>: > >> 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 <[email protected]> >> 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 <[email protected]> >> > 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 < >> > > [email protected]>: >> > > >> > > > 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 < >> [email protected] >> > > >> > > > 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 <[email protected] >> >: >> > > > > >> > > > > > 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 < >> > > > > > [email protected]> 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 < >> > > > [email protected] >> > > > > > >> > > > > > > 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 < >> > > > > > > > [email protected]>: >> > > > > > > > >> > > > > > > > > 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 < >> > > > > > [email protected] >> > > > > > > > >> > > > > > > > > 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 < >> > > > > > > > > > [email protected]>: >> > > > > > > > > > >> > > > > > > > > > > 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 < >> > > > > > > > [email protected] >> > > > > > > > > > >> > > > > > > > > > > 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 < >> > > [email protected] >> > > > >: >> > > > > > > > > > > > >> > > > > > > > > > > > > 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 < >> > > > > > > > > [email protected] >> > > > > > > > > > > >> > > > > > > > > > > > > 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 < >> > > > [email protected] >> > > > > >: >> > > > > > > > > > > > > > >> > > > > > > > > > > > > >> 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 < >> > > > > > > > > > [email protected] >> > > > > > > > > > > > >> > > > > > > > > > > > > >> 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
