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/08/26 04:06:55 UTC

[GitHub] [incubator-doris] weizuo93 opened a new pull request #6513: [Optimize][Clone] Take version count into consideration when choosing src replica for clone task

weizuo93 opened a new pull request #6513:
URL: https://github.com/apache/incubator-doris/pull/6513


   ## Proposed changes
   
   Fix #6512 
   
   If there is missing replica for a tablet, clone task will be executed to restore missing replica from a healthy replica. Src replica selector will randomly choose a healthy replica as src replica.
   
   It's better to choose the health replica with minmun version count as src replica so that it could avoid repetitive compaction task. In addition, replica with less version count is good for query performance.
   
   ## Types of changes
   
   What types of changes does your code introduce to Doris?
   _Put an `x` in the boxes that apply_
   
   - [ ] Bugfix (non-breaking change which fixes an issue)
   - [ ] New feature (non-breaking change which adds functionality)
   - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
   - [ ] Documentation Update (if none of the other choices apply)
   - [ ] Code refactor (Modify the code structure, format the code, etc...)
   - [x] Optimization. Including functional usability improvements and performance improvements.
   - [ ] Dependency. Such as changes related to third-party components.
   - [ ] Other.
   
   


-- 
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] weizuo93 commented on a change in pull request #6513: [Optimize][Clone] Take version count into consideration when choosing src replica for clone task

Posted by GitBox <gi...@apache.org>.
weizuo93 commented on a change in pull request #6513:
URL: https://github.com/apache/incubator-doris/pull/6513#discussion_r697151173



##########
File path: fe/fe-core/src/main/java/org/apache/doris/clone/TabletSchedCtx.java
##########
@@ -499,10 +501,17 @@ public void chooseSrcReplica(Map<Long, PathSlot> backendsWorkingSlots) throws Sc
             
             long srcPathHash = slot.takeSlot(srcReplica.getPathHash());
             if (srcPathHash != -1) {
-                setSrc(srcReplica);
-                return;
+                if (minVersionCount == -1 || srcReplica.getVersionCount() < minVersionCount) {
+                    minVersionCount = srcReplica.getVersionCount();

Review comment:
       > versionCount of a replica is set by report process.
   > It may be -1 if the report is not done yet.
   > So we need to check it.
   
   OK, thanks for your comment.




-- 
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 #6513: [Optimize][Clone] Take version count into consideration when choosing src replica for clone task

Posted by GitBox <gi...@apache.org>.
morningman merged pull request #6513:
URL: https://github.com/apache/incubator-doris/pull/6513


   


-- 
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] caiconghui commented on a change in pull request #6513: [Optimize][Clone] Take version count into consideration when choosing src replica for clone task

Posted by GitBox <gi...@apache.org>.
caiconghui commented on a change in pull request #6513:
URL: https://github.com/apache/incubator-doris/pull/6513#discussion_r698136571



##########
File path: fe/fe-core/src/main/java/org/apache/doris/clone/TabletSchedCtx.java
##########
@@ -499,10 +501,26 @@ public void chooseSrcReplica(Map<Long, PathSlot> backendsWorkingSlots) throws Sc
             
             long srcPathHash = slot.takeSlot(srcReplica.getPathHash());
             if (srcPathHash != -1) {
-                setSrc(srcReplica);
-                return;
+                if (!findSrcReplica) {
+                    // version count is set by report process, so it may not be set yet and default value is -1.
+                    // so we need to check it.
+                    minVersionCount = srcReplica.getVersionCount() == -1 ? Long.MAX_VALUE : srcReplica.getVersionCount();
+                    setSrc(srcReplica);
+                    findSrcReplica = true;
+                } else {
+                    long curVerCount = srcReplica.getVersionCount() == -1 ? Long.MAX_VALUE : srcReplica.getVersionCount();
+                    if (curVerCount < minVersionCount) {
+                        minVersionCount = curVerCount;
+                        setSrc(srcReplica);
+                        findSrcReplica = true;
+                    }
+                }
             }
         }
+
+        if (findSrcReplica) {

Review comment:
       ```suggestion
           if (!findSrcReplica) {
                throw new SchedException(Status.SCHEDULE_FAILED, "unable to find source slot");
           }
   
          
   ```




-- 
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 #6513: [Optimize][Clone] Take version count into consideration when choosing src replica for clone task

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #6513:
URL: https://github.com/apache/incubator-doris/pull/6513#issuecomment-907554902






-- 
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] caiconghui commented on a change in pull request #6513: [Optimize][Clone] Take version count into consideration when choosing src replica for clone task

