Fixes balancing of metadata table

[keith-turner]

No description provided.

[keith-turner]

Have not tested these changes. Made similar changes in 4.0 and manually tested them. We probably need an IT that test balancing the metadata table if there is not one. In 4.0 this issue was preventing the metadata table from balancing across multiple tablet servers.

[ctubbsii]

Why not make it a Map<TableId,TableInfo> instead of Map<String,TableInfo>?

[keith-turner]

Why not make it a Map<TableId,TableInfo> instead of Map<String,TableInfo>?

The code is currently manipulating a Thrift object which can not use the TableId type. A larger refactor would be needed to make that change, could open in issue to look into that.

[keith-turner]

Working on adding an IT for metadata table balancing.

[keith-turner]

Wrote a test and found some additional issues and made fixes to get the test to pass. Converted this PR to draft because I want to go review the changes I made to get the test to pass.

[keith-turner]

After writing the new IT I saw the following problems, with the first being the initial reason I opened this PR.

1. Metadata table was not balancing
2. Sometimes user tables would balance when the metadata table was not balanced
3. Each call to the balancer would balance all tables. For example when balancing the root table it would compute balancing for all tables.

For 1 the changes in 6eab50f fix this. For 2 and 3 the changes in 6de5ef9 fix these. For 2 created a new Migrations class that tracks migrations per data level so that when making balancing decisions it could always know if the previous levels had migrations. For 3 added a new method to the balancer SPI that conveys what tables need to be balanced. This new method support partitioning tables for balancing in different ways and allows balancers to delegate to other balancers with smaller partitions. May also allow different manager processes to parition tables for balancing in the future.

Took this out of draft after the changes in 6de5ef9

The new IT added only test problem 1. For problems 2 and 3, I verified those by running the IT and looking at the manager logs. The test exercises the cause of problems 2 and 3 but will not fail if they occur.

[ddanielr]

Adding a blocker label as this change is needed before finalizing the 2.1.4 release.

[ddanielr]

Applied the BalanceIT test changes to current 2.1 and replicated the breakage.
Then stepped through applying commits individually to understand them.

1. Metadata table was not balancing

Confirmed this is fixed by 6eab50f

3. Each call to the balancer would balance all tables. For example when balancing the root table it would compute balancing for all tables.

This could be fixed in the current code by updating the balance method in TableLoadBalancer.

accumulo/core/src/main/java/org/apache/accumulo/core/spi/balancer/TableLoadBalancer.java

Lines 125 to 133 in aa7944b

	public long balance(BalanceParameters params) {
	long minBalanceTime = 5_000;
	// Iterate over the tables and balance each of them
	final DataLevel currentDataLevel = DataLevel.valueOf(params.currentLevel());
	for (TableId tableId : environment.getTableIdMap().values()) {
	ArrayList<TabletMigration> newMigrations = new ArrayList<>();
	long tableBalanceTime =
	getBalancerForTable(tableId).balance(new BalanceParamsImpl(params.currentStatus(),
	params.currentMigrations(), newMigrations, currentDataLevel));

It just needs to check the datalevel level of the tableId and just skipping that tableID if it does not match the current level.

if (DataLevel.of(tableId) != currentDataLevel) {
       // Don't balance tables that are outside the current data level.
        log.info("Skipping Table {} as it is not in the current data level", tableId);
        continue;
}

2. Sometimes user tables would balance when the metadata table was not balanced

I like the new migrations class but I'd like to understand how it fits into the overall balancing design.
We've made a good deal of changes trying to fix balancing bugs in 2.1 and it might be a good time to take a step back and re-examine it.

[keith-turner]

It just needs to check the datalevel level of the tableId and just skipping that tableID if it does not match the current level.

Sometimes a balancer is expected to work with a smaller set of tables than the entire data level (like when the tablet balancer create a sub balancer). This is why I added the set of tables to balance as a map. This make it less mysterious, instead of using an arbitrary string to somehow determine which subset of tables to balance its given explicitly in a map.

[ddanielr]

All changes from this PR should have been addressed in #5358, #5359, and #5363.