Skip to content

fix(core): remove double offset in GetHistoricalServerStats pagination#1533

Open
CaiJingLong wants to merge 1 commit into
moghtech:mainfrom
CaiJingLong:fix/issue-1532-historical-stats-pagination
Open

fix(core): remove double offset in GetHistoricalServerStats pagination#1533
CaiJingLong wants to merge 1 commit into
moghtech:mainfrom
CaiJingLong:fix/issue-1532-historical-stats-pagination

Conversation

@CaiJingLong

@CaiJingLong CaiJingLong commented Jul 21, 2026

Copy link
Copy Markdown

Related Issue

Closes #1532

Problem

GetHistoricalServerStats applies the page offset twice: 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 { ts: { $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 returns an empty stats array and next_page becomes None, silently losing all older historical data.

Change

  • bin/core/src/api/read/server.rs, GetHistoricalServerStats::resolve: removed the .skip(page * STATS_PER_PAGE) call from FindOptions::builder(). ts_vec already encodes the page offset via the time window shift, so the $in filter is the correct paging mechanism here — no additional skip is needed.

Verification

  • cargo check -p komodo_core passes (no new warnings).
  • cargo fmt -p komodo_core --check passes.
  • Cross-checked against the issue reproduction: page = 0 behavior is unchanged (.skip(0) was already a no-op); page >= 1 is no longer wiped out by the second offset and now returns the documents matching the ts_vec time window, with next_page correctly indicating whether an older window has data.
  • bin/core has 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's 1-min multi-page reproduction steps.

Risk

  • Change is a single-line removal; does not affect the page = 0 path. The ts_vec construction and next_page logic are unchanged. No schema migration, config change, or breaking API change.
  • Note: next_page is Some(page + 1) only when the page returns exactly STATS_PER_PAGE records. If a page returns fewer than 200 due to missing data while an older window still has data, next_page will be None — this is pre-existing behavior, unchanged by this fix, which only removes the double offset that caused the spurious empty results.

Conventions

  • The repository has no explicit CONTRIBUTING.md / CLAUDE.md / AGENTS.md / PR template, so common conventions (Conventional Commits + default PR structure) are used.

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
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.

GetHistoricalServerStats returns empty results after the first page

1 participant