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

   ## Summary
   
   Fixes two correctness bugs in the non-active CRR poller infrastructure 
inside `GetClusterRoleRecordUtil`. JIRA: 
https://issues.apache.org/jira/browse/PHOENIX-7870
   
   ### Bug 1 — Per-HA-group future tracking
   
   The previous implementation kept a single `static volatile 
ScheduledFuture<?> pollerFuture` field that was overwritten by every 
`schedulePoller(...)` invocation regardless of `haGroupName`. When the 
active-CRR detection branch later cancelled `pollerFuture`, it cancelled 
whichever future had been scheduled most recently — which could belong to a 
*different* HA group than the one whose lambda was running.
   
   Fix: replace the static field with a `ConcurrentHashMap<String, 
ScheduledFuture<?>> futureMap` keyed by `haGroupName`. Each HA group's future 
is tracked, looked up, and cancelled independently. Symmetric handling for the 
pre-existing `schedulerMap` (now `final`, and removed from the map on the 
active-CRR cancel path).
   
   ### Bug 2 — url1/url2 alternation each tick
   
   The previous implementation pinned each scheduled poller to the single URL 
passed in at schedule time. If that cluster's RegionServer Endpoint became 
transiently unreachable, the poller could never observe the peer cluster's CRR 
— even after the peer became Active.
   
   Fix: the poller now alternates between `url1` and `url2` each tick. 
Even-numbered ticks (0, 2, 4, ...) target `url1`; odd-numbered ticks target 
`url2`. A failed tick still increments the tick counter so that alternation 
continues uninterrupted on the next iteration.
   
   Method signatures updated:
   - `fetchClusterRoleRecord(String url1, String url2, String primaryUrl, ...)` 
— explicit `primaryUrl` preserves the existing per-call-site ordering at 
`HighAvailabilityGroup.getClusterRoleRecordFromEndpoint`
   - `schedulePoller(String url1, String url2, String haGroupName, ...)` — both 
URLs now in scope when the poller is started
   
   ## Test plan
   
   - [x] New unit test class `GetClusterRoleRecordUtilTest` — 4/4 PASS
     - `testSelectUrlForTickAlternates` — verifies even/odd alternation across 
the first six ticks
     - `testSelectUrlForTickHandlesLargeTickValues` — guards against sign 
issues at large tick values including `Long.MAX_VALUE`
     - `testFutureMapIsolatesEntriesPerHaGroup` — verifies distinct HA groups 
produce distinct future-map entries (Bug 1 invariant)
     - `testCancelOneHaGroupDoesNotCancelOthers` — verifies cancelling one HA 
group's poller leaves peers untouched (Bug 1 behavioural invariant)
   - [x] `mvn install -DskipTests` (full repo) — BUILD SUCCESS
   - [x] `mvn -pl phoenix-core-client compile` — BUILD SUCCESS, 1228 source 
files compiled clean
   - [x] `mvn -pl phoenix-core test -Dtest=GetClusterRoleRecordUtilTest` — 
Tests run: 4, Failures: 0, Errors: 0, Skipped: 0
   
   Branched off `PHOENIX-7562-feature-new` HEAD.
   


-- 
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]

Reply via email to