suvodeep-pyne commented on code in PR #19737:
URL: https://github.com/apache/pinot/pull/19737#discussion_r4213793491


##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/realtime/writer/StatelessRealtimeSegmentWriter.java:
##########
@@ -308,11 +309,8 @@ public void run() {
   public void stopConsumption() {
     if (_consumerThread.isAlive()) {
       _consumerThread.interrupt();
-      try {
-        _consumerThread.join();
-      } catch (InterruptedException e) {
-        _logger.warn("Interrupted while waiting for consumer thread to 
finish");
-      }
+      // Wait even if interrupted, so that the segment is not destroyed while 
the consumer thread is still indexing
+      Uninterruptibles.joinUninterruptibly(_consumerThread);

Review Comment:
   Good catch. Fixed in 9b5a02e24f, with one change from your suggestion. A 
plugin that swallows the interrupt usually clears the flag too, so checking 
`Thread.currentThread().isInterrupted()` in the loop could miss it. Instead, 
`stopConsumption()` sets a volatile `_shouldStop` flag (the same pattern as 
`RealtimeSegmentDataManager`), and the loop checks it before each fetch. 
Consumption therefore ends at the next batch, bounded by the fetch timeout, 
however the plugin handles interrupts. A stopped consumption is not successful 
and records a cause naming the stop. Leaving the `while` loop normally would 
have set `_isSuccess`, which is why it doesn't just add the flag to the `while` 
condition.
   
   I also added the first `StatelessRealtimeSegmentWriterTest`. It waits until 
a fake stream that never reaches the end offset has been fetched, then stops 
consumption while the fake swallows interrupts. It hung before the change and 
now passes in about 0.2s.



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