epam / java-cme-mdp3-handler

Java Market Data Handler for CME Market Data (MDP 3.0)
GNU Lesser General Public License v3.0
75 stars 31 forks source link

GapChannelController numberOfTCPAttempts not reset after successful #54

Closed swarwick closed 5 years ago

swarwick commented 5 years ago

If the GapChannelController is using an executor and the TCP recovery the numberOfTCPAttempts does not get reset to 0 after successfully recovering and the result is that after every 2 TCP recovery events it gets forced into doing a full snapshot recovery.

Is there any reason that we cannot reset the numberOfTCPAttemps to 0 in the TCPRecoveryProcessor run() method if the result is true?

`

    @Override
    public void run() {
        try {
            boolean result = tcpMessageRequester.askForLostMessages(beginSeqNo, endSeqNo, tcpPacketListener);
            if (result) {
                try {
                    lock.lock();
                    switchState(ChannelState.SYNC);
                    processMessagesFromBuffer(feedContext);
                    //Should numberOfTCPAttemps be set to 0 here?
                } finally {
                    lock.unlock();
                }
            } else {
                snapshotRecoveryManager.startRecovery();
            }
        } catch (Exception e) {
            log.error(e.getMessage(), e);
        }
    }

`

iamolever commented 5 years ago

Hi,

Very likely we can reset here. But better to cover it with test(s), even if usage of mocks is expected (to avoid objects witch are unnecessary in the context).

swarwick commented 5 years ago

How do you even go about writing a test for sync/out of sync? I will have a look around to see if there is any existing to follow as a guideline but if you know of a test I should follow please let me know.

swarwick commented 5 years ago

Also is there any good reason the tcp channel connects and disconnects on every replay? Is there any reason we cannot stay connected and reconnect if not? or does CME disconnect after short no activity timeouts? (I have not looked into their spec on this).

iamolever commented 5 years ago

https://www.cmegroup.com/confluence/display/EPICSANDBOX/MDP+3.0+-+TCP+Recovery The responses are sent by CME Group through this same connection and the connection is then closed by CME Group once the replay is complete.

swarwick commented 5 years ago

I submitted a pull request to fix this