-
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
br/stream: Added toolkit for managing migrations #55665
base: master
Are you sure you want to change the base?
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Hi @YuJuncen. Thanks for your PR. PRs from untrusted users cannot be marked as trusted with I understand the commands that are listed here. Instructions 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. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #55665 +/- ##
=================================================
- Coverage 72.8489% 56.3302% -16.5188%
=================================================
Files 1604 1755 +151
Lines 445733 629786 +184053
=================================================
+ Hits 324712 354760 +30048
- Misses 101000 250345 +149345
- Partials 20021 24681 +4660
Flags with carried forward coverage won't be shown. Click here to find out more.
|
PR needs rebase. Instructions 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. |
make bazel prepare Signed-off-by: hillium <yujuncen@pingcap.com> fix linters Signed-off-by: hillium <yujuncen@pingcap.com> try fix the test case Signed-off-by: hillium <yujuncen@pingcap.com>
095f623
to
e183b20
Compare
Signed-off-by: hillium <yujuncen@pingcap.com>
@YuJuncen: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions 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/test-infra repository. I understand the commands that are listed here. |
err := os.Remove(path) | ||
if err != nil && | ||
l.IgnoreEnoentForDelete && | ||
stderrors.Is(err, syscall.ENOENT) { |
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.
local external storage also support windows, so can syscall.ENOENT
also match the error occur in windows system?
HandingMetaEditDone() | ||
} | ||
|
||
func NewProgressBarHooks(console glue.ConsoleOperations) *ProgressBarHooks { |
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.
is it dead code? or will be called outside?
|
||
me := new(pb.MetaEdit) | ||
me.Path = path | ||
for _, ds := range meta.FileGroups { |
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.
should we also append the me.DeleteLogicalFiles
because m.applyMetaEditTo
uses it?
|
||
// MergeAndMigrateTo will merge the migrations from BASE until the specified SN, then migrate to it. | ||
// Finally it writes the new BASE and remove stale migrations from the storage. | ||
func (m MigrationExt) MergeAndMigrateTo( |
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.
I found there are many functions only used in unit test, so I prefer to move these functions to another file with suffix _test.go
to read and check codes for production more clearly.
What problem does this PR solve?
Issue Number: close #55661
Problem Summary:
See the issue.
What changed and how does it work?
We added some new interfaces that helps to manipulate migrations. They are provided by a wrapper over
ExternalStorage
namedMigrationExt
.MergeAndMigrateTo
is the main interface for executing pending migrations.Load
loads the persisted migrations.Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.