xiangfu0 commented on code in PR #19529:
URL: https://github.com/apache/pinot/pull/19529#discussion_r4502079894
##########
pinot-spi/src/main/java/org/apache/pinot/spi/stream/StreamDataDecoder.java:
##########
@@ -28,4 +31,11 @@ public interface StreamDataDecoder {
/// @param message [StreamMessage] that contains the data payload and
optionally, a key and row metadata
/// @return [StreamDataDecoderResult] that either contains the decoded row
or the exception
StreamDataDecoderResult decode(StreamMessage message);
+
+ /// @return the wrapped value [StreamMessageDecoder], or `null` if
unavailable. Lets callers report the concrete
+ /// decoder class without every implementation needing to support it.
+ @Nullable
+ default StreamMessageDecoder getValueDecoder() {
Review Comment:
(MINOR) This SPI addition may not be needed. `RealtimeSegmentDataManager`
already builds the `StreamMessageDecoder` itself in `createMessageDecoder`
(~L2055), so it could store `streamMessageDecoder.getClass().getName()` at
construction and leave `pinot-spi` unchanged. If you keep the accessor, prefer
`StreamMessageDecoder<?>` over the raw type, and note in the docs that it is
for reporting only. Calling `decode` on it directly would skip the wrapper's
key/header handling.
##########
pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/SegmentConsumerInfo.java:
##########
@@ -31,23 +33,41 @@ public class SegmentConsumerInfo {
private final long _lastConsumedTimestamp;
private final Map<String, String> _partitionToOffsetMap;
private final PartitionOffsetInfo _partitionOffsetInfo;
+ // FQCN of the StreamMessageDecoder instantiated on the consumer, or null if
decoder init has not completed.
Review Comment:
(MINOR) The case "null if decoder init has not completed" can't actually
happen. `_streamDataDecoder` is final, and the `RealtimeSegmentDataManager`
constructor throws if decoder creation fails, so any consumer visible through
the API always has a decoder. In practice, null means the server is older
(rolling upgrade) or the `StreamDataDecoder` doesn't expose its value decoder.
Please say that here and in the javadoc at `RealtimeSegmentDataManager` L1507,
so operators running a migration don't read null as "still initializing".
--
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]
