Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get rid of unreliable TimerTest #408

Merged
merged 1 commit into from
Jul 29, 2021
Merged

Get rid of unreliable TimerTest #408

merged 1 commit into from
Jul 29, 2021

Conversation

chrjohn
Copy link
Member

@chrjohn chrjohn commented Jul 28, 2021

Follow-up to #345

IIRC only on the MacOS Java CI jobs we could observe that the next() method was triggered unreliably (usually this should be called every second). As a result the check whether a Heartbeat should be sent was sometimes called too late. This lead to failures in the TimerTest.

2020-12-31T22:09:00.0063950Z <20201231-22:09:00, FIX.4.4:TW->ISLD, incoming> (8=FIX.4.4�9=60�35=A�34=1�49=ISLD�52=20201231-22:09:00.005�56=TW�98=0�108=2�10=143�)
2020-12-31T22:09:00.0065060Z <20201231-22:09:00, FIX.4.4:TW->ISLD, event> (Received logon)
2020-12-31T22:09:00.0066220Z XXX FIX.4.4:TW->ISLD isHeartBeatNeeded() = false
# more than one second pause
2020-12-31T22:09:01.0259920Z XXX FIX.4.4:ISLD->TW isHeartBeatNeeded() = false
2020-12-31T22:09:01.0261070Z XXX FIX.4.4:TW->ISLD isHeartBeatNeeded() = false
# less than one second pause
2020-12-31T22:09:01.9287190Z XXX FIX.4.4:ISLD->TW isHeartBeatNeeded() = false
2020-12-31T22:09:01.9288290Z XXX FIX.4.4:TW->ISLD isHeartBeatNeeded() = false
# more than one second pause
2020-12-31T22:09:03.0341840Z <20201231-22:09:03, FIX.4.4:ISLD->TW, outgoing> (8=FIX.4.4�9=58�35=1�34=2�49=ISLD�52=20201231-22:09:03.029�56=TW�112=TEST�10=186�)
2020-12-31T22:09:03.0343040Z <20201231-22:09:03, FIX.4.4:ISLD->TW, event> (Sent test request TEST)

I decided to get rid of the TimerTest since in the wild this unreliable triggering of heartbeats would be not a big problem and especially the heartbeat interval of 2 seconds that the test used was ridiculously low. So for a normal heartbeat interval of 30 seconds there is no problem when the heartbeat is generated say 100ms too late.

TimerTest was replaced by a simple unit test with a few lines:

@Test
public void testHeartbeatTiming() {
// we set a HB interval of 2 seconds = 2000ms
SessionState state = new SessionState(new Object(), null, 2 /* HB interval */, false, null,
Session.DEFAULT_TEST_REQUEST_DELAY_MULTIPLIER, Session.DEFAULT_HEARTBEAT_TIMEOUT_MULTIPLIER);
long now = System.currentTimeMillis();
timeSource.setSystemTimes(now);
state.setLastSentTime(now);
assertFalse("heartbeat shouldn't be needed yet", state.isHeartBeatNeeded());
timeSource.increment(1000);
assertFalse("heartbeat shouldn't be needed yet", state.isHeartBeatNeeded());
timeSource.increment(1000);
// current time is now 2000ms further since the start, i.e. the HB interval has elapsed
assertTrue("heartbeat should be needed", state.isHeartBeatNeeded());
}

@chrjohn chrjohn added this to the QFJ 3.0.0 milestone Jul 28, 2021
@chrjohn chrjohn changed the title Get rid of unreliable TimerTest. Get rid of unreliable TimerTest Jul 29, 2021
@chrjohn chrjohn merged commit 7b873f7 into master Jul 29, 2021
@chrjohn chrjohn deleted the ditch-timer-test branch July 29, 2021 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant