Changes to public API to expose resource groups #4851
Conversation
Created ServerId objects to represent server addresses and resource groups Modified instance operations to use the new ServerId objects Consolidated all server location lookup code to new ServerIds object A lot of changes to use the new non-deprecated InstanceOperations methods Closes apache#4849
| @@ -779,6 +761,21 @@ private void fetchCompactions() { | |||
| ThriftUtil.returnClient(tserver, context); | |||
| } | |||
| } | |||
| for (ServerId server : context.instanceOperations().getServers(ServerTypeName.COMPACTOR)) { | |||
There was a problem hiding this comment.
I wonder if this change will start to show external compactions on the monitor front page.
core/src/main/java/org/apache/accumulo/core/client/admin/InstanceOperations.java
Show resolved
Hide resolved
| * | ||
| * @param server The ServerId object | ||
| * @return the list of active compactions | ||
| * @since 4.0.0 |
There was a problem hiding this comment.
This could have throws javadoc like getActiveScans does. Could also test passing in unexpected server types in an IT.
| * @param server server type and address | ||
| * @return A stream of active scans on server. | ||
| * @since 4.0.0 | ||
| * @throws IllegalArgumentException when the type of the server is not TABLET_SERVER or |
There was a problem hiding this comment.
Would be nice to test passing unexpected server types to this method in an IT.
|
|
||
| import com.google.common.base.Preconditions; | ||
|
|
||
| public abstract class ServerId implements Comparable<ServerId> { |
There was a problem hiding this comment.
This should have javadoc w/ a since tag. Also the child types should have a since tag.
| */ | ||
| package org.apache.accumulo.core.client.admin.servers; | ||
|
|
||
| public enum ServerTypeName { |
| private final String host; | ||
| private final int port; | ||
|
|
||
| protected ServerId(ServerTypeName type, String resourceGroup, String host, int port) { |
There was a problem hiding this comment.
This suggestion of making the constructor package private goes w/ making the child classes final.
| protected ServerId(ServerTypeName type, String resourceGroup, String host, int port) { | |
| ServerId(ServerTypeName type, String resourceGroup, String host, int port) { |
| @@ -145,6 +145,8 @@ enum Type { | |||
| String getAddress(); | |||
|
|
|||
| int getPort(); | |||
|
|
|||
| String getResourceGroup(); | |||
There was a problem hiding this comment.
We should open a follow on issue to look into deprecating and replacing this CompactionHost interface in favor of the new ServerId type.
|
|
||
| import com.google.common.net.HostAndPort; | ||
|
|
||
| public class ServerIds { |
There was a problem hiding this comment.
I didn't really look at this class, let me know if there is anything in it you would like a 2nd pair of eyes on.
| break; | ||
| } | ||
| if (test == null) { | ||
| return results; |
There was a problem hiding this comment.
Could avoid returning mutable sets.
| return results; | |
| return Collections.unmodifiableSet(results); |
For the stream I think Collectors has a function for collecting to an unmodifiable set.
| final Set<ServerId> results = new HashSet<>(); | ||
| switch (type) { | ||
| case COMPACTOR: | ||
| results.addAll(context.getServerIdResolver().getCompactors()); |
There was a problem hiding this comment.
Not sure this is worthwhile, wondering if would be useful to push the predicate down. Please ignore this comment if it does not make sense.
| results.addAll(context.getServerIdResolver().getCompactors()); | |
| results.addAll(context.getServerIdResolver(test).getCompactors()); |
|
Full IT build passed |
|
Merged in |
Created ServerId objects to represent server addresses and resource groups Modified instance operations to use the new ServerId objects Consolidated all server location lookup code to new ServerIds object A lot of changes to use the new non-deprecated InstanceOperations methods
Closes #4849