[GitHub] [pinot] KKcorps commented on a diff in pull request #9511: Handle exception in realtime decoder gracefully

[GitBox <gi...@apache.org>]

KKcorps commented on code in PR #9511:
URL: https://github.com/apache/pinot/pull/9511#discussion_r985442978


##########
pinot-spi/src/main/java/org/apache/pinot/spi/config/table/ingestion/IngestionConfig.java:
##########
@@ -51,6 +51,10 @@ public class IngestionConfig extends BaseJsonConfig {
   @JsonPropertyDescription("Configs related to skip any row which has error and continue during ingestion")
   private boolean _continueOnError;
 
+  @JsonPropertyDescription("If set to true, the records with GenericRow.INCOMPLETE_RECORD_KEY will not be consumed."
+      + "This can be helpful if user only wants to see correct data in the table")

Review Comment:
   This applies to more cases then just decoder. e.g. if an error occurs during converting one of the columns data type, then if `continueOnError` is true, we simply replace it with default value. Hence it will be easier for users to filter such records with this flag.



##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/LLRealtimeSegmentDataManager.java:
##########
@@ -543,23 +543,32 @@ private boolean processStreamEvents(MessageBatch messagesAndOffsets, long idlePi
       // Decode message
       StreamDataDecoderResult decodedRow = _streamDataDecoder.decode(messagesAndOffsets.getStreamMessage(index));
       RowMetadata msgMetadata = messagesAndOffsets.getStreamMessage(index).getMetadata();
+      GenericRow decoderResult;
       if (decodedRow.getException() != null) {

Review Comment:
   Makes sense. We can treat metadata vs data seperately. Are there any cases where metadata errors are critical?



##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/creator/SegmentGeneratorConfig.java:
##########
@@ -261,6 +265,17 @@ public static Schema updateSchemaWithTimestampIndexes(Schema schema,
     return newSchema;
   }
 
+  public static Schema updateSchemaWithDerivedFlags(Schema schema) {

Review Comment:
   Don't like this method name. Suggestions welcome on renaming it (or if it is needed at all)



##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/creator/SegmentGeneratorConfig.java:
##########
@@ -261,6 +265,17 @@ public static Schema updateSchemaWithTimestampIndexes(Schema schema,
     return newSchema;
   }
 
+  public static Schema updateSchemaWithDerivedFlags(Schema schema) {

Review Comment:
   I don't like this method's name. Suggestions welcome on renaming it (or if it is needed at all)



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org