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

   ### What changes were proposed in this pull request?
   
   Builds on PHOENIX-7884 (lag tracking refactor, #2506) with four new 
operational metrics for the eventually-consistent (EC) index 
`IndexCDCConsumer`, plus two correctness fixes in the lag watermark.
   
   **New metrics** (all per-table + global, following the existing dual-emit 
pattern in `MetricsIndexCDCConsumerSourceImpl`):
   
   | Metric | Type | Purpose |
   |---|---|---|
   | `cdcEventSkippedCount` | Counter | Increments at the 
`processCDCBatchGenerated` give-up site when `maxDataVisibilityRetries` is 
exhausted and the consumer permanently advances past unprocessable CDC events. 
Surfaces silent data divergence between the data table and its EC indexes. |
   | `cdcParentReplayActiveRegions` | Gauge | "How many regions on this RS are 
currently in post-split / post-merge parent-region replay for this table?" Lets 
operators distinguish the by-design lag-spike during catch-up from a broken 
consumer. Incremented in `run()` around the top-level 
`replayAndCompleteParentRegions` call (outside the recursive descent so 
ancestor recursion does not double-count). |
   | `cdcParentReplayDuration` | Histogram (ms) | One sample per ancestor 
partition when `processPartitionToCompletion` reaches a terminal state (marked 
COMPLETE here, or observed COMPLETE-by-sibling). Stopped/interrupted exits emit 
nothing. |
   | `cdcConsumerActiveRegions` | Gauge | "How many consumers are in 
steady-state poll loop for this table on this RS?" Incremented immediately 
before the main `while (!stopped)` loop in `run()` and decremented in a 
`finally`, so it strictly reflects steady-state processing and is semantically 
disjoint from `cdcParentReplayActiveRegions`. Sum of the two gauges = "consumer 
is doing useful work". |
   
   **Lag-tracking fixes** (built on PHOENIX-7884 watermark plumbing):
   
   - `processCDCBatchGenerated` give-up path: pull 
`progress.recordProcessed(newLastTimestamp)` out of the 
`!batchStates.isEmpty()` gate into the existing `newLastTimestamp > 
lastProcessedTimestamp` gate so the in-memory watermark advances in lockstep 
with the durable tracker. Previously the watermark stayed stale until the next 
empty poll or successful batch, causing `cdcIndexUpdateLag` to over-report.
   - `processCDCBatch` inner loop: when the CDC scan returns rows that are all 
empty `IndexMutations` protos (no-op CDC entries), advance 
`progress.recordProcessed(newLastTimestamp)` — we have definitively scanned 
past those timestamps and the watermark would otherwise stay fixed for the 
burn-through.
   - Bump `DEFAULT_LAG_SAMPLE_INTERVAL_MS` from 1000 ms to 5000 ms to cut 
background histogram-update load on RegionServers hosting many EC-indexed 
regions. Tunable via `phoenix.index.cdc.consumer.lag.sample.interval.ms`.
   
   ### Why are the changes needed?
   
   After PHOENIX-7884 the lag histogram became more accurate but several 
operational blind spots remained:
   
   1. **Silent data divergence**: the give-up branch in 
`processCDCBatchGenerated` permanently drops index updates with only a WARN log 
— no metric an SRE can alert on.
   2. **Post-split lag spikes are indistinguishable from broken consumers**: 
parent-region replay deliberately does not advance the freshness watermark 
(parent freshness ≠ child freshness), so `cdcIndexUpdateLag` inflates by design 
during catch-up; nothing else fires to disambiguate it.
   3. **Liveness ambiguity**: a daemon consumer thread that exits cleanly (no 
EC index) is indistinguishable from one that crashed mid-run, and the lag 
histogram is silent in both cases.
   4. The give-up-path watermark staleness and no-op-burn-through watermark 
stagnation (both fixed here) caused the new lag metric to over-report under 
exactly the conditions where accuracy matters most.
   
   ### Does this PR introduce _any_ user-facing change?
   
   No user-facing API/behavior change. New JMX metrics are additive (under 
`RegionServer,sub=IndexCDCConsumer`). One default config value changed 
(`phoenix.index.cdc.consumer.lag.sample.interval.ms` 1000 → 5000), overridable 
via existing config knob.
   
   ### How was this patch tested?
   
   - `mvn -pl phoenix-core-client,phoenix-core-server spotless:apply` (no 
further changes required).
   - `mvn -pl phoenix-core-client,phoenix-core-server -am install -DskipTests` 
clean.
   - No new test infrastructure added for the new metrics — they are additive 
per-table+global counters/gauges/histogram following the established dual-emit 
pattern, wired at unique, single-purpose call sites (one each for the skip 
counter and the histogram; symmetric inc/dec in `finally` blocks for the two 
gauges).
   
   ### Was this patch authored or co-authored using generative AI tooling?
   
   Generated-by: Cursor (Claude Opus 4.7)
   


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