xiangfu0 opened a new pull request, #18971: URL: https://github.com/apache/pinot/pull/18971
## Problem Fixes #18836. The controller's *"View consuming segments info"* panel column **"Max Partition Availability Lag (ms)"** (and the `MAX_RECORD_AVAILABILITY_LAG_MS` metric) showed a huge, nonsensical value — essentially the current Unix time in ms (~`1.7e12`) — whenever a record's upstream ingestion timestamp was unavailable. The availability lag is computed on the server as: ```java availabilityLag = lastProcessedTimeMs - recordIngestionTimeMs; ``` The guard only checked `lastProcessedTimeMs > 0`; it never validated `recordIngestionTimeMs`. When a record carries no valid timestamp — Kafka `NO_TIMESTAMP` (`-1`), the unset `StreamMessageMetadata` builder default (`Long.MIN_VALUE`), or `0` — the subtraction produces an epoch-sized (or overflowed) number that is a valid `Long`, so it sails past the controller's `NumberFormatException` skip and the UI's `Number(av) > -1` filter and gets displayed as the "lag". ## Fix In all three concrete `StreamMetadataProvider.getCurrentPartitionLagState` implementations (`pinot-kafka-3.0`, `pinot-kafka-4.0`, `pinot-kinesis`): - Guard the availability-lag computation on `getRecordIngestionTimeMs() > 0`, so an invalid/unset ingestion time reports the SPI-defined `PartitionLagState.NOT_CALCULATED` sentinel instead of a bogus number. - Replace the previously hard-coded `"UNKNOWN"` fallback (for **both** offset lag and availability lag) with `PartitionLagState.NOT_CALCULATED`, to align the plugins with the SPI's own default sentinel and let `RealtimeConsumerMonitor` use its fast-path skip instead of catching a `NumberFormatException`. ## Backward compatibility The observable fallback string in the `/consumingSegmentsInfo` REST maps changes from `"UNKNOWN"` to `"NOT_CALCULATED"`. All in-tree consumers already treat both identically: - `RealtimeConsumerMonitor` skips `NOT_CALCULATED` and try/catches `Long.parseLong` for anything else. - The UI's `Number(lag)` yields `NaN` for both and filters it out. External dashboards that scrape the literal string `"UNKNOWN"` from this endpoint should be updated to expect `"NOT_CALCULATED"`. ## Testing Added a regression test per module (`KafkaStreamMetadataProviderTest` ×2, `KinesisStreamMetadataProviderTest`) covering: - a valid ingestion time → numeric availability lag, - unset (`Long.MIN_VALUE`), `NO_TIMESTAMP` (`-1`), and `0` ingestion times → `NOT_CALCULATED`, - the Kafka offset-lag fallback (unknown upstream offset) → `NOT_CALCULATED`. Each test fails on the pre-fix code (it produces the bogus value) and passes after the fix. `spotless`, `checkstyle`, and `license` checks pass on all three modules. ## Notes Pulsar and the SPI base class have no availability-lag computation, so no sibling implementation is left on the buggy pattern. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
