fix(mito): use 1day as default time partition duration #6202
Conversation
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
evenyag
reviewed
May 29, 2025
## Add Helper Functions and Enhance Compaction Tests - **Refactor Compaction Logic**: Introduced helper functions `flush` and `compact` in `compaction_test.rs` to streamline compaction operations. - **Enhance Compaction Tests**: Added a new test `test_infer_compaction_time_window` in `compaction_test.rs` to verify compaction time window inference. - **Testing Improvements**: Added `#[cfg(test)]` attribute to `new_multi_partitions` in `time_partition.rs` to ensure it's only included in test builds.
- **Refactor `TimePartition` Struct**: Removed unnecessary comments regarding `time_range` in `time_partition.rs`. - **Enhance `TimePartitions` Functionality**: Added a method `part_duration_or_default` to provide a default partition duration in `time_partition.rs`. - **Update SQL Test Cases**: Modified SQL operations and expected results in `scan_big_varchar.result` and `scan_big_varchar.sql` to reflect changes in data manipulation logic.
### Update Time Partition Default Duration - **Refactor Default Duration**: Introduced `INITIAL_TIME_WINDOW` constant to define the default time window duration as `Duration::from_days(1)`. This change replaces multiple instances of the hardcoded default duration across the `time_partition.rs` file. - **Files Affected**: `time_partition.rs`
## Update Partition Duration Handling - **`time_partition.rs`**: Refactored `part_duration` to be non-optional, removing `Option` wrapper. Updated logic to use `unwrap_or` with `INITIAL_TIME_WINDOW` where necessary. Adjusted related methods and tests to accommodate this change. - **`version.rs` (memtable and region)**: Updated handling of `part_duration` to align with changes in `time_partition.rs`, ensuring consistent use of non-optional `Duration`.
v0y4g3r
force-pushed
the
fix/default-time-window
branch
from
911482e to
47abc22
Compare
evenyag
approved these changes
Jun 4, 2025
### Improve Error Context in `time_partition.rs` - Enhanced error context message in `time_partition.rs` to provide clearer information on partition time range issues, including bucket size details. Signed-off-by: Lei, HUANG <[email protected]>
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.
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?
This PR changes the default partition duration from infinite to 1day.
TimeParitionsaims to split inserted data according to a "part_duration". Originally the part_duration is not present, so all rows are written to a default time partition without time range. When the first compaction is triggered, GreptiemDB will infer a proper time window according to flushed files.However this would cause problem when the initial data covers a very large time span, for example, 10 years. Then we would end up with an SST file covers a large time span, which would further make it overlap with possibly every new SST files.
In this PR, we make 1day the default
part_durationwhen there's no compaction time window specified while creating tables. This default value should suit most cases.PR Checklist
Please convert it to a draft if some of the following conditions are not met.