Re: Review Request 55194: HIVE-15541: Hive OOM when ATSHook enabled and ATS goes down

[Jason Dere]

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

(Updated Jan. 19, 2017, 12:33 a.m.)


Review request for hive.


Changes
---

Create a separate executor to handle sending to ATS, so that the existing 
executor (now just creating the ATS events) does not get blocked on send calls.


Bugs: HIVE-15541
https://issues.apache.org/jira/browse/HIVE-15541


Repository: hive-git


Description
---

Create the ATSHook executor with a bounded queue capacity


Diffs (updated)
-

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 96928db 
  ql/src/java/org/apache/hadoop/hive/ql/hooks/ATSHook.java 3651c9c 

Diff: https://reviews.apache.org/r/55194/diff/


Testing
---


Thanks,

Jason Dere

Re: Review Request 55194: HIVE-15541: Hive OOM when ATSHook enabled and ATS goes down

[Jason Dere]

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

(Updated Jan. 10, 2017, 9:09 p.m.)


Review request for hive.


Changes
---

Move creation of ATS events to outside of the ATS logging thread, to reduce the 
amount of context being saved by the work queued to the ExecutorService in case 
it's blocked.


Bugs: HIVE-15541
https://issues.apache.org/jira/browse/HIVE-15541


Repository: hive-git


Description
---

Create the ATSHook executor with a bounded queue capacity


Diffs (updated)
-

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 16f6c1c 
  ql/src/java/org/apache/hadoop/hive/ql/hooks/ATSHook.java 3651c9c 

Diff: https://reviews.apache.org/r/55194/diff/


Testing
---


Thanks,

Jason Dere

Re: Review Request 55194: HIVE-15541: Hive OOM when ATSHook enabled and ATS goes down

[Barna Zsombor Klara]

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


Ship it!




Thank you for the update. I have no more nitpicks.

- Barna Zsombor Klara


On Jan. 5, 2017, 6:45 p.m., Jason Dere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55194/
> ---
> 
> (Updated Jan. 5, 2017, 6:45 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-15541
> https://issues.apache.org/jira/browse/HIVE-15541
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Create the ATSHook executor with a bounded queue capacity
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 47db0c0 
>   ql/src/java/org/apache/hadoop/hive/ql/hooks/ATSHook.java 3651c9c 
> 
> Diff: https://reviews.apache.org/r/55194/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jason Dere
> 
>

Re: Review Request 55194: HIVE-15541: Hive OOM when ATSHook enabled and ATS goes down

[Jason Dere]

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

(Updated Jan. 5, 2017, 6:45 p.m.)


Review request for hive.


Changes
---

Changing log level to WARN for caught exceptions


Bugs: HIVE-15541
https://issues.apache.org/jira/browse/HIVE-15541


Repository: hive-git


Description
---

Create the ATSHook executor with a bounded queue capacity


Diffs (updated)
-

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 47db0c0 
  ql/src/java/org/apache/hadoop/hive/ql/hooks/ATSHook.java 3651c9c 

Diff: https://reviews.apache.org/r/55194/diff/


Testing
---


Thanks,

Jason Dere

Re: Review Request 55194: HIVE-15541: Hive OOM when ATSHook enabled and ATS goes down

[Jason Dere]

> On Jan. 5, 2017, 9:38 a.m., Barna Zsombor Klara wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/hooks/ATSHook.java, line 208
> > 
> >
> > Should this be logged at INFO lvl? I understand that the hive query can 
> > still be executed just because we couldn't send the data to Yarn, but 
> > shouldn't it be at least a warning since something went wrong?

Makes sense, will change to warning level logs


- Jason


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


On Jan. 5, 2017, 2:28 a.m., Jason Dere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55194/
> ---
> 
> (Updated Jan. 5, 2017, 2:28 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-15541
> https://issues.apache.org/jira/browse/HIVE-15541
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Create the ATSHook executor with a bounded queue capacity
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 47db0c0 
>   ql/src/java/org/apache/hadoop/hive/ql/hooks/ATSHook.java 3651c9c 
> 
> Diff: https://reviews.apache.org/r/55194/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jason Dere
> 
>

Re: Review Request 55194: HIVE-15541: Hive OOM when ATSHook enabled and ATS goes down

[Barna Zsombor Klara]

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




ql/src/java/org/apache/hadoop/hive/ql/hooks/ATSHook.java (line 208)


Should this be logged at INFO lvl? I understand that the hive query can 
still be executed just because we couldn't send the data to Yarn, but shouldn't 
it be at least a warning since something went wrong?



ql/src/java/org/apache/hadoop/hive/ql/hooks/ATSHook.java (line 213)


Same as line 208. Should this be a warning instead?


Thanks for the patch. LGTM, just minor nits.

- Barna Zsombor Klara


On Jan. 5, 2017, 2:28 a.m., Jason Dere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55194/
> ---
> 
> (Updated Jan. 5, 2017, 2:28 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-15541
> https://issues.apache.org/jira/browse/HIVE-15541
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Create the ATSHook executor with a bounded queue capacity
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 47db0c0 
>   ql/src/java/org/apache/hadoop/hive/ql/hooks/ATSHook.java 3651c9c 
> 
> Diff: https://reviews.apache.org/r/55194/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jason Dere
> 
>

Review Request 55194: HIVE-15541: Hive OOM when ATSHook enabled and ATS goes down

[Jason Dere]

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

Review request for hive.


Bugs: HIVE-15541
https://issues.apache.org/jira/browse/HIVE-15541


Repository: hive-git


Description
---

Create the ATSHook executor with a bounded queue capacity


Diffs
-

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 47db0c0 
  ql/src/java/org/apache/hadoop/hive/ql/hooks/ATSHook.java 3651c9c 

Diff: https://reviews.apache.org/r/55194/diff/


Testing
---


Thanks,

Jason Dere