Re: Danger (?) change of DiscoveryCustomEvent in GridDhtPartitionsExchangeFuture#onDone
Alexey, thank you for the advice! On Thu, Oct 4, 2018 at 1:25 PM Alexey Goncharuk wrote: > > Vyacheslav, > > I think it would be more correct to capture all required state that will be > further used in a custom object and use it later in service processor. > Nullifying the field is an explicit action that was taken to reduce memory > consumption on server nodes, so we cannot simply drop it. Another solution > is reference counting and nullifying the field only after all parties > finished processing the message, but it looks like an overengineering to me. > > ср, 3 окт. 2018 г. в 15:00, Vyacheslav Daradur : > > > Alexey, thank you for the answer. I'd ask your advice about the > > following question: > > > > New Service Grid implementation listens to messages: > > * ChangeGlobalStateMessage - to perform activation/deactivation actions; > > * DynamicCacheChangeBatch - to handle caches stopping to undeploy > > related affinity services; > > * CacheAffinityChangeMessage - to recalculate assignments for affinity > > services in case of late affinity. > > > > It's important to handle these messages in order from disco-spi that > > means SG is storing them in own exchange queue to process. > > In some cases, PME may nullify this message earlier than they will be > > processed by SG. > > > > Could I exclude these messages from PME nullifying? > > > > On Wed, Oct 3, 2018 at 11:26 AM Alexey Goncharuk > > wrote: > > > > > > Vyacheslav, > > > > > > Thanks for investigating this. User code should never listen to system > > > custom events because this is an internal API and it's a subject to > > change. > > > If there is anything a user interested in, the corresponding public event > > > should be added. > > > > > > Nullifying the event in this case looks ok for me. > > > > > > вс, 30 сент. 2018 г. в 11:45, Vyacheslav Daradur : > > > > > > > I think that I understand a reason for doing this: > > > > The most custom events which handle in > > > > 'GridDhtPartitionsExchangeFuture' are using only in PME flow and > > > > reason is release them for GC as soon as possible. > > > > > > > > But there are some other systems which can listen to the same events, > > > > for example, to perform activation/deactivation actions them should > > > > handle [ChangeGlobalStateMessage, ChangeGlobalStateFinishMessage] > > > > which can be reset to 'null' by PME earlier then they will be handled > > > > by other systems. > > > > > > > > I'd suggest do not reset to 'null' custom messages in > > > > 'DiscoveryCustomEvent ' (at least without properly logic from the > > > > discovery-spi side). > > > > > > > > Thoughts? > > > > > > > > > > > > > > > > On Sat, Sep 29, 2018 at 11:43 PM Vyacheslav Daradur < > > daradu...@gmail.com> > > > > wrote: > > > > > > > > > > Hi Igniters! > > > > > > > > > > I think I found an illegal behavior in > > > > > GridDhtPartitionsExchangeFuture#onDone, the following code is called > > > > > here: > > > > > ((DiscoveryCustomEvent)firstDiscoEvt).customMessage(null); > > > > > > > > > > That means a global instance of 'DiscoveryCustomEvent' is being > > > > > mutated outside discovery-spi infrastructure. It also means that > > > > > discovery listeners receive 'DiscoveryCustomEvent' with 'null' field > > > > > instead of 'CustomMessage' which they may rely on. > > > > > > > > > > Could someone confirm if it is wrong behavior and should be fixed? > > > > > > > > > > -- > > > > > Best Regards, Vyacheslav D. > > > > > > > > > > > > > > > > -- > > > > Best Regards, Vyacheslav D. > > > > > > > > > > > > -- > > Best Regards, Vyacheslav D. > > -- Best Regards, Vyacheslav D.
Re: Danger (?) change of DiscoveryCustomEvent in GridDhtPartitionsExchangeFuture#onDone
Vyacheslav, I think it would be more correct to capture all required state that will be further used in a custom object and use it later in service processor. Nullifying the field is an explicit action that was taken to reduce memory consumption on server nodes, so we cannot simply drop it. Another solution is reference counting and nullifying the field only after all parties finished processing the message, but it looks like an overengineering to me. ср, 3 окт. 2018 г. в 15:00, Vyacheslav Daradur : > Alexey, thank you for the answer. I'd ask your advice about the > following question: > > New Service Grid implementation listens to messages: > * ChangeGlobalStateMessage - to perform activation/deactivation actions; > * DynamicCacheChangeBatch - to handle caches stopping to undeploy > related affinity services; > * CacheAffinityChangeMessage - to recalculate assignments for affinity > services in case of late affinity. > > It's important to handle these messages in order from disco-spi that > means SG is storing them in own exchange queue to process. > In some cases, PME may nullify this message earlier than they will be > processed by SG. > > Could I exclude these messages from PME nullifying? > > On Wed, Oct 3, 2018 at 11:26 AM Alexey Goncharuk > wrote: > > > > Vyacheslav, > > > > Thanks for investigating this. User code should never listen to system > > custom events because this is an internal API and it's a subject to > change. > > If there is anything a user interested in, the corresponding public event > > should be added. > > > > Nullifying the event in this case looks ok for me. > > > > вс, 30 сент. 2018 г. в 11:45, Vyacheslav Daradur : > > > > > I think that I understand a reason for doing this: > > > The most custom events which handle in > > > 'GridDhtPartitionsExchangeFuture' are using only in PME flow and > > > reason is release them for GC as soon as possible. > > > > > > But there are some other systems which can listen to the same events, > > > for example, to perform activation/deactivation actions them should > > > handle [ChangeGlobalStateMessage, ChangeGlobalStateFinishMessage] > > > which can be reset to 'null' by PME earlier then they will be handled > > > by other systems. > > > > > > I'd suggest do not reset to 'null' custom messages in > > > 'DiscoveryCustomEvent ' (at least without properly logic from the > > > discovery-spi side). > > > > > > Thoughts? > > > > > > > > > > > > On Sat, Sep 29, 2018 at 11:43 PM Vyacheslav Daradur < > daradu...@gmail.com> > > > wrote: > > > > > > > > Hi Igniters! > > > > > > > > I think I found an illegal behavior in > > > > GridDhtPartitionsExchangeFuture#onDone, the following code is called > > > > here: > > > > ((DiscoveryCustomEvent)firstDiscoEvt).customMessage(null); > > > > > > > > That means a global instance of 'DiscoveryCustomEvent' is being > > > > mutated outside discovery-spi infrastructure. It also means that > > > > discovery listeners receive 'DiscoveryCustomEvent' with 'null' field > > > > instead of 'CustomMessage' which they may rely on. > > > > > > > > Could someone confirm if it is wrong behavior and should be fixed? > > > > > > > > -- > > > > Best Regards, Vyacheslav D. > > > > > > > > > > > > -- > > > Best Regards, Vyacheslav D. > > > > > > > -- > Best Regards, Vyacheslav D. >
Re: Danger (?) change of DiscoveryCustomEvent in GridDhtPartitionsExchangeFuture#onDone
Alexey, thank you for the answer. I'd ask your advice about the following question: New Service Grid implementation listens to messages: * ChangeGlobalStateMessage - to perform activation/deactivation actions; * DynamicCacheChangeBatch - to handle caches stopping to undeploy related affinity services; * CacheAffinityChangeMessage - to recalculate assignments for affinity services in case of late affinity. It's important to handle these messages in order from disco-spi that means SG is storing them in own exchange queue to process. In some cases, PME may nullify this message earlier than they will be processed by SG. Could I exclude these messages from PME nullifying? On Wed, Oct 3, 2018 at 11:26 AM Alexey Goncharuk wrote: > > Vyacheslav, > > Thanks for investigating this. User code should never listen to system > custom events because this is an internal API and it's a subject to change. > If there is anything a user interested in, the corresponding public event > should be added. > > Nullifying the event in this case looks ok for me. > > вс, 30 сент. 2018 г. в 11:45, Vyacheslav Daradur : > > > I think that I understand a reason for doing this: > > The most custom events which handle in > > 'GridDhtPartitionsExchangeFuture' are using only in PME flow and > > reason is release them for GC as soon as possible. > > > > But there are some other systems which can listen to the same events, > > for example, to perform activation/deactivation actions them should > > handle [ChangeGlobalStateMessage, ChangeGlobalStateFinishMessage] > > which can be reset to 'null' by PME earlier then they will be handled > > by other systems. > > > > I'd suggest do not reset to 'null' custom messages in > > 'DiscoveryCustomEvent ' (at least without properly logic from the > > discovery-spi side). > > > > Thoughts? > > > > > > > > On Sat, Sep 29, 2018 at 11:43 PM Vyacheslav Daradur > > wrote: > > > > > > Hi Igniters! > > > > > > I think I found an illegal behavior in > > > GridDhtPartitionsExchangeFuture#onDone, the following code is called > > > here: > > > ((DiscoveryCustomEvent)firstDiscoEvt).customMessage(null); > > > > > > That means a global instance of 'DiscoveryCustomEvent' is being > > > mutated outside discovery-spi infrastructure. It also means that > > > discovery listeners receive 'DiscoveryCustomEvent' with 'null' field > > > instead of 'CustomMessage' which they may rely on. > > > > > > Could someone confirm if it is wrong behavior and should be fixed? > > > > > > -- > > > Best Regards, Vyacheslav D. > > > > > > > > -- > > Best Regards, Vyacheslav D. > > -- Best Regards, Vyacheslav D.
Re: Danger (?) change of DiscoveryCustomEvent in GridDhtPartitionsExchangeFuture#onDone
Vyacheslav, Thanks for investigating this. User code should never listen to system custom events because this is an internal API and it's a subject to change. If there is anything a user interested in, the corresponding public event should be added. Nullifying the event in this case looks ok for me. вс, 30 сент. 2018 г. в 11:45, Vyacheslav Daradur : > I think that I understand a reason for doing this: > The most custom events which handle in > 'GridDhtPartitionsExchangeFuture' are using only in PME flow and > reason is release them for GC as soon as possible. > > But there are some other systems which can listen to the same events, > for example, to perform activation/deactivation actions them should > handle [ChangeGlobalStateMessage, ChangeGlobalStateFinishMessage] > which can be reset to 'null' by PME earlier then they will be handled > by other systems. > > I'd suggest do not reset to 'null' custom messages in > 'DiscoveryCustomEvent ' (at least without properly logic from the > discovery-spi side). > > Thoughts? > > > > On Sat, Sep 29, 2018 at 11:43 PM Vyacheslav Daradur > wrote: > > > > Hi Igniters! > > > > I think I found an illegal behavior in > > GridDhtPartitionsExchangeFuture#onDone, the following code is called > > here: > > ((DiscoveryCustomEvent)firstDiscoEvt).customMessage(null); > > > > That means a global instance of 'DiscoveryCustomEvent' is being > > mutated outside discovery-spi infrastructure. It also means that > > discovery listeners receive 'DiscoveryCustomEvent' with 'null' field > > instead of 'CustomMessage' which they may rely on. > > > > Could someone confirm if it is wrong behavior and should be fixed? > > > > -- > > Best Regards, Vyacheslav D. > > > > -- > Best Regards, Vyacheslav D. >
Re: Danger (?) change of DiscoveryCustomEvent in GridDhtPartitionsExchangeFuture#onDone
I think that I understand a reason for doing this: The most custom events which handle in 'GridDhtPartitionsExchangeFuture' are using only in PME flow and reason is release them for GC as soon as possible. But there are some other systems which can listen to the same events, for example, to perform activation/deactivation actions them should handle [ChangeGlobalStateMessage, ChangeGlobalStateFinishMessage] which can be reset to 'null' by PME earlier then they will be handled by other systems. I'd suggest do not reset to 'null' custom messages in 'DiscoveryCustomEvent ' (at least without properly logic from the discovery-spi side). Thoughts? On Sat, Sep 29, 2018 at 11:43 PM Vyacheslav Daradur wrote: > > Hi Igniters! > > I think I found an illegal behavior in > GridDhtPartitionsExchangeFuture#onDone, the following code is called > here: > ((DiscoveryCustomEvent)firstDiscoEvt).customMessage(null); > > That means a global instance of 'DiscoveryCustomEvent' is being > mutated outside discovery-spi infrastructure. It also means that > discovery listeners receive 'DiscoveryCustomEvent' with 'null' field > instead of 'CustomMessage' which they may rely on. > > Could someone confirm if it is wrong behavior and should be fixed? > > -- > Best Regards, Vyacheslav D. -- Best Regards, Vyacheslav D.
