[
https://issues.apache.org/jira/browse/IGNITE-12568?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17022048#comment-17022048
]
Andrey N. Gura commented on IGNITE-12568:
-----------------------------------------
[~ibessonov] [~agoncharuk] It seems that encapsulation is not problem if we'll
introduce some method that will return all message types and message suppliers.
{{MessageFactory}} interface is part of public API and it is a problem. So I
propose the following:
Introduce {{MessageFactoryEx}} interface as temp solution (should be changed in
Apache Ignite 3.0)
{code:java}
public interface MessageFactoryEx extends MessageFactory {
/**
* Returns set of all message suppliers with their direct types.
*/
Set<Tuple<Short, Supplier<Message>>> messages();
}
{code}
{{GridIoMessageFactory}} will invoke {{messages()}} method for all extensions
if they implements {{MessageFactory}} interface.
Another solution requires two entities. {{IgniteMessageFactory}} class that is
aggregate for all message factories and {{MessageFactoryProvider}} that is
message suppliers provider:
{code:java}
public interface MessageFactoryProvider extends MessageFactory {
/**
* Register all message suppliers.
*/
void registerAll(IgniteMessageFactory factory);
}
{code}
{code:java}
/**
* Implementation of this interface
*/
public class IgniteMessageFactory implements MessageFactoryProvider {
private final Supplier<Message>[] msgSuppliers = new
Supplier<Message>[2^16];
// Here GridIoMessageFactory also will passed as exts.
public IgniteMessageFactory(MessageFactory[] exts) {
// Pseudocode
Set<MessageFactory> oldFactories = new HashSet();
// Iter over exts and register all messages form newly introsuced
providers
// if ext instaceof MessageFactoryProvider
// ext.registerAll(this);
// else
// oldFactories.add(ext);
// brute force
// iter over msgSuppliers array and for empty elements try to create
messages via oldFactories
// if oldFactory.create(type) != null
registerMesage(type, oldFactory::create)
}
/**
* Creates and returns messages instances of given type.
*/
public Message create(short type) {
// implementation
}
/**
* Registers supplier for given message type. This method should be called
by MessageFactoryEx implementation.
*/
public void registerMessage(short type, Supplier<Message> supplier) {
// implementation
}
public void registerAll(IgniteMessageFactory factory) {
registerMessage(1, SomeMessage::new)
...
}
}
{code}
This solution provides backward compatibility and allows detect messages with
the same direct type.
WDYT?
> 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
>
> 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}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
