From 2154e02b9cc23a1213cebc6429c6cf87289ebd15 Mon Sep 17 00:00:00 2001 From: lokiore Date: Thu, 23 Jul 2026 17:51:11 -0700 Subject: [PATCH] PHOENIX-7966 :- Fix flaky HA IT transitClusterRole: expect the settled peer role HA integration tests intermittently time out (~1 in 5 runs) inside the transitClusterRole test helper when a group transitions under the FAILOVER or PARALLEL policy with both clusters live. The helper writes an expected ClusterRoleRecord to both clusters and then waits for the refreshed record to equal it. When the local cluster moves toward standby (ACTIVE_TO_STANDBY), a live peer's failover-management listener autonomously advances a STANDBY peer to STANDBY_TO_ACTIVE. Naming the transient pre-advance role in the wait therefore races that autonomous transition and intermittently never matches, timing out. Two complementary, deterministic fixes, each matching its call site's intent: * FailoverPhoenixConnectionIT and HAGroupMetricsIT (setup-then-assert sites): the {ACTIVE_TO_STANDBY, STANDBY} transit is only a stepping stone to a later settled state that the test actually asserts on, so keep the settle-wait but name the SETTLED peer role STANDBY_TO_ACTIVE -- a fixed point in the peer/local transition maps -- so the record matches on the first poll regardless of listener timing. * ParallelPhoenixConnectionIT.testOneClusterATSRoleWithStandby (assert-on-the- transient-pair site): {ACTIVE_TO_STANDBY, STANDBY} is itself the state under test and is not reachable as a stable pair with both clusters live, so no settled role can be named without changing what the test exercises. Skip the settle-wait (doRefresh=false) instead; the parallel connection is still exercised against the ACTIVE_TO_STANDBY cluster pair and must complete without error. The now-unused single-argument overload is removed. The transitClusterRole Javadoc and the cluster-down / cluster-restart variants are annotated to document the peer-advance exposure and why each is or is not subject to it. Test-only change; no production code is touched. Generated-by: Claude Code (Opus 4.8 (1M context)) --- .../jdbc/FailoverPhoenixConnectionIT.java | 10 ++++++++-- .../apache/phoenix/jdbc/HAGroupMetricsIT.java | 10 ++++++++-- .../jdbc/HighAvailabilityTestingUtility.java | 17 +++++++++++++++-- .../jdbc/ParallelPhoenixConnectionIT.java | 12 +++++++----- 4 files changed, 38 insertions(+), 11 deletions(-) diff --git a/phoenix-core/src/it/java/org/apache/phoenix/jdbc/FailoverPhoenixConnectionIT.java b/phoenix-core/src/it/java/org/apache/phoenix/jdbc/FailoverPhoenixConnectionIT.java index 38e66bf4f5e..d41cc0e91cc 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/jdbc/FailoverPhoenixConnectionIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/jdbc/FailoverPhoenixConnectionIT.java @@ -196,7 +196,10 @@ public void testConnectionCreationFailsIfNoActiveCluster() throws Exception { doTestBasicOperationsWithConnection(conn, tableName, haGroupName); } - CLUSTERS.transitClusterRole(haGroup, ClusterRole.ACTIVE_TO_STANDBY, ClusterRole.STANDBY); + // role2 = the SETTLED peer role: local ACTIVE_TO_STANDBY drives the peer + // STANDBY -> STANDBY_TO_ACTIVE, so the waitFor must expect the settled state. + CLUSTERS.transitClusterRole(haGroup, ClusterRole.ACTIVE_TO_STANDBY, + ClusterRole.STANDBY_TO_ACTIVE); CLUSTERS.transitClusterRole(haGroup, ClusterRole.STANDBY, ClusterRole.STANDBY); try { @@ -565,7 +568,10 @@ public void testFailoverConnectionExplicitlyTimeout() throws Exception { Connection conn = createFailoverConnection(); doTestBasicOperationsWithConnection(conn, tableName, haGroupName); - CLUSTERS.transitClusterRole(haGroup, ClusterRole.ACTIVE_TO_STANDBY, ClusterRole.STANDBY); + // role2 = the SETTLED peer role: local ACTIVE_TO_STANDBY drives the peer + // STANDBY -> STANDBY_TO_ACTIVE, so the waitFor must expect the settled state. + CLUSTERS.transitClusterRole(haGroup, ClusterRole.ACTIVE_TO_STANDBY, + ClusterRole.STANDBY_TO_ACTIVE); CLUSTERS.transitClusterRole(haGroup, ClusterRole.STANDBY, ClusterRole.STANDBY); try { diff --git a/phoenix-core/src/it/java/org/apache/phoenix/jdbc/HAGroupMetricsIT.java b/phoenix-core/src/it/java/org/apache/phoenix/jdbc/HAGroupMetricsIT.java index 088ccff183a..9589204cb53 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/jdbc/HAGroupMetricsIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/jdbc/HAGroupMetricsIT.java @@ -133,7 +133,10 @@ public void testStaleCrrDetectedCount() throws Exception { try (Connection conn = DriverManager.getConnection(CLUSTERS.getJdbcHAUrl(), clientProperties)) { conn.createStatement().execute("UPSERT INTO " + tableName + " VALUES (2, 2)"); conn.commit(); - CLUSTERS.transitClusterRole(haGroup, ClusterRole.ACTIVE_TO_STANDBY, ClusterRole.STANDBY); + // role2 = the SETTLED peer role: local ACTIVE_TO_STANDBY drives the peer + // STANDBY -> STANDBY_TO_ACTIVE, so the waitFor must expect the settled state. + CLUSTERS.transitClusterRole(haGroup, ClusterRole.ACTIVE_TO_STANDBY, + ClusterRole.STANDBY_TO_ACTIVE); // doRefreshHAGroup=false: keep haGroup's CRR snapshot stale on purpose so that // the next mutation drives StaleClusterRoleRecordException through wrapActionDuringFailover, // exercising the GLOBAL_HA_STALE_CRR_DETECTED_COUNT increment path. @@ -250,7 +253,10 @@ public void testPollerTickCount() throws Exception { long beforeTicks = GLOBAL_HA_POLLER_TICK_COUNT.getMetric().getValue(); long beforeFailures = GLOBAL_HA_POLLER_TICK_FAILURES.getMetric().getValue(); - CLUSTERS.transitClusterRole(haGroup, ClusterRole.ACTIVE_TO_STANDBY, ClusterRole.STANDBY); + // role2 = the SETTLED peer role: local ACTIVE_TO_STANDBY drives the peer + // STANDBY -> STANDBY_TO_ACTIVE, so the waitFor must expect the settled state. + CLUSTERS.transitClusterRole(haGroup, ClusterRole.ACTIVE_TO_STANDBY, + ClusterRole.STANDBY_TO_ACTIVE); CLUSTERS.transitClusterRole(haGroup, ClusterRole.STANDBY, ClusterRole.STANDBY); // Allow at least 2 poller ticks at default interval to land. diff --git a/phoenix-core/src/it/java/org/apache/phoenix/jdbc/HighAvailabilityTestingUtility.java b/phoenix-core/src/it/java/org/apache/phoenix/jdbc/HighAvailabilityTestingUtility.java index d27c5ba4f70..f27803d91e4 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/jdbc/HighAvailabilityTestingUtility.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/jdbc/HighAvailabilityTestingUtility.java @@ -423,9 +423,13 @@ public void transitClusterRole(HighAvailabilityGroup haGroup, ClusterRole role1, * Update HAGroup's clusterRoleRecord info to new roles (Update HAGroupStoreRecord and SYSTEM * Tables) and then do the refresh of the clusterRoleRecord in HAGroup based on the param * provided. + *

