You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2021/09/22 04:04:19 UTC
[GitHub] [incubator-doris] xy720 opened a new pull request #6705: [Bugs] Fix many bugs in sync job
xy720 opened a new pull request #6705:
URL: https://github.com/apache/incubator-doris/pull/6705
## Proposed changes
QA and I found a lot of bugs during sync job's functional testing. This pr will fix them as follows:
1、Fix bug that the sync jobs are not cancelled after deleting the database.
2、The MySQL and Doris tables should have a one-to-one correspondence. If they are not, they should fail when creating the task.
3、When the cluster has multiple FE, the non-master will core when replay create the sync job.
4、Inconsistent data when updating key column
5、Failed to synchronize data when there are multiple tables in single sync job.
6、After restarting the master, resuming the suspended syncjob will fail.
## Types of changes
What types of changes does your code introduce to Doris?
_Put an `x` in the boxes that apply_
- [x] Bugfix (non-breaking change which fixes an issue)
- [x] Code refactor (Modify the code structure, format the code, etc...)
## Checklist
_Put an `x` in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code._
- [x] I have created an issue on (Fix #6704 ) and described the bug/feature there in detail
- [x] Compiling and unit tests pass locally with my changes
## Further comments
If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman merged pull request #6705: [Bugs] Fix the bugs list of sync job
Posted by GitBox <gi...@apache.org>.
morningman merged pull request #6705:
URL: https://github.com/apache/incubator-doris/pull/6705
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] xy720 commented on a change in pull request #6705: [Bugs] Fix the bugs list of sync job
Posted by GitBox <gi...@apache.org>.
xy720 commented on a change in pull request #6705:
URL: https://github.com/apache/incubator-doris/pull/6705#discussion_r714538955
##########
File path: fe/fe-core/src/main/java/org/apache/doris/load/sync/SyncJobManager.java
##########
@@ -281,9 +296,6 @@ public void readField(DataInput in) throws IOException {
int size = in.readInt();
for (int i = 0; i < size; i++) {
SyncJob syncJob = SyncJob.read(in);
- if (!syncJob.isCompleted()) {
- syncJob.updateState(JobState.PENDING, true);
- }
unprotectedAddSyncJob(syncJob);
Review comment:
readfield方法理论上只会由checkpoint线程来调用，写入的catalog和现在FE运行时的catalog不是一个对象，所以这里不会有线程安全的问题。我们只需要在创建syncjob的时候检查同名，就可以保证checkpoint时不会出现重复名称了，这一检查逻辑在DdlExecutor里。
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] xy720 commented on a change in pull request #6705: [Bugs] Fix the bugs list of sync job
Posted by GitBox <gi...@apache.org>.
xy720 commented on a change in pull request #6705:
URL: https://github.com/apache/incubator-doris/pull/6705#discussion_r714535353
##########
File path: fe/fe-core/src/main/java/org/apache/doris/load/sync/SyncJob.java
##########
@@ -152,6 +186,65 @@ public synchronized void updateState(JobState newState, boolean isReplay) {
}
}
+ private void checkStateTransform(JobState newState) throws UserException {
+ CHECK: {
+ switch (jobState) {
+ case RUNNING:
+ if (newState == JobState.RUNNING) {
+ break CHECK;
Review comment:
ok，改成直接throw了
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] xy720 commented on a change in pull request #6705: [Bugs] Fix the bugs list of sync job
Posted by GitBox <gi...@apache.org>.
xy720 commented on a change in pull request #6705:
URL: https://github.com/apache/incubator-doris/pull/6705#discussion_r714549884
##########
File path: fe/fe-core/src/main/java/org/apache/doris/load/sync/SyncLifeCycle.java
##########
@@ -55,10 +55,6 @@ public void run() {
}
public void stop() {
- if (!isStart()) {
- throw new RuntimeException(this.getClass().getName() + " isn't start , please check");
Review comment:
改成了如果重复stop，后面的逻辑都会跳过
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] pengxiangyu commented on a change in pull request #6705: [Bugs] Fix the bugs list of sync job
Posted by GitBox <gi...@apache.org>.
pengxiangyu commented on a change in pull request #6705:
URL: https://github.com/apache/incubator-doris/pull/6705#discussion_r714506283
##########
File path: fe/fe-core/src/main/java/org/apache/doris/load/sync/SyncJob.java
##########
@@ -152,6 +186,65 @@ public synchronized void updateState(JobState newState, boolean isReplay) {
}
}
+ private void checkStateTransform(JobState newState) throws UserException {
+ CHECK: {
+ switch (jobState) {
+ case RUNNING:
+ if (newState == JobState.RUNNING) {
+ break CHECK;
Review comment:
这里直接throw比较好，这样不需要外面的CHECK，结尾处也不需要再return了。逻辑比较清晰
##########
File path: fe/fe-core/src/main/java/org/apache/doris/load/sync/SyncJobManager.java
##########
@@ -281,9 +296,6 @@ public void readField(DataInput in) throws IOException {
int size = in.readInt();
for (int i = 0; i < size; i++) {
SyncJob syncJob = SyncJob.read(in);
- if (!syncJob.isCompleted()) {
- syncJob.updateState(JobState.PENDING, true);
- }
unprotectedAddSyncJob(syncJob);
Review comment:
这里不用加锁么？调用unprotectedAddSyncJob的时候没有检查同名jobName是否已存在，应该要做下检查吧，否则同名任务会被直接覆盖。
##########
File path: fe/fe-core/src/main/java/org/apache/doris/load/sync/SyncJob.java
##########
@@ -152,6 +186,65 @@ public synchronized void updateState(JobState newState, boolean isReplay) {
}
}
+ private void checkStateTransform(JobState newState) throws UserException {
+ CHECK: {
+ switch (jobState) {
+ case RUNNING:
+ if (newState == JobState.RUNNING) {
+ break CHECK;
+ }
+ break;
+ case PAUSED:
+ if (newState == JobState.PAUSED || newState == JobState.RUNNING) {
+ break CHECK;
Review comment:
同上，这一段整体改造一下吧。
##########
File path: fe/fe-core/src/main/java/org/apache/doris/load/sync/SyncLifeCycle.java
##########
@@ -55,10 +55,6 @@ public void run() {
}
public void stop() {
- if (!isStart()) {
- throw new RuntimeException(this.getClass().getName() + " isn't start , please check");
Review comment:
这块外面没有catch这个异常的地方，没什么意义。如果isStart()为false也存在重复stop的情况，建议直接按stop成功来处理。
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] github-actions[bot] commented on pull request #6705: [Bugs] Fix the bugs list of sync job
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #6705:
URL: https://github.com/apache/incubator-doris/pull/6705#issuecomment-926298645
PR approved by at least one committer and no changes requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] xy720 commented on a change in pull request #6705: [Bugs] Fix the bugs list of sync job
Posted by GitBox <gi...@apache.org>.
xy720 commented on a change in pull request #6705:
URL: https://github.com/apache/incubator-doris/pull/6705#discussion_r714539835
##########
File path: fe/fe-core/src/main/java/org/apache/doris/load/sync/SyncLifeCycle.java
##########
@@ -55,10 +55,6 @@ public void run() {
}
public void stop() {
- if (!isStart()) {
- throw new RuntimeException(this.getClass().getName() + " isn't start , please check");
Review comment:
没错，这一块我去掉抛出异常，就是想重复的stop都按成功处理。
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] github-actions[bot] commented on pull request #6705: [Bugs] Fix the bugs list of sync job
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #6705:
URL: https://github.com/apache/incubator-doris/pull/6705#issuecomment-925682992
PR approved by anyone and no changes requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org