> On Nov. 19, 2019, 6:53 p.m., Slim Bouguerra wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/hooks/HiveProtoLoggingHook.java
> > Line 217 (original), 219 (patched)
> > <https://reviews.apache.org/r/71784/diff/1/?file=2173907#file2173907line220>
> >
> >     how this can solve the issue ?
> >     Seems like it is doing the same thing 
> >     
> >     `Executors.newSingleThreadScheduledExecutor`
> >     is the same as what you are doing
> >     ```
> >     public static ScheduledExecutorService 
> > newSingleThreadScheduledExecutor(ThreadFactory threadFactory) {
> >             return new DelegatedScheduledExecutorService
> >                 (new ScheduledThreadPoolExecutor(1, threadFactory));
> >         }
> >     ```

This not the fix. This change is only needed to get back the proper type so 
that I can invoke the getQueue() method. I can't do that on 
ScheduledExecutorService which is interface that is returned by 
Executors.newSingleThreadScheduledExecutor. I need the concreate class 
(ScheduledThreadPoolExecutor).


> On Nov. 19, 2019, 6:53 p.m., Slim Bouguerra wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/hooks/HiveProtoLoggingHook.java
> > Lines 274 (patched)
> > <https://reviews.apache.org/r/71784/diff/1/?file=2173907#file2173907line275>
> >
> >     looking at the java code i see that it is using a bounded queue so not 
> > sure what you mean by unbounded ?
> >     can you please clarify ?

No, unfortunatelly it uses an unbounded DelayedWorkQueue internally and it 
cannot be changed.


> On Nov. 19, 2019, 6:53 p.m., Slim Bouguerra wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/hooks/HiveProtoLoggingHook.java
> > Lines 279 (patched)
> > <https://reviews.apache.org/r/71784/diff/1/?file=2173907#file2173907line280>
> >
> >     i am still not sure how this is going to work?
> >     the original code was dropping events when the queue is full that is 
> > the case where you see the `RejectedExecutionException`

RejectedExecutionException was never thrown with the original code because of 
the unbounded queue. The queue continued to be larger. In the heap dump there 
17000 elements in the queue totally and it takes about 2.5g space.


- Attila


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


On Nov. 19, 2019, 3:43 p.m., Attila Magyar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71784/
> -----------------------------------------------------------
> 
> (Updated Nov. 19, 2019, 3:43 p.m.)
> 
> 
> Review request for hive, Laszlo Bodor, Harish Jaiprakash, Mustafa Iman, and 
> Panos Garefalakis.
> 
> 
> Bugs: HIVE-22514
>     https://issues.apache.org/jira/browse/HIVE-22514
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HiveProtoLoggingHook uses a ScheduledThreadPoolExecutor to submit writer 
> tasks and to periodically handle rollover. The builtin 
> ScheduledThreadPoolExecutor uses a unbounded queue which cannot be replaced 
> from the outside. If log events are generated at a very fast rate this queue 
> can grow large.
> 
> Since ScheduledThreadPoolExecutor does not support changing the default 
> unbounded queue to a bounded one, the queue capacity is checked manually by 
> the patch.
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java a7687d59004 
>   ql/src/java/org/apache/hadoop/hive/ql/hooks/HiveProtoLoggingHook.java 
> 8eab54859bf 
>   ql/src/test/org/apache/hadoop/hive/ql/hooks/TestHiveProtoLoggingHook.java 
> 450a0b544d6 
> 
> 
> Diff: https://reviews.apache.org/r/71784/diff/1/
> 
> 
> Testing
> -------
> 
> unittest
> 
> 
> Thanks,
> 
> Attila Magyar
> 
>

Reply via email to