[ 
https://issues.apache.org/jira/browse/IGNITE-13098?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17168827#comment-17168827
 ] 

Ignite TC Bot commented on IGNITE-13098:
----------------------------------------

{panel:title=Branch: [pull/8057/head] Base: [master] : Possible Blockers 
(2)|borderStyle=dashed|borderColor=#ccc|titleBGColor=#F7D6C1}
{color:#d04437}SPI{color} [[tests 
1|https://ci.ignite.apache.org/viewLog.html?buildId=5503233]]
* IgniteSpiTestSuite: TcpCommunicationStatisticsTest.testStatistics - Test has 
low fail rate in base branch 0,0% and is not flaky

{color:#d04437}Cache 6{color} [[tests 
1|https://ci.ignite.apache.org/viewLog.html?buildId=5503508]]
* IgniteCacheTestSuite6: 
IgniteExchangeLatchManagerCoordinatorFailTest.testCoordinatorFail1 - Test has 
low fail rate in base branch 0,0% and is not flaky

{panel}
{panel:title=Branch: [pull/8057/head] Base: [master] : New Tests 
(8)|borderStyle=dashed|borderColor=#ccc|titleBGColor=#D6F7C1}
{color:#00008b}Service Grid{color} [[tests 
4|https://ci.ignite.apache.org/viewLog.html?buildId=5501136]]
* {color:#013220}IgniteServiceGridTestSuite: 
ServiceDeploymentProcessIdSelfTest.topologyVersion[Test event=IgniteBiTuple 
[val1=DiscoveryEvent [evtNode=176233d1-4667-491e-adfd-0774678cd0c2, topVer=0, 
msgTemplate=null, span=null, nodeId8=e18a88e9, msg=, type=NODE_JOINED, 
tstamp=1596117993384], val2=AffinityTopologyVersion 
[topVer=3432296147845163674, minorTopVer=0]]] - PASSED{color}
* {color:#013220}IgniteServiceGridTestSuite: 
ServiceDeploymentProcessIdSelfTest.requestId[Test event=IgniteBiTuple 
[val1=DiscoveryEvent [evtNode=176233d1-4667-491e-adfd-0774678cd0c2, topVer=0, 
msgTemplate=null, span=null, nodeId8=e18a88e9, msg=, type=NODE_JOINED, 
tstamp=1596117993384], val2=AffinityTopologyVersion 
[topVer=3432296147845163674, minorTopVer=0]]] - PASSED{color}
* {color:#013220}IgniteServiceGridTestSuite: 
ServiceDeploymentProcessIdSelfTest.topologyVersion[Test event=IgniteBiTuple 
[val1=DiscoveryCustomEvent [customMsg=ServiceChangeBatchRequest 
[id=fa4e510a371-aff0a1db-0880-4bc9-9a19-3d6a4893fbdf, reqs=SingletonList 
[ServiceUndeploymentRequest []]], affTopVer=null, super=DiscoveryEvent 
[evtNode=8b0a15bd-5d97-45c3-930e-04e640926b8b, topVer=0, msgTemplate=null, 
span=null, nodeId8=8b0a15bd, msg=null, type=DISCOVERY_CUSTOM_EVT, 
tstamp=1596117993384]], val2=AffinityTopologyVersion 
[topVer=-7209573037103367773, minorTopVer=0]]] - PASSED{color}
* {color:#013220}IgniteServiceGridTestSuite: 
ServiceDeploymentProcessIdSelfTest.requestId[Test event=IgniteBiTuple 
[val1=DiscoveryCustomEvent [customMsg=ServiceChangeBatchRequest 
[id=fa4e510a371-aff0a1db-0880-4bc9-9a19-3d6a4893fbdf, reqs=SingletonList 
[ServiceUndeploymentRequest []]], affTopVer=null, super=DiscoveryEvent 
[evtNode=8b0a15bd-5d97-45c3-930e-04e640926b8b, topVer=0, msgTemplate=null, 
span=null, nodeId8=8b0a15bd, msg=null, type=DISCOVERY_CUSTOM_EVT, 
tstamp=1596117993384]], val2=AffinityTopologyVersion 
[topVer=-7209573037103367773, minorTopVer=0]]] - PASSED{color}

{color:#00008b}Service Grid (legacy mode){color} [[tests 
4|https://ci.ignite.apache.org/viewLog.html?buildId=5501137]]
* {color:#013220}IgniteServiceGridTestSuite: 
ServiceDeploymentProcessIdSelfTest.topologyVersion[Test event=IgniteBiTuple 
[val1=DiscoveryEvent [evtNode=e059769f-17e0-4527-b8a9-e9bfdde5d323, topVer=0, 
msgTemplate=null, span=null, nodeId8=0b19b3c9, msg=, type=NODE_JOINED, 
tstamp=1596118053728], val2=AffinityTopologyVersion [topVer=626854468514918512, 
minorTopVer=0]]] - PASSED{color}
* {color:#013220}IgniteServiceGridTestSuite: 
ServiceDeploymentProcessIdSelfTest.requestId[Test event=IgniteBiTuple 
[val1=DiscoveryEvent [evtNode=e059769f-17e0-4527-b8a9-e9bfdde5d323, topVer=0, 
msgTemplate=null, span=null, nodeId8=0b19b3c9, msg=, type=NODE_JOINED, 
tstamp=1596118053728], val2=AffinityTopologyVersion [topVer=626854468514918512, 
minorTopVer=0]]] - PASSED{color}
* {color:#013220}IgniteServiceGridTestSuite: 
ServiceDeploymentProcessIdSelfTest.topologyVersion[Test event=IgniteBiTuple 
[val1=DiscoveryCustomEvent [customMsg=ServiceChangeBatchRequest 
[id=02c9610a371-c143ee92-c429-4c53-8a68-3fec028e8023, reqs=SingletonList 
[ServiceUndeploymentRequest []]], affTopVer=null, super=DiscoveryEvent 
[evtNode=dd4526af-4480-49aa-9ead-7eb3f50f4bc8, topVer=0, msgTemplate=null, 
span=null, nodeId8=dd4526af, msg=null, type=DISCOVERY_CUSTOM_EVT, 
tstamp=1596118053728]], val2=AffinityTopologyVersion 
[topVer=315740683572685142, minorTopVer=0]]] - PASSED{color}
* {color:#013220}IgniteServiceGridTestSuite: 
ServiceDeploymentProcessIdSelfTest.requestId[Test event=IgniteBiTuple 
[val1=DiscoveryCustomEvent [customMsg=ServiceChangeBatchRequest 
[id=02c9610a371-c143ee92-c429-4c53-8a68-3fec028e8023, reqs=SingletonList 
[ServiceUndeploymentRequest []]], affTopVer=null, super=DiscoveryEvent 
[evtNode=dd4526af-4480-49aa-9ead-7eb3f50f4bc8, topVer=0, msgTemplate=null, 
span=null, nodeId8=dd4526af, msg=null, type=DISCOVERY_CUSTOM_EVT, 
tstamp=1596118053728]], val2=AffinityTopologyVersion 
[topVer=315740683572685142, minorTopVer=0]]] - PASSED{color}

{panel}
[TeamCity *--> Run :: All* 
Results|https://ci.ignite.apache.org/viewLog.html?buildId=5501159&buildTypeId=IgniteTests24Java8_RunAll]

> 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
>             Fix For: 2.10
>
>          Time Spent: 10m
>  Remaining Estimate: 0h
>
> 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)

Reply via email to