PHOENIX-7995 Tag client HA metrics with the HA group and add missing HA/CRR/failover client metrics - #2609
Open
lokiore wants to merge 4 commits into
Open
Conversation
…th and add a connection-failed counter The HA failover observability metrics added under PHOENIX-7872 recorded HA_FAILOVER_DURATION_MS inside FailoverPhoenixConnection.failover(long). That method is only reached through wrapActionDuringFailover -> FailoverPolicy .shouldFailover(), which returns false under the default ExplicitFailoverPolicy, or through the explicit static failover(Connection, long) helper. Neither runs during an autonomous, CRR-driven failover, so the duration metric never moved in production. Move the duration measurement to the path that actually drives failovers: refreshClusterRoleRecord, where the cluster-role transition is 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, or interrupt), avoiding a silent metric miss if a future exit path is added. The now-dead timing in failover(long) is removed. Add 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). This tracks real active-cluster connection failures regardless of the configured failover policy. Tests: three unit tests in HighAvailabilityGroupTest -- a counted role-flip transition records both HA_FAILOVER_COUNT and an HA_FAILOVER_DURATION_MS sample on the CRR-write path; a failed connectActive increments the connection-failed counter; a successful connectActive leaves it unchanged. HighAvailabilityGroupTest 16/16, FailoverPhoenixConnectionTest 8/8. Generated-by: Claude Code (Opus 4.8) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
lokiore
force-pushed
the
PHOENIX-7995-ha-group-metrics
branch
from
August 26, 2026 17:24
4d58ed6 to
1d67d42
Compare
lokiore
marked this pull request as ready for review
August 26, 2026 17:26
Contributor
|
…HA/CRR/failover client metrics
The ZK-less HA client emits all HA/CRR/failover metrics as JVM-global
GLOBAL_HA_* counters, so a JVM connected to more than one HA group cannot
attribute a failover, stale-CRR detection, or poller failure to a specific
group. It also lacks client counters for several HA/CRR/failover events that
are relevant on the ZK-less path.
Per-HA-group tagging
--------------------
Add HAGroupClientMetricsSource, a per-group Hadoop Metrics2 source (one per HA
group name) that stamps a "ha_group" tag carrying the group name and appends the
quoted group name to its JMX context so each group registers as a distinct
source/MBean. HAGroupMetricsManager is the process-wide registry: it lazily
creates a source per group (only when global client metrics are enabled),
routes per-group increment/update, and detaches the source on HA-group close so
the same group can re-register later. Creating a source registers it with the
metrics system and can throw; getOrCreate swallows that failure (logs and
returns null) so metrics registration never breaks a caller such as
HighAvailabilityGroup.init(). Emission is dual: every group-attributable
GLOBAL_HA_* increment is mirrored to the group's source, and the JVM-global
counters continue to emit unchanged.
The tag key lives in a new module-neutral MetricConstants.HA_GROUP_TAG_NAME
("ha_group") referenced by both this client source and the server-side
HAGroupStoreMetricsSource, so both sides tag with the same key and can be
filtered together downstream.
New client counters
-------------------
- HA_FAILOVER_CONNECTION_CREATED_COUNTER: FailoverPhoenixConnection instances
successfully created against the active cluster; one increment per constructed
connection. This is not a matched pair with the connection-failed counter,
which increments on every failed active-connect attempt across all callers
(including each failover retry), so failed/(created+failed) is not a rate.
- HA_ROLE_TRANSITION_FAILED_COUNTER: cluster-role-transition dispatch failures
(execution error or timeout) on the CRR-write path.
- CRR_TRANSITION_COUNT: cluster-role-record transitions applied per HA policy,
including transitions into a no-active state (distinct from HA_FAILOVER_COUNT,
which counts only transitions that establish/move an ACTIVE cluster).
Group-attributable emission is wired at the existing sites in
HighAvailabilityGroup (failover count/duration, CRR refresh, connect-failed, and
the two new transition metrics), FailoverPhoenixConnection (connection created,
stale-CRR, mutation-blocked), HighAvailabilityPolicy (parallel fallback),
ParallelPhoenixContext/ParallelPhoenixUtil (parallel connection created/error,
task timeout), and GetClusterRoleRecordUtil (poller tick count/failures). The
JVM-shared parallel-executor pool metrics and the HA_CRR_CACHE_AGE_MS gauge are
deliberately excluded from the per-group source (JVM-shared / not a counter).
Tests: HAGroupClientMetricsSourceTest (6) and HAGroupMetricsManagerTest (7)
cover the ha_group tag/quoting, per-counter increment/update, per-group
isolation, distinct registered sources, and detach/re-create.
HighAvailabilityGroupTest additionally asserts the per-group (ha_group-tagged)
counters mirror the JVM-global counters on a real counted transition
(HA_FAILOVER_COUNT + CRR_TRANSITION_COUNT) and on a failed / successful
connectActive (HA_FAILOVER_CONNECTION_FAILED_COUNTER), and that init
pre-registers the group's source while close detaches it. 30/30 pass across the
three.
Generated-by: Claude Code (Opus 4.8)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
lokiore
force-pushed
the
PHOENIX-7995-ha-group-metrics
branch
from
August 26, 2026 20:07
1d67d42 to
a956ba4
Compare
| // Count every applied CRR transition, including transitions into a no-active state. Distinct | ||
| // from HA_FAILOVER_COUNT, which counts only transitions that establish/move an ACTIVE | ||
| // cluster. | ||
| GLOBAL_HA_CRR_TRANSITION_COUNT.increment(); |
There was a problem hiding this comment.
This is to count for number of requests of transition or the actual number of transitions applied?
Currently it increments before the transition is successful, so in case transition is not applied, counter would still be incremented.
| * The HA metrics attributable to a single HA group; each emission site has a | ||
| * {@code HighAvailabilityGroup} in scope. | ||
| */ | ||
| public static final MetricType[] METRIC_TYPES = new MetricType[] { HA_FAILOVER_COUNT, |
There was a problem hiding this comment.
nit: Not used outside this class, so can be private (and also unmodifiable - to avoid accidental updates)
…Policy and rename to CRR_TRANSITION_DURATION_MS Address review on the placement of the cluster-role-record transition metrics so the ZK-less client counts and times the same event the transit work actually reacts to, mirroring the reference wiring. Move the transition count and duration -------------------------------------- Previously GLOBAL_HA_CRR_TRANSITION_COUNT (+ its per-group mirror) and the transition duration were incremented in HighAvailabilityGroup on the caller thread around the dispatch/future.get() boundary in refreshClusterRoleRecord. That over-counts: refreshClusterRoleRecord can clear its isNewerThan + hasSameInfo guards and dispatch a strictly-newer CRR whose registry, urls, and roles are all identical (a pure version bump), which is not a transition. Move both metrics into HighAvailabilityPolicy#transitClusterRoleRecord, the single place the transit work runs, and gate them on a real change: increment the count and record the duration only when registry, url, or role actually changed; a pure version bump logs and returns without touching either metric. Duration is recorded in a finally so a transit that fails partway still contributes its time-to-failure, while HA_ROLE_TRANSITION_FAILED_COUNTER (recorded by the caller on dispatch failure/timeout) distinguishes failed transits. HA_FAILOVER_COUNT stays on the caller thread via shouldCountFailover, unchanged. Rename HA_FAILOVER_DURATION_MS -> CRR_TRANSITION_DURATION_MS ----------------------------------------------------------- The duration now measures every applied registry/url/role transition, not only failovers, so it is renamed CRR_TRANSITION_DURATION_MS and paired with CRR_TRANSITION_COUNT (GLOBAL_HA_CRR_TRANSITION_DURATION_MS on the JVM-global side). MetricType descriptions are updated for accuracy and the HAGroupClientMetricsSource METRIC_TYPES list is reordered so HA_FAILOVER_COUNT leads and the paired CRR transition metrics sit together. Tests ----- HighAvailabilityGroupTest adds testPureVersionBumpDoesNotCountOrTimeTransition, asserting a strictly-newer version with identical registry/url/role is still adopted but increments neither CRR_TRANSITION_COUNT nor HA_FAILOVER_COUNT and records no CRR_TRANSITION_DURATION_MS sample (global and per-group). Existing source/manager unit tests and HAGroupMetricsIT are updated to the new name. Generated-by: Claude Code (Opus 4.8) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ments The per-HA-group metric wiring added to ParallelPhoenixContext (connection created/error counters) and ParallelPhoenixUtil (parallel task timeout counter) eagerly dereferenced the HA group via haGroup.getName(). When the HA group is null the dereference threw an NPE on the parallel connection path. ParallelPhoenixUtilTest and ParallelPhoenixResultSetTest build a ParallelPhoenixContext with a null HA group in a static field, so the NPE escaped the static initializer as ExceptionInInitializerError, failing all 11 methods in those two classes with NoClassDefFoundError at class load. Guard the group-name dereference at all three sites; a null group yields a null name, which HAGroupMetricsManager already treats as a safe no-op (the per-group metric is simply skipped). Metric emission must never break the connection path. Generated-by: Claude Code (Opus 4.8) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
tkhurana
approved these changes
Aug 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
This PR makes the ZK-less HA client's HA/CRR/failover metrics sliceable per HA group, and fills gaps in the client-side HA metric coverage. It builds on (and supersedes) #2605, which is folded in here as its own PHOENIX-7872 commit.
1. Per-HA-group metrics tag. A new per-group Hadoop Metrics2 source,
HAGroupClientMetricsSource, registers one metrics2 source per HA group. Each instance:,haGroup=<ObjectName.quote(name)>to its JMX context so every group registers as a distinct source / MBean, andha_grouptag carrying the (unquoted) group name so the series can be sliced per HA group downstream.HAGroupMetricsManageris a small registry (one source per group name) wired intoHighAvailabilityGrouplifecycle: a source is created on group init and detached on group close. Emission is dual: the existing JVM-globalGLOBAL_HA_*counters continue to emit unchanged, and the same event is additionally recorded on the per-group source. The tag keyha_groupis defined in the module-neutralorg.apache.phoenix.metrics.MetricConstants(HA_GROUP_TAG_NAME, introduced by #2606 / PHOENIX-7991) and referenced by the new client source, so the client shares the exact tag key already used by the server-sideHAGroupStoreMetricsSource. The JMX ObjectName property key stayshaGroup(matching #2606); only the metrics2 tag isha_group.The per-group set is intentionally limited to metrics attributable to a single HA group (each emission site has a
HighAvailabilityGroupin scope). The JVM-shared parallel-executor pool metrics and theHA_CRR_CACHE_AGE_MSgauge are excluded.2. Missing client counters. New
MetricTypes and theirGLOBAL_HA_*wrappers, emitted (dual) at the relevant client sites:HA_FAILOVER_CONNECTION_CREATED_COUNTER— aFailoverPhoenixConnectionwas successfully created against the active cluster.HA_FAILOVER_CONNECTION_FAILED_COUNTER— a connect-to-active attempt threw (no active cluster, demoted mid-connect, or connect error). (from PHOENIX-7872 Addendum record HA failover duration on the CRR-write path and add a connection-failed counter #2605 / PHOENIX-7872)HA_ROLE_TRANSITION_FAILED_COUNTER— a cluster-role-transition dispatch failed while applying a new CRR.CRR_TRANSITION_COUNT— a CRR transition was applied per HA policy, including transitions into a no-active state (distinct fromHA_FAILOVER_COUNT, which counts only transitions that establish/move an ACTIVE cluster).Existing counters (
HA_FAILOVER_COUNT,HA_FAILOVER_DURATION_MS,HA_STALE_CRR_DETECTED_COUNT,HA_MUTATION_BLOCKED_COUNT,HA_CRR_REFRESH_COUNT, theHA_PARALLEL_*connection/task counters, and the poller-tick counters) are additionally recorded per-group at their existing emission sites.The #2605 change (record
HA_FAILOVER_DURATION_MSon the CRR-write path rather than the deadfailover()path, and add the connection-failed counter) is preserved as its own PHOENIX-7872-attributed commit at the base of this branch; #2605 will be closed as superseded by this PR.Why are the changes needed?
On the ZK-less HA client the JVM-global
GLOBAL_HA_*counters aggregate across every HA group in the process, so a JVM serving more than one HA group cannot attribute failover / stale-CRR / mutation-blocked / transition activity to a specific group. Tagging each series withha_groupmakes per-group dashboards and alerting possible while keeping the existing global counters intact for backward compatibility. The additional counters close observability gaps around connection creation/failure and role-transition outcomes that had no client-side metric.Does this PR introduce any user-facing change?
No behavioral change. New client-side metrics are added (new
MetricTypeentries and theirGLOBAL_HA_*wrappers) and a new per-groupha_group-tagged metrics2 source is registered; the existing global HA counters are unchanged. No SQL, API, or wire-format change.How was this patch tested?
New unit tests (run under surefire):
HAGroupClientMetricsSourceTest— theha_grouptag carries the unquoted group name; the JMX context is quoted per group; increment/update are per-counter; unknown metric types are ignored; each group is a distinct registered source; unregister frees the source name for reuse.HAGroupMetricsManagerTest—getOrCreateis idempotent and registers a source; null/empty group names are a no-op; two groups never cross-count; each group gets a distinct tagged source;updateaccumulates per group;removedetaches the source; re-create after remove rebuilds a fresh source.HighAvailabilityGroupTestadditions cover the CRR-write-path duration/connection-failed emission (from PHOENIX-7872 Addendum record HA failover duration on the CRR-write path and add a connection-failed counter #2605 / PHOENIX-7872).mvn spotless:applywas run before pushing; the full module build compiles clean.Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 4.8)