[IGNITE-12568] MessageFactory implementations refactoring

• Type: Improvement
• Status: Resolved
• Priority: Major
• Resolution: Fixed
• Fix Version/s: 2.9
• Assignee: Andrey N. Gura
• Reporter: Andrey N. Gura
• Created: 22/Jan/20 14:37
• Updated: 14/Feb/20 12:25
• Resolved: 04/Feb/20 15:17

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:

    @Override public Message create(short type) {
        Message msg = null;

        switch (type) {
            case -61:
                msg = new IgniteDiagnosticMessage();

                break;

            case -53:
                msg = new SchemaOperationStatusMessage();

                break;

                // etc.
    }

After refactoring it will looks like:

    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();
    }

Issue Links

• Dependent: IGNITE-11927 [IEP-35] Add ability to enable\disable subset of metrics
• is related to: IGNITE-12682 Remove IgniteMessageFactoryImpl.registerCustom() method
• links to: GitHub Pull Request #7302