> On Nov. 19, 2019, 10:24 p.m., Panos Garefalakis wrote: > > ql/src/java/org/apache/hadoop/hive/ql/hooks/HiveProtoLoggingHook.java > > Lines 192 (patched) > > <https://reviews.apache.org/r/71784/diff/1/?file=2173907#file2173907line193> > > > > The solution makes sense to me, however maybe we need to investigate > > further why the queueCapacity change (which is similar to what you are > > proposing) was reverted in HIVE-20746? > > > > Also this logwriter is used to track all protobuf messages right? Is it > > acceptable to drop messages here? > > Panos Garefalakis wrote: > Update: Seems like HIVE-20746 only makes sure that logFile is closed at > the end of the day (even when no events are triggered) -- so the remaining > question is if its acceptible to start dropping messages here (because even > if we drop the messages the events are still going to happen)
@harishjp is already added to the review but he is on vacation so I don't know if'll respond or not. But my impression is that removing the capacity limit was not the goal but a side effect when the ScheduledThreadPoolExecutor was added. Before HIVE-20746 we were dropping events. HIVE-20746 was about making sure that the file is closed not about making sure that there are no drops. As far as I understand these events are for external systems like DAS. The event is still visible in the log so they're not lost. But @harishjp will hopefully confirm or refute this. - Attila ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71784/#review218694 ----------------------------------------------------------- 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 > >
