[PR] PHOENIX-7872 Addendum record HA failover duration on the CRR-write path and add a connection-failed counter [phoenix]

["lokiore (via GitHub)" <gi...@apache.org>]

lokiore opened a new pull request, #2605:
URL: https://github.com/apache/phoenix/pull/2605

   ### What changes were proposed in this pull request?
   
   Follow-up to the HA failover observability metrics added under PHOENIX-7872, correcting where two metrics are recorded on the HA client path.
   
   - **`HA_FAILOVER_DURATION_MS` moved to the CRR-write path.** It was recorded inside `FailoverPhoenixConnection.failover(long)`, which is only reached through `wrapActionDuringFailover` → `FailoverPolicy.shouldFailover()` (returns `false` under the default `ExplicitFailoverPolicy`) or the explicit static `failover(Connection, long)` helper — neither runs during an autonomous, cluster-role-record-driven failover. The measurement now lives in `HighAvailabilityGroup.refreshClusterRoleRecord`, where the cluster-role transition is actually dispatched and where `HA_FAILOVER_COUNT` is already gated by `shouldCountFailover`. The dispatch block is wrapped in a `try/finally` so the duration is recorded on **every** exit (success, timeout, policy failure, interrupt), which avoids a silent metric miss if a future exit path is added. The now-dead timing in `failover(long)` is removed.
   - **New `HA_FAILOVER_CONNECTION_FAILED_COUNTER`** (`GLOBAL_HA_FAILOVER_CONNECTION_FAILED_COUNTER`), incremented at the single `SQLException` throw funnel in `connectActive` (no active cluster, cluster demoted mid-connect, or the underlying connect threw).
   
   Class touch points:
   - `FailoverPhoenixConnection` — remove dead duration timing + now-unused import.
   - `HighAvailabilityGroup` — record duration on the transition-dispatch `try/finally`; increment the new counter on the `connectActive` throw funnel.
   - `MetricType` / `GlobalClientMetrics` — add the new counter constant; correct the `HA_FAILOVER_DURATION_MS` description to reflect the CRR-write recording site.
   
   ### Why are the changes needed?
   
   As placed, `HA_FAILOVER_DURATION_MS` never moved in production: the default `ExplicitFailoverPolicy.shouldFailover()` returns `false`, so `FailoverPhoenixConnection.failover(long)` is not auto-invoked during a real, CRR-driven failover. Recording the duration on `refreshClusterRoleRecord` ties it to the path that actually drives failovers, alongside the existing `HA_FAILOVER_COUNT`. The new connection-failed counter gives operators visibility into failed active-cluster connection attempts independent of the configured failover policy.
   
   ### Does this PR introduce _any_ user-facing change?
   
   No. Client-side metric placement/addition only; no API or query-behavior change. These metrics are on an unreleased feature branch.
   
   ### How was this patch tested?
   
   Three unit tests added to `HighAvailabilityGroupTest`:
   - `testCountedTransitionRecordsFailoverCountAndDuration` — a counted role-flip transition driven through `refreshClusterRoleRecord` increments `HA_FAILOVER_COUNT` **and** records an `HA_FAILOVER_DURATION_MS` sample on the CRR-write path.
   - `testConnectActiveFailureIncrementsFailedCounter` — a failed `connectActive` (no active cluster) increments the connection-failed counter on its `SQLException` funnel.
   - `testConnectActiveSuccessLeavesFailedCounterUnchanged` — a successful `connectActive` leaves the counter unchanged (non-vacuous negative assertion).
   
   Results: `HighAvailabilityGroupTest` 16/16, `FailoverPhoenixConnectionTest` 8/8. `spotless:check` clean on the touched files.
   
   ### Was this patch authored or co-authored using generative AI tooling?
   
   Generated-by: Claude Code (Opus 4.8)
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)


-- 
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: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] PHOENIX-7872 Addendum record HA failover duration on the CRR-write path and add a connection-failed counter [phoenix]

["lokiore (via GitHub)" <gi...@apache.org>]

lokiore commented on PR #2605:
URL: https://github.com/apache/phoenix/pull/2605#issuecomment-5403174719

   Superseded by #2609 (PHOENIX-7995). This addendum is folded into that PR as its own PHOENIX-7872-attributed commit at the base of the branch, so the CRR-write-path failover-duration fix and the connection-failed counter ship there alongside the per-HA-group metric tagging. Closing to consolidate review in one place.


-- 
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: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] PHOENIX-7872 Addendum record HA failover duration on the CRR-write path and add a connection-failed counter [phoenix]

["lokiore (via GitHub)" <gi...@apache.org>]

lokiore closed pull request #2605: PHOENIX-7872 Addendum record HA failover duration on the CRR-write path and add a connection-failed counter
URL: https://github.com/apache/phoenix/pull/2605


-- 
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: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org