Copilot commented on code in PR #8733:
URL: https://github.com/apache/hbase/pull/8733#discussion_r4221982362
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupBasedLoadBalancer.java:
##########
@@ -99,6 +100,44 @@ public class RSGroupBasedLoadBalancer implements
LoadBalancer {
private volatile boolean fallbackEnabled = false;
+ @RestrictedApi(explanation = "Should only be called in tests", link = "",
+ allowedOnPath = ".*/src/test/.*")
+ public void setFallbackEnabledForTest(boolean enabled) {
+ this.fallbackEnabled = enabled;
+ }
+
+ // Test-only call-tracking flags, set true whenever the corresponding
assignment method below is
+ // invoked. Reset via resetAssignmentCallFlagsForTest() before the operation
under test.
+ private volatile boolean roundRobinAssignmentInvoked = false;
+ private volatile boolean retainAssignmentInvoked = false;
+ private volatile boolean randomAssignmentInvoked = false;
+
+ @RestrictedApi(explanation = "Should only be called in tests", link = "",
+ allowedOnPath = ".*/src/test/.*")
+ public boolean isRoundRobinAssignmentInvoked() {
+ return roundRobinAssignmentInvoked;
+ }
Review Comment:
These test-only helpers are `public` on a production class, which increases
the public API surface and can be used unintentionally in non-test code
(especially if Error Prone isn't enforced in all builds). If feasible, prefer
reducing visibility (e.g., package-private) and keeping tests in the same
package, or using a test-only accessor mechanism (e.g., `@VisibleForTesting` +
non-public members, or a test subclass/instrumentation hook) to avoid exposing
these methods as part of the runtime API.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfoManagerImpl.java:
##########
@@ -247,37 +253,32 @@ private RSGroupInfoManagerImpl(MasterServices
masterServices) {
this.script = new RSGroupMappingScript(masterServices.getConfiguration());
}
- private synchronized void updateDefaultServers() {
- LOG.info("Updating default servers.");
- Map<String, RSGroupInfo> newGroupMap =
Maps.newHashMap(holder.groupName2Group);
- RSGroupInfo oldDefaultGroupInfo = getRSGroup(RSGroupInfo.DEFAULT_GROUP);
- assert oldDefaultGroupInfo != null;
- RSGroupInfo newDefaultGroupInfo =
- new RSGroupInfo(RSGroupInfo.DEFAULT_GROUP, getDefaultServers());
- newDefaultGroupInfo.addAllTables(oldDefaultGroupInfo.getTables());
- newGroupMap.put(RSGroupInfo.DEFAULT_GROUP, newDefaultGroupInfo);
- // do not need to persist, as we do not persist default group.
- resetRSGroupMap(newGroupMap);
- LOG.info("Updated default servers, {} servers",
newDefaultGroupInfo.getServers().size());
- if (LOG.isDebugEnabled()) {
- LOG.debug("New default servers list: {}",
newDefaultGroupInfo.getServers());
+ /**
+ * Called synchronously from {@code ServerListener#serverAdded}/{@code
#serverRemoved}. Server
+ * membership of 'default' and of every regex-governed group is never
persisted, so it is
+ * recomputed from the live servers and applied in memory only -- no {@code
hbase:rsgroup}/ZK I/O.
+ */
+ private synchronized void handleServerEvent() {
+ Map<String, RSGroupInfo> currentGroups = holder.groupName2Group;
+ Map<String, SortedSet<Address>> autoManagedServers =
+ computeAutoManagedRSGroupServers(currentGroups.values());
+ Map<String, RSGroupInfo> newGroupMap = Maps.newHashMap(currentGroups);
+ applyAutoManagedRSGroupServers(newGroupMap, autoManagedServers);
+ dropRegexGovernedServersFromOtherGroups(newGroupMap, autoManagedServers);
+ for (Map.Entry<String, RSGroupInfo> entry : newGroupMap.entrySet()) {
+ if
(!entry.getValue().getServers().equals(currentGroups.get(entry.getKey()).getServers()))
{
+ resetRSGroupMap(newGroupMap);
+ LOG.info("Updated auto-managed RSGroup servers, {} servers",
+
autoManagedServers.values().stream().mapToInt(SortedSet::size).sum());
+ return;
+ }
}
+ LOG.info("No changes in auto-managed RSGroup server membership.");
Review Comment:
A number of logs here are emitted at INFO on every RS join/leave (and
potentially on every move via getRegexGroupMap/checkMoveAgreesWithRegex paths).
In busy clusters this can generate significant log volume and noise (e.g.,
per-hostname match logs, full resolved maps, and 'No changes' messages).
Consider downgrading the high-frequency/diagnostic logs to DEBUG/TRACE (e.g.,
getMatchingRSGroupNames, 'Resolved regex...' map dump, 'Computed...' dump, and
the 'No changes...' line), while keeping WARN/INFO only for actionable events
(invalid regex, overlapping matches, empty default, etc.).
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfoManagerImpl.java:
##########
@@ -742,30 +831,267 @@ private void updateCacheOfRSGroups(final Set<String>
currentGroups) {
this.prevRSGroups.addAll(currentGroups);
}
- // Called by ServerEventsListenerThread. Presume it has lock on this manager
when it runs.
- private SortedSet<Address> getDefaultServers() {
- return getDefaultServers(listRSGroups()/* get from rsGroupMap */);
+ /**
+ * Parses {@code hbase.rsgroup.regex.<groupname>=<regex>} entries out of
{@code conf}. Entries
+ * with an invalid group name, targeting the reserved {@link
RSGroupInfo#DEFAULT_GROUP}, or with
+ * an unparsable regex are logged and skipped.
+ */
+ static Map<String, Pattern> getRegexGroupMap(Configuration conf) {
+ Map<String, String> rsGroupNameToRegexMap =
conf.getPropsWithPrefix(RS_GROUP_REGEX_PREFIX);
+ Map<String, Pattern> rsGroupNameToPatternMap = new HashMap<>();
+ for (Map.Entry<String, String> e : rsGroupNameToRegexMap.entrySet()) {
Review Comment:
A number of logs here are emitted at INFO on every RS join/leave (and
potentially on every move via getRegexGroupMap/checkMoveAgreesWithRegex paths).
In busy clusters this can generate significant log volume and noise (e.g.,
per-hostname match logs, full resolved maps, and 'No changes' messages).
Consider downgrading the high-frequency/diagnostic logs to DEBUG/TRACE (e.g.,
getMatchingRSGroupNames, 'Resolved regex...' map dump, 'Computed...' dump, and
the 'No changes...' line), while keeping WARN/INFO only for actionable events
(invalid regex, overlapping matches, empty default, etc.).
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfoManagerImpl.java:
##########
@@ -742,30 +831,267 @@ private void updateCacheOfRSGroups(final Set<String>
currentGroups) {
this.prevRSGroups.addAll(currentGroups);
}
- // Called by ServerEventsListenerThread. Presume it has lock on this manager
when it runs.
- private SortedSet<Address> getDefaultServers() {
- return getDefaultServers(listRSGroups()/* get from rsGroupMap */);
+ /**
+ * Parses {@code hbase.rsgroup.regex.<groupname>=<regex>} entries out of
{@code conf}. Entries
+ * with an invalid group name, targeting the reserved {@link
RSGroupInfo#DEFAULT_GROUP}, or with
+ * an unparsable regex are logged and skipped.
+ */
+ static Map<String, Pattern> getRegexGroupMap(Configuration conf) {
+ Map<String, String> rsGroupNameToRegexMap =
conf.getPropsWithPrefix(RS_GROUP_REGEX_PREFIX);
+ Map<String, Pattern> rsGroupNameToPatternMap = new HashMap<>();
+ for (Map.Entry<String, String> e : rsGroupNameToRegexMap.entrySet()) {
+ if (e.getValue() == null) {
+ // getPropsWithPrefix reads names and values separately, so an entry
removed by a concurrent
+ // configuration reload can show up here with a null value. Treat it
as unset.
+ continue;
+ }
+ if (!GROUP_NAME_PATTERN.matcher(e.getKey()).matches()) {
+ LOG.warn("Ignoring {}{} -- '{}' is not a valid RSGroup name (only
alphanumeric characters "
+ + "and underscore allowed)", RS_GROUP_REGEX_PREFIX, e.getKey(),
e.getKey());
+ continue;
+ }
+ if (RSGroupInfo.DEFAULT_GROUP.equals(e.getKey())) {
+ LOG.warn("Ignoring {}{} -- regex-based membership cannot target the
reserved '{}' group",
+ RS_GROUP_REGEX_PREFIX, e.getKey(), RSGroupInfo.DEFAULT_GROUP);
+ continue;
+ }
+ try {
+ rsGroupNameToPatternMap.put(e.getKey(), Pattern.compile(e.getValue()));
+ } catch (PatternSyntaxException ex) {
+ LOG.warn("Invalid regex '{}' for RSGroup '{}' ({}{}); ignoring this
entry", e.getValue(),
+ e.getKey(), RS_GROUP_REGEX_PREFIX, e.getKey());
+ }
+ }
+ LOG.info("Resolved regex-based RSGroup membership config: {}",
rsGroupNameToPatternMap);
+ return rsGroupNameToPatternMap;
}
- // Called by ServerEventsListenerThread. Presume it has lock on this manager
when it runs.
- private SortedSet<Address> getDefaultServers(List<RSGroupInfo>
rsGroupInfoList) {
- // Build a list of servers in other groups than default group, from
rsGroupMap
- Set<Address> serversInOtherGroup = new HashSet<>();
- for (RSGroupInfo group : rsGroupInfoList) {
- if (!RSGroupInfo.DEFAULT_GROUP.equals(group.getName())) { // not default
group
- serversInOtherGroup.addAll(group.getServers());
+ private static List<String> getMatchingRSGroupNames(String hostname,
+ Map<String, Pattern> rsGroupNameToPatternMap) {
+ List<String> rsGroupNames = new ArrayList<>();
+ for (Map.Entry<String, Pattern> e : rsGroupNameToPatternMap.entrySet()) {
+ if (e.getValue().matcher(hostname).matches()) {
+ rsGroupNames.add(e.getKey());
}
}
+ LOG.info("Hostname '{}' matches RSGroup name(s) {}", hostname,
rsGroupNames);
+ return rsGroupNames;
+ }
Review Comment:
A number of logs here are emitted at INFO on every RS join/leave (and
potentially on every move via getRegexGroupMap/checkMoveAgreesWithRegex paths).
In busy clusters this can generate significant log volume and noise (e.g.,
per-hostname match logs, full resolved maps, and 'No changes' messages).
Consider downgrading the high-frequency/diagnostic logs to DEBUG/TRACE (e.g.,
getMatchingRSGroupNames, 'Resolved regex...' map dump, 'Computed...' dump, and
the 'No changes...' line), while keeping WARN/INFO only for actionable events
(invalid regex, overlapping matches, empty default, etc.).
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfoManagerImpl.java:
##########
@@ -742,30 +831,267 @@ private void updateCacheOfRSGroups(final Set<String>
currentGroups) {
this.prevRSGroups.addAll(currentGroups);
}
- // Called by ServerEventsListenerThread. Presume it has lock on this manager
when it runs.
- private SortedSet<Address> getDefaultServers() {
- return getDefaultServers(listRSGroups()/* get from rsGroupMap */);
+ /**
+ * Parses {@code hbase.rsgroup.regex.<groupname>=<regex>} entries out of
{@code conf}. Entries
+ * with an invalid group name, targeting the reserved {@link
RSGroupInfo#DEFAULT_GROUP}, or with
+ * an unparsable regex are logged and skipped.
+ */
+ static Map<String, Pattern> getRegexGroupMap(Configuration conf) {
+ Map<String, String> rsGroupNameToRegexMap =
conf.getPropsWithPrefix(RS_GROUP_REGEX_PREFIX);
+ Map<String, Pattern> rsGroupNameToPatternMap = new HashMap<>();
+ for (Map.Entry<String, String> e : rsGroupNameToRegexMap.entrySet()) {
+ if (e.getValue() == null) {
+ // getPropsWithPrefix reads names and values separately, so an entry
removed by a concurrent
+ // configuration reload can show up here with a null value. Treat it
as unset.
+ continue;
+ }
+ if (!GROUP_NAME_PATTERN.matcher(e.getKey()).matches()) {
+ LOG.warn("Ignoring {}{} -- '{}' is not a valid RSGroup name (only
alphanumeric characters "
+ + "and underscore allowed)", RS_GROUP_REGEX_PREFIX, e.getKey(),
e.getKey());
+ continue;
+ }
+ if (RSGroupInfo.DEFAULT_GROUP.equals(e.getKey())) {
+ LOG.warn("Ignoring {}{} -- regex-based membership cannot target the
reserved '{}' group",
+ RS_GROUP_REGEX_PREFIX, e.getKey(), RSGroupInfo.DEFAULT_GROUP);
+ continue;
+ }
+ try {
+ rsGroupNameToPatternMap.put(e.getKey(), Pattern.compile(e.getValue()));
+ } catch (PatternSyntaxException ex) {
+ LOG.warn("Invalid regex '{}' for RSGroup '{}' ({}{}); ignoring this
entry", e.getValue(),
+ e.getKey(), RS_GROUP_REGEX_PREFIX, e.getKey());
+ }
+ }
+ LOG.info("Resolved regex-based RSGroup membership config: {}",
rsGroupNameToPatternMap);
+ return rsGroupNameToPatternMap;
}
- // Called by ServerEventsListenerThread. Presume it has lock on this manager
when it runs.
- private SortedSet<Address> getDefaultServers(List<RSGroupInfo>
rsGroupInfoList) {
- // Build a list of servers in other groups than default group, from
rsGroupMap
- Set<Address> serversInOtherGroup = new HashSet<>();
- for (RSGroupInfo group : rsGroupInfoList) {
- if (!RSGroupInfo.DEFAULT_GROUP.equals(group.getName())) { // not default
group
- serversInOtherGroup.addAll(group.getServers());
+ private static List<String> getMatchingRSGroupNames(String hostname,
+ Map<String, Pattern> rsGroupNameToPatternMap) {
+ List<String> rsGroupNames = new ArrayList<>();
+ for (Map.Entry<String, Pattern> e : rsGroupNameToPatternMap.entrySet()) {
+ if (e.getValue().matcher(hostname).matches()) {
+ rsGroupNames.add(e.getKey());
}
}
+ LOG.info("Hostname '{}' matches RSGroup name(s) {}", hostname,
rsGroupNames);
+ return rsGroupNames;
+ }
- // Get all online servers from Zookeeper and find out servers in default
group
- SortedSet<Address> defaultServers = Sets.newTreeSet();
- for (ServerName serverName :
masterServices.getServerManager().getOnlineServers().keySet()) {
- Address server = Address.fromParts(serverName.getHostname(),
serverName.getPort());
- if (!serversInOtherGroup.contains(server)) { // not in other groups
- defaultServers.add(server);
+ /**
+ * Resolves every server in {@code onlineServers} whose hostname
unambiguously matches exactly one
+ * entry of {@code rsGroupNameToPatternMap} naming a group in {@code
existingGroupNames}. A server
+ * matching multiple regexes, or matching a regex for a group that does not
(yet) exist, is
+ * omitted (warn-only) rather than resolved.
+ */
+ static Map<Address, String> resolveServerAddrToRSGroupName(Set<Address>
onlineServers,
+ Map<String, Pattern> rsGroupNameToPatternMap, Set<String>
existingGroupNames) {
+ Map<Address, String> serverAddrToRSGroupName = new HashMap<>();
+ if (rsGroupNameToPatternMap.isEmpty()) {
+ return serverAddrToRSGroupName;
+ }
+ Set<String> nonExistingRSGroupNames = new HashSet<>();
+ for (Address server : onlineServers) {
+ List<String> matchingRSGroupNames =
+ getMatchingRSGroupNames(server.getHostName(), rsGroupNameToPatternMap);
+ if (matchingRSGroupNames.isEmpty()) {
+ continue;
+ }
+ if (matchingRSGroupNames.size() > 1) {
+ LOG.warn("Server {} hostname matches regexes for multiple RSGroups {};
treating it as "
+ + "unmatched by regex (falls back to default) until the overlapping "
+ + "hbase.rsgroup.regex.* entries are fixed", server,
matchingRSGroupNames);
+ continue;
+ }
+ String rsGroupName = matchingRSGroupNames.get(0);
+ if (existingGroupNames.contains(rsGroupName)) {
+ serverAddrToRSGroupName.put(server, rsGroupName);
+ } else if (nonExistingRSGroupNames.add(rsGroupName)) {
+ LOG.warn(
+ "Config {}{} matches server {} but RSGroup '{}' does not exist --
create it with "
+ + "addRSGroup first; treating this server as unmatched by regex
until then",
+ RS_GROUP_REGEX_PREFIX, rsGroupName, server, rsGroupName);
}
}
- return defaultServers;
+ LOG.info("Resolved server address to RSGroup name map: {}",
serverAddrToRSGroupName);
+ return serverAddrToRSGroupName;
+ }
+
+ /** Returns {@code true} if every online server is either admin-managed or
regex-matched. */
+ static boolean wouldEmptyDefaultGroup(Set<Address> onlineServers,
+ Set<Address> adminManagedServers, Set<Address> regexMatchedServers) {
+ for (Address server : onlineServers) {
+ if (!adminManagedServers.contains(server) &&
!regexMatchedServers.contains(server)) {
+ return false;
+ }
+ }
+ return !onlineServers.isEmpty();
+ }
+
+ static final class RegexBasedRSGroupMembershipResolution {
+ final Set<Address> adminManagedServers;
+ final Map<Address, String> regexMatchedServers;
+
+ RegexBasedRSGroupMembershipResolution(Set<Address> adminManagedServers,
+ Map<Address, String> regexMatchedServers) {
+ this.adminManagedServers = adminManagedServers;
+ this.regexMatchedServers = regexMatchedServers;
+ }
+ }
+
+ static RegexBasedRSGroupMembershipResolution
resolveRegexBasedRSGroupMembership(
+ Map<String, Pattern> rsGroupNameToPatternMap, Set<Address> onlineServers,
+ Collection<RSGroupInfo> existingGroups) {
+ Set<String> existingGroupNames =
+
existingGroups.stream().map(RSGroupInfo::getName).collect(Collectors.toSet());
+ Set<Address> adminManagedServers = new HashSet<>();
+ for (RSGroupInfo existingGroup : existingGroups) {
+ if (
+ !RSGroupInfo.DEFAULT_GROUP.equals(existingGroup.getName())
+ && !rsGroupNameToPatternMap.containsKey(existingGroup.getName())
+ ) {
+ adminManagedServers.addAll(existingGroup.getServers());
+ }
+ }
+ Map<Address, String> regexMatchedServers =
+ resolveServerAddrToRSGroupName(onlineServers, rsGroupNameToPatternMap,
existingGroupNames);
+ // A server stored in an admin-managed group that a regex now claims
belongs to the regex group.
+ adminManagedServers.removeAll(regexMatchedServers.keySet());
+ if (wouldEmptyDefaultGroup(onlineServers, adminManagedServers,
regexMatchedServers.keySet())) {
+ // Regex placement is still applied: every server lands in its correct
group. Operators are
+ // expected to configure regexes carefully, so this is a warning, not an
error.
+ LOG.warn(
+ "Regex-based RSGroup membership leaves RSGroup '{}' with no online
servers -- every "
+ + "online server is either admin-managed or matches a {}* entry.
Tables in RSGroup '{}' "
+ + "(including system tables, unless they are in another RSGroup)
have no server of "
+ + "their own to be assigned to; set {} to let them fall back to
servers of other "
+ + "RSGroups",
+ RSGroupInfo.DEFAULT_GROUP, RS_GROUP_REGEX_PREFIX,
RSGroupInfo.DEFAULT_GROUP,
+ RSGroupBasedLoadBalancer.FALLBACK_GROUP_ENABLE_KEY);
+ }
+ LOG.info(
+ "Regex-based RSGroup membership resolution: onlineServers={}, "
+ + "adminManagedServers={}, regexMatchedServers={}",
+ onlineServers, adminManagedServers, regexMatchedServers);
+ return new RegexBasedRSGroupMembershipResolution(adminManagedServers,
regexMatchedServers);
+ }
+
+ private Map<String, SortedSet<Address>>
+ computeAutoManagedRSGroupServers(Collection<RSGroupInfo> existingGroups) {
+ LOG.info("Computing auto-managed RSGroup server membership.");
+ Set<Address> onlineServers = getOnlineServers();
+ Map<String, Pattern> rsGroupNameToPatternMap =
+ getRegexGroupMap(masterServices.getConfiguration());
+ RegexBasedRSGroupMembershipResolution resolution =
+ resolveRegexBasedRSGroupMembership(rsGroupNameToPatternMap,
onlineServers, existingGroups);
+
+ Set<String> existingGroupNames =
+
existingGroups.stream().map(RSGroupInfo::getName).collect(Collectors.toSet());
+ Map<String, SortedSet<Address>> result = new HashMap<>();
+ for (String rsGroupName : rsGroupNameToPatternMap.keySet()) {
+ if (existingGroupNames.contains(rsGroupName)) {
+ result.put(rsGroupName, new TreeSet<>());
+ }
+ }
+ result.put(RSGroupInfo.DEFAULT_GROUP, new TreeSet<>());
+
+ for (Address server : onlineServers) {
+ String regexGroupName = resolution.regexMatchedServers.get(server);
+ if (regexGroupName != null) {
+ result.get(regexGroupName).add(server);
+ continue;
+ }
+ if (resolution.adminManagedServers.contains(server)) {
+ continue;
+ }
+ result.get(RSGroupInfo.DEFAULT_GROUP).add(server);
+ }
+ for (Map.Entry<String, SortedSet<Address>> entry : result.entrySet()) {
+ if (entry.getValue().isEmpty() &&
!RSGroupInfo.DEFAULT_GROUP.equals(entry.getKey())) {
+ LOG.warn(
+ "Regex-governed RSGroup '{}' has no online servers; any table
assigned to it has "
+ + "nowhere to go (check {}{} and the online servers' hostnames)",
+ entry.getKey(), RS_GROUP_REGEX_PREFIX, entry.getKey());
+ }
+ }
+ LOG.info("Computed auto-managed RSGroup server membership: {}", result);
Review Comment:
A number of logs here are emitted at INFO on every RS join/leave (and
potentially on every move via getRegexGroupMap/checkMoveAgreesWithRegex paths).
In busy clusters this can generate significant log volume and noise (e.g.,
per-hostname match logs, full resolved maps, and 'No changes' messages).
Consider downgrading the high-frequency/diagnostic logs to DEBUG/TRACE (e.g.,
getMatchingRSGroupNames, 'Resolved regex...' map dump, 'Computed...' dump, and
the 'No changes...' line), while keeping WARN/INFO only for actionable events
(invalid regex, overlapping matches, empty default, etc.).
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfoManagerImpl.java:
##########
@@ -881,7 +1207,7 @@ private void multiMutate(List<Mutation> mutations) throws
IOException {
}
private void checkGroupName(String groupName) throws ConstraintException {
- if (!groupName.matches("[a-zA-Z0-9_]+")) {
+ if (!GROUP_NAME_PATTERN.matcher(groupName).matches()) {
throw new ConstraintException("RSGroup name should only contain
alphanumeric characters");
Review Comment:
The validation pattern allows underscores (`[a-zA-Z0-9_]+`), but the
exception message says 'only contain alphanumeric characters'. Updating the
message to include underscores would make the error clearer and consistent with
the actual constraint.
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRegexBasedRSGroupMembership.java:
##########
@@ -0,0 +1,1239 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.rsgroup;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.io.IOException;
+import java.io.StringWriter;
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.HConstants;
+import org.apache.hadoop.hbase.LocalHBaseCluster;
+import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.SingleProcessHBaseCluster;
+import org.apache.hadoop.hbase.client.Get;
+import org.apache.hadoop.hbase.client.RegionInfo;
+import org.apache.hadoop.hbase.client.Result;
+import org.apache.hadoop.hbase.client.Table;
+import org.apache.hadoop.hbase.net.Address;
+import org.apache.hadoop.hbase.testclassification.MediumTests;
+import org.apache.hadoop.hbase.testclassification.RSGroupTests;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.hadoop.hbase.util.DNS;
+import org.apache.hadoop.hbase.util.JVMClusterUtil;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.hbase.thirdparty.com.google.common.collect.Sets;
+
+import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil;
+import org.apache.hadoop.hbase.shaded.protobuf.generated.RSGroupProtos;
+
+/**
+ * Black-box tests for regex-based automatic RSGroup membership
+ * (hbase.rsgroup.regex.<groupname>=<regex>), driven entirely
through the public
+ * {@link org.apache.hadoop.hbase.client.Admin} RSGroup API, live RegionServer
add/remove against
+ * the minicluster, and captured log output -- no reflection or visibility
changes against
+ * {@link RSGroupInfoManagerImpl}.
+ */
+@Tag(RSGroupTests.TAG)
+@Tag(MediumTests.TAG)
+public class TestRegexBasedRSGroupMembership extends TestRSGroupsBase {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(TestRegexBasedRSGroupMembership.class);
+ private static final String REGEX_PREFIX = "hbase.rsgroup.regex.";
+
+ private final List<JVMClusterUtil.RegionServerThread> fakeRegionServers =
new ArrayList<>();
+ private final Set<String> regexConfigKeysSet = new HashSet<>();
+
+ @BeforeAll
+ public static void setUp() throws Exception {
+ setUpTestBeforeClass();
+ }
+
+ @AfterAll
+ public static void tearDown() throws Exception {
+ tearDownAfterClass();
+ }
+
+ @BeforeEach
+ public void beforeMethod(TestInfo testInfo) throws Exception {
+ setUpBeforeMethod(testInfo);
+ }
+
+ @AfterEach
+ public void afterMethod() throws Exception {
+ // Stop any fake-hostname RS this test started before handing back to the
shared teardown,
+ // which only ever starts servers to top back up to NUM_SLAVES_BASE, never
stops extras.
+ for (JVMClusterUtil.RegionServerThread rst : new
ArrayList<>(fakeRegionServers)) {
+ stopFakeRegionServer(rst);
+ }
+ // The MASTER/ADMIN Configuration objects are shared across every test
method in this class;
+ // undo any regex config this test added to both so it can't leak into the
next method.
+ for (String key : regexConfigKeysSet) {
+ MASTER.getConfiguration().unset(key);
+ ADMIN.getConfiguration().unset(key);
+ }
+ regexConfigKeysSet.clear();
+ tearDownAfterMethod();
+ RSGroupBasedLoadBalancer balancer = (RSGroupBasedLoadBalancer)
MASTER.getLoadBalancer();
+ balancer.resetAssignmentCallFlagsForTest();
+ balancer.setFallbackEnabledForTest(false);
+ }
+
+ // ============================== helpers ==============================
+
+ /**
+ * Starts a genuinely new RegionServer process (thread) in the shared
minicluster that reports
+ * itself under a caller-chosen hostname, so regex membership rules can
target it individually.
+ * The base cluster's real RS all share one real, resolvable hostname (this
machine's address);
+ * the RPC services constructor unconditionally resolves whatever hostname
is configured
+ * (independent of any RPC bind-address override), so the caller-chosen
hostname here must itself
+ * be a real, resolvable, bindable literal -- "127.0.0.1" and "localhost"
both work and are
+ * guaranteed distinct from the base cluster's real hostname, which is
exactly what every test in
+ * this class relies on to get a controllable, non-shared identity.
+ */
+ private JVMClusterUtil.RegionServerThread startFakeHostnameRS(String
hostname) throws Exception {
+ Configuration rsConf = new Configuration(TEST_UTIL.getConfiguration());
+ rsConf.set(DNS.UNSAFE_RS_HOSTNAME_KEY, hostname);
+ rsConf.set("hbase.regionserver.ipc.address", "localhost");
+ // The shared TEST_UTIL configuration's regionserver port is never
rewritten to an ephemeral
+ // value by minicluster startup (the base RS get their distinct ports
through a separate,
+ // per-instance mechanism), so without this override every fake RS started
here would try to
+ // bind the literal default port (16020) and collide with any other fake
RS still alive.
+ rsConf.set(HConstants.REGIONSERVER_PORT, "0");
+ // Same issue for the embedded web UI (default port 16030); these fake RS
don't need one.
+ rsConf.set(HConstants.REGIONSERVER_INFO_PORT, "-1");
+ LocalHBaseCluster localCluster = ((SingleProcessHBaseCluster)
CLUSTER).hbaseCluster;
+ JVMClusterUtil.RegionServerThread rst =
+ localCluster.addRegionServer(rsConf,
localCluster.getRegionServers().size());
+ rst.start();
+ rst.waitForServerOnline();
+ fakeRegionServers.add(rst);
+ final ServerName sn = rst.getRegionServer().getServerName();
+ TEST_UTIL.waitFor(WAIT_TIMEOUT,
+ () -> MASTER.getServerManager().getOnlineServersList().contains(sn));
+ LOG.info("Started fake-hostname RS {}", sn);
+ return rst;
+ }
+
+ private void stopFakeRegionServer(JVMClusterUtil.RegionServerThread rst)
throws Exception {
+ fakeRegionServers.remove(rst);
+ final ServerName sn = rst.getRegionServer().getServerName();
+ rst.getRegionServer().stop("test cleanup");
+ TEST_UTIL.waitFor(WAIT_TIMEOUT,
+ () -> !MASTER.getServerManager().getOnlineServersList().contains(sn));
+ LOG.info("Stopped fake-hostname RS {}", sn);
+ }
+
+ private void killFakeRegionServer(JVMClusterUtil.RegionServerThread rst)
throws Exception {
+ fakeRegionServers.remove(rst);
+ final ServerName sn = rst.getRegionServer().getServerName();
+ ((SingleProcessHBaseCluster) CLUSTER).killRegionServer(sn);
+ TEST_UTIL.waitFor(WAIT_TIMEOUT,
+ () -> !MASTER.getServerManager().getOnlineServersList().contains(sn));
+ LOG.info("Killed fake-hostname RS {}", sn);
+ }
+
+ /**
+ * Server-membership recompute happens synchronously in the {@code
serverAdded}/
+ * {@code serverRemoved} callback -- it is only ever triggered by an actual
server add/remove
+ * event, a bare config mutation is not enough. Forces such an event
(add+remove a bystander RS)
+ * so a config change just made takes effect without requiring an explicit
RSGroup admin RPC. Only
+ * used by tests that *want* the automatic reconciliation to run and correct
things; the trigger
+ * RS's own transient membership doesn't affect assertions elsewhere since
those only check for
+ * specific other servers' addresses.
+ */
+ private void triggerListenerCycle() throws Exception {
+ JVMClusterUtil.RegionServerThread trigger =
startFakeHostnameRS("127.0.0.1");
+ stopFakeRegionServer(trigger);
+ }
+
+ /** Reads the server set of a group straight from its row in the
hbase:rsgroup table. */
+ private Set<Address> readStoredServers(String groupName) throws IOException {
+ try (
+ Table table =
TEST_UTIL.getConnection().getTable(RSGroupInfoManagerImpl.RSGROUP_TABLE_NAME)) {
+ Result result = table.get(new Get(Bytes.toBytes(groupName)));
+ byte[] value = result.getValue(RSGroupInfoManagerImpl.META_FAMILY_BYTES,
+ RSGroupInfoManagerImpl.META_QUALIFIER_BYTES);
+ if (value == null) {
+ return new HashSet<>();
+ }
+ return new HashSet<>(
+
ProtobufUtil.toGroupInfo(RSGroupProtos.RSGroupInfo.parseFrom(value)).getServers());
+ }
+ }
+
+ /**
+ * Fails over to a freshly started master (the minicluster's masters are
threads): a backup is
+ * started, the active master is stopped, and the shared MASTER/ADMIN
handles are re-pointed. The
+ * base class keeps the master's embedded RS in a "master" group, whose
stored address would go
+ * stale, so the master is first moved back to 'default' (whose membership
is live-computed); the
+ * next cleanup places the new master in the "master" group again.
+ */
+ private void restartMaster() throws Exception {
+ SingleProcessHBaseCluster cluster = TEST_UTIL.getMiniHBaseCluster();
+ int oldIndex = -1;
+ List<JVMClusterUtil.MasterThread> masters = cluster.getMasterThreads();
+ for (int i = 0; i < masters.size(); i++) {
+ if (masters.get(i).getMaster() == MASTER) {
+ oldIndex = i;
+ }
+ }
+ cluster.startMaster();
+ cluster.stopMaster(oldIndex);
+ cluster.waitOnMaster(oldIndex);
+ assertTrue(cluster.waitForActiveAndReadyMaster(WAIT_TIMEOUT));
+ // Clients bootstrap from the configured master address, which is now the
dead master's.
+ TEST_UTIL.getConfiguration().set(HConstants.MASTER_ADDRS_KEY,
+ cluster.getMaster().getServerName().getAddress().toString());
+ TEST_UTIL.closeConnection();
+ ADMIN.close();
+ initialize();
+ }
+
+ private void setRegex(String groupName, String regex) {
+ String key = REGEX_PREFIX + groupName;
+ MASTER.getConfiguration().set(key, regex);
+ // ADMIN's own Configuration is a distinct object from MASTER's (the
minicluster deep-copies
+ // configuration per daemon), so mirror the change there too --
VerifyingRSGroupAdmin#verify
+ // reads it to know which servers' membership is regex-driven.
+ ADMIN.getConfiguration().set(key, regex);
+ regexConfigKeysSet.add(key);
+ }
+
+ /**
+ * Tears down a regex-governed group. Explicit moves that contradict an
active regex are rejected,
+ * so the regex is first dropped on the master only -- ADMIN's copy still
tells
+ * VerifyingRSGroupAdmin which servers' membership is regex-driven -- and
then the group is
+ * emptied and removed.
+ */
+ private void removeRegexGroup(String groupName) throws Exception {
+ String key = REGEX_PREFIX + groupName;
+ MASTER.getConfiguration().unset(key);
+ try {
+ removeGroup(groupName);
+ } finally {
+ clearRegex(groupName);
+ }
+ }
+
+ private void clearRegex(String groupName) {
+ String key = REGEX_PREFIX + groupName;
+ MASTER.getConfiguration().unset(key);
+ ADMIN.getConfiguration().unset(key);
+ regexConfigKeysSet.remove(key);
+ }
+
+ private Address addressOf(JVMClusterUtil.RegionServerThread rst) {
+ return rst.getRegionServer().getServerName().getAddress();
+ }
+
+ private static class LogCapturer {
+ private final StringWriter sw = new StringWriter();
+ private final org.apache.logging.log4j.core.appender.WriterAppender
appender;
+ private final org.apache.logging.log4j.core.Logger logger;
+
+ LogCapturer(org.apache.logging.log4j.core.Logger logger) {
+ this.logger = logger;
+ this.appender =
org.apache.logging.log4j.core.appender.WriterAppender.newBuilder()
+ .setName("test-regex-membership").setTarget(sw).build();
+ this.appender.start();
+ this.logger.addAppender(this.appender);
+ }
+
+ String getOutput() {
+ return sw.toString();
+ }
+
+ void stopCapturing() {
+ this.logger.removeAppender(this.appender);
Review Comment:
LogCapturer#stopCapturing removes the appender from the logger but does not
stop the appender itself. Calling `this.appender.stop()` (and optionally
clearing/closing the writer if needed) would better ensure resources are
released and avoids potential cross-test leakage when many tests attach
appenders.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupBasedLoadBalancer.java:
##########
@@ -99,6 +100,44 @@ public class RSGroupBasedLoadBalancer implements
LoadBalancer {
private volatile boolean fallbackEnabled = false;
+ @RestrictedApi(explanation = "Should only be called in tests", link = "",
+ allowedOnPath = ".*/src/test/.*")
+ public void setFallbackEnabledForTest(boolean enabled) {
+ this.fallbackEnabled = enabled;
+ }
+
+ // Test-only call-tracking flags, set true whenever the corresponding
assignment method below is
+ // invoked. Reset via resetAssignmentCallFlagsForTest() before the operation
under test.
+ private volatile boolean roundRobinAssignmentInvoked = false;
+ private volatile boolean retainAssignmentInvoked = false;
+ private volatile boolean randomAssignmentInvoked = false;
+
+ @RestrictedApi(explanation = "Should only be called in tests", link = "",
+ allowedOnPath = ".*/src/test/.*")
+ public boolean isRoundRobinAssignmentInvoked() {
+ return roundRobinAssignmentInvoked;
+ }
+
+ @RestrictedApi(explanation = "Should only be called in tests", link = "",
+ allowedOnPath = ".*/src/test/.*")
+ public boolean isRetainAssignmentInvoked() {
+ return retainAssignmentInvoked;
+ }
+
+ @RestrictedApi(explanation = "Should only be called in tests", link = "",
+ allowedOnPath = ".*/src/test/.*")
+ public boolean isRandomAssignmentInvoked() {
+ return randomAssignmentInvoked;
+ }
+
+ @RestrictedApi(explanation = "Should only be called in tests", link = "",
+ allowedOnPath = ".*/src/test/.*")
+ public void resetAssignmentCallFlagsForTest() {
+ roundRobinAssignmentInvoked = false;
+ retainAssignmentInvoked = false;
+ randomAssignmentInvoked = false;
+ }
Review Comment:
These test-only helpers are `public` on a production class, which increases
the public API surface and can be used unintentionally in non-test code
(especially if Error Prone isn't enforced in all builds). If feasible, prefer
reducing visibility (e.g., package-private) and keeping tests in the same
package, or using a test-only accessor mechanism (e.g., `@VisibleForTesting` +
non-public members, or a test subclass/instrumentation hook) to avoid exposing
these methods as part of the runtime API.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfoManagerImpl.java:
##########
@@ -742,30 +831,267 @@ private void updateCacheOfRSGroups(final Set<String>
currentGroups) {
this.prevRSGroups.addAll(currentGroups);
}
- // Called by ServerEventsListenerThread. Presume it has lock on this manager
when it runs.
- private SortedSet<Address> getDefaultServers() {
- return getDefaultServers(listRSGroups()/* get from rsGroupMap */);
+ /**
+ * Parses {@code hbase.rsgroup.regex.<groupname>=<regex>} entries out of
{@code conf}. Entries
+ * with an invalid group name, targeting the reserved {@link
RSGroupInfo#DEFAULT_GROUP}, or with
+ * an unparsable regex are logged and skipped.
+ */
+ static Map<String, Pattern> getRegexGroupMap(Configuration conf) {
+ Map<String, String> rsGroupNameToRegexMap =
conf.getPropsWithPrefix(RS_GROUP_REGEX_PREFIX);
+ Map<String, Pattern> rsGroupNameToPatternMap = new HashMap<>();
+ for (Map.Entry<String, String> e : rsGroupNameToRegexMap.entrySet()) {
+ if (e.getValue() == null) {
+ // getPropsWithPrefix reads names and values separately, so an entry
removed by a concurrent
+ // configuration reload can show up here with a null value. Treat it
as unset.
+ continue;
+ }
+ if (!GROUP_NAME_PATTERN.matcher(e.getKey()).matches()) {
+ LOG.warn("Ignoring {}{} -- '{}' is not a valid RSGroup name (only
alphanumeric characters "
+ + "and underscore allowed)", RS_GROUP_REGEX_PREFIX, e.getKey(),
e.getKey());
+ continue;
+ }
+ if (RSGroupInfo.DEFAULT_GROUP.equals(e.getKey())) {
+ LOG.warn("Ignoring {}{} -- regex-based membership cannot target the
reserved '{}' group",
+ RS_GROUP_REGEX_PREFIX, e.getKey(), RSGroupInfo.DEFAULT_GROUP);
+ continue;
+ }
+ try {
+ rsGroupNameToPatternMap.put(e.getKey(), Pattern.compile(e.getValue()));
+ } catch (PatternSyntaxException ex) {
+ LOG.warn("Invalid regex '{}' for RSGroup '{}' ({}{}); ignoring this
entry", e.getValue(),
+ e.getKey(), RS_GROUP_REGEX_PREFIX, e.getKey());
+ }
+ }
+ LOG.info("Resolved regex-based RSGroup membership config: {}",
rsGroupNameToPatternMap);
+ return rsGroupNameToPatternMap;
}
- // Called by ServerEventsListenerThread. Presume it has lock on this manager
when it runs.
- private SortedSet<Address> getDefaultServers(List<RSGroupInfo>
rsGroupInfoList) {
- // Build a list of servers in other groups than default group, from
rsGroupMap
- Set<Address> serversInOtherGroup = new HashSet<>();
- for (RSGroupInfo group : rsGroupInfoList) {
- if (!RSGroupInfo.DEFAULT_GROUP.equals(group.getName())) { // not default
group
- serversInOtherGroup.addAll(group.getServers());
+ private static List<String> getMatchingRSGroupNames(String hostname,
+ Map<String, Pattern> rsGroupNameToPatternMap) {
+ List<String> rsGroupNames = new ArrayList<>();
+ for (Map.Entry<String, Pattern> e : rsGroupNameToPatternMap.entrySet()) {
+ if (e.getValue().matcher(hostname).matches()) {
+ rsGroupNames.add(e.getKey());
}
}
+ LOG.info("Hostname '{}' matches RSGroup name(s) {}", hostname,
rsGroupNames);
+ return rsGroupNames;
+ }
- // Get all online servers from Zookeeper and find out servers in default
group
- SortedSet<Address> defaultServers = Sets.newTreeSet();
- for (ServerName serverName :
masterServices.getServerManager().getOnlineServers().keySet()) {
- Address server = Address.fromParts(serverName.getHostname(),
serverName.getPort());
- if (!serversInOtherGroup.contains(server)) { // not in other groups
- defaultServers.add(server);
+ /**
+ * Resolves every server in {@code onlineServers} whose hostname
unambiguously matches exactly one
+ * entry of {@code rsGroupNameToPatternMap} naming a group in {@code
existingGroupNames}. A server
+ * matching multiple regexes, or matching a regex for a group that does not
(yet) exist, is
+ * omitted (warn-only) rather than resolved.
+ */
+ static Map<Address, String> resolveServerAddrToRSGroupName(Set<Address>
onlineServers,
+ Map<String, Pattern> rsGroupNameToPatternMap, Set<String>
existingGroupNames) {
+ Map<Address, String> serverAddrToRSGroupName = new HashMap<>();
+ if (rsGroupNameToPatternMap.isEmpty()) {
+ return serverAddrToRSGroupName;
+ }
+ Set<String> nonExistingRSGroupNames = new HashSet<>();
+ for (Address server : onlineServers) {
+ List<String> matchingRSGroupNames =
+ getMatchingRSGroupNames(server.getHostName(), rsGroupNameToPatternMap);
+ if (matchingRSGroupNames.isEmpty()) {
+ continue;
+ }
+ if (matchingRSGroupNames.size() > 1) {
+ LOG.warn("Server {} hostname matches regexes for multiple RSGroups {};
treating it as "
+ + "unmatched by regex (falls back to default) until the overlapping "
+ + "hbase.rsgroup.regex.* entries are fixed", server,
matchingRSGroupNames);
+ continue;
+ }
+ String rsGroupName = matchingRSGroupNames.get(0);
+ if (existingGroupNames.contains(rsGroupName)) {
+ serverAddrToRSGroupName.put(server, rsGroupName);
+ } else if (nonExistingRSGroupNames.add(rsGroupName)) {
+ LOG.warn(
+ "Config {}{} matches server {} but RSGroup '{}' does not exist --
create it with "
+ + "addRSGroup first; treating this server as unmatched by regex
until then",
+ RS_GROUP_REGEX_PREFIX, rsGroupName, server, rsGroupName);
}
}
- return defaultServers;
+ LOG.info("Resolved server address to RSGroup name map: {}",
serverAddrToRSGroupName);
+ return serverAddrToRSGroupName;
+ }
+
+ /** Returns {@code true} if every online server is either admin-managed or
regex-matched. */
+ static boolean wouldEmptyDefaultGroup(Set<Address> onlineServers,
+ Set<Address> adminManagedServers, Set<Address> regexMatchedServers) {
+ for (Address server : onlineServers) {
+ if (!adminManagedServers.contains(server) &&
!regexMatchedServers.contains(server)) {
+ return false;
+ }
+ }
+ return !onlineServers.isEmpty();
+ }
+
+ static final class RegexBasedRSGroupMembershipResolution {
+ final Set<Address> adminManagedServers;
+ final Map<Address, String> regexMatchedServers;
+
+ RegexBasedRSGroupMembershipResolution(Set<Address> adminManagedServers,
+ Map<Address, String> regexMatchedServers) {
+ this.adminManagedServers = adminManagedServers;
+ this.regexMatchedServers = regexMatchedServers;
+ }
+ }
+
+ static RegexBasedRSGroupMembershipResolution
resolveRegexBasedRSGroupMembership(
+ Map<String, Pattern> rsGroupNameToPatternMap, Set<Address> onlineServers,
+ Collection<RSGroupInfo> existingGroups) {
+ Set<String> existingGroupNames =
+
existingGroups.stream().map(RSGroupInfo::getName).collect(Collectors.toSet());
+ Set<Address> adminManagedServers = new HashSet<>();
+ for (RSGroupInfo existingGroup : existingGroups) {
+ if (
+ !RSGroupInfo.DEFAULT_GROUP.equals(existingGroup.getName())
+ && !rsGroupNameToPatternMap.containsKey(existingGroup.getName())
+ ) {
+ adminManagedServers.addAll(existingGroup.getServers());
+ }
+ }
+ Map<Address, String> regexMatchedServers =
+ resolveServerAddrToRSGroupName(onlineServers, rsGroupNameToPatternMap,
existingGroupNames);
+ // A server stored in an admin-managed group that a regex now claims
belongs to the regex group.
+ adminManagedServers.removeAll(regexMatchedServers.keySet());
+ if (wouldEmptyDefaultGroup(onlineServers, adminManagedServers,
regexMatchedServers.keySet())) {
+ // Regex placement is still applied: every server lands in its correct
group. Operators are
+ // expected to configure regexes carefully, so this is a warning, not an
error.
+ LOG.warn(
+ "Regex-based RSGroup membership leaves RSGroup '{}' with no online
servers -- every "
+ + "online server is either admin-managed or matches a {}* entry.
Tables in RSGroup '{}' "
+ + "(including system tables, unless they are in another RSGroup)
have no server of "
+ + "their own to be assigned to; set {} to let them fall back to
servers of other "
+ + "RSGroups",
+ RSGroupInfo.DEFAULT_GROUP, RS_GROUP_REGEX_PREFIX,
RSGroupInfo.DEFAULT_GROUP,
+ RSGroupBasedLoadBalancer.FALLBACK_GROUP_ENABLE_KEY);
+ }
+ LOG.info(
+ "Regex-based RSGroup membership resolution: onlineServers={}, "
+ + "adminManagedServers={}, regexMatchedServers={}",
+ onlineServers, adminManagedServers, regexMatchedServers);
+ return new RegexBasedRSGroupMembershipResolution(adminManagedServers,
regexMatchedServers);
+ }
+
+ private Map<String, SortedSet<Address>>
+ computeAutoManagedRSGroupServers(Collection<RSGroupInfo> existingGroups) {
+ LOG.info("Computing auto-managed RSGroup server membership.");
Review Comment:
A number of logs here are emitted at INFO on every RS join/leave (and
potentially on every move via getRegexGroupMap/checkMoveAgreesWithRegex paths).
In busy clusters this can generate significant log volume and noise (e.g.,
per-hostname match logs, full resolved maps, and 'No changes' messages).
Consider downgrading the high-frequency/diagnostic logs to DEBUG/TRACE (e.g.,
getMatchingRSGroupNames, 'Resolved regex...' map dump, 'Computed...' dump, and
the 'No changes...' line), while keeping WARN/INFO only for actionable events
(invalid regex, overlapping matches, empty default, etc.).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
