[
https://issues.apache.org/jira/browse/IGNITE-13098?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Stepachev Maksim updated IGNITE-13098:
--------------------------------------
Description:
h2. Description
This ticket describes requirements for TcpCommunicationSpi refactoring. The
main goal is to split the class without changing behavior and public API.
*Actual problem:*
CurrentlyTcpCommunicationSpi has over 5K lines and includes about15+ inner
classes like:
# ShmemAcceptWorker
# SHMemHandshakeClosure
# ShmemWorker
# CommunicationDiscoveryEventListener
# CommunicationWorker
# ConnectFuture
# ConnectGateway
# ConnectionKey
# ConnectionPolicy
# DisconnectedSessionInfo
# FirstConnectionPolicy
# HandshakeTimeoutObject
# RoundRobinConnectionPolicy
# TcpCommunicationConnectionCheckFuture
# TcpCommunicationSpiMBeanImpl
In addition, it contains logic of client connection life cycle, nio server
handler, and handshake handler.
The classes above have cyclic dependencies and high coupling.The whole
mechanism works because classes have access to each other via parent class
references. As a result, initialization of class isn't consistent. By
consistent I mean that class created via constructor is ready to be used. All
of the classes work with context and shareproperties everywhere.
Many methods of TcpCommunicationSpi don’t have a single responsibility. Example
is getNodeAttribute:,it makes client reservation, takes the IP address of the
node and provides attributes.
It works fine and we usually don’t have reasons to change anything. But if you
want to create a test that has a little different behavior than a blocking
message, you can't mock or change the behavior of inner classes. For example,
test covering change in the handshake process. Some people make test methods in
public API like "closeConnections" or "openSocketChannel" because the current
design isn't fine for it. It also takes a lot of time for test development for
minor changes.
*Solution:*
The scope of work is big and communication spi is place which should be changed
carefully. I recommend to make this refactoring step by step.
* The first idea is to split the parent class into independent classes and
move them to the internal package. We should achieveSOLID when it’s done.
* Extract spread logic to appropriate classes like ClientPool,
HandshakeHandler, etc.
* Make a common transfer object for TCSpi configuration.
* Make dependencies direct if it is possible.
* Initialize all dependencies in one place.
* Make child classes context-free.
* Try to do classes more testable.
* Use the idea of dependency injection without a framework for it.
*Benefits:*
With the ability to write truly jUnit-style tests and cover functionality with
better testing we get a way to easier develop new features and optimizations
needed in such low-level components as TcpCommunicationSpi.
Examples of features that improve usability of Apache Ignite a lot are: inverse
communication connection with optimizations and connection multiplexing. Both
of the features could be used in environments with restricted network
connectivity (e.g. when connections between nodes could be established only in
one direction).
was:TcpCommunicationSpi split to independent classes
> TcpCommunicationSpi split to independent classes
> ------------------------------------------------
>
> Key: IGNITE-13098
> URL: https://issues.apache.org/jira/browse/IGNITE-13098
> Project: Ignite
> Issue Type: Bug
> Environment: TcpCommunicationSpi split to independent classes
> Reporter: Stepachev Maksim
> Assignee: Stepachev Maksim
> Priority: Major
>
> h2. Description
> This ticket describes requirements for TcpCommunicationSpi refactoring. The
> main goal is to split the class without changing behavior and public API.
> *Actual problem:*
> CurrentlyTcpCommunicationSpi has over 5K lines and includes about15+ inner
> classes like:
> # ShmemAcceptWorker
> # SHMemHandshakeClosure
> # ShmemWorker
> # CommunicationDiscoveryEventListener
> # CommunicationWorker
> # ConnectFuture
> # ConnectGateway
> # ConnectionKey
> # ConnectionPolicy
> # DisconnectedSessionInfo
> # FirstConnectionPolicy
> # HandshakeTimeoutObject
> # RoundRobinConnectionPolicy
> # TcpCommunicationConnectionCheckFuture
> # TcpCommunicationSpiMBeanImpl
> In addition, it contains logic of client connection life cycle, nio server
> handler, and handshake handler.
> The classes above have cyclic dependencies and high coupling.The whole
> mechanism works because classes have access to each other via parent class
> references. As a result, initialization of class isn't consistent. By
> consistent I mean that class created via constructor is ready to be used. All
> of the classes work with context and shareproperties everywhere.
> Many methods of TcpCommunicationSpi don’t have a single responsibility.
> Example is getNodeAttribute:,it makes client reservation, takes the IP
> address of the node and provides attributes.
> It works fine and we usually don’t have reasons to change anything. But if
> you want to create a test that has a little different behavior than a
> blocking message, you can't mock or change the behavior of inner classes. For
> example, test covering change in the handshake process. Some people make test
> methods in public API like "closeConnections" or "openSocketChannel" because
> the current design isn't fine for it. It also takes a lot of time for test
> development for minor changes.
> *Solution:*
> The scope of work is big and communication spi is place which should be
> changed carefully. I recommend to make this refactoring step by step.
> * The first idea is to split the parent class into independent classes and
> move them to the internal package. We should achieveSOLID when it’s done.
> * Extract spread logic to appropriate classes like ClientPool,
> HandshakeHandler, etc.
> * Make a common transfer object for TCSpi configuration.
> * Make dependencies direct if it is possible.
> * Initialize all dependencies in one place.
> * Make child classes context-free.
> * Try to do classes more testable.
> * Use the idea of dependency injection without a framework for it.
> *Benefits:*
> With the ability to write truly jUnit-style tests and cover functionality
> with better testing we get a way to easier develop new features and
> optimizations needed in such low-level components as TcpCommunicationSpi.
> Examples of features that improve usability of Apache Ignite a lot are:
> inverse communication connection with optimizations and connection
> multiplexing. Both of the features could be used in environments with
> restricted network connectivity (e.g. when connections between nodes could be
> established only in one direction).
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
