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 2c9bb7477d PHOENIX-7775 ReplicationLogGroup initialization fixes
(#2386)
2c9bb7477d is described below
commit 2c9bb7477df365f9071e7323b682fdadb1c7b409
Author: tkhurana <[email protected]>
AuthorDate: Sat Feb 28 10:59:10 2026 -0800
PHOENIX-7775 ReplicationLogGroup initialization fixes (#2386)
---
.../phoenix/replication/ReplicationLogGroup.java | 16 +++++++--
.../MetricsReplicationLogGroupSourceFactory.java | 39 ++++++++++++++++++++++
.../replication/ReplicationLogGroupTest.java | 29 ++++++++++++++++
3 files changed, 81 insertions(+), 3 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 b7a5ca0228..0b31a3e940 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
@@ -41,6 +41,7 @@ import java.net.URISyntaxException;
import java.util.ArrayList;
import java.util.EnumSet;
import java.util.List;
+import java.util.Map;
import java.util.Optional;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ConcurrentHashMap;
@@ -61,7 +62,7 @@ import org.apache.phoenix.jdbc.HAGroupStoreManager;
import org.apache.phoenix.jdbc.HAGroupStoreRecord;
import org.apache.phoenix.jdbc.HAGroupStoreRecord.HAGroupState;
import org.apache.phoenix.replication.metrics.MetricsReplicationLogGroupSource;
-import
org.apache.phoenix.replication.metrics.MetricsReplicationLogGroupSourceImpl;
+import
org.apache.phoenix.replication.metrics.MetricsReplicationLogGroupSourceFactory;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -394,7 +395,16 @@ public class ReplicationLogGroup {
*/
protected ReplicationLogGroup(Configuration conf, ServerName serverName,
String haGroupName,
HAGroupStoreManager haGroupStoreManager) {
- this.conf = conf;
+ // conf object from coprocessor is instance of
+ // org.apache.hadoop.hbase.coprocessor.ReadOnlyConfiguration and we need
to modify it when
+ // we send rpc to namenode so copying it
+ // Clone configuration by iterating all entries because
ReadOnlyConfiguration wraps the
+ // original config so you can't use the Configuration(other) constructor
to create a clone
+ Configuration clonedConf = new Configuration();
+ for (Map.Entry<String, String> entry : conf) {
+ clonedConf.set(entry.getKey(), entry.getValue());
+ }
+ this.conf = clonedConf;
this.serverName = serverName;
this.haGroupName = haGroupName;
this.haGroupStoreManager = haGroupStoreManager;
@@ -717,7 +727,7 @@ public class ReplicationLogGroup {
/** Create a new metrics source for monitoring operations. */
protected MetricsReplicationLogGroupSource createMetricsSource() {
- return new MetricsReplicationLogGroupSourceImpl(haGroupName);
+ return
MetricsReplicationLogGroupSourceFactory.getInstanceForLogGroup(haGroupName);
}
/**
diff --git
a/phoenix-core-server/src/main/java/org/apache/phoenix/replication/metrics/MetricsReplicationLogGroupSourceFactory.java
b/phoenix-core-server/src/main/java/org/apache/phoenix/replication/metrics/MetricsReplicationLogGroupSourceFactory.java
new file mode 100644
index 0000000000..8c3a083ade
--- /dev/null
+++
b/phoenix-core-server/src/main/java/org/apache/phoenix/replication/metrics/MetricsReplicationLogGroupSourceFactory.java
@@ -0,0 +1,39 @@
+/*
+ * 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.phoenix.replication.metrics;
+
+import java.util.concurrent.ConcurrentHashMap;
+
+/**
+ * Factory class for creating log forwarder metrics
+ */
+public class MetricsReplicationLogGroupSourceFactory {
+
+ private MetricsReplicationLogGroupSourceFactory() {
+ // Utility class, no instantiation
+ }
+
+ /** Cache of ReplicationLogTrackerForwarderImpl instances by HA Group ID */
+ private static final ConcurrentHashMap<String,
+ MetricsReplicationLogGroupSource> LOG_GROUP_INSTANCES = new
ConcurrentHashMap<>();
+
+ public static MetricsReplicationLogGroupSource getInstanceForLogGroup(String
haGroupName) {
+ return LOG_GROUP_INSTANCES.computeIfAbsent(haGroupName,
+ k -> new MetricsReplicationLogGroupSourceImpl(haGroupName));
+ }
+}
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 68fa17196f..39109364c0 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
@@ -33,6 +33,7 @@ import static org.mockito.Mockito.atLeast;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.timeout;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
@@ -1264,6 +1265,34 @@ public class ReplicationLogGroupTest extends
ReplicationLogBaseTest {
inOrder.verify(storeAndForwardWriter, times(1)).sync();
}
+ /**
+ * Tests that multiple instances of the same HA group don't attempt to
register the same jmx
+ * metrics instance which is not allowed.
+ */
+ @Test
+ public void testMetricsCaching() throws Exception {
+ String hagroup1 = "group1";
+ // create a HA group which creates the metrics jmx instance
+ ReplicationLogGroup group1 =
+ spy(new ReplicationLogGroup(conf, serverName, hagroup1,
haGroupStoreManager));
+ // HA group initialization fails so the HA group is not cached
+ doThrow(new IOException("Simulate HAGroup initialization
error")).when(group1).init();
+ try {
+ group1.init();
+ fail("HAGroup initialization should have failed");
+ } catch (IOException e) {
+ // expected
+ }
+ // retry creating another instance of the same HA Group
+ ReplicationLogGroup group2 =
+ new ReplicationLogGroup(conf, serverName, hagroup1, haGroupStoreManager);
+ // this time initialization succeeds
+ group2.init();
+ // metrics instance is the same in both the instances
+ assertEquals(group1.metrics, group2.metrics);
+ group2.close();
+ }
+
// @Test
public void testAppendTimeoutWhileSyncPending() throws Exception {
final String tableName = "TESTTBL";
