feat: Improve search speed by chunking long time range searches into smaller incremental search windows. #1125
Conversation
🦋 Changeset detectedLatest commit: dd4e2d9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…smaller incremental search windows.
MikeShi42
force-pushed
the
mikeshi/window-large-time-range-searches
branch
from
3ee375f to
4e5b55e
Compare
Stably Runner - Test Suite - 'Smoke Test'Test Suite Run Result: 🟢 Success (4/4 tests passed) [dashboard] This comment was generated from stably-runner-action |
There was a problem hiding this comment.
Pull Request Overview
This PR implements a progressive time windowing strategy to improve search performance for long time range queries by chunking them into smaller incremental search windows (6h, 6h, 12h, 24h).
- Refactors pagination to support time-windowed searches with progressive bucketing
- Updates UI to display loading progress with current search date
- Adds changeset documenting the feature enhancement
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/app/src/hooks/useOffsetPaginatedQuery.tsx | Implements time windowing logic and updates pagination to work with windowed queries |
| packages/app/src/components/DBRowTable.tsx | Adds loading date display to show search progress |
| .changeset/stale-brooms-sparkle.md | Documents the feature change |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| meta: data.pages[0].meta, | ||
| data: flattenPages(data.pages), | ||
| chSql: data.pages[0].chSql, | ||
| window: data.pages[data.pages.length - 1].window, |
There was a problem hiding this comment.
Using the last page's window in flattenData may not represent the correct time window for the flattened data. Consider using the first page's window or creating a merged window that spans from the first to last page's time range.
| if (window == null) { | ||
| throw new Error('Invalid time window for page param'); |
There was a problem hiding this comment.
The error message is too generic. It should include the windowIndex value to help with debugging: Invalid time window for page param with windowIndex: ${pageParam.windowIndex}
| if (window == null) { | |
| throw new Error('Invalid time window for page param'); | |
| throw new Error(`Invalid time window for page param with windowIndex: ${pageParam.windowIndex}`); |
|
Looks good in my limited testing. Code is clean too. I have a PR to add some unit tests, which I've written as a way to test this. |
|
tests pr #1132 |
Closes HDX-2407 This PR fixes searches which sort in ascending time order or do not order based on time. With this change, time-based "chunking" of queries (#1125) will only be used when the results are ordered by time. Further, when ordering by time _ascending_, the chunks will load in ascending time order (rather than descending time order).
Resolves HDX-1035