You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by StephanEwen <gi...@git.apache.org> on 2015/08/11 16:12:21 UTC
[GitHub] flink pull request: [FLINK-2509] Add class loader info to exceptio...
GitHub user StephanEwen opened a pull request:
https://github.com/apache/flink/pull/1008
[FLINK-2509] Add class loader info to exception message when user code classes are not found
This pull request adds the following type of info to the exception message:
```
Cannot load user class: com.foo.bar.SomeClass
ClassLoader info: URL ClassLoader: -- http://localhost:26712/some/file/path -- file: '/tmp/flink-url-test2825910185710886145.tmp' (valid JAR) -- file: '/tmp/flink-url-test4914469684250387785.tmp' (invalid JAR: error in opening zip file) -- file: '/tmp/flink-url-test5462325335282720117.tmp' (missing)
```
This should help diagnosing why sometimes classes cannot be resolved, even though the JAR files seem valid.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/StephanEwen/incubator-flink class_loader_info
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/1008.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1008
----
commit 00e123709f304bd7f602989f28b111b8d282ebb1
Author: Stephan Ewen <se...@apache.org>
Date: 2015-08-11T14:07:22Z
[FLINK-2509] [runtime] Add class loader info into the exception message when user code classes are not found.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2509] Add class loader info to exceptio...
Posted by hsaputra <gi...@git.apache.org>.
Github user hsaputra commented on a diff in the pull request:
https://github.com/apache/flink/pull/1008#discussion_r36825210
--- Diff: flink-staging/flink-streaming/flink-streaming-core/src/main/java/org/apache/flink/streaming/api/graph/StreamConfig.java ---
@@ -198,12 +199,26 @@ public void setStreamOperator(StreamOperator<?> operator) {
}
}
}
-
- @SuppressWarnings({ "unchecked" })
+
public <T> T getStreamOperator(ClassLoader cl) {
try {
- return (T) InstantiationUtil.readObjectFromConfig(this.config, SERIALIZEDUDF, cl);
- } catch (Exception e) {
+ @SuppressWarnings("unchecked")
+ T result = (T) InstantiationUtil.readObjectFromConfig(this.config, SERIALIZEDUDF, cl);
+ return result;
+ }
+ catch (ClassNotFoundException e) {
+ String classLoaderInfo = ClassLoaderUtil.getUserCodeClassLoaderInfo(cl);
+ boolean loadableDoubleCheck = ClassLoaderUtil.validateClassLoadable(e, cl);
+
+ String exceptionMessage = "Cannot load user class: " + e.getMessage()
+ + "\nClassLoader info: " + classLoaderInfo +
+ (loadableDoubleCheck ?
+ "Class was actually found in classloader - deserialization issue." :
+ "Class not resolveable through given classloader.");
--- End diff --
small nit on message: Class was not resolvable
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2509] Add class loader info to exceptio...
Posted by hsaputra <gi...@git.apache.org>.
Github user hsaputra commented on the pull request:
https://github.com/apache/flink/pull/1008#issuecomment-130362926
HI @StephanEwen why is this PR is merged without addressing existing comments?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2509] Add class loader info to exceptio...
Posted by uce <gi...@git.apache.org>.
Github user uce commented on the pull request:
https://github.com/apache/flink/pull/1008#issuecomment-130206616
Very nice addition! :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2509] Add class loader info to exceptio...
Posted by hsaputra <gi...@git.apache.org>.
Github user hsaputra commented on the pull request:
https://github.com/apache/flink/pull/1008#issuecomment-130154250
Agree with Max, this should be useful.
Other than small nit on message +1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2509] Add class loader info to exceptio...
Posted by mxm <gi...@git.apache.org>.
Github user mxm commented on the pull request:
https://github.com/apache/flink/pull/1008#issuecomment-129947817
This is great and will be very helpful while debugging! Only the output is very hard to read. How about this? Just some new lines and indention:
```
Cannot load user class: com.foo.bar.SomeClass
ClassLoader info:
URL ClassLoader:
url: 'http://localhost:26712/some/file/path'
file: '/tmp/flink-url-test2825910185710886145.tmp' (valid JAR)
file: '/tmp/flink-url-test4914469684250387785.tmp' (invalid JAR: error in opening zip file)
file: '/tmp/flink-url-test5462325335282720117.tmp' (missing)
```
I think users are less prone to read Exception messages when they have to scroll horizontally. We usually don't include newlines in our Exceptions.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2509] Add class loader info to exceptio...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1008#issuecomment-129955067
Good comment, will adjust this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2509] Add class loader info to exceptio...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/1008
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2509] Add class loader info to exceptio...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1008#issuecomment-130369569
I thought not, but you are right, I pushed the wrong branch.
Sorry, git-fail on my side!
I'll push an update in a bit...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2509] Add class loader info to exceptio...
Posted by hsaputra <gi...@git.apache.org>.
Github user hsaputra commented on the pull request:
https://github.com/apache/flink/pull/1008#issuecomment-130388161
No worries =)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2509] Add class loader info to exceptio...
Posted by uce <gi...@git.apache.org>.
Github user uce commented on a diff in the pull request:
https://github.com/apache/flink/pull/1008#discussion_r36834144
--- Diff: flink-runtime/src/test/java/org/apache/flink/runtime/util/ClassLoaderUtilsTest.java ---
@@ -0,0 +1,131 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.runtime.util;
+
+import static org.junit.Assert.*;
+
+import org.junit.Test;
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.net.URL;
+import java.net.URLClassLoader;
+import java.util.jar.JarFile;
+
+/**
+ * Tests that validate the {@link ClassLoaderUtil}.
+ */
+public class ClassLoaderUtilsTest {
+
+ @Test
+ public void testWithURLClassLoader() {
+ File validJar = null;
+ File invalidJar = null;
+
+ try {
+ // file with jar contents
+ validJar = File.createTempFile("flink-url-test", ".tmp");
+ JarFileCreator jarFileCreator = new JarFileCreator(validJar);
+ jarFileCreator.addClass(ClassLoaderUtilsTest.class);
+ jarFileCreator.createJarFile();
+
+ // validate that the JAR is correct and the test setup is not broken
+ try {
+ new JarFile(validJar.getAbsolutePath());
+ }
+ catch (Exception e) {
+ e.printStackTrace();
+ fail("test setup broken: cannot create a valid jar file");
+ }
+
+ // file with some random contents
+ invalidJar = File.createTempFile("flink-url-test", ".tmp");
+ try (FileOutputStream invalidout = new FileOutputStream(invalidJar)) {
+ invalidout.write(new byte[] { -1, 1, -2, 3, -3, 4, });
+ }
+
+ // non existing file
+ File nonExisting = File.createTempFile("flink-url-test", ".tmp");
+ assertTrue("Cannot create and delete temp file", nonExisting.delete());
+
+
+ // create a URL classloader with
+ // - a HTTP URL
+ // - a file URL for an existing jar file
+ // - a file URL for an existing file that is not a jar file
+ // - a file URL for a non-existing file
+
+ URL[] urls = {
+ new URL("http", "localhost", 26712, "/some/file/path"),
+ new URL("file", null, validJar.getAbsolutePath()),
+ new URL("file", null, invalidJar.getAbsolutePath()),
+ new URL("file", null, nonExisting.getAbsolutePath()),
+ };
+
+ URLClassLoader loader = new URLClassLoader(urls, getClass().getClassLoader());
+ String info = ClassLoaderUtil.getUserCodeClassLoaderInfo(loader);
+
+ assertTrue(info.indexOf("/some/file/path") > 0);
+ assertTrue(info.indexOf(validJar.getAbsolutePath() + "' (valid") > 0);
+ assertTrue(info.indexOf(invalidJar.getAbsolutePath() + "' (invalid JAR") > 0);
+ assertTrue(info.indexOf(nonExisting.getAbsolutePath() + "' (missing") > 0);
+
+ System.out.println(info);
+ }
+ catch (Exception e) {
+ e.printStackTrace();
+ fail(e.getMessage());
+ }
+ finally {
+ if (validJar != null) {
+ //noinspection ResultOfMethodCallIgnored
+ validJar.delete();
+ }
+ if (invalidJar != null) {
+ //noinspection ResultOfMethodCallIgnored
+ invalidJar.delete();
+ }
+ }
+ }
+
+ @Test
+ public void testWithAppClassLoader() {
+ try {
+ // must return something when invoked with 'null'
--- End diff --
copy paste comment misleading :smiley:
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---