ritegarg opened a new pull request, #2587:
URL: https://github.com/apache/phoenix/pull/2587
Six control-plane fixes found while qualifying consistent HA on a
two-cluster setup.
### What changes were proposed in this pull request?
* **Deliver events for recreated znodes.** Both the local
`HAGroupStoreClient` cache and `PeerClusterWatcher` de-duplicated on
ZooKeeper's data version alone. That version resets to `0` when a znode is
deleted and recreated, so the new incarnation compared as stale and was
dropped. A shared `HAGroupStoreCacheUtil.isNewerRevision` now orders on
`(czxid, version)`: a changed `czxid` is a new incarnation and is accepted at
version `0`; within one incarnation only a higher version is accepted. A
recreated local record is redelivered once even when its state is unchanged, so
`ABORT_TO_STANDBY`, `ABORT_TO_ACTIVE_IN_SYNC`, and
`ABORT_TO_ACTIVE_NOT_IN_SYNC` still drive their completion transitions. The
peer watcher's single forced redelivery after reconnect is unchanged, and
`closeConnection()` clears the watermark so a reconfigured peer ensemble starts
clean.
* **Keep the cached client through a transient local ZooKeeper outage.**
`getInstanceForZkUrl` now creates a client only when none is mapped, instead of
also replacing any instance whose `isHealthy` is false. Unhealthy instances
still fail closed on every read and write and recover on
`CONNECTION_RECONNECTED`; initial construction failures are still closed, never
registered, and returned as `null`; and `close()` remains the way an entry
leaves the map.
* **Validate admin input.** `--policy` is parsed through
`HighAvailabilityPolicy` before dry-run, reads, or writes, and stored as the
canonical enum name. `create` is now strictly create-only: an existing HA-group
name returns `RET_VALIDATION_ERROR` and writes nothing.
* **Gate failover and abort on pair convergence.** `pollForStateTransition`
requires local `STANDBY` together with peer `ACTIVE_IN_SYNC` or
`ACTIVE_NOT_IN_SYNC`, and reports both last-seen states on timeout.
* **Correct CLI guidance.** Malformed ZooKeeper URLs no longer suggest
`--force`; top-level `help`/`-h`/`--help` exit `0`; and a version-only `update`
explains that a configuration field is also required.
* **Validate configuration before allocating resources.**
`phoenix.ha.group.store.client.initialization.timeout.ms` and
`phoenix.ha.group.store.sync.interval.seconds` must be positive, checked before
any admin, cache, executor, or cache-map entry exists, with the key and value
in the message. Peer-cache retry and legacy CRR reconciliation keep `<= 0` as
disable. `HAGroupStoreManager` translates the resulting
`IllegalArgumentException` into `IOException`.
### Why are the changes needed?
* A peer znode delete/recreate left failover management reading stale peer
state indefinitely, because version `0` never compared newer than the last
delivered version. The same gap applied locally, where a recreated abort marker
is an actionable signal rather than a steady state.
* A brief local ZooKeeper blip silently disabled failover automation.
`HAGroupStoreManager` guards failover setup with a once-per-HA-group flag and
`FailoverManagementListener` captures the client it was built with, so a
replacement came up with an empty subscriber set and no listener was ever
re-registered — failover and replay reactions were dead until process restart,
with nothing logged.
* An unsupported policy string was persisted and only failed later at the
unguarded `HighAvailabilityPolicy.valueOf` call sites, far from the operator
who typed it. A `create` whose URLs, roles, or version conflicted with the
stored row reported success while changing nothing.
* Abort could report success while the peer was still in
`ABORT_TO_ACTIVE_IN_SYNC`. Cluster roles alone cannot distinguish this, because
the abort states already map to the final `STANDBY`/`ACTIVE` roles.
* The malformed-URL message advertised a `--force` bypass that ZooKeeper URL
validation never honors, and `help` exited non-zero.
* A zero or negative required interval surfaced as an unexplained `null`
after the full initialization timeout. Left unchecked as an unchecked
exception, it could also abort a RegionServer through
`BaseScannerRegionObserver.preScannerOpen`, since `CoprocessorHost` aborts on
any non-`IOException` throwable by default.
### Does this PR introduce _any_ user-facing change?
Yes, within the unreleased consistent-failover feature branch (no change vs
released Phoenix):
* `phoenix-consistentha-admin-tool create` now returns `4` for an existing
HA-group name instead of `0`; help text no longer describes it as idempotent.
* An unsupported `--policy` value is rejected with the supported list
instead of being stored.
* `initiate-failover` and `abort-failover` return success only after both
clusters converge; a local-only transition now times out with both states
reported.
* Malformed ZooKeeper URL errors state that `--force` cannot bypass
validation; top-level `help`/`-h`/`--help` exit `0`.
* A non-positive client initialization timeout or system-table sync interval
now fails fast with the offending key and value.
No SQL, JDBC, wire-format, or persisted-record changes.
### How was this patch tested?
* Unit tests:
`mvn -pl phoenix-core -am -DskipITs
-Dtest='PeerClusterWatcherTest,PhoenixHAAdminToolTest,HAGroupStoreClientConfigurationTest'
-DnumForkedUT=1 test`
* Integration tests, each run in full:
`HAGroupStoreClientIT`, `HAGroupStoreManagerIT`, `PhoenixHAAdminToolIT`,
`HAGroupStoreMetricsIT`, `HAGroupStateSubscriptionIT`,
`ReplicationLogDiscoveryReplayTestIT`, `ReplicationLogReplayServiceTestIT`,
`HighAvailabilityGroupIT`, `HighAvailabilityGroup2IT`, `HAGroupMetricsIT`,
`PhoenixHAAdminIT`, `ParallelPhoenixConnectionFallbackIT`,
`PhoenixRegionServerEndpointWithConsistentFailoverIT`
* Result: 10 unit tests and 191 integration tests passed, 0 failures.
New coverage: a peer znode recreated at version `0` is delivered exactly
once while duplicates and same-incarnation replays are suppressed; a recreated
local znode redelivers an unchanged abort state; a client subscribed before a
local ZooKeeper outage is the same instance afterwards and its listener still
fires; policy validation rejects before dry-run and writes; `create` rejects an
existing name without mutation; the stable-pair predicate rejects transitional,
abort, degraded, offline, unknown, and offline-peer states; help exit codes;
and the required/optional configuration boundaries.
`HAGroupStoreMetricsIT.testLateInitializationFailureResetsGauges` was
updated to construct the client directly, since configuration validation now
makes that failure unreachable through the factory; it still exercises the same
post-gauge-registration failure window.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Cursor
Made with [Cursor](https://cursor.com)
--
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]
