This is an automated email from the ASF dual-hosted git repository.
tkhurana pushed a commit to branch PHOENIX-7562-feature-new
in repository https://gitbox.apache.org/repos/asf/phoenix.git
The following commit(s) were added to refs/heads/PHOENIX-7562-feature-new by
this push:
new c0ed9893d6 PHOENIX-7983 Key ReplicationLogGroup cache on (port,
startcode) instead of host-inclusive ServerName (#2595)
c0ed9893d6 is described below
commit c0ed9893d67d0d38dac726bc9c18840de1664655
Author: tkhurana <[email protected]>
AuthorDate: Tue Aug 11 13:52:06 2026 -0700
PHOENIX-7983 Key ReplicationLogGroup cache on (port, startcode) instead of
host-inclusive ServerName (#2595)
The same physical RegionServer can be observed under two ServerName
spellings that differ only in host -- a declared FQDN (from
hbase.unsafe.regionserver.hostname) versus the pod IP the master hands
back at reportForDuty when reverse-DNS is enabled. Different coprocessor
environments surface different spellings for the same server (prewarm
PhoenixRegionServerEndpoint vs write-path IndexRegionObserver), so keying
INSTANCES on serverName.getServerName() split one logical group into two
independent instances, each with its own Disruptor and sequence counter.
Key on (port, startcode) instead, which excludes the host: two distinct
live RegionServers can never share a port on the same host at the same
time, so mini-cluster ITs running several RS in one JVM stay distinct
(each binds its own ephemeral port), while the single-RS case collapses
the FQDN/IP spellings to one entry. closeAllForServer() matches on the
same canonical identity.
---
.../phoenix/replication/ReplicationLogGroup.java | 29 +++++++++++++++--
.../apache/phoenix/jdbc/HAGroupStoreManagerIT.java | 4 +--
.../replication/ReplicationLogGroupTest.java | 37 ++++++++++++++++++++++
3 files changed, 66 insertions(+), 4 deletions(-)
diff --git
a/phoenix-core-server/src/main/java/org/apache/phoenix/replication/ReplicationLogGroup.java
b/phoenix-core-server/src/main/java/org/apache/phoenix/replication/ReplicationLogGroup.java
index 845c181e70..d4218d69da 100644
---
a/phoenix-core-server/src/main/java/org/apache/phoenix/replication/ReplicationLogGroup.java
+++
b/phoenix-core-server/src/main/java/org/apache/phoenix/replication/ReplicationLogGroup.java
@@ -218,8 +218,29 @@ public class ReplicationLogGroup {
protected static final ConcurrentHashMap<String, ReplicationLogGroup>
INSTANCES =
new ConcurrentHashMap<>();
+ /**
+ * Canonical, DNS-independent identity of a RegionServer within this JVM's
{@link #INSTANCES}
+ * cache. Uses {@code (port, startcode)} and deliberately EXCLUDES the
hostname.
+ * <p>
+ * The same physical RS can be observed under two {@link ServerName}
spellings that differ only in
+ * host — e.g. a declared FQDN (from {@code
hbase.unsafe.regionserver.hostname}) versus the pod IP
+ * the master hands back at reportForDuty when reverse-DNS is enabled.
Different coprocessor
+ * environments then surface different spellings for the same server (prewarm
+ * {@code PhoenixRegionServerEndpoint} vs write-path {@code
IndexRegionObserver}). Keying on
+ * {@code serverName.getServerName()} (host-inclusive) split one logical
group into two
+ * independent instances, each with its own Disruptor and sequence counter.
+ * <p>
+ * {@code (port, startcode)} is a safe discriminator: two distinct live
RegionServers can never
+ * share a port on the same host at the same time, so in the mini-cluster
ITs that run several
+ * RegionServers in one JVM they remain distinct (each binds its own
ephemeral port), while the
+ * single-RS production/kind case collapses the FQDN/IP spellings to one
entry.
+ */
+ private static String serverIdentity(ServerName serverName) {
+ return serverName.getPort() + "," + serverName.getStartcode();
+ }
+
private static String instanceKey(ServerName serverName, String haGroupName)
{
- return serverName.getServerName() + "|" + haGroupName;
+ return serverIdentity(serverName) + "|" + haGroupName;
}
protected final Configuration conf;
@@ -1008,8 +1029,12 @@ public class ReplicationLogGroup {
LOG.info("Closing all ReplicationLogGroup instances for server {}, total
count={}", serverName,
INSTANCES.size());
List<ReplicationLogGroup> groups = new ArrayList<>(INSTANCES.values());
+ String identity = serverIdentity(serverName);
for (ReplicationLogGroup group : groups) {
- if (group.serverName.equals(serverName)) {
+ // Match on the canonical (port, startcode) identity, not the
host-inclusive ServerName, so a
+ // caller passing one host spelling still purges a group cached under
the other (see
+ // serverIdentity).
+ if (serverIdentity(group.serverName).equals(identity)) {
try {
group.close();
} catch (Exception e) {
diff --git
a/phoenix-core/src/it/java/org/apache/phoenix/jdbc/HAGroupStoreManagerIT.java
b/phoenix-core/src/it/java/org/apache/phoenix/jdbc/HAGroupStoreManagerIT.java
index 18e83aa9d5..912ac32ce4 100644
---
a/phoenix-core/src/it/java/org/apache/phoenix/jdbc/HAGroupStoreManagerIT.java
+++
b/phoenix-core/src/it/java/org/apache/phoenix/jdbc/HAGroupStoreManagerIT.java
@@ -700,8 +700,8 @@ public class HAGroupStoreManagerIT extends HABaseIT {
HAGroupStoreRecord.HAGroupState.STANDBY);
HRegionServer regionServer =
CLUSTERS.getHBaseCluster1().getHBaseCluster().getRegionServer(0);
- replicationLogGroup = ReplicationLogGroup.get(cluster1Conf,
regionServer.getServerName(),
- haGroupName, cluster1HAManager).get();
+ replicationLogGroup = ReplicationLogGroup
+ .get(cluster1Conf, regionServer.getServerName(), haGroupName,
cluster1HAManager).get();
replicationLogReplay = ReplicationLogReplay.get(cluster2Conf,
haGroupName);
replicationLogReplay.startReplay();
diff --git
a/phoenix-core/src/test/java/org/apache/phoenix/replication/ReplicationLogGroupTest.java
b/phoenix-core/src/test/java/org/apache/phoenix/replication/ReplicationLogGroupTest.java
index 22db791b73..4ebb8c3c07 100644
---
a/phoenix-core/src/test/java/org/apache/phoenix/replication/ReplicationLogGroupTest.java
+++
b/phoenix-core/src/test/java/org/apache/phoenix/replication/ReplicationLogGroupTest.java
@@ -1348,6 +1348,43 @@ public class ReplicationLogGroupTest extends
ReplicationLogBaseTest {
g2_1.close();
}
+ /**
+ * The instance cache is keyed on (port, startcode), NOT the host-inclusive
ServerName string. The
+ * same physical RegionServer can be observed under two ServerName spellings
that differ only in
+ * host — e.g. a declared FQDN vs the pod IP the master hands back at
reportForDuty — from
+ * different coprocessor environments. Both must resolve to ONE
ReplicationLogGroup; a
+ * host-inclusive key split it into two independent instances (two
Disruptors / sequence
+ * counters).
+ */
+ @Test
+ public void testCachingIsHostSpellingIndependent() throws Exception {
+ final String haGroupId = "testHAGroupHostSpelling";
+ // Two spellings of the SAME physical server: same port + startcode, host
differs (FQDN vs IP).
+ ServerName asFqdn = ServerName.valueOf("rs-0.rs.ns.svc.cluster.local",
serverName.getPort(),
+ serverName.getStartcode());
+ ServerName asIp =
+ ServerName.valueOf("10.244.1.25", serverName.getPort(),
serverName.getStartcode());
+
+ ReplicationLogGroup viaFqdn =
+ ReplicationLogGroup.get(conf, asFqdn, haGroupId,
haGroupStoreManager).get();
+ ReplicationLogGroup viaIp =
+ ReplicationLogGroup.get(conf, asIp, haGroupId,
haGroupStoreManager).get();
+
+ assertSame("Two host spellings of the same (port, startcode) must share
one instance", viaFqdn,
+ viaIp);
+
+ // A genuinely different server (distinct port — two live RS can never
share a port on a host,
+ // e.g. mini-cluster ITs) must still get its own instance.
+ ServerName otherServer =
+ ServerName.valueOf(asIp.getHostname(), serverName.getPort() + 1,
serverName.getStartcode());
+ ReplicationLogGroup viaOther =
+ ReplicationLogGroup.get(conf, otherServer, haGroupId,
haGroupStoreManager).get();
+ assertNotEquals("A distinct port must yield a distinct instance", viaFqdn,
viaOther);
+
+ viaFqdn.close();
+ viaOther.close();
+ }
+
/**
* Tests that close() removes the instance from the cache. Verifies that
after closing, a new call
* to get() creates a new instance.
