> On Dec. 2, 2019, 12:43 p.m., Panos Garefalakis wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionPoolManager.java
> > Line 533 (original), 533 (patched)
> > <https://reviews.apache.org/r/71845/diff/1/?file=2180361#file2180361line533>
> >
> >     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?

After another look it seems that updateSessions method is only called within 
synchronized blocks so modifying the list while copying is not an issue. The 
only question is then if it is ok for the TriggerValidatorRunnables to use a 
slightly not-up-to-date version of the sessionState.


- Panos


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71845/#review218875
-----------------------------------------------------------


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

Reply via email to