fix(evpp): reconnect stats correct in onclose during a failing retry loop (follow-up to #862) - #863
Merged
Merged
Conversation
…loop Follow-up to #862 (#615). That PR merged an earlier revision where isReconnect() returned reconn_success_cnt_ > 0, so while auto-reconnect was still failing (no success yet), isReconnect() read false in onclose -- not what users expect when checking 'am I in a reconnect loop?'. Track the cycle via reconn_retry_cnt_, updated when an attempt is SCHEDULED (in startReconnect), not only on success. It mirrors reconn_setting-> cur_retry_cnt but survives the reset done on a successful connect, so it is correct in all timings: first connect (false), onclose while retrying (true), and onopen/onConnection after a reconnect succeeds (true). API: bool isReconnect(); // reconn_retry_cnt_ > 0 uint32_t reconnectRetries(); // attempts in the current reconnect cycle uint32_t reconnectSuccessCount(); // number of successful auto-reconnects Renames the earlier lastReconnectRetries() -> reconnectRetries() (its meaning is now 'attempts in the current cycle', updated on schedule). Stats reset in setReconnect(NULL) (closesocket() too); reconfiguring keeps them.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #862 (#615). #862 merged an earlier revision of the reconnect-stats change; the final refinement discussed on the issue didn't make it in. This PR completes it.
Problem in the merged version
isReconnect()returnedreconn_success_cnt_ > 0. So while auto-reconnect is still failing (no successful reconnect yet),isReconnect()reads false inonclose— which is wrong when the user is asking "am I in a reconnect loop?". It also only counted on success, solastReconnectRetries()was 0 during the failing loop.Fix
Track the cycle via
reconn_retry_cnt_, updated when an attempt is scheduled (instartReconnect), not only on success. It mirrorsreconn_setting->cur_retry_cntbut survives the reset done on a successful connect, so the state is correct in all three timings.Renames the just-added
lastReconnectRetries()→reconnectRetries(), because its meaning is now "attempts in the current reconnect cycle" (updated on schedule, valid during the failing loop), not "failed attempts before the last success". Since #862 landed very recently, adjusting this name now avoids shipping the confusing semantics. Stats reset insetReconnect(NULL)(whichclosesocket()also does); reconfiguring with a non-NULL setting keeps them.Verified
make libhvbuilds clean.