Re: Review Request 71845: ConcurrentModificationException in TriggerValidatorRunnable stops trigger processing

[Panos Garefalakis via Review Board]

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

Re: Review Request 71845: ConcurrentModificationException in TriggerValidatorRunnable stops trigger processing

[Panos Garefalakis via Review Board]

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


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

Review Request 71845: ConcurrentModificationException in TriggerValidatorRunnable stops trigger processing

[Attila Magyar]

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

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