swaminathanmanish opened a new pull request, #19161:
URL: https://github.com/apache/pinot/pull/19161
## Problem
Real-time tables ingesting from **transactional Kafka topics** raise false
`StreamDataLoss` alerts. `KafkaPartitionLevelConsumer` flagged data loss
whenever the
first returned record offset was greater than the requested `startOffset`
(under
`read_uncommitted`):
```java
boolean hasDataLoss = !_isReadCommitted && firstOffset > startOffset;
```
Transactional producers write **commit/abort control records** that consume
offsets in
the log but are never delivered to the consumer. A perfectly healthy,
contiguous stream
of user records therefore has offset gaps, which this check misread as data
loss.
## Fix
Only report data loss when the requested `startOffset` is actually below the
broker's
**log start offset** (`Consumer.beginningOffsets`) — i.e. records at/after
`startOffset`
were genuinely deleted by retention or truncation:
```java
boolean hasDataLoss = false;
if (!_isReadCommitted && firstOffset > startOffset) {
hasDataLoss = getLogStartOffset(timeoutMs) > startOffset;
}
```
- The extra broker round-trip only happens on the rare gap path, never on
the contiguous
hot path or under `read_committed`.
- If the log start offset can't be determined, we default to **no data
loss** so a
transient broker hiccup can't manufacture a false alert.
- This is the resolution anticipated by the pre-existing `// TODO: fetch
earliest offset
from topic` comment.
Applied to both `pinot-kafka-3.0` and `pinot-kafka-4.0`.
## Tests
New `KafkaPartitionLevelConsumerDataLossTest` in each module (Mockito
harness mirroring
`KafkaPartitionLevelConsumerSeekTest`), 5 cases each:
| Scenario | Expected |
|---|---|
| Transactional gap, data still retained (`logStart <= startOffset`) | no
loss |
| `startOffset` below log start (real truncation) | **loss** |
| Contiguous batch (asserts `beginningOffsets` is never called) | no loss |
| `read_committed` gap (never called) | no loss |
| `beginningOffsets` lookup fails | no loss |
Verified the tests fail against the old code (the two false-positive cases
flip to
`true`) and pass with the fix. `checkstyle` and `spotless` clean on both
modules.
--
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]
