[ https://issues.apache.org/jira/browse/GEODE-8589?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Kirk Lund updated GEODE-8589: ----------------------------- Affects Version/s: (was: 1.13.0) (was: 1.14.0) > make locatorsStopWaitingForLocatorWaitTimeIfAllLocatorsContacted test > deterministic > ----------------------------------------------------------------------------------- > > Key: GEODE-8589 > URL: https://issues.apache.org/jira/browse/GEODE-8589 > Project: Geode > Issue Type: Improvement > Components: membership > Affects Versions: 1.15.0 > Reporter: Bill Burcham > Priority: Major > Labels: membership, pull-request-available > > A recent commit added a new test: > {{MembershipIntegrationTest.locatorsStopWaitingForLocatorWaitTimeIfAllLocatorsContacted()}} > That's a good test. But it would be better if it ran faster and was less > susceptible to timing issues. The problem is that the logic we are trying to > test, {{GMSJoinLeave.leave()}} uses {{System.currentTimeMillis()}} to get the > current time and it uses {{Thread.sleep()}} to sleep: > {code:java} > long now = System.currentTimeMillis(); > ... > if (state.joinedMembersContacted <= 0 && ((now >= > locatorGiveUpTime && > tries >= minimumRetriesBeforeBecomingCoordinator) || > state.locatorsContacted >= locators.size())) { > ... > if (System.currentTimeMillis() > giveupTime) { > break; > } > ... > Thread.sleep(retrySleep); > ... > giveupTime = System.currentTimeMillis() + timeout; > ... > if (!this.isJoined) { > logger.debug("giving up attempting to join the distributed system > after " > + (System.currentTimeMillis() - startTime) + "ms"); > } > ... > if (!this.isJoined && state.hasContactedAJoinedLocator) { > throw new MemberStartupException("Unable to join the distributed > system in " > + (System.currentTimeMillis() - startTime) + "ms"); > } > {code} > The opportunity is to _inject_ objects that handle these two functions (time > keeping and sleeping). This will enable us to artifically control these in > our test! Sleeping doesn't have to take any time at all. And we can make time > pass as quickly as we want to. > For an example of how this can be done, see [PR > #5422|https://github.com/apache/geode/pull/5422] for GEODE-6950. > This particular test > ({{locatorsStopWaitingForLocatorWaitTimeIfAllLocatorsContacted()}}) is a > little bit more involved than that one in that more objects are involved. In > addition to {{GMSJoinLeave}}, other classes involved in the test also need > current time and need to sleep. > When this ticket is complete: > * functional interfaces {{MillisecondProvider}} and {{Sleeper}}, currently > defined inside {{PrimaryHandler}} will be moved higher in the (internal) > hierarchy for use by other membership classes > * {{GMSJoinLeave}} will take a {{MillisecondProvider}} and {{Sleeper}} in its > constructor and will delegate to those instead of calling > {{System.currentTimeMillis()}} and {{Thread.sleep()}} directly > * TBD other classes may require injection of {{MillisecondProvider}} and > {{Sleeper}} > * TBD other changes may be necessary in cases where collaborating classes > currently spin up threads or synchronize between threads e.g. calling > {{wait()}} > * {{locatorsStopWaitingForLocatorWaitTimeIfAllLocatorsContacted()}} will no > longer employ {{Thread.sleep()}} nor will it call > {{CompletableFuture.get(long timeout, TimeUnit unit)}}—instead it will > operate deterministically (ideally in the same thread but not necessarily) > with respect to the unit under test. -- This message was sent by Atlassian Jira (v8.20.7#820007)