Copilot commented on code in PR #19296:
URL: https://github.com/apache/pinot/pull/19296#discussion_r3810307934


##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeSegmentDataManager.java:
##########
@@ -462,12 +462,16 @@ private void handleTransientStreamErrors(Exception e)
           _consecutiveErrorCount, e);
       throw e;
     } else {
-      if (_shouldStop && (e instanceof InterruptedException || e.getCause() 
instanceof InterruptedException)) {
-        _segmentLogger.debug("Interrupted to stop consumption", e);
-      } else {
-        _segmentLogger.warn("Stream transient exception when fetching 
messages, retrying (count={})",
-            _consecutiveErrorCount, e);
+      if (_shouldStop) {
+        // Consumption is being stopped (the transient exception is usually 
the interrupt from the stop), and the
+        // consume loop exits on the next check, so skip the retry sleep and 
the stream consumer recreation. Closing
+        // the current consumer from the interrupted consumer thread would 
just fail with another interrupt; it is
+        // closed by the regular shutdown path instead.
+        _segmentLogger.debug("Interrupted to stop consumption, skipping stream 
consumer recreation", e);
+        return;
       }
+      _segmentLogger.warn("Stream transient exception when fetching messages, 
retrying (count={})",
+          _consecutiveErrorCount, e);
       Uninterruptibles.sleepUninterruptibly(1, TimeUnit.SECONDS);
       recreateStreamConsumer("Too many transient errors");

Review Comment:
   `stop()` can set `_shouldStop` and interrupt the consumer after this read, 
especially during the one-second uninterruptible retry sleep. 
`sleepUninterruptibly` then restores the interrupt flag and the code still 
calls `recreateStreamConsumer()`, which closes the Kafka consumer on the 
interrupted thread and reproduces the shutdown error this change is intended to 
eliminate. Re-check the stop flag after the sleep before recreating.
   
   This issue also appears on line 470 of the same file.



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

Reply via email to