Posted by GitBox <gi...@apache.org>.
caiconghui commented on a change in pull request #6513:
URL: https://github.com/apache/incubator-doris/pull/6513#discussion_r698136801



##########
File path: fe/fe-core/src/main/java/org/apache/doris/clone/TabletSchedCtx.java
##########
@@ -499,10 +501,26 @@ public void chooseSrcReplica(Map<Long, PathSlot> backendsWorkingSlots) throws Sc
             
             long srcPathHash = slot.takeSlot(srcReplica.getPathHash());
             if (srcPathHash != -1) {
-                setSrc(srcReplica);
-                return;
+                if (!findSrcReplica) {

Review comment:
       remove the first if block




-- 
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] weizuo93 commented on a change in pull request #6513: [Optimize][Clone] Take version count into consideration when choosing src replica for clone task

Posted by GitBox <gi...@apache.org>.
weizuo93 commented on a change in pull request #6513:
URL: https://github.com/apache/incubator-doris/pull/6513#discussion_r698200356



##########
File path: fe/fe-core/src/main/java/org/apache/doris/clone/TabletSchedCtx.java
##########
@@ -499,10 +501,26 @@ public void chooseSrcReplica(Map<Long, PathSlot> backendsWorkingSlots) throws Sc
             
             long srcPathHash = slot.takeSlot(srcReplica.getPathHash());
             if (srcPathHash != -1) {
-                setSrc(srcReplica);
-                return;
+                if (!findSrcReplica) {
+                    // version count is set by report process, so it may not be set yet and default value is -1.
+                    // so we need to check it.
+                    minVersionCount = srcReplica.getVersionCount() == -1 ? Long.MAX_VALUE : srcReplica.getVersionCount();
+                    setSrc(srcReplica);
+                    findSrcReplica = true;
+                } else {
+                    long curVerCount = srcReplica.getVersionCount() == -1 ? Long.MAX_VALUE : srcReplica.getVersionCount();
+                    if (curVerCount < minVersionCount) {
+                        minVersionCount = curVerCount;
+                        setSrc(srcReplica);
+                        findSrcReplica = true;
+                    }
+                }
             }
         }
+
+        if (findSrcReplica) {

Review comment:
       It's necessary to record first candidate replica in the `if` block.




-- 
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] weizuo93 commented on a change in pull request #6513: [Optimize][Clone] Take version count into consideration when choosing src replica for clone task

Posted by GitBox <gi...@apache.org>.
weizuo93 commented on a change in pull request #6513:
URL: https://github.com/apache/incubator-doris/pull/6513#discussion_r698200483



##########
File path: fe/fe-core/src/main/java/org/apache/doris/clone/TabletSchedCtx.java
##########
@@ -499,10 +501,26 @@ public void chooseSrcReplica(Map<Long, PathSlot> backendsWorkingSlots) throws Sc
             
             long srcPathHash = slot.takeSlot(srcReplica.getPathHash());
             if (srcPathHash != -1) {
-                setSrc(srcReplica);
-                return;
+                if (!findSrcReplica) {

Review comment:
       It's necessary to record first candidate replica in the `if` block.




-- 
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 commented on a change in pull request #6513: [Optimize][Clone] Take version count into consideration when choosing src replica for clone task

Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #6513:
URL: https://github.com/apache/incubator-doris/pull/6513#discussion_r696726743



##########
File path: fe/fe-core/src/main/java/org/apache/doris/clone/TabletSchedCtx.java
##########
@@ -499,10 +501,17 @@ public void chooseSrcReplica(Map<Long, PathSlot> backendsWorkingSlots) throws Sc
             
             long srcPathHash = slot.takeSlot(srcReplica.getPathHash());
             if (srcPathHash != -1) {
-                setSrc(srcReplica);
-                return;
+                if (minVersionCount == -1 || srcReplica.getVersionCount() < minVersionCount) {
+                    minVersionCount = srcReplica.getVersionCount();

Review comment:
       versionCount of a replica is set by report process.
   It may be -1 if the report is not done yet.
   So we need to check it.




-- 
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