You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by XuTingjun <gi...@git.apache.org> on 2015/02/12 12:37:20 UTC
[GitHub] spark pull request: [SPARK-5770] Fix bug: Use addJar() to upload a...
GitHub user XuTingjun opened a pull request:
https://github.com/apache/spark/pull/4565
[SPARK-5770] Fix bug: Use addJar() to upload a new jar file to executor, it can't be added to classloader
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/XuTingjun/spark fileOverWrite
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/4565.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 #4565
----
commit 519a3bd7c1458d3112440a929d7dfecf46576ead
Author: Xu Tingjun <xu...@huawei.com>
Date: 2015-02-12T11:24:36Z
fix bug
----
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5770] Fix bug: Use addJar() to upload a...
Posted by XuTingjun <gi...@git.apache.org>.
Github user XuTingjun commented on the pull request:
https://github.com/apache/spark/pull/4565#issuecomment-74060311
I have test it in spark-shell. Yearh the new jar will overwrite the old one in local dir, but the classloader has not updated. They are inconsistent. It will confuse users.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5770] Fix bug: Use addJar() to upload a...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4565#issuecomment-74191089
Can one of the admins verify this patch?
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5770] Fix bug: Use addJar() to upload a...
Posted by XuTingjun <gi...@git.apache.org>.
GitHub user XuTingjun reopened a pull request:
https://github.com/apache/spark/pull/4565
[SPARK-5770] Fix bug: Use addJar() to upload a new jar file to executor, it can't be added to classloader
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/XuTingjun/spark fileOverWrite
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/4565.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 #4565
----
commit 519a3bd7c1458d3112440a929d7dfecf46576ead
Author: Xu Tingjun <xu...@huawei.com>
Date: 2015-02-12T11:24:36Z
fix bug
----
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5770] Fix bug: Use addJar() to upload a...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/4565#issuecomment-74064507
OK, in any event, this change can't be made. It affects other code using this method.
I am not sure (?) that this is the intended effect of `addJar`. You can add new jars; overwriting probably doesn't make the classes unload, yes. That could be an argument for disallowing overwrite for this use case, but not an argument for removing the functionality from this method for everyone.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5770] Fix bug: Use addJar() to upload a...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/4565#issuecomment-74058430
This change doesn't make sense; it does not seem to relate to the issue you are reporting. It sounds like you are exercising the case where overwrite is enabled, but you've taken out the logic to handle it.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5770] forbid user to overwrite jar usin...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/4565#issuecomment-74414488
@XuTingjun I'm not sure what you're responding to. I'm first suggesting that this code change can be simpler and doesn't seem to need a new flag. But I'm also suggesting that restricting this behavior may cause errors when the user doesn't care about reloading classes. Maybe it can be a warning only.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5770] Fix bug: Use addJar() to upload a...
Posted by XuTingjun <gi...@git.apache.org>.
Github user XuTingjun closed the pull request at:
https://github.com/apache/spark/pull/4565
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5770] Fix bug: Use addJar() to upload a...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/4565#issuecomment-74065394
I suppose I'm saying that this kind of thing doesn't belong in `Utils`. It's not generally a problem to try to copy and overwrite files. If anything I think you're arguing that overwrite should be set to false just for JARs. It is false by default.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5770] Fix bug: Use addJar() to upload a...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/4565#issuecomment-74059776
The current code path appears to correctly overwrite the old file with a new one. If you're saying you think it doesn't, have you debugged way? I don't think you can just take this out.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5770] forbid user to overwrite jar usin...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/4565#issuecomment-74235582
Rather than add a new flag, why not just disallow overwrite in these cases?
I'd like someone else to review the logic too before forbidding this. That is, I would not want this to cause an error in a common case, just because a jar changed, but where the caller does not care about updating the classloader.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5770] forbid user to overwrite jar usin...
Posted by XuTingjun <gi...@git.apache.org>.
Github user XuTingjun closed the pull request at:
https://github.com/apache/spark/pull/4565
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5770] Fix bug: Use addJar() to upload a...
Posted by XuTingjun <gi...@git.apache.org>.
Github user XuTingjun commented on the pull request:
https://github.com/apache/spark/pull/4565#issuecomment-74064920
I also think it's controversial, but can we make the log more reasonable?
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5770] Fix bug: Use addJar() to upload a...
Posted by XuTingjun <gi...@git.apache.org>.
Github user XuTingjun commented on the pull request:
https://github.com/apache/spark/pull/4565#issuecomment-74190761
Though it's false by default, but "spark.files.overwrite" is exposed to user. So I think we should set overwrite just false only to JARS?
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5770] Fix bug: Use addJar() to upload a...
Posted by XuTingjun <gi...@git.apache.org>.
Github user XuTingjun commented on the pull request:
https://github.com/apache/spark/pull/4565#issuecomment-74059653
Sorry, I think it's very difficult to unload the old jar and load the new jar to the classloader, maybe no solution.
So I think the good way is restricting users do this thing, and throw the exception to tell the user.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5770] Fix bug: Use addJar() to upload a...
Posted by XuTingjun <gi...@git.apache.org>.
Github user XuTingjun commented on the pull request:
https://github.com/apache/spark/pull/4565#issuecomment-74199949
@srowen,
I think overwrite jar file has no meaning, because now code doesn't support loader new jar file. So I think forbid use to do this is more reasonable.
I add a param "isJar" to distinguish file overwite and jar overwrite.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5770] Fix bug: Use addJar() to upload a...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4565#issuecomment-74058013
Can one of the admins verify this patch?
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5770] forbid user to overwrite jar usin...
Posted by XuTingjun <gi...@git.apache.org>.
Github user XuTingjun commented on the pull request:
https://github.com/apache/spark/pull/4565#issuecomment-74399664
Thanks, @srowen. I think there have the users who starts a long-running sparkcontext, and add jars to run different cases.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org