lokiore opened a new pull request, #2589: URL: https://github.com/apache/phoenix/pull/2589
## PHOENIX-7973 JIRA: https://issues.apache.org/jira/browse/PHOENIX-7973 Targets the `PHOENIX-7562-feature-new` (consistent failover) feature branch. ### Problem `HighAvailabilityGroup.getClusterRoleRecordFromEndpoint()` queried cluster 1 first and returned its `ClusterRoleRecord` (CRR) **immediately whenever it had no `UNKNOWN` role**, without ever consulting cluster 2. CRR admin-version propagation across the two clusters' RegionServers is **not synchronized**, so at startup or during an in-flight admin/failover transition one endpoint can momentarily serve a **lower admin version** (or an `UNKNOWN` role) than its peer. In that window the client adopted the staler, lower-version record and silently reverted to an older cluster-role view. The refresh path (`refreshClusterRoleRecord`) guards only with `ClusterRoleRecord.equals()` — which **ignores `version`** — and never called the existing `isNewerThan()` helper, so nothing detected or prevented the downgrade. ### Fix `getClusterRoleRecordFromEndpoint()` now **always fetches the CRR from both cluster endpoints** and reconciles them via a new package-private static helper `reconcileClusterRoleRecords(r1, r2)`: 1. A record **without** an `UNKNOWN` role beats one **with** an `UNKNOWN` role (an `UNKNOWN` role means that endpoint could not resolve the cluster roles — e.g. a ZooKeeper problem — and is not usable for routing, so it must never win on version alone). 2. Among records in the **same** category, the **higher admin `version`** wins (the admin version only advances on an operator-driven change, so higher is strictly fresher). 3. On a tie, the peer (cluster 2) record is returned; both are equivalent for routing. This is a **strict superset** of the previous `UNKNOWN`-only handling: the earlier "non-UNKNOWN wins" and "UNKNOWN-vs-UNKNOWN → higher version" outcomes still hold, and the new logic additionally prevents the steady-state stale-revert. If the peer endpoint is **unreachable**, cluster 1's record is used as-is (exception consumed with a `WARN`). The outer CRR-not-found / exception fallback to cluster 2 is unchanged. ### Performance Adds **one endpoint RPC on the CRR refresh path only**. CRR is fetched on connect/refresh and cached in `roleRecord`; it is not on the per-query hot path, so there is no meaningful performance impact. ### Scope Client-side only. **No API or wire-format change.** ### Testing New unit test `HighAvailabilityGroupTest#testReconcileClusterRoleRecords` (mirrors the existing `shouldCountFailover` unit-test pattern): - higher version wins when both records are usable — the regression guard for the stale-revert bug - order-independence (same result regardless of which endpoint is passed first) - non-`UNKNOWN` beats `UNKNOWN` even with a lower version (both argument orders) - `UNKNOWN` vs `UNKNOWN` → higher version wins (preserves prior behavior) `HighAvailabilityGroupTest` → 3/3 pass. `phoenix-core-client` + `phoenix-core` test-compile clean; `spotless:check` clean on both touched files. -- 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]