+ * Pass the role each cluster SETTLES at, not a transient pre-advance role: local + * ACTIVE_TO_STANDBY drives the peer's listener to advance a STANDBY peer to STANDBY_TO_ACTIVE, + * so naming the pre-advance role makes the equality wait race the autonomous transition. * @param haGroup the HA group name - * @param role1 cluster role for the first cluster in the group - * @param role2 cluster role for the second cluster in the group + * @param role1 settled cluster role for the first cluster in the group + * @param role2 settled cluster role for the second cluster in the group */ public void transitClusterRole(HighAvailabilityGroup haGroup, ClusterRole role1, ClusterRole role2, boolean doRefreshHAGroup, HighAvailabilityPolicy policy) throws Exception { @@ -487,6 +491,9 @@ public void invalidateHAGroupStoreClientForCluster(String haGroupName, Configura public void transitClusterRoleWithCluster1Down(HighAvailabilityGroup haGroup, ClusterRole role1, ClusterRole role2) throws Exception { + // Immune to the peer-advance race: the down cluster is client-invalidated (role + // UNKNOWN), so no live peer listener can advance the survivor past the expected + // record; no settled-role naming needed. String haGroupName = haGroup.getGroupInfo().getName(); final Pair currentRecord2 = haAdmin2.getHAGroupStoreRecordInZooKeeper(haGroupName); @@ -519,6 +526,9 @@ public void transitClusterRoleWithCluster1Down(HighAvailabilityGroup haGroup, Cl public void transitClusterRoleWithCluster2Down(HighAvailabilityGroup haGroup, ClusterRole role1, ClusterRole role2) throws Exception { + // Immune to the peer-advance race: the down cluster is client-invalidated (role + // UNKNOWN), so no live peer listener can advance the survivor past the expected + // record; no settled-role naming needed. String haGroupName = haGroup.getGroupInfo().getName(); final Pair currentRecord1 = haAdmin1.getHAGroupStoreRecordInZooKeeper(haGroupName); @@ -550,6 +560,9 @@ public void transitClusterRoleWithCluster2Down(HighAvailabilityGroup haGroup, Cl public void refreshClusterRoleRecordAfterClusterRestart(HighAvailabilityGroup haGroup, ClusterRole role1, ClusterRole role2, int clusterIndex) throws Exception { + // Both clusters live, so this shares transitClusterRole's peer-advance exposure: a + // transitional role pair must name the SETTLED peer role. Current callers pass steady + // pairs, so they are safe. String haGroupName = haGroup.getGroupInfo().getName(); // TODO:- Inducing version change, but how would we know that a URL is changed // as we don't store url in ZNodes so it doesn't increase version for that change diff --git a/phoenix-core/src/it/java/org/apache/phoenix/jdbc/ParallelPhoenixConnectionIT.java b/phoenix-core/src/it/java/org/apache/phoenix/jdbc/ParallelPhoenixConnectionIT.java index 38821276a48..84ff5199d4b 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/jdbc/ParallelPhoenixConnectionIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/jdbc/ParallelPhoenixConnectionIT.java @@ -421,14 +421,16 @@ public void testOneClusterATSRoleWithActive() throws Exception { */ @Test public void testOneClusterATSRoleWithStandby() throws Exception { - testOneClusterATSRole(ClusterRole.STANDBY); - } - - private void testOneClusterATSRole(ClusterRole otherRole) throws Exception { - testOneClusterATSRole(otherRole, true); + testOneClusterATSRole(ClusterRole.STANDBY, false); } private void testOneClusterATSRole(ClusterRole otherRole, boolean doRefresh) throws Exception { + // doRefresh=false: {ACTIVE_TO_STANDBY, STANDBY} (and {ACTIVE_TO_STANDBY, ACTIVE}) is not a + // stable role pair with both clusters live -- a live peer under a local ACTIVE_TO_STANDBY + // autonomously advances (STANDBY -> STANDBY_TO_ACTIVE; ACTIVE keeps the local draining), so + // an equality waitFor against the named transient pair races that advance and intermittently + // times out. Skipping the settle-wait avoids the race; the PARALLEL connection is still + // exercised against the ACTIVE_TO_STANDBY cluster pair and must complete without error. CLUSTERS.transitClusterRole(haGroup, ClusterRole.ACTIVE_TO_STANDBY, otherRole, doRefresh, PARALLEL); try (Connection conn = getParallelConnection()) {