-
Notifications
You must be signed in to change notification settings - Fork 471
Add multi-range Tablet info API #6027
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
keith-turner
left 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.
This looks good, was there anything else you were planning to do before taking out of draft?
core/src/main/java/org/apache/accumulo/core/clientImpl/TabletInformationCollector.java
Show resolved
Hide resolved
core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java
Show resolved
Hide resolved
test/src/main/java/org/apache/accumulo/test/TableOperationsIT.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/apache/accumulo/core/spi/balancer/BalancerEnvironment.java
Show resolved
Hide resolved
The main thing I wanted to look into was how we can improve things in Maybe this shouldn't block things here though since this new code is already an improvement and we can look into adding that new feature in a follow on. |
There is something similar to this here but it take extents and not row ranges. It expects those exact extents to exist. The impl for this uses a batch scanner. So we could have something like that, however the batch scanner will deliver things out of order. As this is done now, maybe it gives things back in the order the ranges were given? I think what is here is a good start. Could have a follow on issue for optimization, but maybe that could only be done if needed. |
|
Re optimizing this, for now we could make the javadoc mention there is no expected order for returned tablets. That leaves open future optimizations that would return tablets in arbitrary order. |
|
@keith-turner I marked this ready for review. I think things are ready to go as-is. Two potential follow on tasks that could improve things here are:
|
fbd4197 to
c08b4e0
Compare
Fixes #5667
This PR adds the following:
multi-range tablet info API:
TableOperations.getTabletInformation()which now accepts aList<Range>and the single range overload now delegates to this new methodBalancerEnvironment.getTabletInformation(TableId, List<Range>, TabletInformation.Field...)so balancer plugins can request only the tablets they needCreated new
TabletInformationCollectorclass to centralize logicFeilds and reads tablets via AmpleTableOperationsImplandBalancerEnvironmentImplnow delegate toTabletInformationCollectorfor both single- and multi-range calls