-
Notifications
You must be signed in to change notification settings - Fork 188
Conversation
Codecov Report
@@ Coverage Diff @@
## master #722 +/- ##
================================================
- Coverage 57.0981% 57.0248% -0.0734%
================================================
Files 205 205
Lines 21104 21189 +85
================================================
+ Hits 12050 12083 +33
- Misses 7890 7931 +41
- Partials 1164 1175 +11 |
syncer/sharding-meta/shardmeta.go
Outdated
@@ -178,25 +178,23 @@ func (meta *ShardingMeta) checkItemExists(item *DDLItem) (int, bool) { | |||
// active: whether the DDL will be processed in this round | |||
func (meta *ShardingMeta) AddItem(item *DDLItem) (active bool, err error) { | |||
index, exists := meta.checkItemExists(item) | |||
if exists { | |||
return index == meta.activeIdx, nil |
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 easy to add a negative test case for #719?
one more question, can we reset shard meta when resuming?
/run-all-tests |
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, and which modify related to the description check ddl conflict even the ddl exist
?
"\"result\": true" 3 | ||
|
||
cp $cur/conf/dm-task.yaml $WORK_DIR/task.yaml | ||
echo "ignore-checking-items: [\"all\"]" >> $WORK_DIR/task.yaml |
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 need to ignore all checks? how about adding 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.
add comment in 644a504
We have a discuss in #582 (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
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
cherry pick to release-1.0 failed |
@GMHDBJD please cherry-pick it manually again. 😢 |
What problem does this PR solve?
#719 #582
What is changed and how it works?
ErrSyncerShardDDLConflict
as UnresumableErrorTests
Related changes