You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org> on 2018/10/05 06:48:24 UTC
[Impala-ASF-CR] IMPALA-7668: Proper clean up of URLClassLoader
Bharath Vissapragada has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11594
Change subject: IMPALA-7668: Proper clean up of URLClassLoader
......................................................................
IMPALA-7668: Proper clean up of URLClassLoader
Starting with Java 7 URLClassLoader implements a close() method that cleans
up any open files and helps avoid bugs like fd leaks inside the class loader.
Additionally it also helps load updated class versions of the classes that are
loaded already by prior instances.
This commit makes sure that the URLClassLoader is close()'d in a few
places in the code.
Testing: Tricky to automate the tests for this behavior, so no new tests were added.
Change-Id: I5c5100ef5c5a97d92d94fb68daab622f0aa30158
---
M fe/src/main/java/org/apache/impala/extdatasource/ExternalDataSourceExecutor.java
M fe/src/main/java/org/apache/impala/hive/executor/UdfExecutor.java
M fe/src/main/java/org/apache/impala/util/FunctionUtils.java
3 files changed, 53 insertions(+), 40 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/94/11594/1
--
To view, visit http://gerrit.cloudera.org:8080/11594
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5c5100ef5c5a97d92d94fb68daab622f0aa30158
Gerrit-Change-Number: 11594
Gerrit-PatchSet: 1
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
[Impala-ASF-CR] IMPALA-7668: Proper clean up of URLClassLoader
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/11594 )
Change subject: IMPALA-7668: Proper clean up of URLClassLoader
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11594/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11594/1//COMMIT_MSG@17
PS1, Line 17: Testing: Tricky to automate the tests for this behavior, so no new
> The bug came about with a resource leak with UDFs, yes?
The issue though is that I was not able to repro it locally (tried a few Java versions) in Impala.
This bug report was motivated by a user running an older version of Impala and we suspect that a proper clean up could've avoided it. A colleague of mine could reproduce it a standalone class [1] but it doesn't repro it on my machine for some reason.
[1] https://gist.github.com/bharathv/8afb592a202694b8db2842a2376112cc
http://gerrit.cloudera.org:8080/#/c/11594/1/fe/src/main/java/org/apache/impala/extdatasource/ExternalDataSourceExecutor.java
File fe/src/main/java/org/apache/impala/extdatasource/ExternalDataSourceExecutor.java:
http://gerrit.cloudera.org:8080/#/c/11594/1/fe/src/main/java/org/apache/impala/extdatasource/ExternalDataSourceExecutor.java@148
PS1, Line 148: URLClassLoader loader = URLClassLoader.newInstance( new URL[] { url },
: getClass().getClassLoader());
: c = Class.forName(className_, true, loader);
: if (!ArrayUtils.contains(c.getInterfaces(), apiVersion_.getApiInterface())) {
: throw new ImpalaRuntimeException(String.format(
: "Class '%s' does not implement interface '%s' required for API version %s",
: className_, apiVersion_.getApiInterface().getName(), apiVersion_.name()));
: }
: // Only cache the class if the init string starts with CACHE_CLASS_PREFIX
: if (initString_ != null && initString_.startsWith(CACHE_CLASS_PREFIX)) {
: cachedClasses_.put(cacheMapKey, c);
: }
: i
I've undone these changes because we basically cache the "class" objects here which are lazily instantiated using the provided loader. Closing the loader here eagerly means that we cannot resolve the dependent classes later using the closed loader. This was rightly caught by unit tests and easily reproducible.
ClassLoader seems to be having a "protected" method that eagerly resolves all the dependent classes, fwiw.
Also given the fact that this datasource interface is not being actively developed (or used I believe), I don't think it is worth putting any more effort into optimizing it.
[1] https://docs.oracle.com/javase/7/docs/api/java/lang/ClassLoader.html#loadClass(java.lang.String,%20boolean)
--
To view, visit http://gerrit.cloudera.org:8080/11594
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5c5100ef5c5a97d92d94fb68daab622f0aa30158
Gerrit-Change-Number: 11594
Gerrit-PatchSet: 2
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 Oct 2018 00:42:13 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7668: Proper clean up of URLClassLoader
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11594 )
Change subject: IMPALA-7668: Proper clean up of URLClassLoader
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1133/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/11594
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5c5100ef5c5a97d92d94fb68daab622f0aa30158
Gerrit-Change-Number: 11594
Gerrit-PatchSet: 3
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 Oct 2018 18:32:12 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7668: Proper clean up of URLClassLoader
Posted by "Philip Zeyliger (Code Review)" <ge...@cloudera.org>.
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/11594 )
Change subject: IMPALA-7668: Proper clean up of URLClassLoader
......................................................................
Patch Set 2: Code-Review+2
(2 comments)
Thanks. Looks fine to me. Some minor nits below, but feel free to resolve them however you see fit.
http://gerrit.cloudera.org:8080/#/c/11594/2/fe/src/main/java/org/apache/impala/hive/executor/UdfExecutor.java
File fe/src/main/java/org/apache/impala/hive/executor/UdfExecutor.java:
http://gerrit.cloudera.org:8080/#/c/11594/2/fe/src/main/java/org/apache/impala/hive/executor/UdfExecutor.java@584
PS2, Line 584: loader = getClassLoader(jarPath);
What's weird here is that if jarPath is null then this returns the system class loader. Now, probably the system class loader isn't a URLClassLoader, but it's still a little uncomfortable.
Do you have any ideas? We could avoid cleaning up if jarPath == null to at least be sure.
http://gerrit.cloudera.org:8080/#/c/11594/2/fe/src/main/java/org/apache/impala/util/FunctionUtils.java
File fe/src/main/java/org/apache/impala/util/FunctionUtils.java:
http://gerrit.cloudera.org:8080/#/c/11594/2/fe/src/main/java/org/apache/impala/util/FunctionUtils.java@97
PS2, Line 97: URL[] classLoaderUrls = new URL[] {new URL(localJarPath.toString())};
The equivalent line in UdfExecutor.java is:
URL url = new File(jarPath).toURI().toURL();
Should that one be simplified to this one?
--
To view, visit http://gerrit.cloudera.org:8080/11594
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5c5100ef5c5a97d92d94fb68daab622f0aa30158
Gerrit-Change-Number: 11594
Gerrit-PatchSet: 2
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 Oct 2018 14:58:32 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7668: Proper clean up of URLClassLoader
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Hello Philip Zeyliger, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11594
to look at the new patch set (#2).
Change subject: IMPALA-7668: Proper clean up of URLClassLoader
......................................................................
IMPALA-7668: Proper clean up of URLClassLoader
Starting with Java 7 URLClassLoader implements a close() method that
cleans up any open files and helps avoid bugs like fd leaks inside the
class loader. Additionally it also helps load updated class versions
of the classes that are loaded already by prior instances.
This commit makes sure that the URLClassLoader is close()'d in a few
places in the code.
Testing: Tricky to automate the tests for this behavior, so no new
tests were added.
Change-Id: I5c5100ef5c5a97d92d94fb68daab622f0aa30158
---
M fe/src/main/java/org/apache/impala/extdatasource/ExternalDataSourceExecutor.java
M fe/src/main/java/org/apache/impala/hive/executor/UdfExecutor.java
M fe/src/main/java/org/apache/impala/util/FunctionUtils.java
3 files changed, 43 insertions(+), 31 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/94/11594/2
--
To view, visit http://gerrit.cloudera.org:8080/11594
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5c5100ef5c5a97d92d94fb68daab622f0aa30158
Gerrit-Change-Number: 11594
Gerrit-PatchSet: 2
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
[Impala-ASF-CR] IMPALA-7668: Proper clean up of URLClassLoader
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11594 )
Change subject: IMPALA-7668: Proper clean up of URLClassLoader
......................................................................
Patch Set 3: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/11594
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5c5100ef5c5a97d92d94fb68daab622f0aa30158
Gerrit-Change-Number: 11594
Gerrit-PatchSet: 3
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 Oct 2018 21:50:15 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7668: Proper clean up of URLClassLoader
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/11594 )
Change subject: IMPALA-7668: Proper clean up of URLClassLoader
......................................................................
Patch Set 3: Code-Review+2
(2 comments)
Carrying +2.
http://gerrit.cloudera.org:8080/#/c/11594/2/fe/src/main/java/org/apache/impala/hive/executor/UdfExecutor.java
File fe/src/main/java/org/apache/impala/hive/executor/UdfExecutor.java:
http://gerrit.cloudera.org:8080/#/c/11594/2/fe/src/main/java/org/apache/impala/hive/executor/UdfExecutor.java@584
PS2, Line 584: loader = getClassLoader(jarPath);
> What's weird here is that if jarPath is null then this returns the system c
Tightened the check in the cleanup. It is not super clear if 'jarPath' can ever be null.
- I looked at the git history and the original code review that added this. It has stayed the same way since the beginning.
- I added a Preconditions.checkNotNull(jarPath) at the beginning of this method and re-ran a bunch of UDF tests and none hit the check.
http://gerrit.cloudera.org:8080/#/c/11594/2/fe/src/main/java/org/apache/impala/util/FunctionUtils.java
File fe/src/main/java/org/apache/impala/util/FunctionUtils.java:
http://gerrit.cloudera.org:8080/#/c/11594/2/fe/src/main/java/org/apache/impala/util/FunctionUtils.java@97
PS2, Line 97: URL[] classLoaderUrls = new URL[] {new URL(localJarPath.toString())};
> The equivalent line in UdfExecutor.java is:
The issue is that we qualify the filesystem prefix here in L86. In UdfExecutor we don't do that. Doing a File.toURI.... adds it. Leaving it as is for now.
--
To view, visit http://gerrit.cloudera.org:8080/11594
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5c5100ef5c5a97d92d94fb68daab622f0aa30158
Gerrit-Change-Number: 11594
Gerrit-PatchSet: 3
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 Oct 2018 17:58:57 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7668: Proper clean up of URLClassLoader
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11594 )
Change subject: IMPALA-7668: Proper clean up of URLClassLoader
......................................................................
Patch Set 1:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3279/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/11594
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5c5100ef5c5a97d92d94fb68daab622f0aa30158
Gerrit-Change-Number: 11594
Gerrit-PatchSet: 1
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 05 Oct 2018 07:17:36 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7668: Proper clean up of URLClassLoader
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11594 )
Change subject: IMPALA-7668: Proper clean up of URLClassLoader
......................................................................
IMPALA-7668: Proper clean up of URLClassLoader
Starting with Java 7 URLClassLoader implements a close() method that
cleans up any open files and helps avoid bugs like fd leaks inside the
class loader. Additionally it also helps load updated class versions
of the classes that are loaded already by prior instances.
This commit makes sure that the URLClassLoader is close()'d in a few
places in the code.
Testing: Tricky to automate the tests for this behavior, so no new
tests were added.
Change-Id: I5c5100ef5c5a97d92d94fb68daab622f0aa30158
Reviewed-on: http://gerrit.cloudera.org:8080/11594
Reviewed-by: Bharath Vissapragada <bh...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/extdatasource/ExternalDataSourceExecutor.java
M fe/src/main/java/org/apache/impala/hive/executor/UdfExecutor.java
M fe/src/main/java/org/apache/impala/util/FunctionUtils.java
3 files changed, 43 insertions(+), 31 deletions(-)
Approvals:
Bharath Vissapragada: Looks good to me, approved
Impala Public Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/11594
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I5c5100ef5c5a97d92d94fb68daab622f0aa30158
Gerrit-Change-Number: 11594
Gerrit-PatchSet: 4
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
[Impala-ASF-CR] IMPALA-7668: Proper clean up of URLClassLoader
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11594 )
Change subject: IMPALA-7668: Proper clean up of URLClassLoader
......................................................................
Patch Set 2: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/11594
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5c5100ef5c5a97d92d94fb68daab622f0aa30158
Gerrit-Change-Number: 11594
Gerrit-PatchSet: 2
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 Oct 2018 05:04:55 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7668: Proper clean up of URLClassLoader
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11594 )
Change subject: IMPALA-7668: Proper clean up of URLClassLoader
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1130/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/11594
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5c5100ef5c5a97d92d94fb68daab622f0aa30158
Gerrit-Change-Number: 11594
Gerrit-PatchSet: 2
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 Oct 2018 01:28:16 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7668: Proper clean up of URLClassLoader
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11594 )
Change subject: IMPALA-7668: Proper clean up of URLClassLoader
......................................................................
Patch Set 1: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3279/
--
To view, visit http://gerrit.cloudera.org:8080/11594
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5c5100ef5c5a97d92d94fb68daab622f0aa30158
Gerrit-Change-Number: 11594
Gerrit-PatchSet: 1
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 05 Oct 2018 11:12:37 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7668: Proper clean up of URLClassLoader
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11594 )
Change subject: IMPALA-7668: Proper clean up of URLClassLoader
......................................................................
Patch Set 2:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3344/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/11594
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5c5100ef5c5a97d92d94fb68daab622f0aa30158
Gerrit-Change-Number: 11594
Gerrit-PatchSet: 2
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 Oct 2018 01:10:04 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7668: Proper clean up of URLClassLoader
Posted by "Philip Zeyliger (Code Review)" <ge...@cloudera.org>.
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/11594 )
Change subject: IMPALA-7668: Proper clean up of URLClassLoader
......................................................................
Patch Set 1:
> (1 comment)
I just realized the leak is actually "fds", so we can run "lsof" or just listdir(/proc/<pid>/fd) to make sure using UDFs isn't leaking resources.
--
To view, visit http://gerrit.cloudera.org:8080/11594
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5c5100ef5c5a97d92d94fb68daab622f0aa30158
Gerrit-Change-Number: 11594
Gerrit-PatchSet: 1
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Comment-Date: Fri, 05 Oct 2018 16:58:03 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7668: Proper clean up of URLClassLoader
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11594 )
Change subject: IMPALA-7668: Proper clean up of URLClassLoader
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/958/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/11594
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5c5100ef5c5a97d92d94fb68daab622f0aa30158
Gerrit-Change-Number: 11594
Gerrit-PatchSet: 1
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 05 Oct 2018 07:25:34 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7668: Proper clean up of URLClassLoader
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Hello Philip Zeyliger, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11594
to look at the new patch set (#3).
Change subject: IMPALA-7668: Proper clean up of URLClassLoader
......................................................................
IMPALA-7668: Proper clean up of URLClassLoader
Starting with Java 7 URLClassLoader implements a close() method that
cleans up any open files and helps avoid bugs like fd leaks inside the
class loader. Additionally it also helps load updated class versions
of the classes that are loaded already by prior instances.
This commit makes sure that the URLClassLoader is close()'d in a few
places in the code.
Testing: Tricky to automate the tests for this behavior, so no new
tests were added.
Change-Id: I5c5100ef5c5a97d92d94fb68daab622f0aa30158
---
M fe/src/main/java/org/apache/impala/extdatasource/ExternalDataSourceExecutor.java
M fe/src/main/java/org/apache/impala/hive/executor/UdfExecutor.java
M fe/src/main/java/org/apache/impala/util/FunctionUtils.java
3 files changed, 43 insertions(+), 31 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/94/11594/3
--
To view, visit http://gerrit.cloudera.org:8080/11594
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5c5100ef5c5a97d92d94fb68daab622f0aa30158
Gerrit-Change-Number: 11594
Gerrit-PatchSet: 3
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
[Impala-ASF-CR] IMPALA-7668: Proper clean up of URLClassLoader
Posted by "Philip Zeyliger (Code Review)" <ge...@cloudera.org>.
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/11594 )
Change subject: IMPALA-7668: Proper clean up of URLClassLoader
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11594/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11594/1//COMMIT_MSG@17
PS1, Line 17: Testing: Tricky to automate the tests for this behavior, so no new tests were added.
The bug came about with a resource leak with UDFs, yes?
Could we write a custom cluster test that loads a UDF, and then use jmap -dump:histo (the "live" version) to make sure that the UDF is garbage collected? Presumably it wasn't before hand?
--
To view, visit http://gerrit.cloudera.org:8080/11594
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5c5100ef5c5a97d92d94fb68daab622f0aa30158
Gerrit-Change-Number: 11594
Gerrit-PatchSet: 1
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Comment-Date: Fri, 05 Oct 2018 16:43:18 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7668: Proper clean up of URLClassLoader
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11594 )
Change subject: IMPALA-7668: Proper clean up of URLClassLoader
......................................................................
Patch Set 3:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3345/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/11594
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5c5100ef5c5a97d92d94fb68daab622f0aa30158
Gerrit-Change-Number: 11594
Gerrit-PatchSet: 3
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 Oct 2018 17:59:07 +0000
Gerrit-HasComments: No