This is an automated email from the ASF dual-hosted git repository.
dlmarion pushed a commit to branch 2.1
in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/2.1 by this push:
new b9e227b712 Fixed issue with balancer property updates (#5575)
b9e227b712 is described below
commit b9e227b71268fa81322adf6ad405bc89e4aa86a3
Author: Dave Marion <dlmar...@apache.org>
AuthorDate: Tue May 27 15:06:31 2025 -0400
Fixed issue with balancer property updates (#5575)
The change in #5530 added code to clear the
property cache in initializeBalancer because
the property change was not immediately seen.
BrokenBalancerIT was changing the property
then checking the balance to see if the new
balancer had taken effect.
This commit changes BrokenBalancerIT to give
it more time in the test thread for the
property change to take effect in the Manager
and changes how the Manager responds to system
property changes so that the property cache
invalidation can be removed.
Related to #5530
---
.../java/org/apache/accumulo/manager/Manager.java | 36 ++++++++++++----------
.../manager/ManagerClientServiceHandler.java | 16 ----------
.../org/apache/accumulo/test/BrokenBalancerIT.java | 10 ++++--
3 files changed, 27 insertions(+), 35 deletions(-)
diff --git
a/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java
b/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java
index c69142fd80..6a7ecb86fd 100644
--- a/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java
+++ b/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java
@@ -201,7 +201,7 @@ public class Manager extends AbstractServer implements
LiveTServerSet.Listener,
ServiceLock managerLock = null;
private TServer clientService = null;
- private volatile TabletBalancer tabletBalancer;
+ private volatile TabletBalancer tabletBalancer = null;
private final BalancerEnvironment balancerEnvironment;
private final BalancerMetrics balancerMetrics = new BalancerMetrics();
@@ -1022,6 +1022,9 @@ public class Manager extends AbstractServer implements
LiveTServerSet.Listener,
private long balanceTablets() {
+ // Check for balancer property change
+ initializeBalancer();
+
final int tabletsNotHosted = notHosted();
BalanceParamsImpl params = null;
long wait = 0;
@@ -1917,29 +1920,29 @@ public class Manager extends AbstractServer implements
LiveTServerSet.Listener,
return upgradeCoordinator.getStatus() !=
UpgradeCoordinator.UpgradeStatus.COMPLETE;
}
- void initializeBalancer() {
+ private void initializeBalancer() {
+ String configuredBalancerClass =
getConfiguration().get(Property.MANAGER_TABLET_BALANCER);
try {
- getContext().getPropStore().getCache().removeAll();
- getConfiguration().invalidateCache();
- log.debug("Attempting to reinitialize balancer using class {}",
- getConfiguration().get(Property.MANAGER_TABLET_BALANCER));
- var localTabletBalancer =
Property.createInstanceFromPropertyName(getConfiguration(),
- Property.MANAGER_TABLET_BALANCER, TabletBalancer.class, new
DoNothingBalancer());
- localTabletBalancer.init(balancerEnvironment);
- tabletBalancer = localTabletBalancer;
+ if (tabletBalancer == null
+ ||
!tabletBalancer.getClass().getName().equals(configuredBalancerClass)) {
+ log.debug("Attempting to initialize balancer using class {}, was {}",
+ configuredBalancerClass,
+ tabletBalancer == null ? "null" :
tabletBalancer.getClass().getName());
+ var localTabletBalancer =
Property.createInstanceFromPropertyName(getConfiguration(),
+ Property.MANAGER_TABLET_BALANCER, TabletBalancer.class, new
DoNothingBalancer());
+ localTabletBalancer.init(balancerEnvironment);
+ tabletBalancer = localTabletBalancer;
+ log.info("tablet balancer changed to {}",
localTabletBalancer.getClass().getName());
+ }
} catch (Exception e) {
- log.warn("Failed to create balancer {} using {} instead",
- getConfiguration().get(Property.MANAGER_TABLET_BALANCER),
DoNothingBalancer.class, e);
+ log.warn("Failed to create balancer {} using {} instead",
configuredBalancerClass,
+ DoNothingBalancer.class, e);
var localTabletBalancer = new DoNothingBalancer();
localTabletBalancer.init(balancerEnvironment);
tabletBalancer = localTabletBalancer;
}
}
- Class<?> getBalancerClass() {
- return tabletBalancer.getClass();
- }
-
void getAssignments(SortedMap<TServerInstance,TabletServerStatus>
currentStatus,
Map<KeyExtent,UnassignedTablet> unassigned,
Map<KeyExtent,TServerInstance> assignedOut) {
AssignmentParamsImpl params =
AssignmentParamsImpl.fromThrift(currentStatus,
@@ -1953,5 +1956,4 @@ public class Manager extends AbstractServer implements
LiveTServerSet.Listener,
public ServiceLock getLock() {
return managerLock;
}
-
}
diff --git
a/server/manager/src/main/java/org/apache/accumulo/manager/ManagerClientServiceHandler.java
b/server/manager/src/main/java/org/apache/accumulo/manager/ManagerClientServiceHandler.java
index 5a5b022e31..505eefab7f 100644
---
a/server/manager/src/main/java/org/apache/accumulo/manager/ManagerClientServiceHandler.java
+++
b/server/manager/src/main/java/org/apache/accumulo/manager/ManagerClientServiceHandler.java
@@ -52,8 +52,6 @@ import
org.apache.accumulo.core.clientImpl.thrift.ThriftConcurrentModificationEx
import
org.apache.accumulo.core.clientImpl.thrift.ThriftNotActiveServiceException;
import org.apache.accumulo.core.clientImpl.thrift.ThriftSecurityException;
import
org.apache.accumulo.core.clientImpl.thrift.ThriftTableOperationException;
-import org.apache.accumulo.core.conf.DeprecatedPropertyUtil;
-import org.apache.accumulo.core.conf.Property;
import org.apache.accumulo.core.data.Key;
import org.apache.accumulo.core.data.NamespaceId;
import org.apache.accumulo.core.data.Range;
@@ -431,7 +429,6 @@ public class ManagerClientServiceHandler implements
ManagerClientService.Iface {
try {
SystemPropUtil.removeSystemProperty(manager.getContext(), property);
- updatePlugins(property);
} catch (Exception e) {
Manager.log.error("Problem removing config property in zookeeper", e);
throw new RuntimeException(e.getMessage());
@@ -447,7 +444,6 @@ public class ManagerClientServiceHandler implements
ManagerClientService.Iface {
try {
SystemPropUtil.setSystemProperty(manager.getContext(), property, value);
- updatePlugins(property);
} catch (IllegalArgumentException iae) {
Manager.log.error("Problem setting invalid property", iae);
throw new ThriftPropertyException(property, value, "Property is
invalid");
@@ -467,9 +463,6 @@ public class ManagerClientServiceHandler implements
ManagerClientService.Iface {
try {
SystemPropUtil.modifyProperties(manager.getContext(),
properties.getVersion(),
properties.getProperties());
- for (Map.Entry<String,String> entry :
properties.getProperties().entrySet()) {
- updatePlugins(entry.getKey());
- }
} catch (IllegalArgumentException iae) {
Manager.log.error("Problem setting invalid property", iae);
throw new ThriftPropertyException("Modify properties", "failed",
iae.getMessage());
@@ -591,15 +584,6 @@ public class ManagerClientServiceHandler implements
ManagerClientService.Iface {
}
}
- private void updatePlugins(String property) {
- // resolve without warning; any warnings should have already occurred
- String resolved = DeprecatedPropertyUtil.getReplacementName(property,
(log, replacement) -> {});
- if (resolved.equals(Property.MANAGER_TABLET_BALANCER.getKey())) {
- manager.initializeBalancer();
- log.info("tablet balancer changed to {}",
manager.getBalancerClass().getName());
- }
- }
-
@Override
public void waitForBalance(TInfo tinfo) {
manager.waitForBalance();
diff --git a/test/src/main/java/org/apache/accumulo/test/BrokenBalancerIT.java
b/test/src/main/java/org/apache/accumulo/test/BrokenBalancerIT.java
index fafdaf9c5b..9cde44080e 100644
--- a/test/src/main/java/org/apache/accumulo/test/BrokenBalancerIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/BrokenBalancerIT.java
@@ -119,7 +119,13 @@ public class BrokenBalancerIT extends ConfigurableMacBase {
getCluster().getConfig().setNumTservers(5);
getCluster().getClusterControl().start(ServerType.TABLET_SERVER);
- UtilWaitThread.sleep(5000);
+ Wait.waitFor(() -> c.instanceOperations().getTabletServers().size() ==
5);
+ Wait.waitFor(() -> c.instanceOperations().getSystemConfiguration()
+
.get(Property.MANAGER_TABLET_BALANCER.getKey()).equals(balancerClass));
+ c.instanceOperations().waitForBalance();
+
+ // Give enough time for property change and Status Thread in Manager
+ UtilWaitThread.sleep(30000);
// should not have balanced across the two new tservers
assertEquals(2, BalanceIT.countLocations(c, tableName).size());
@@ -130,7 +136,7 @@ public class BrokenBalancerIT extends ConfigurableMacBase {
TableLoadBalancer.class.getName());
// should eventually balance across all 5 tabletsevers
- Wait.waitFor(() -> 5 == BalanceIT.countLocations(c, tableName).size());
+ Wait.waitFor(() -> 5 == BalanceIT.countLocations(c, tableName).size(),
60_000);
}
}
}
