PHOENIX-7562 Add HAGroupStore Metrics2 observability#2576
Conversation
Expose a focused per-group health, state, failure, and listener-latency contract without coupling metrics to HA decisions. Co-authored-by: Cursor <cursoragent@cursor.com>
Reset gauges after late initialization failures and clarify the exported monitoring contract, metric lifetimes, and accepted best-effort semantics. Co-authored-by: Cursor <cursoragent@cursor.com>
| pstmt.executeUpdate(); | ||
| conn.commit(); | ||
| } catch (Exception e) { | ||
| metricsSource.incrementSystemTableSyncFailedCount(); |
There was a problem hiding this comment.
So this metric is only accounting for write failures but it is ignoring all other failures which can happen before the call even reaches here.
There was a problem hiding this comment.
Addressed in 64a625c. The metric now covers the complete synchronization attempt: failures while preparing or writing SYSTEM.HA_GROUP are counted in the helper, while periodic failures before the write are counted by the outer sync handler. The paths are mutually exclusive, so each failed attempt increments once.
| } catch (IOException | SQLException e) { | ||
| long syncIntervalSeconds = conf.getLong(HA_GROUP_STORE_SYNC_INTERVAL_SECONDS, | ||
| DEFAULT_HA_GROUP_STORE_SYNC_INTERVAL_SECONDS); | ||
| LOGGER.error("Failed to sync ZK data to system table for HA group on best effort basis {}," |
There was a problem hiding this comment.
Just noticed that we are not logging the exception here
There was a problem hiding this comment.
Addressed in 64a625c. Both synchronization error logs now pass the exception as the trailing SLF4J argument, so the full stack trace and root cause are preserved.
| @@ -880,4 +890,4 @@ | |||
| } catch (IOException | SQLException e) { | |||
There was a problem hiding this comment.
There are other reasons which can cause exceptions but the metric is not tracking that.
There was a problem hiding this comment.
Addressed in 64a625c. Periodic synchronization now catches and counts ZK reads, peer reads, policy/URL preparation, system-table comparison, and write failures. Write-helper failures are swallowed after counting and return false, preventing double-counting and false success logs.
Count preparation and write failures exactly once, preserve periodic retries after runtime errors, and include exception stack traces in sync logs. Co-authored-by: Cursor <cursoragent@cursor.com>
2 participants
Expose focused per-HA-group observability without coupling metrics to HA decisions.
What changes were proposed in this pull request?
Why are the changes needed?
HAGroupStore currently relies on logs for cache health, peer reachability, state divergence, fail-closed behavior, background failures, and slow synchronous listeners. These metrics provide alertable current state and targeted diagnostics while remaining independent of HA control flow.
Does this PR introduce any user-facing change?
Yes. RegionServers expose new HAGroupStore Metrics2/JMX metrics. There is no SQL, JDBC, or HA decision behavior change.
How was this patch tested?
mvn -pl phoenix-core -am -DskipITs -Dtest=HAGroupStoreRecordTest,HAGroupStoreMetricsSourceImplTest -Dsurefire.failIfNoSpecifiedTests=false -DfailIfNoTests=false testmvn -pl phoenix-core -am -Dtest=NoSuchUnitTest -Dsurefire.failIfNoSpecifiedTests=false -DfailIfNoTests=false -Dit.test=HAGroupStoreMetricsIT -DskipParallelStatsEnabledTests=true -DskipParallelStatsDisabledTests=true verifyWas this patch authored or co-authored using generative AI tooling?
Generated-by: Cursor 3.9.16