[accumulo] 01/03: Revert "Revert "Ensure that HostRegexTableLoadBalancer propery changes are picked up""

[ibella]

This is an automated email from the ASF dual-hosted git repository.

ibella pushed a commit to branch 1.10
in repository https://gitbox.apache.org/repos/asf/accumulo.git

commit 8bb474908817228b51571f80c342b8f7ea9e2f9b
Author: Ivan Bella <i...@bella.name>
AuthorDate: Wed Apr 7 16:21:27 2021 +0000

    Revert "Revert "Ensure that HostRegexTableLoadBalancer propery changes are 
picked up""
    
    This reverts commit 30fd56eb3e0ef5f6cd3d7c86c2c133b48eb53f21.
---
 .../master/balancer/HostRegexTableLoadBalancer.java  | 20 ++++++++++++++++----
 ...ostRegexTableLoadBalancerReconfigurationTest.java | 19 +++++++++++++++++++
 2 files changed, 35 insertions(+), 4 deletions(-)

diff --git 
a/server/base/src/main/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancer.java
 
b/server/base/src/main/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancer.java
index 3934b7f..246568f 100644
--- 
a/server/base/src/main/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancer.java
+++ 
b/server/base/src/main/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancer.java
@@ -220,7 +220,7 @@ public class HostRegexTableLoadBalancer extends 
TableLoadBalancer implements Con
    * @param conf
    *          server configuration
    */
-  protected void parseConfiguration(ServerConfiguration conf) {
+  protected void parseTableConfiguration(ServerConfiguration conf) {
     TableOperations t = getTableOperations();
     if (null == t) {
       throw new RuntimeException("Table Operations cannot be null");
@@ -252,6 +252,14 @@ public class HostRegexTableLoadBalancer extends 
TableLoadBalancer implements Con
     tableIdToTableName = ImmutableMap.copyOf(tableIdToTableNameBuilder);
     poolNameToRegexPattern = 
ImmutableMap.copyOf(poolNameToRegexPatternBuilder);
 
+    LOG.info("{}", this);
+  }
+
+  public void parseSystemConfiguration(ServerConfiguration conf) {
+    // reparse the table configuration properties in case a table property is 
being
+    // changed at the system level.
+    parseTableConfiguration(conf);
+
     String oobProperty = 
conf.getConfiguration().get(HOST_BALANCER_OOB_CHECK_KEY);
     if (null != oobProperty) {
       oobCheckMillis = AccumuloConfiguration.getTimeInMillis(oobProperty);
@@ -317,13 +325,15 @@ public class HostRegexTableLoadBalancer extends 
TableLoadBalancer implements Con
   @Override
   public void init(ServerConfigurationFactory conf) {
     super.init(conf);
-    parseConfiguration(conf);
+    parseSystemConfiguration(conf);
   }
 
   @Override
   public void getAssignments(SortedMap<TServerInstance,TabletServerStatus> 
current,
       Map<KeyExtent,TServerInstance> unassigned, 
Map<KeyExtent,TServerInstance> assignments) {
 
+    parseSystemConfiguration(this.configuration);
+
     Map<String,SortedMap<TServerInstance,TabletServerStatus>> pools = 
splitCurrentByRegex(current);
     // group the unassigned into tables
     Map<String,Map<KeyExtent,TServerInstance>> groupedUnassigned = new 
HashMap<>();
@@ -368,6 +378,8 @@ public class HostRegexTableLoadBalancer extends 
TableLoadBalancer implements Con
     if (t == null)
       return minBalanceTime;
 
+    parseSystemConfiguration(this.configuration);
+
     Map<String,String> tableIdMap = t.tableIdMap();
     long now = System.currentTimeMillis();
 
@@ -543,12 +555,12 @@ public class HostRegexTableLoadBalancer extends 
TableLoadBalancer implements Con
 
   @Override
   public void propertyChanged(String key) {
-    parseConfiguration(this.configuration);
+    parseTableConfiguration(this.configuration);
   }
 
   @Override
   public void propertiesChanged() {
-    parseConfiguration(this.configuration);
+    parseTableConfiguration(this.configuration);
   }
 
   @Override
diff --git 
a/server/base/src/test/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancerReconfigurationTest.java
 
b/server/base/src/test/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancerReconfigurationTest.java
index aec9167..a863a81 100644
--- 
a/server/base/src/test/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancerReconfigurationTest.java
+++ 
b/server/base/src/test/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancerReconfigurationTest.java
@@ -96,6 +96,25 @@ public class HostRegexTableLoadBalancerReconfigurationTest
           || migration.newServer.host().startsWith("192.168.0.4")
           || migration.newServer.host().startsWith("192.168.0.5"));
     }
+
+    // checking that system properties are reread when balance is called
+    assertEquals(7000, this.getOobCheckMillis());
+    assertEquals(10, this.getMaxOutstandingMigrations());
+    assertEquals(4, this.getMaxMigrations());
+
+    
DEFAULT_TABLE_PROPERTIES.put(HostRegexTableLoadBalancer.HOST_BALANCER_OOB_CHECK_KEY,
 "1m");
+    DEFAULT_TABLE_PROPERTIES
+        
.put(HostRegexTableLoadBalancer.HOST_BALANCER_OUTSTANDING_MIGRATIONS_KEY, "50");
+    
DEFAULT_TABLE_PROPERTIES.put(HostRegexTableLoadBalancer.HOST_BALANCER_REGEX_MAX_MIGRATIONS_KEY,
+        "1000");
+    migrations.clear();
+    migrationsOut.clear();
+    this.balance(Collections.unmodifiableSortedMap(allTabletServers), 
migrations, migrationsOut);
+
+    // now we should see the new values
+    assertEquals(60000, this.getOobCheckMillis());
+    assertEquals(50, this.getMaxOutstandingMigrations());
+    assertEquals(1000, this.getMaxMigrations());
   }
 
   @Override