fix(core): remove double offset in GetHistoricalServerStats pagination#1533
Open
CaiJingLong wants to merge 1 commit into
Open
fix(core): remove double offset in GetHistoricalServerStats pagination#1533CaiJingLong wants to merge 1 commit into
CaiJingLong wants to merge 1 commit into
Conversation
The page offset was applied twice when paginating historical server stats: once when building ts_vec (shifting the time window back by granularity * STATS_PER_PAGE * page), and again via .skip(page * STATS_PER_PAGE) on the MongoDB query. Since the $in: ts_vec filter already restricts the result to at most STATS_PER_PAGE documents for that page, the subsequent .skip(page * STATS_PER_PAGE) discards every matched document when page >= 1, so every page after the first always returned an empty stats array. Remove the redundant .skip() call; the $in filter is the correct paging mechanism here. Closes moghtech#1532
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.
Related Issue
Closes #1532
Problem
GetHistoricalServerStatsapplies thepageoffset twice: once when buildingts_vec(shifting the time window back bygranularity * STATS_PER_PAGE * page), and again via.skip(page * STATS_PER_PAGE)on the MongoDB query. Since the{ ts: { $in: ts_vec } }filter already restricts the result to at mostSTATS_PER_PAGEdocuments for that page, the subsequent.skip(page * STATS_PER_PAGE)discards every matched document whenpage >= 1, so every page after the first always returns an emptystatsarray andnext_pagebecomesNone, silently losing all older historical data.Change
bin/core/src/api/read/server.rs,GetHistoricalServerStats::resolve: removed the.skip(page * STATS_PER_PAGE)call fromFindOptions::builder().ts_vecalready encodes the page offset via the time window shift, so the$infilter is the correct paging mechanism here — no additional skip is needed.Verification
cargo check -p komodo_corepasses (no new warnings).cargo fmt -p komodo_core --checkpasses.page = 0behavior is unchanged (.skip(0)was already a no-op);page >= 1is no longer wiped out by the second offset and now returns the documents matching thets_vectime window, withnext_pagecorrectly indicating whether an older window has data.bin/corehas no existing unit-test infrastructure and the function depends on a live MongoDB runtime, so no new tests were added; the fix can be verified manually via the issue's1-minmulti-page reproduction steps.Risk
page = 0path. Thets_vecconstruction andnext_pagelogic are unchanged. No schema migration, config change, or breaking API change.next_pageisSome(page + 1)only when the page returns exactlySTATS_PER_PAGErecords. If a page returns fewer than 200 due to missing data while an older window still has data,next_pagewill beNone— this is pre-existing behavior, unchanged by this fix, which only removes the double offset that caused the spurious empty results.Conventions
CONTRIBUTING.md/CLAUDE.md/AGENTS.md/ PR template, so common conventions (Conventional Commits + default PR structure) are used.