Skip to content

fix: use measurementId as location after offloading#801

Open
alexey-yarmosh wants to merge 1 commit intomasterfrom
offline-offloaded
Open

fix: use measurementId as location after offloading#801
alexey-yarmosh wants to merge 1 commit intomasterfrom
offline-offloaded

Conversation

@alexey-yarmosh
Copy link
Member

Fixes #783

@coderabbitai
Copy link

coderabbitai bot commented Feb 16, 2026

Walkthrough

This PR introduces centralized measurement retrieval logic in MeasurementStore that can fetch from either Redis or an offloader database. A new generic helper method getFromRedisOrOffloader handles the retrieval pattern, while getMeasurement and getMeasurementString provide typed accessors for measurement records and strings respectively. The implementation checks the offloader first for older or offloaded measurements and falls back to Redis for cache hits or recent data. When offloaded data is retrieved, optional parsing functions are applied if provided. Unit tests cover offloader preference, Redis fallback scenarios, error cases, and the distinction between recent and older measurements.

Possibly related PRs

  • PR 761: Adds offloader-aware measurement retrieval with similar fallback patterns between offloader and Redis, plus offloaded-key expiration handling.

Suggested reviewers

  • MartinKolarik
🚥 Pre-merge checks | ✅ 6
✅ Passed checks (6 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: implementing logic to use measurementId as location after offloading, which directly addresses the core objective.
Description check ✅ Passed The description links to issue #783, which is directly related to the changeset's objective of handling measurements after offloading.
Linked Issues check ✅ Passed The PR implements retrieval of offloaded measurements from the offloader/database when Redis lacks the measurement object, directly fulfilling issue #783's requirement to fetch measurements from SQL when gp:m:${id}:ip exists in Redis but the measurement does not.
Out of Scope Changes check ✅ Passed All changes focus on implementing offloader retrieval logic and comprehensive unit tests, remaining within the scope of addressing issue #783's requirement to support measurementId-as-location after offloading.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into master

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch offline-offloaded

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/measurement/store.ts (1)

71-101: ⚠️ Potential issue | 🟡 Minor

Add offloader support to getMeasurements for fallback retries.

The bulk getMeasurements method (line 103-105) queries only Redis and lacks offloader support. When a database insert fails and triggers a fallback retry (lines 119-127 in store-offloader.ts), insertBatchToDbByIds calls getMeasurements to re-fetch those measurements (line 177). For measurements older than 30 minutes that have already been offloaded to the database, this query will return null values and the retry will fail.

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.

Cannot use measurementId as location after offloading

1 participant

Comments