Skip to content

feat: scan with sst minimal sequence#6051

Merged
MichaelScofield merged 3 commits intomainfrom
feat/scan-with-sst-min-sequence
May 8, 2025
Merged

feat: scan with sst minimal sequence#6051
MichaelScofield merged 3 commits intomainfrom
feat/scan-with-sst-min-sequence

Conversation

@MichaelScofield
Copy link
Copy Markdown
Collaborator

I hereby agree to the terms of the GreptimeDB CLA.

Refer to a related PR or issue link (optional)

What's changed and what's your intention?

add a "minimal sequence" in scan request to filter out some unwanted SST files, useful in our "leader-read" feature

PR Checklist

Please convert it to a draft if some of the following conditions are not met.

  • I have written the necessary rustdoc comments.
  • I have added the necessary unit tests and integration tests.
  • This PR requires documentation updates.
  • API changes are backward compatible.
  • Schema or data changes are backward compatible.

@MichaelScofield MichaelScofield requested review from a team and waynexia as code owners May 6, 2025 07:38
@github-actions github-actions Bot added the docs-not-required This change does not impact docs. label May 6, 2025
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces functionality to filter out unwanted SST files during a scan operation by adding a new "sst_min_sequence" field to the scan request, which is propagated through various components of the system. Key changes include:

  • Adding the "sst_min_sequence" field and its documentation in the ScanRequest.
  • Propagating and processing the field in QueryContext and within the scan logic in the engine.
  • Updating tests to verify that the minimal SST sequence filtering works as expected.
  • Bumping the greptime-proto dependency version in Cargo.toml.

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/store-api/src/storage/requests.rs Added sst_min_sequence field with accompanying docs
src/session/src/context.rs Updated QueryContext and its builder to include sst_min_sequences
src/query/src/dummy_catalog.rs Adjusted TableProviderFactory to create TableProvider with new scan request
src/mito2/src/read/scan_region.rs Integrated sst_min_sequence filtering into scan logic
src/mito2/src/engine/scan_test.rs Added tests for sst_min_sequence filtering
src/mito2/src/engine/projection_test.rs Updated test to use Default for ScanRequest values
src/mito2/src/engine.rs Included new scan test module
src/metric-engine/src/metadata_region.rs Updated ScanRequest creation to use Default values
src/common/query/src/request.rs Added derive(Clone, Debug) to QueryRequest
Cargo.toml Upgraded greptime-proto dependency version

Comment thread src/store-api/src/storage/requests.rs Outdated
@MichaelScofield
Copy link
Copy Markdown
Collaborator Author

proto change: GreptimeTeam/greptime-proto#234

Comment thread Cargo.toml Outdated
Copy link
Copy Markdown
Contributor

@v0y4g3r v0y4g3r left a comment

Choose a reason for hiding this comment

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

LGTM

@v0y4g3r
Copy link
Copy Markdown
Contributor

v0y4g3r commented May 7, 2025

Please merge the proto pr and change the proto ref in Cargo.toml, then this PR can be merged.

MichaelScofield and others added 3 commits May 7, 2025 21:01
@MichaelScofield MichaelScofield force-pushed the feat/scan-with-sst-min-sequence branch from 3c5f6fa to e8f356a Compare May 7, 2025 13:06
@MichaelScofield MichaelScofield added this pull request to the merge queue May 8, 2025
Merged via the queue into main with commit e787007 May 8, 2025
68 of 74 checks passed
@MichaelScofield MichaelScofield deleted the feat/scan-with-sst-min-sequence branch May 8, 2025 01:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs-not-required This change does not impact docs.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants