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/01/15 05:04:30 UTC
[GitHub] [incubator-doris] liutang123 opened a new pull request #5250: Fix 5243 Skip repair replica not in colocate group.
liutang123 opened a new pull request #5250:
URL: https://github.com/apache/incubator-doris/pull/5250
## Proposed changes
Fix #5243 Skip repair replica not in colocate group.
## 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)
- [] 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...)
## Checklist
- [x] I have created an issue on (Fix #5243) and described the bug/feature there in detail
- [x] Compiling and unit tests pass locally with my changes
- [] I have added tests that prove my fix is effective or that my feature works
- [] If this change need a document change, I have updated the document
- [] Any dependent changes have been merged
## Further comments
----------------------------------------------------------------
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.
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] kangkaisen commented on pull request #5250: Fix 5243 Skip repair replica not in colocate group.
Posted by GitBox <gi...@apache.org>.
kangkaisen commented on pull request #5250:
URL: https://github.com/apache/incubator-doris/pull/5250#issuecomment-760758056
@liutang123 Hi, Please add a UT for getColocateHealthStatus. BTW. The `visibleVersionHash` ars could be removed.
----------------------------------------------------------------
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.
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] wangbo merged pull request #5250: Fix 5243 Skip repair replica not in colocate group.
Posted by GitBox <gi...@apache.org>.
wangbo merged pull request #5250:
URL: https://github.com/apache/incubator-doris/pull/5250
----------------------------------------------------------------
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.
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 #5250: Fix 5243 Skip repair replica not in colocate group.
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #5250:
URL: https://github.com/apache/incubator-doris/pull/5250#discussion_r559090364
##########
File path: fe/fe-core/src/main/java/org/apache/doris/catalog/Tablet.java
##########
@@ -547,14 +547,16 @@ public TabletStatus getColocateHealthStatus(long visibleVersion, long visibleVer
// 1. check if replicas' backends are mismatch
Set<Long> replicaBackendIds = getBackendIds();
- for (Long backendId : backendsSet) {
- if (!replicaBackendIds.contains(backendId)) {
- return TabletStatus.COLOCATE_MISMATCH;
- }
+ if (!replicaBackendIds.containsAll(backendsSet)) {
+ return TabletStatus.COLOCATE_MISMATCH;
}
// 2. check version completeness
for (Replica replica : replicas) {
+ if (!backendsSet.contains(replica.getBackendId())) {
+ // We don't care about replicas that are not in backendsSet
+ continue;
+ }
Review comment:
Why not fall down to COLOCATE_REDUNDANT?
----------------------------------------------------------------
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.
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 #5250: Fix 5243 Skip repair replica not in colocate group.
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #5250:
URL: https://github.com/apache/incubator-doris/pull/5250#discussion_r559093890
##########
File path: fe/fe-core/src/main/java/org/apache/doris/catalog/Tablet.java
##########
@@ -547,14 +547,16 @@ public TabletStatus getColocateHealthStatus(long visibleVersion, long visibleVer
// 1. check if replicas' backends are mismatch
Set<Long> replicaBackendIds = getBackendIds();
- for (Long backendId : backendsSet) {
- if (!replicaBackendIds.contains(backendId)) {
- return TabletStatus.COLOCATE_MISMATCH;
- }
+ if (!replicaBackendIds.containsAll(backendsSet)) {
+ return TabletStatus.COLOCATE_MISMATCH;
}
// 2. check version completeness
for (Replica replica : replicas) {
+ if (!backendsSet.contains(replica.getBackendId())) {
+ // We don't care about replicas that are not in backendsSet
+ continue;
+ }
Review comment:
Oh I miss understood
----------------------------------------------------------------
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.
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 #5250: Fix 5243 Skip repair replica not in colocate group.
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #5250:
URL: https://github.com/apache/incubator-doris/pull/5250#discussion_r567218260
##########
File path: fe/fe-core/src/main/java/org/apache/doris/catalog/Tablet.java
##########
@@ -542,19 +542,20 @@ public long getDataSize(boolean singleReplica) {
* No need to check if backend is available. We consider all backends in 'backendsSet' are available,
* If not, unavailable backends will be relocated by CalocateTableBalancer first.
*/
- public TabletStatus getColocateHealthStatus(long visibleVersion, long visibleVersionHash,
- int replicationNum, Set<Long> backendsSet) {
+ public TabletStatus getColocateHealthStatus(long visibleVersion, int replicationNum, Set<Long> backendsSet) {
// 1. check if replicas' backends are mismatch
Set<Long> replicaBackendIds = getBackendIds();
- for (Long backendId : backendsSet) {
- if (!replicaBackendIds.contains(backendId)) {
- return TabletStatus.COLOCATE_MISMATCH;
- }
+ if (!replicaBackendIds.containsAll(backendsSet)) {
+ return TabletStatus.COLOCATE_MISMATCH;
}
// 2. check version completeness
for (Replica replica : replicas) {
+ if (!backendsSet.contains(replica.getBackendId())) {
+ // We don't care about replicas that are not in backendsSet
Review comment:
Hi @liutang123 , modify 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.
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] wangbo commented on a change in pull request #5250: Fix 5243 Skip repair replica not in colocate group.
Posted by GitBox <gi...@apache.org>.
wangbo commented on a change in pull request #5250:
URL: https://github.com/apache/incubator-doris/pull/5250#discussion_r565845937
##########
File path: fe/fe-core/src/main/java/org/apache/doris/catalog/Tablet.java
##########
@@ -542,19 +542,20 @@ public long getDataSize(boolean singleReplica) {
* No need to check if backend is available. We consider all backends in 'backendsSet' are available,
* If not, unavailable backends will be relocated by CalocateTableBalancer first.
*/
- public TabletStatus getColocateHealthStatus(long visibleVersion, long visibleVersionHash,
- int replicationNum, Set<Long> backendsSet) {
+ public TabletStatus getColocateHealthStatus(long visibleVersion, int replicationNum, Set<Long> backendsSet) {
// 1. check if replicas' backends are mismatch
Set<Long> replicaBackendIds = getBackendIds();
- for (Long backendId : backendsSet) {
- if (!replicaBackendIds.contains(backendId)) {
- return TabletStatus.COLOCATE_MISMATCH;
- }
+ if (!replicaBackendIds.containsAll(backendsSet)) {
+ return TabletStatus.COLOCATE_MISMATCH;
}
// 2. check version completeness
for (Replica replica : replicas) {
+ if (!backendsSet.contains(replica.getBackendId())) {
+ // We don't care about replicas that are not in backendsSet
Review comment:
```suggestion
// We don't care about replicas that are not in backendsSet.
// eg: replicaBackendIds=(1,2,3,4); backendsSet=(1,2,3), then replica 4 should be skipped here and then goto ```COLOCATE_REDUNDANT``` in step 3
```
----------------------------------------------------------------
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.
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