log restore: filter backupmeta file by ts to speed up pitr#61347
log restore: filter backupmeta file by ts to speed up pitr#61347ti-chi-bot[bot] merged 7 commits intopingcap:masterfrom
Conversation
|
Skipping CI for Draft Pull Request. |
|
Hi @3pointer. Thanks for your PR. PRs from untrusted users cannot be marked as trusted with I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Signed-off-by: 3pointer <luancheng@pingcap.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #61347 +/- ##
================================================
+ Coverage 73.1815% 74.7513% +1.5698%
================================================
Files 1726 1729 +3
Lines 478595 489546 +10951
================================================
+ Hits 350243 365942 +15699
+ Misses 106890 101173 -5717
- Partials 21462 22431 +969
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This PR improves PITR performance by introducing timestamp‐based filtering for backup meta files. Key changes include new filtering logic via FilterPathByTs, corresponding updates in metadata scanning functions, and new unit tests verifying the filtering behavior.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| br/pkg/stream/stream_misc_test.go | Added comprehensive tests for the new FilterPathByTs function. |
| br/pkg/stream/stream_mgr.go | Introduced FilterPathByTs for timestamp-based filtering and updated FastUnmarshalMetaData usage. |
| br/pkg/stream/stream_metas.go | Adjusted FastUnmarshalMetaData invocation to match the new function signature. |
| br/pkg/restore/log_client/log_file_manager.go | Updated meta file processing to incorporate the new filtering logic and removed a redundant parameter. |
| br/pkg/restore/log_client/BUILD.bazel | Added a dependency on the tikv oracle package. |
| minTs, _ := strconv.ParseUint(matches[1], 10, 64) | ||
| maxTs, _ := strconv.ParseUint(matches[2], 10, 64) | ||
| minDefaultTs, _ := strconv.ParseUint(matches[3], 10, 64) |
There was a problem hiding this comment.
Consider handling potential errors from strconv.ParseUint instead of ignoring them, to prevent unintended behavior if the filename format is malformed.
| minTs, _ := strconv.ParseUint(matches[1], 10, 64) | |
| maxTs, _ := strconv.ParseUint(matches[2], 10, 64) | |
| minDefaultTs, _ := strconv.ParseUint(matches[3], 10, 64) | |
| minTs, err := strconv.ParseUint(matches[1], 10, 64) | |
| if err != nil { | |
| log.Error("failed to parse minTs from filename", zap.String("file", path), zap.Error(err)) | |
| return path | |
| } | |
| maxTs, err := strconv.ParseUint(matches[2], 10, 64) | |
| if err != nil { | |
| log.Error("failed to parse maxTs from filename", zap.String("file", path), zap.Error(err)) | |
| return path | |
| } | |
| minDefaultTs, err := strconv.ParseUint(matches[3], 10, 64) | |
| if err != nil { | |
| log.Error("failed to parse minDefaultTs from filename", zap.String("file", path), zap.Error(err)) | |
| return path | |
| } |
| streamBackupGlobalCheckpointPrefix = "v1/global_checkpoint" | ||
| ) | ||
|
|
||
| var metaPattern = regexp.MustCompile(`^(\d+)-(\d+)-(\d+)-[^/]+$`) |
There was a problem hiding this comment.
Perhaps we can encode the filename as hex so they can be compared byte by byte?
|
/hold |
|
/unhold |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Leavrth, YuJuncen The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
|
In response to a cherrypick label: new pull request created to branch |
…1347) (pingcap#62115) X-Original-Commit: b9e2ec9
What problem does this PR solve?
Issue Number: ref #61318
Problem Summary:
What changed and how does it work?
Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.