dlmarion commented on code in PR #3603:
URL: https://github.com/apache/accumulo/pull/3603#discussion_r1268406946
##########
core/src/main/java/org/apache/accumulo/core/spi/balancer/TabletBalancer.java:
##########
@@ -135,13 +134,43 @@ interface BalanceParameters {
long balance(BalanceParameters params);
/**
- * Get the ResourceGroup name for this tablet
+ * Provides access to information related to a tablet that is current
assigned to a tablet server.
*
- * @param tabletId id of tablet
- * @return resource group name
* @since 4.0.0
*/
- default String getResourceGroup(TabletId tabletId) {
- return Constants.DEFAULT_RESOURCE_GROUP_NAME;
+ interface CurrentAssignment {
+ TabletId getTablet();
+
+ TabletServerId getTabletServer();
+
+ String getResourceGroup();
+ }
+
+ /**
+ * <p>
+ * The manager periodically scans all tablets looking for tablets that are
assigned to dead tablet
+ * servers or unassigned. During the scan this method is also called for
tablets that are
+ * currently assigned to a live tserver to see if they should be unassigned
and reassigned. If
+ * this method returns true the tablet will be unloaded from the tablet
sever and then later the
+ * tablet will be passed to {@link #getAssignments(AssignmentParameters)}.
+ * </p>
+ *
+ * <p>
+ * One example use case for this method is a balancer that partitions tablet
servers into groups.
+ * If the balancers config is changed such that a table that was assigned to
tablet server group A
+ * should now be assigned to tablet server B, then this method can return
true for the tablets in
+ * that table assigned to tablet server group A. After those tablets are
unloaded and passed to
+ * the {@link #getAssignments(AssignmentParameters)} method it can send them
to tablet server
+ * group B.
+ * </p>
+ *
+ * <p>
+ * Thjs new method may be used instead of or in addition to {@link
#balance(BalanceParameters)}
Review Comment:
```suggestion
* This new method may be used instead of or in addition to {@link
#balance(BalanceParameters)}
```
##########
core/src/main/java/org/apache/accumulo/core/spi/balancer/HostRegexTableLoadBalancer.java:
##########
@@ -561,4 +561,25 @@ private static String limitTen(Collection<?> iterable) {
.collect(Collectors.joining(", ", "[", "]"));
}
+ // TODO this is a proof of concept, needs to be flushed out
+ private String getPoolForTserver(TabletServerId tsii) {
Review Comment:
I'm of the opinion that `HostRegexTableLoadBalancer` can be deprecated in
3.0 and removed in `elasticity`. The TableLoadBalancer supports the use-case of
balancing within groups. The groups can be assigned to TabletServers on startup
using the `tserver.group` property. This can be done with `cluster.yaml` and
`accumulo-cluster` or manually (which supports non-orchestrated (bare-metal)
and orchestrated (k8s) deployments).
##########
core/src/main/java/org/apache/accumulo/core/spi/balancer/TabletBalancer.java:
##########
@@ -135,13 +134,43 @@ interface BalanceParameters {
long balance(BalanceParameters params);
/**
- * Get the ResourceGroup name for this tablet
+ * Provides access to information related to a tablet that is current
assigned to a tablet server.
Review Comment:
```suggestion
* Provides access to information related to a tablet that is currently
assigned to a tablet server.
```
##########
core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java:
##########
@@ -446,11 +447,33 @@ public TabletState getTabletState(Set<TServerInstance>
liveTServers, TabletBalan
} else if (current != null) {
if (liveTServers.contains(current.getServerInstance())) {
if (balancer != null) {
- String resourceGroup = balancer.getResourceGroup(new
TabletIdImpl(extent));
- log.trace("Resource Group for extent {} is {}", extent,
resourceGroup);
- Set<TabletServerId> tservers =
currentTServerGrouping.get(resourceGroup);
- if (tservers == null
- || !tservers.contains(new
TabletServerIdImpl(current.getServerInstance()))) {
+ Location finalCurrent = current;
+ var tsii = new TabletServerIdImpl(finalCurrent.getServerInstance());
+ // TODO the passed in map needs to be keyed on tserver to avoid
linear search
+ // TODO do not use default constant..
+ String tserversGroup =
+ currentTServerGrouping.entrySet().stream().filter(e ->
e.getValue().contains(tsii))
+ .map(Entry::getKey).findFirst().orElse("default");
Review Comment:
```suggestion
.map(Entry::getKey).findFirst().orElse(Constants.DEFAULT_RESOURCE_GROUP_NAME);
```
##########
core/src/main/java/org/apache/accumulo/core/spi/balancer/TabletBalancer.java:
##########
@@ -135,13 +134,43 @@ interface BalanceParameters {
long balance(BalanceParameters params);
/**
- * Get the ResourceGroup name for this tablet
+ * Provides access to information related to a tablet that is current
assigned to a tablet server.
*
- * @param tabletId id of tablet
- * @return resource group name
* @since 4.0.0
*/
- default String getResourceGroup(TabletId tabletId) {
- return Constants.DEFAULT_RESOURCE_GROUP_NAME;
+ interface CurrentAssignment {
+ TabletId getTablet();
+
+ TabletServerId getTabletServer();
+
+ String getResourceGroup();
+ }
+
+ /**
+ * <p>
+ * The manager periodically scans all tablets looking for tablets that are
assigned to dead tablet
+ * servers or unassigned. During the scan this method is also called for
tablets that are
+ * currently assigned to a live tserver to see if they should be unassigned
and reassigned. If
+ * this method returns true the tablet will be unloaded from the tablet
sever and then later the
+ * tablet will be passed to {@link #getAssignments(AssignmentParameters)}.
+ * </p>
+ *
+ * <p>
+ * One example use case for this method is a balancer that partitions tablet
servers into groups.
+ * If the balancers config is changed such that a table that was assigned to
tablet server group A
+ * should now be assigned to tablet server B, then this method can return
true for the tablets in
+ * that table assigned to tablet server group A. After those tablets are
unloaded and passed to
+ * the {@link #getAssignments(AssignmentParameters)} method it can send them
to tablet server
Review Comment:
```suggestion
* the {@link #getAssignments(AssignmentParameters)} method it can
reassign them to tablet server
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
