[ https://issues.apache.org/jira/browse/IGNITE-13098?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17170720#comment-17170720 ]
Ignite TC Bot commented on IGNITE-13098: ---------------------------------------- {panel:title=Branch: [pull/8057/head] Base: [master] : No blockers found!|borderStyle=dashed|borderColor=#ccc|titleBGColor=#D6F7C1}{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=5510335]] * {color:#013220}IgniteServiceGridTestSuite: ServiceDeploymentProcessIdSelfTest.topologyVersion[Test event=IgniteBiTuple [val1=DiscoveryCustomEvent [customMsg=ServiceChangeBatchRequest [id=46acc15b371-74cb2904-83ac-4347-96c9-27716e55ae2a, reqs=SingletonList [ServiceUndeploymentRequest []]], affTopVer=null, super=DiscoveryEvent [evtNode=d7d06a3c-62c6-4a8a-abc7-65fe008865b0, topVer=0, msgTemplate=null, span=null, nodeId8=d7d06a3c, msg=null, type=DISCOVERY_CUSTOM_EVT, tstamp=1596470772632]], val2=AffinityTopologyVersion [topVer=-6491574591164370428, minorTopVer=0]]] - PASSED{color} * {color:#013220}IgniteServiceGridTestSuite: ServiceDeploymentProcessIdSelfTest.requestId[Test event=IgniteBiTuple [val1=DiscoveryCustomEvent [customMsg=ServiceChangeBatchRequest [id=46acc15b371-74cb2904-83ac-4347-96c9-27716e55ae2a, reqs=SingletonList [ServiceUndeploymentRequest []]], affTopVer=null, super=DiscoveryEvent [evtNode=d7d06a3c-62c6-4a8a-abc7-65fe008865b0, topVer=0, msgTemplate=null, span=null, nodeId8=d7d06a3c, msg=null, type=DISCOVERY_CUSTOM_EVT, tstamp=1596470772632]], val2=AffinityTopologyVersion [topVer=-6491574591164370428, minorTopVer=0]]] - PASSED{color} * {color:#013220}IgniteServiceGridTestSuite: ServiceDeploymentProcessIdSelfTest.topologyVersion[Test event=IgniteBiTuple [val1=DiscoveryEvent [evtNode=dd270f16-4836-4fa7-9f9c-b9b8be3961ce, topVer=0, msgTemplate=null, span=null, nodeId8=372ef472, msg=, type=NODE_JOINED, tstamp=1596470772632], val2=AffinityTopologyVersion [topVer=-8763859702679294079, minorTopVer=0]]] - PASSED{color} * {color:#013220}IgniteServiceGridTestSuite: ServiceDeploymentProcessIdSelfTest.requestId[Test event=IgniteBiTuple [val1=DiscoveryEvent [evtNode=dd270f16-4836-4fa7-9f9c-b9b8be3961ce, topVer=0, msgTemplate=null, span=null, nodeId8=372ef472, msg=, type=NODE_JOINED, tstamp=1596470772632], val2=AffinityTopologyVersion [topVer=-8763859702679294079, minorTopVer=0]]] - PASSED{color} {color:#00008b}Service Grid (legacy mode){color} [[tests 4|https://ci.ignite.apache.org/viewLog.html?buildId=5510336]] * {color:#013220}IgniteServiceGridTestSuite: ServiceDeploymentProcessIdSelfTest.topologyVersion[Test event=IgniteBiTuple [val1=DiscoveryEvent [evtNode=dc8cf342-6566-4b21-963e-9e7c97c6f990, topVer=0, msgTemplate=null, span=null, nodeId8=7e1f90b0, msg=, type=NODE_JOINED, tstamp=1596470697262], val2=AffinityTopologyVersion [topVer=4988132546913497014, minorTopVer=0]]] - PASSED{color} * {color:#013220}IgniteServiceGridTestSuite: ServiceDeploymentProcessIdSelfTest.requestId[Test event=IgniteBiTuple [val1=DiscoveryEvent [evtNode=dc8cf342-6566-4b21-963e-9e7c97c6f990, topVer=0, msgTemplate=null, span=null, nodeId8=7e1f90b0, msg=, type=NODE_JOINED, tstamp=1596470697262], val2=AffinityTopologyVersion [topVer=4988132546913497014, minorTopVer=0]]] - PASSED{color} * {color:#013220}IgniteServiceGridTestSuite: ServiceDeploymentProcessIdSelfTest.topologyVersion[Test event=IgniteBiTuple [val1=DiscoveryCustomEvent [customMsg=ServiceChangeBatchRequest [id=23d9115b371-c0207a62-1b23-4f7a-bc98-94d1344b04d0, reqs=SingletonList [ServiceUndeploymentRequest []]], affTopVer=null, super=DiscoveryEvent [evtNode=c20579c7-aadb-4f7f-82e1-9bef66bda40a, topVer=0, msgTemplate=null, span=null, nodeId8=c20579c7, msg=null, type=DISCOVERY_CUSTOM_EVT, tstamp=1596470697262]], val2=AffinityTopologyVersion [topVer=8457328299814147369, minorTopVer=0]]] - PASSED{color} * {color:#013220}IgniteServiceGridTestSuite: ServiceDeploymentProcessIdSelfTest.requestId[Test event=IgniteBiTuple [val1=DiscoveryCustomEvent [customMsg=ServiceChangeBatchRequest [id=23d9115b371-c0207a62-1b23-4f7a-bc98-94d1344b04d0, reqs=SingletonList [ServiceUndeploymentRequest []]], affTopVer=null, super=DiscoveryEvent [evtNode=c20579c7-aadb-4f7f-82e1-9bef66bda40a, topVer=0, msgTemplate=null, span=null, nodeId8=c20579c7, msg=null, type=DISCOVERY_CUSTOM_EVT, tstamp=1596470697262]], val2=AffinityTopologyVersion [topVer=8457328299814147369, minorTopVer=0]]] - PASSED{color} {panel} [TeamCity *--> Run :: All* Results|https://ci.ignite.apache.org/viewLog.html?buildId=5510358&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)