-
Notifications
You must be signed in to change notification settings - Fork 24
[to #48] try to fix scheduler problem #90
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
Conversation
Signed-off-by: zeminzhou <zhouzemin@ping.com>
|
TODO: Unit Test. |
Signed-off-by: zeminzhou <zhouzemin@ping.com>
Signed-off-by: zeminzhou <zhouzemin@ping.com>
Codecov Report
@@ Coverage Diff @@
## main #90 +/- ##
================================================
+ Coverage 53.4580% 61.2819% +7.8239%
================================================
Files 197 180 -17
Lines 19838 14275 -5563
================================================
- Hits 10605 8748 -1857
+ Misses 8265 4910 -3355
+ Partials 968 617 -351
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| return shouldUpdateState, nil | ||
| } | ||
|
|
||
| func (s *oldScheduler) diffCurrentKeySpans(currentKeySpans map[model.KeySpanID]regionspan.Span) (map[model.KeySpanID]struct{}, []model.KeySpanID) { |
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.
Please add unit test for this method.
| // syncKeySpansWithCurrentKeySpans iterates all current keyspans to check whether it should be listened or not. | ||
| // this function will return schedulerJob to make sure all keyspans will be listened. | ||
| func (s *oldScheduler) syncKeySpansWithCurrentKeySpans() ([]*schedulerJob, error) { | ||
| func (s *oldScheduler) syncKeySpansWithCurrentKeySpans(newKeySpans map[model.KeySpanID]struct{}, needRemoveKeySpans []model.KeySpanID) ([]*schedulerJob, 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.
Suggest to add unit test for this method.
| return nil | ||
| } | ||
|
|
||
| func (p *processor) checkRelatedKeyspans(relatedKeySpans []model.KeySpanLocation) bool { |
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.
Please add unit test for this method.
Signed-off-by: zeminzhou <zhouzemin@ping.com>
Signed-off-by: zeminzhou <zhouzemin@ping.com>
Signed-off-by: zeminzhou <zhouzemin@pingcap.com>
cdc/cdc/model/owner.go
Outdated
| // WorkloadInfo records the workload info of a keyspan | ||
| type WorkloadInfo struct { | ||
| Workload uint64 `json:"workload"` | ||
| Workload int64 `json:"workload"` |
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.
Why change to int64? Can it be negative?
Signed-off-by: zeminzhou <zhouzemin@pingcap.com>
cdc/pkg/scheduler/workload.go
Outdated
|
|
||
| func (w workloads) SelectIdleCapture() model.CaptureID { | ||
| minWorkload := uint64(math.MaxUint64) | ||
| minWorkload := uint64(math.MaxInt64) |
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.
maybe revert this change?
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.
Done
pingyu
left a comment
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.
LGTM~
haojinming
left a comment
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.
LGTM~
Signed-off-by: zeminzhou zhouzemin@ping.com
What problem does this PR solve?
Keyspans to add & keyspans to remove, must be overlapped. So to be correct, I think we must finish remove jobs first, and then dispatch the add jobs.here
Issue Number: to #48
Problem Description:
Keyspans to add & keyspans to remove, must be overlapped. So to be correct, I think we must finish remove jobs first, and then dispatch the add jobs
What is changed and how does it work?
After each acquisition of the current keyspan, compare the current keyspan with the keyspan of the previous period to establish a corresponding relationship newKeySpan -> needRemoveKeySpan. The running and deployment of the newKeySpan task needs to wait for all the needRemoveKeySpan tasks to stop. This check will be done in Processor.
Code changes