Fix #1790: discard test poll results from a finished sweep - #1806
Open
4RH1T3CT0R7 wants to merge 1 commit into
Open
Fix #1790: discard test poll results from a finished sweep#18064RH1T3CT0R7 wants to merge 1 commit into
4RH1T3CT0R7 wants to merge 1 commit into
Conversation
The ResultPoller ticks that drain partial results check sessionGen_ once, at the top of the tick, and then block in QueryURLTest / QueryIPTest / QueryCurrentSpeedTests / QueryCountryTestResults. ~ResultPoller only sets the stop flag and deliberately does not join, so a tick already inside its RPC outlives the sweep that created it. By the time that RPC returns the next sweep may have started: seedLatencyTest() has zeroed testProgress, and the tick then calls addTestProgress() once per result it drained. The new sweep's counter is credited with the previous run's leftovers, so the panel shows more than totalProfiles and the percentage runs past 100%. That is why it reproduces on the second Url Test Selected rather than the first. The same window misfiles results: BuildTestConfig numbers chain outbounds positionally (hopTag prefix-1, prefix-2, ...) and restarts the counter for every batch, so a stale tag2entID resolves a new sweep's tag to whatever profile held that slot before, and applyUrlResult writes the latency onto the wrong row. Re-check sessionGen_ after each query returns, before crediting progress or touching profiles. No profile data is lost by dropping a late drain: the owning sweep's Test / IPTest / SpeedTest response carries the full result set and its final pass applies all of it. The counter is a separate matter - the final pass never calls addTestProgress, so results a poll never reached already go uncounted today and the bar can stop short of 100%. This does not change that; it stops the count from being credited to the wrong run. pollSpeedTest and pollCountryTest take the generation as an argument since they run the query themselves.
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
1 participant
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.
~ResultPollerdoesn't join its tick, so a poll in a 30s RPC outlives the sweep that started it. It checkssessionGen_going in, not coming out, and credits what it drains to the sweep running now. That's the >100% bar:seedLatencyTest()has already zeroedtestProgressand the late tick callsaddTestProgress()anyway. Quieter half: chain tags are positional and the counter restarts each batch, so a staletag2entIDcan write a latency onto the wrong profile.Fix re-checks
sessionGen_after each query returns, before crediting progress or touching a profile. Dropping a late drain loses nothing, the owning sweep's response carries the full result set.One gap:
sessionGen_bumps per sweep, not per batch, so past 100 profiles a poll can still misattribute within a sweep. Happy to add afetch_addinrunBatchif you want that too.Closes #1790