Skip to content

Conversation

@David-Engel
Copy link
Collaborator

@David-Engel David-Engel commented Apr 7, 2022

The ICR feature brought in a previous timeout implementation that had drawbacks. This changes ICR to use the current SharedTimer, which is much more efficient.

This PR obsoletes #1782 and #1789 since it completely removes the class being updated in those two PRs.

EDIT: While testing, discovered that ICR also fails to reconnect a connection that has already been recovered once by ICR. This is also fixed in this PR.

@David-Engel David-Engel added this to the 11.1.1 milestone Apr 7, 2022
@lilgreenbird lilgreenbird changed the title Refactor ICR timeout to use existing SharedTimer Refactor Idle Connection Resiliency timeout to use existing SharedTimer Apr 8, 2022
eReceived = null;
stopRequested = false;
if (loggerExternal.isLoggable(Level.FINER)) {
loggerExternal.finer("ICR reconnect thread initialized. Connection retry count = " + connectRetryCount
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't use ICR acronym? (multiple occurences)

while ((connectRetryCount != 0) && (!stopRequested) && keepRetrying) {
while ((connectRetryCount > 0) && (!stopRequested) && keepRetrying) {
if (loggerExternal.isLoggable(Level.FINER)) {
loggerExternal.finer("Running ICR reconnect for command: " + command.toString());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] combine both to just 1 call?

Jeffery-Wasty
Jeffery-Wasty previously approved these changes Apr 12, 2022
VeryVerySpicy
VeryVerySpicy previously approved these changes Apr 12, 2022
tkyc
tkyc previously approved these changes Apr 12, 2022
@David-Engel David-Engel dismissed stale reviews from tkyc, VeryVerySpicy, and Jeffery-Wasty via 46326d8 April 13, 2022 22:23
This was referenced May 10, 2022
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.

5 participants