-
Notifications
You must be signed in to change notification settings - Fork 5.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
log-backup: filter out unnecessary metadata files #36656
Conversation
Signed-off-by: Yu Juncen <yujuncen@pingcap.com>
Signed-off-by: Yu Juncen <yujuncen@pingcap.com>
[REVIEW NOTIFICATION] This pull request has been approved by:
To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by submitting an approval review. |
Signed-off-by: Yu Juncen <yujuncen@pingcap.com>
Signed-off-by: Yu Juncen <yujuncen@pingcap.com>
Code Coverage Details: https://codecov.io/github/pingcap/tidb/commit/888e8fbfac647337ab5c72e460f013d6b2835d81 |
Signed-off-by: Yu Juncen <yujuncen@pingcap.com>
Signed-off-by: Yu Juncen <yujuncen@pingcap.com>
Signed-off-by: Yu Juncen <yujuncen@pingcap.com>
Signed-off-by: Yu Juncen <yujuncen@pingcap.com>
/rebuild |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rest LGTM
@@ -25,8 +26,7 @@ type StreamMetadataSet struct { | |||
BeforeDoWriteBack func(path string, last, current *backuppb.Metadata) (skip bool) | |||
} | |||
|
|||
// LoadFrom loads data from an external storage into the stream metadata set. | |||
func (ms *StreamMetadataSet) LoadFrom(ctx context.Context, s storage.ExternalStorage) error { | |||
func (ms *StreamMetadataSet) LoadUntil(ctx context.Context, s storage.ExternalStorage, until uint64) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can add some comments
Signed-off-by: Yu Juncen <yujuncen@pingcap.com>
/hold |
Signed-off-by: Yu Juncen <yujuncen@pingcap.com>
/unhold |
/merge |
This pull request has been accepted and is ready to merge. Commit hash: cdafeff
|
/run-check_dev |
1 similar comment
/run-check_dev |
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
cherry pick to release-6.2 in PR #36708 |
TiDB MergeCI notify
|
What problem does this PR solve?
Issue Number: close #36648
Problem Summary:
Even we are going to truncate / restore a subset of all logs, the current implementation would try to read all metadata.
This may lead to OOM when there are too many meta files.
What is changed and how it works?
After this PR, when we executing
restore point
orlog truncate
, we would filter out metadata that is obviously should not be considered. They are:For
restore point
:MinTS > RestoreTS || MaxTS < ShiftedStartTS
For
log truncate
:MinTS > UntilTS
Check List
Tests
The image above shows the different memory usage between restoring full log and restoring a subset of log.
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.