Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add option for balancing a single table at a time when using host regex balancer #4523

Draft
wants to merge 2 commits into
base: 2.1
Choose a base branch
from
+57 −3
Draft

Add option for balancing a single table at a time when using host regex balancer #4523

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c3e1c96
Add option for balancing a single table at a time when using host regex
cshannon May 3, 2024
6a2b43d
Merge branch '2.1' into accumulo-4521
cshannon May 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 23 additions & 3 deletions core/src/main/java/org/apache/accumulo/core/spi/balancer/HostRegexTableLoadBalancer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,11 @@
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Optional;
import java.util.Set;
import java.util.SortedMap;
import java.util.TreeMap;
import java.util.TreeSet;
import java.util.function.Supplier;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -110,6 +112,9 @@ public class HostRegexTableLoadBalancer extends TableLoadBalancer {
private static final int DEFAULT_OUTSTANDING_MIGRATIONS = 0;
public static final String HOST_BALANCER_OUTSTANDING_MIGRATIONS_KEY =
PROP_PREFIX + "balancer.host.regex.max.outstanding.migrations";
public static final String HOST_BALANCER_REGEX_CONCURRENT_TABLES_KEY =
PROP_PREFIX + "balancer.host.regex.concurrent.tables";
private static final boolean HOST_BALANCER_REGEX_CONCURRENT_TABLES_DEFAULT = true;

private static Map<String,String> getRegexes(PluginEnvironment.Configuration conf) {
Map<String,String> regexes = new HashMap<>();
Expand All @@ -121,7 +126,8 @@ private static Map<String,String> getRegexes(PluginEnvironment.Configuration con
if (customProp.getKey().equals(HOST_BALANCER_OOB_CHECK_KEY)
|| customProp.getKey().equals(HOST_BALANCER_REGEX_USING_IPS_KEY)
|| customProp.getKey().equals(HOST_BALANCER_REGEX_MAX_MIGRATIONS_KEY)
|| customProp.getKey().equals(HOST_BALANCER_OUTSTANDING_MIGRATIONS_KEY)) {
|| customProp.getKey().equals(HOST_BALANCER_OUTSTANDING_MIGRATIONS_KEY)
|| customProp.getKey().equals(HOST_BALANCER_REGEX_CONCURRENT_TABLES_KEY)) {
continue;
}
String tableName = customProp.getKey().substring(HOST_BALANCER_PREFIX.length());
Expand All @@ -146,6 +152,7 @@ static class HrtlbConf {
private boolean isIpBasedRegex = false;
private final Map<String,String> regexes;
private final Map<String,Pattern> poolNameToRegexPattern;
private final boolean concurrentTables;

HrtlbConf(PluginEnvironment.Configuration conf) {
System.out.println("building hrtlb conf");
Expand All @@ -165,6 +172,8 @@ static class HrtlbConf {
if (outstanding != null) {
maxOutstandingMigrations = Integer.parseInt(outstanding);
}
concurrentTables = Optional.ofNullable(conf.get(HOST_BALANCER_REGEX_CONCURRENT_TABLES_KEY))
.map(Boolean::parseBoolean).orElse(HOST_BALANCER_REGEX_CONCURRENT_TABLES_DEFAULT);

this.regexes = getRegexes(conf);

Expand Down Expand Up @@ -466,6 +475,13 @@ public long balance(BalanceParameters params) {
LOG.trace("Sample up to 10 outstanding migrations: {}", limitTen(migrations));
}
return minBalanceTime;
} else if (!myConf.concurrentTables) {
LOG.warn("Not balancing tables due to {} existing migrations and {}} is set to false",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be at debug? It seems that it will be common and would be expected to sort itself out eventually. It would only be a concern is it was persistent? Could the message include something to show that it may be making progress? Current id being processed?

Is this a log message that quickly flood the log?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this should likely be debug or trace, I just copied it from above and didn't pay attention but that would definitely flood things pretty quickly and it is expected.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems another case where we really could benefit from being able to log 1 of Number of 1 every T seconds occurrences of the same message. It is not helpful to keep repeating that same message frequently, but having it appear occasionally helps troubleshoot cases where something in not making progress - might be out of scope for this, but could be worthwhile.

migrations.size(), HOST_BALANCER_REGEX_CONCURRENT_TABLES_KEY);
if (LOG.isTraceEnabled()) {
LOG.trace("Sample up to 10 outstanding migrations: {}", limitTen(migrations));
}
return minBalanceTime;
}

LOG.debug("Current outstanding migrations of {} being applied", migrations.size());
Expand Down Expand Up @@ -493,7 +509,8 @@ public long balance(BalanceParameters params) {
migrationsFromLastPass.clear();
}

for (TableId tableId : tableIdMap.values()) {
// Sort the tableIds so we process them in order always
for (TableId tableId : new TreeSet<>(tableIdMap.values())) {
String tableName = tableIdToTableName.get(tableId);
String regexTableName = getPoolNameForTable(tableName);
SortedMap<TabletServerId,TServerStatus> currentView = currentGrouped.get(regexTableName);
Expand All @@ -517,7 +534,10 @@ public long balance(BalanceParameters params) {
}

migrationsOut.addAll(newMigrations);
if (migrationsOut.size() >= myConf.maxTServerMigrations) {
// Break if concurrentTables is false and migrations are found as we only are processing
// one table at a time. Also break if the migrations count reaches maxTServerMigrations
if ((!myConf.concurrentTables && !migrationsOut.isEmpty())
|| (migrationsOut.size() >= myConf.maxTServerMigrations)) {
break;
}
}
Expand Down
34 changes: 34 additions & 0 deletions core/src/test/java/org/apache/accumulo/core/spi/balancer/HostRegexTableLoadBalancerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,40 @@ public void testOutOfBoundsTablets() {
assertEquals(2, migrationsOut.size());
}

/**
* Test that balance will only process 1 table at a time if migrations are found before returning
*/
@Test
public void testBalanceConcurrentTablesFalse() throws InterruptedException {
List<TabletMigration> migrationsOut = new ArrayList<>();
HashMap<String,String> props = new HashMap<>(DEFAULT_TABLE_PROPERTIES);
// Only balance 1 table at a time
props.put(HostRegexTableLoadBalancer.HOST_BALANCER_REGEX_CONCURRENT_TABLES_KEY, "false");
// Disable max migrations for this test
props.put(HostRegexTableLoadBalancer.HOST_BALANCER_REGEX_MAX_MIGRATIONS_KEY, "0");
init(props);
Set<TabletId> migrations = new HashSet<>();

long wait =
this.balance(new BalanceParamsImpl(Collections.unmodifiableSortedMap(createCurrent(15)),
migrations, migrationsOut));
assertEquals(20000, wait);
// should oly balance four tablets we are only balancing one table at a time.
assertEquals(4, migrationsOut.size());

// now balance again passing in the new migrations
for (TabletMigration m : migrationsOut) {
migrations.add(m.getTablet());
}

migrationsOut.clear();
wait = this.balance(new BalanceParamsImpl(Collections.unmodifiableSortedMap(createCurrent(15)),
migrations, migrationsOut));
assertEquals(20000, wait);
// should not balance due to existing migrations and concurrent is false
assertEquals(0, migrationsOut.size());
}

@Override
public List<TabletStatistics> getOnlineTabletsForTable(TabletServerId tserver, TableId tableId) {
// Report incorrect information so that balance will create an assignment
Expand Down