----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71845/#review218875 -----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionPoolManager.java Line 533 (original), 533 (patched) <https://reviews.apache.org/r/71845/#comment306788> Hey Attila, Good catch, however I believe there is still the case of modifying the list while copying it right? How about synching this copy or using collections.synchronizedList? - Panos Garefalakis On Nov. 29, 2019, 9:57 a.m., Attila Magyar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71845/ > ----------------------------------------------------------- > > (Updated Nov. 29, 2019, 9:57 a.m.) > > > Review request for hive, Laszlo Bodor, prasanthj, and Slim Bouguerra. > > > Bugs: HIVE-22502 > https://issues.apache.org/jira/browse/HIVE-22502 > > > Repository: hive-git > > > Description > ------- > > A ConcurrentModificationException was thrown from the main loop of > TriggerValidatorRunnable. > > > The ConcurrentModificationException happened because an other thread (from > TezSessionPoolManager) updated the sessions list while the > TriggerValidatorRunnable was iterating over it. > > The sessions list is updated by TezSessionPoolManager when opening or closing > a session. These operations are synchronized but the iteration in > TriggerValidatorRunnable is not. > > The TriggerValidatorRunnable is executed frequently (it is scheduled at a > 500ms rate by default) therefore I was reluctant put the whole iteration into > a synchronized block. Opening and closing a session happens not so often so I > decided to make a copy of the sessions list before passing it to the > TriggerValidatorRunnable. Let me know if you think otherwise. > > > Diffs > ----- > > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionPoolManager.java > 7c0a1fe120b > > > Diff: https://reviews.apache.org/r/71845/diff/1/ > > > Testing > ------- > > qtest > > > Thanks, > > Attila Magyar > >