[
https://issues.apache.org/jira/browse/IGNITE-12568?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Andrey N. Gura updated IGNITE-12568:
------------------------------------
Description:
Currently existing {{MessageFactory}} implementations (at least
{{GridIoMessageFactory}} and {{GridH2ValueMessageFactory}}) have serious
problem: it is possible to register several messages with the same direct type.
It could lead to the cluster freeze due to unexpected unmarshaling error.
*Solution:*
Each message should be registered and during registration we can check that
there is no registered message with the same type. Otherwise we should not
start node and print error message.
*Proposed implementation:*
Instead of {{switch-case}} block new array of size 2^16 should be created.
Each array cell should contain message default constructor reference or lambda
which is responsible for message instance creation. So during message
unmarshaling system just lookup ctor-ref or lambda from array by index and
invoke it in order to create proper message instance.
All message factory extensions and custom message should be registered at the
same array before communication SPI will be started.
If we try to register message with direct type for which already exists
registered message then node start process must be terminated (directly, with
out any failure handlers).
If we get message with unknown direct type (there is now registered message for
this direct type) then failure handler be invoked but this is topic for
discussion and should be considered separately.
It could affect perfromance so we should run microbenchmark for this change.
Additional benefit of this change is improving code readability. At present we
have the following code:
{code:java}
@Override public Message create(short type) {
Message msg = null;
switch (type) {
case -61:
msg = new IgniteDiagnosticMessage();
break;
case -53:
msg = new SchemaOperationStatusMessage();
break;
// etc.
}
{code}
After refactoring it will looks like:
{code:java}
private Supplier<Message>[] msgCtrs;
public GridIoMessageFactory(MessageFactory[] ext) {
this.ext = ext;
registerMessage(-61, IgniteDiagnosticMessage::new)
registerMessage(-53, SchemaOperationStatusMessage::new)
}
@Override public Message create(short type) {
return msgCtros[type].get();
}
{code}
was:
Currently existing {{MessageFactory}} implementations (at least
{{GridIoMessageFactory}} and {{GridH2ValueMessageFactory}}) have serious
problem: it is possible to register several messages with the same direct type.
It could lead to the cluster freeze due to unexpected unmarshaling error.
*Solution:*
Each message should be registered and during registration we can check that
there is no registered message with the same type. Otherwise we should not
start node and print error message.
*Proposed implementation:*
Instead of {{switch-case}} block new array of size 2^16 should be created.
Each array cell should contain message default constructor reference or lambda
which is responsible for message instance creation. So during message
unmarshaling system just lookup ctor-ref or lambda from array by index and
invoke it in order to create proper message instance.
All message factory extensions and custom message should be registered at the
same array before communication SPI will be started.
If we try to register message with direct type for which already exists
registered message then node start process must be terminated (directly, with
out any failure handlers).
If we get message with unknown direct type (there is now registered message for
this direct type) then failure handler be invoked.
It could affect perfromance so we should run microbenchmark for this change.
Additional benefit of this change is improving code readability. At present we
have the following code:
{code:java}
@Override public Message create(short type) {
Message msg = null;
switch (type) {
case -61:
msg = new IgniteDiagnosticMessage();
break;
case -53:
msg = new SchemaOperationStatusMessage();
break;
// etc.
}
{code}
After refactoring it will looks like:
{code:java}
private Supplier<Message>[] msgCtrs;
public GridIoMessageFactory(MessageFactory[] ext) {
this.ext = ext;
registerMessage(-61, IgniteDiagnosticMessage::new)
registerMessage(-53, SchemaOperationStatusMessage::new)
}
@Override public Message create(short type) {
return msgCtros[type].get();
}
{code}
> MessageFactory implementations refactoring
> ------------------------------------------
>
> Key: IGNITE-12568
> URL: https://issues.apache.org/jira/browse/IGNITE-12568
> Project: Ignite
> Issue Type: Improvement
> Reporter: Andrey N. Gura
> Assignee: Andrey N. Gura
> Priority: Major
> Fix For: 2.9
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> Currently existing {{MessageFactory}} implementations (at least
> {{GridIoMessageFactory}} and {{GridH2ValueMessageFactory}}) have serious
> problem: it is possible to register several messages with the same direct
> type. It could lead to the cluster freeze due to unexpected unmarshaling
> error.
> *Solution:*
> Each message should be registered and during registration we can check that
> there is no registered message with the same type. Otherwise we should not
> start node and print error message.
> *Proposed implementation:*
> Instead of {{switch-case}} block new array of size 2^16 should be created.
> Each array cell should contain message default constructor reference or
> lambda which is responsible for message instance creation. So during message
> unmarshaling system just lookup ctor-ref or lambda from array by index and
> invoke it in order to create proper message instance.
> All message factory extensions and custom message should be registered at the
> same array before communication SPI will be started.
> If we try to register message with direct type for which already exists
> registered message then node start process must be terminated (directly, with
> out any failure handlers).
> If we get message with unknown direct type (there is now registered message
> for this direct type) then failure handler be invoked but this is topic for
> discussion and should be considered separately.
> It could affect perfromance so we should run microbenchmark for this change.
> Additional benefit of this change is improving code readability. At present
> we have the following code:
> {code:java}
> @Override public Message create(short type) {
> Message msg = null;
> switch (type) {
> case -61:
> msg = new IgniteDiagnosticMessage();
> break;
> case -53:
> msg = new SchemaOperationStatusMessage();
> break;
> // etc.
> }
> {code}
> After refactoring it will looks like:
> {code:java}
> private Supplier<Message>[] msgCtrs;
> public GridIoMessageFactory(MessageFactory[] ext) {
> this.ext = ext;
> registerMessage(-61, IgniteDiagnosticMessage::new)
> registerMessage(-53, SchemaOperationStatusMessage::new)
> }
> @Override public Message create(short type) {
> return msgCtros[type].get();
> }
> {code}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
