You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "swaminathanmanish (via GitHub)" <gi...@apache.org> on 2024/05/02 05:11:38 UTC

Re: [PR] Introduce retries while creating stream message decoder for more robustness [pinot]

swaminathanmanish commented on code in PR #13036:
URL: https://github.com/apache/pinot/pull/13036#discussion_r1585479559


##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeSegmentDataManager.java:
##########
@@ -1504,14 +1510,24 @@ public RealtimeSegmentDataManager(SegmentZKMetadata segmentZKMetadata, TableConf
 
     // Create message decoder
     Set<String> fieldsToRead = IngestionUtils.getFieldsForRecordExtractor(_tableConfig.getIngestionConfig(), _schema);
+    RetryPolicy retryPolicy = RetryPolicies.exponentialBackoffRetryPolicy(5, 1000L, 1.2f);

Review Comment:
   Curious how we picked these values ? 



##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeSegmentDataManager.java:
##########
@@ -1504,14 +1510,24 @@ public RealtimeSegmentDataManager(SegmentZKMetadata segmentZKMetadata, TableConf
 
     // Create message decoder
     Set<String> fieldsToRead = IngestionUtils.getFieldsForRecordExtractor(_tableConfig.getIngestionConfig(), _schema);
+    RetryPolicy retryPolicy = RetryPolicies.exponentialBackoffRetryPolicy(5, 1000L, 1.2f);
+    AtomicReference<StreamDataDecoder> localStreamDataDecoder = new AtomicReference<>();

Review Comment:
   is this done because only a reference is allowed in lambda?



##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeSegmentDataManager.java:
##########
@@ -1504,14 +1510,24 @@ public RealtimeSegmentDataManager(SegmentZKMetadata segmentZKMetadata, TableConf
 
     // Create message decoder
     Set<String> fieldsToRead = IngestionUtils.getFieldsForRecordExtractor(_tableConfig.getIngestionConfig(), _schema);
+    RetryPolicy retryPolicy = RetryPolicies.exponentialBackoffRetryPolicy(5, 1000L, 1.2f);
+    AtomicReference<StreamDataDecoder> localStreamDataDecoder = new AtomicReference<>();
     try {
-      StreamMessageDecoder streamMessageDecoder = createMessageDecoder(fieldsToRead);
-      _streamDataDecoder = new StreamDataDecoderImpl(streamMessageDecoder);
+      retryPolicy.attempt(() -> {
+        try {
+          StreamMessageDecoder streamMessageDecoder = createMessageDecoder(fieldsToRead);
+          localStreamDataDecoder.set(new StreamDataDecoderImpl(streamMessageDecoder));
+          return true;
+        } catch (Exception e) {
+          return false;

Review Comment:
   Could you log the exception as well



-- 
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