You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "dlmarion (via GitHub)" <gi...@apache.org> on 2024/04/05 16:16:08 UTC
[PR] Add tablet ranges to coordinator msg in ManagerClientServiceHandler [accumulo]
dlmarion opened a new pull request, #4433:
URL: https://github.com/apache/accumulo/pull/4433
(no 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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add tablet ranges to coordinator msg in ManagerClientServiceHandler [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #4433:
URL: https://github.com/apache/accumulo/pull/4433#discussion_r1554208536
##########
server/manager/src/main/java/org/apache/accumulo/manager/ManagerClientServiceHandler.java:
##########
@@ -648,9 +649,12 @@ public void requestTabletHosting(TInfo tinfo, TCredentials credentials, String t
inProgress.forEach(hostingRequestInProgress::remove);
}
- // ELASTICITY_TODO pass ranges of individual tablets
- manager.getEventCoordinator().event(success, "Tablet hosting requested for %d tablets in %s",
- success.size(), tableId);
+ final List<Range> ranges = new ArrayList<>(success.size());
+ success.forEach(ke -> ranges.add(ke.toDataRange()));
+ final List<Range> tabletRanges = Range.mergeOverlapping(ranges);
+ manager.getEventCoordinator().event(success,
+ "Tablet hosting requested for %d tablets in ranges %s in table %s", success.size(),
+ tabletRanges, tableId);
Review Comment:
I think we could just remove the comment and leave this code as is. Looking at the comment, I made it and it was mistake. I think I saw the table id in the method call and assumed that was causing an entire table scan, however that is not the case the tableId is just for logging. The first argument to the method `success` does seem to already be individual tablets that need attention.
```suggestion
manager.getEventCoordinator().event(success, "Tablet hosting requested for %d tablets in %s",
success.size(), tableId);
```
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add tablet ranges to coordinator msg in ManagerClientServiceHandler [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #4433:
URL: https://github.com/apache/accumulo/pull/4433#discussion_r1554210556
##########
server/manager/src/main/java/org/apache/accumulo/manager/ManagerClientServiceHandler.java:
##########
@@ -648,9 +649,12 @@ public void requestTabletHosting(TInfo tinfo, TCredentials credentials, String t
inProgress.forEach(hostingRequestInProgress::remove);
}
- // ELASTICITY_TODO pass ranges of individual tablets
- manager.getEventCoordinator().event(success, "Tablet hosting requested for %d tablets in %s",
- success.size(), tableId);
+ final List<Range> ranges = new ArrayList<>(success.size());
+ success.forEach(ke -> ranges.add(ke.toDataRange()));
+ final List<Range> tabletRanges = Range.mergeOverlapping(ranges);
+ manager.getEventCoordinator().event(success,
+ "Tablet hosting requested for %d tablets in ranges %s in table %s", success.size(),
+ tabletRanges, tableId);
Review Comment:
Also, there is existing logging earlier in the method for each tablet that needs to be hosted.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add tablet ranges to coordinator msg in ManagerClientServiceHandler [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion merged PR #4433:
URL: https://github.com/apache/accumulo/pull/4433
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org