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 getExitingExecutorService.
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. On Thu, Nov 8, 2018 at 1:02 PM Lukasz Cwik <[email protected]> wrote: > 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 <[email protected]> wrote: > >> 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 >> <https://github.com/apache/beam/blob/279a05604b83a54e8e5a79e13d8761f94841f326/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/util/GcsUtil.java#L551> >> wrapper, >> which leaks memory. The question is, why is that wrapper necessary >> if executor.shutdown(); is later unconditionally called? >> >
smime.p7s
Description: S/MIME Cryptographic Signature
