ctubbsii commented on a change in pull request #401: Fix ChaoticBalancerIT
URL: https://github.com/apache/accumulo/pull/401#discussion_r174987369
##########
File path:
test/src/test/java/org/apache/accumulo/test/functional/ChaoticBalancerIT.java
##########
@@ -42,6 +42,7 @@ public void configureMiniCluster(MiniAccumuloConfigImpl cfg,
Configuration hadoo
Map<String,String> siteConfig = cfg.getSiteConfig();
siteConfig.put(Property.TSERV_MAXMEM.getKey(), "10K");
siteConfig.put(Property.TSERV_MAJC_DELAY.getKey(), "0");
+ siteConfig.put(Property.TABLE_LOAD_BALANCER.getKey(),
ChaoticLoadBalancer.class.getName());
Review comment:
I think the idea of this test was not just to test the `ChaoticBalancer`,
but also to test the per-table balancer configuration feature. What you've done
here should work, but it's probably best to set the property at table creation
time using `NewTableConfiguration` rather than move it to the site
configuration. That way, we can still test that per-table configuration path
for having a custom balancer for a specific table and you still have the
property set before the table is brought online for the first time.
(Also, I think it probably doesn't matter... but I'm not sure what the
impact of setting this balancer for all tables will have. We may take
precautions so it doesn't affect the metadata tables, but I'm not sure.)
Also, I saw a recent failure of `RegexGroupBalancerIT`, and I believe it has
the same problem you were trying to fix here. Might be best to fix both.
:smiley_cat:
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
