ddcprg commented on code in PR #9163:
URL: https://github.com/apache/pinot/pull/9163#discussion_r938509887
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/LLRealtimeSegmentDataManager.java:
##########
@@ -548,9 +549,14 @@ private boolean processStreamEvents(MessageBatch
messagesAndOffsets, long idlePi
// this can be overridden by the decoder if there is a better indicator
in the message payload
RowMetadata msgMetadata = messagesAndOffsets.getMetadataAtIndex(index);
- GenericRow decodedRow = _messageDecoder
- .decode(messagesAndOffsets.getMessageAtIndex(index),
messagesAndOffsets.getMessageOffsetAtIndex(index),
- messagesAndOffsets.getMessageLengthAtIndex(index), reuse);
+ GenericRow decodedRow = null;
Review Comment:
agree, I think the idea is that the decoder encapsulates retry logic and
error handling and only throw an exception when it is exhausted to let the
data manager retry ingestion. The decoder should return `null` if the payload
cannot be decoded.
We have added documentation to `StreamMessageDecoder` however there are
already OOTB decoders which break this contract and users may still want to use
them - this could also be the case of third-party plugins. We could make the
decoders backwards incompatible, treat the issue as a bug and ignore this
change instead but I think this may still be handy.
--
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]
