You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by tzulitai <gi...@git.apache.org> on 2017/05/25 09:00:18 UTC
[GitHub] flink pull request #3987: [FLINK-6714] [runtime] Use user classloader for op...
GitHub user tzulitai opened a pull request:
https://github.com/apache/flink/pull/3987
[FLINK-6714] [runtime] Use user classloader for operator state copying on snapshots
Now that the operator state backend creates a deep copy of the state during the synchronous part of async checkpoints, it needs to set the user classloader as the thread context classloader, otherwise serializers that uses serialization for copying will use the wrong classloader when deserializing the copy.
A test is added to {{OperatorStateBackendTest}} to verify that the user classloader is properly used on copy.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/tzulitai/flink FLINK-6714
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/3987.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 #3987
----
commit 516fd085d640d56ea2ef7be7f0b11f6ad43e92f5
Author: Tzu-Li (Gordon) Tai <tz...@apache.org>
Date: 2017-05-25T08:54:08Z
[FLINK-6714] [runtime] Use user classloader for operator state copying on snapshots
----
---
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 issue #3987: [FLINK-6714] [runtime] Use user classloader for operator ...
Posted by rmetzger <gi...@git.apache.org>.
Github user rmetzger commented on the issue:
https://github.com/apache/flink/pull/3987
+1 to merge
---
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 #3987: [FLINK-6714] [runtime] Use user classloader for op...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/3987
---
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 issue #3987: [FLINK-6714] [runtime] Use user classloader for operator ...
Posted by tzulitai <gi...@git.apache.org>.
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/3987
Thanks for the review @rmetzger @EronWright.
I'll address Eron's comment while merging.
---
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 #3987: [FLINK-6714] [runtime] Use user classloader for op...
Posted by EronWright <gi...@git.apache.org>.
Github user EronWright commented on a diff in the pull request:
https://github.com/apache/flink/pull/3987#discussion_r118530090
--- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/state/DefaultOperatorStateBackend.java ---
@@ -185,13 +185,18 @@ public void dispose() {
new HashMap<>(registeredStates.size());
// eagerly create deep copies of the list states in the sync phase, so that we can use them in the async writing
- for (Map.Entry<String, PartitionableListState<?>> entry : this.registeredStates.entrySet()) {
-
- PartitionableListState<?> listState = entry.getValue();
- if (null != listState) {
- listState = listState.deepCopy();
+ ClassLoader snapshotClassLoader = Thread.currentThread().getContextClassLoader();
+ try {
+ Thread.currentThread().setContextClassLoader(userClassloader);
--- End diff --
(nitpick) `Thread.currentThread().setContextClassLoader(userClassloader)` should occur before the `try` block is entered. Setting the TCCL occurs in about a half-dozen places in Flink, and this is inconsistent with most of them.
---
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 issue #3987: [FLINK-6714] [runtime] Use user classloader for operator ...
Posted by tzulitai <gi...@git.apache.org>.
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/3987
Manually verified that https://issues.apache.org/jira/browse/FLINK-6515 should be properly fixed with this change.
---
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.
---