chesnokoff commented on code in PR #13642:
URL: https://github.com/apache/ignite/pull/13642#discussion_r4181657411


##########
modules/core/src/main/java/org/apache/ignite/internal/processors/performancestatistics/FilePerformanceStatisticsWriter.java:
##########


Review Comment:
   PerformanceStatisticsSelfTest#testCacheOperation sometimes fails on JDK 21
   
(https://ci2.ignite.apache.org/test/1357538850106251069?currentProjectId=IgniteTests24Java8&branch=%3Cdefault%3E)
 because last records are lost from the performance statistics file. On JDK 17 
the test is OK.
   
   Reason is https://bugs.openjdk.org/browse/JDK-8323782 because it changed 
order of steps in Thread.interrupt():
   
   Before: 1) close the channel, 2) wake up the thread
   After:  1) wake up the thread, 2) close the channel
   
   Now one `Thread.interrupt` can hit the writer thread two times:
   
   1) `stop` interrupts the FileWriter thread. The thread was in `wait`
   2) The thread wakes up, goes to ```catch (InterruptedException e)``` and 
calls `flush` which starts `FileChannel.write`
   3) Only now `Thread.interrupt` checks the channel and closes it
   4) `write` fails with ClosedByInterruptException. The exception is ignored, 
so the rest of
      the buffer is lost. Then `fileIo.force` in `stop` logs `Failed to fsync 
the performance
      statistics file`.
   
   Let's not interrupt the writer thread and just notify it about cancelling
   
   patch with fix is green for more than 30 runs: 
https://ci2.ignite.apache.org/test/1357538850106251069?currentProjectId=IgniteTests24Java8&branch=pull%2F13642%2Fhead



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

Reply via email to