Re: BEAM-6018: memory leak in thread pool instantiation

2018-11-09 Thread Lukasz Cwik
Thanks for the context Dan, that was helpful. On Fri, Nov 9, 2018 at 10:09 AM Udi Meiri wrote: > The reasoning unbounded threadpool is explained as: > /* The SDK requires an unbounded thread pool because a step may create X > writers > * each requiring their own thread to perform the writes

Re: BEAM-6018: memory leak in thread pool instantiation

2018-11-09 Thread Udi Meiri
The reasoning unbounded threadpool is explained as: /* The SDK requires an unbounded thread pool because a step may create X writers * each requiring their own thread to perform the writes otherwise a writer may * block causing deadlock for the step because the writers buffer is full. * Also, the

Re: BEAM-6018: memory leak in thread pool instantiation

2018-11-08 Thread Dan Halperin
> > > On Thu, Nov 8, 2018 at 2:12 PM Udi Meiri wrote: > >> Both options risk delaying worker shutdown if the executor's shutdown() >> hasn't been called, which is I guess why the executor in GcsOptions.java >> creates daemon threads. >> > My guess (and it really is a guess at this point) is that

Re: BEAM-6018: memory leak in thread pool instantiation

2018-11-08 Thread Dan Halperin
Hey Udi, Thanks for the commit comment . I'll try to dump any (old) mental context I have left.. We were trying to find the right point in a space of: * enough parallelism to speed things up - more

Re: BEAM-6018: memory leak in thread pool instantiation

2018-11-08 Thread Udi Meiri
My thought was to use 1 executor per GcsUtil instance (or 1 per process as you suggest), with a possible performance hit since I don't know how often these batch copy and remove operations are called. The other option is to leave things as they mostly are, and only remove the call to

Re: BEAM-6018: memory leak in thread pool instantiation

2018-11-08 Thread Lukasz Cwik
Not certain, it looks like we should have been caching the executor within the GcsUtil as a static instance instead of creating one each time. Could have been missed during code review / slow code changes over time. GcsUtil is not well "loved". On Thu, Nov 8, 2018 at 11:00 AM Udi Meiri wrote: >

BEAM-6018: memory leak in thread pool instantiation

2018-11-08 Thread Udi Meiri
HI, I've identified a memory leak when GcsUtil.java instantiates a ThreadPoolExecutor (https://issues.apache.org/jira/browse/BEAM-6018). The code uses the getExitingExecutorService