[jira] [Commented] (IGNITE-5038) BinaryMarshaller might need to use context class loader for deserialization
[ https://issues.apache.org/jira/browse/IGNITE-5038?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17177674#comment-17177674 ] Igor Seliverstov commented on IGNITE-5038: -- [~ivan.glukos]I went across the PR, generally I'm OK with changes. > BinaryMarshaller might need to use context class loader for deserialization > --- > > Key: IGNITE-5038 > URL: https://issues.apache.org/jira/browse/IGNITE-5038 > Project: Ignite > Issue Type: Improvement > Components: binary >Affects Versions: 2.0 >Reporter: Dmitry Karachentsev >Assignee: Ivan Rakov >Priority: Major > Labels: features > Attachments: results-compound-20170802.zip, > results-compound-20170808.zip > > Time Spent: 1h 40m > Remaining Estimate: 0h > > There is a special use case discussed on the dev list: > http://apache-ignite-developers.2346864.n4.nabble.com/Re-BinaryObjectImpl-deserializeValue-with-specific-ClassLoader-td17126.html#a17224 > According to the use case, BinaryMarshaller might need to try to deserialize > an object using a context class loader if it failed to do so with a custom > classloader (`IgniteConfiguration.getClassLoader()`) or the system one. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (IGNITE-5038) BinaryMarshaller might need to use context class loader for deserialization
[ https://issues.apache.org/jira/browse/IGNITE-5038?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17177646#comment-17177646 ] Vladislav Pyatkov commented on IGNITE-5038: --- [~ivan.glukos] I left a couple of comments in PR. Both of them carry of optional character. > BinaryMarshaller might need to use context class loader for deserialization > --- > > Key: IGNITE-5038 > URL: https://issues.apache.org/jira/browse/IGNITE-5038 > Project: Ignite > Issue Type: Improvement > Components: binary >Affects Versions: 2.0 >Reporter: Dmitry Karachentsev >Assignee: Ivan Rakov >Priority: Major > Labels: features > Attachments: results-compound-20170802.zip, > results-compound-20170808.zip > > Time Spent: 1h 40m > Remaining Estimate: 0h > > There is a special use case discussed on the dev list: > http://apache-ignite-developers.2346864.n4.nabble.com/Re-BinaryObjectImpl-deserializeValue-with-specific-ClassLoader-td17126.html#a17224 > According to the use case, BinaryMarshaller might need to try to deserialize > an object using a context class loader if it failed to do so with a custom > classloader (`IgniteConfiguration.getClassLoader()`) or the system one. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (IGNITE-5038) BinaryMarshaller might need to use context class loader for deserialization
[
https://issues.apache.org/jira/browse/IGNITE-5038?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17177086#comment-17177086
]
Ignite TC Bot commented on IGNITE-5038:
---
{panel:title=Branch: [pull/8146/head] Base: [master] : No blockers
found!|borderStyle=dashed|borderColor=#ccc|titleBGColor=#D6F7C1}{panel}
{panel:title=Branch: [pull/8146/head] Base: [master] : New Tests
(16)|borderStyle=dashed|borderColor=#ccc|titleBGColor=#D6F7C1}
{color:#8b}Cache 3{color} [[tests
4|https://ci.ignite.apache.org/viewLog.html?buildId=5535463]]
* {color:#013220}IgniteBinaryObjectsCacheTestSuite3:
BinaryClassLoaderMultiJvmTest.testClientLoadClassFromBinary - PASSED{color}
* {color:#013220}IgniteBinaryObjectsCacheTestSuite3:
BinaryClassLoaderMultiJvmTest.testLoadClassFromBinary - PASSED{color}
* {color:#013220}IgniteBinaryObjectsCacheTestSuite3:
BinaryClassLoaderTest.testClientLoadClassFromBinary - PASSED{color}
* {color:#013220}IgniteBinaryObjectsCacheTestSuite3:
BinaryClassLoaderTest.testLoadClassFromBinary - PASSED{color}
{color:#8b}Service Grid{color} [[tests
4|https://ci.ignite.apache.org/viewLog.html?buildId=5535494]]
* {color:#013220}IgniteServiceGridTestSuite:
ServiceDeploymentProcessIdSelfTest.requestId[Test event=IgniteBiTuple
[val1=DiscoveryCustomEvent [customMsg=ServiceChangeBatchRequest
[id=ba70143e371-1060fb3f-d8a1-44a9-8374-2460100ff1f1, reqs=SingletonList
[ServiceUndeploymentRequest []]], affTopVer=null, super=DiscoveryEvent
[evtNode=93bb11d9-6db9-4183-8ca7-fe5bd1fc48d4, topVer=0, msgTemplate=null,
span=null, nodeId8=93bb11d9, msg=null, type=DISCOVERY_CUSTOM_EVT,
tstamp=1597245556647]], val2=AffinityTopologyVersion
[topVer=7712774926676635059, minorTopVer=0]]] - PASSED{color}
* {color:#013220}IgniteServiceGridTestSuite:
ServiceDeploymentProcessIdSelfTest.topologyVersion[Test event=IgniteBiTuple
[val1=DiscoveryEvent [evtNode=f569d24e-8439-493d-ae79-4cfda46347c3, topVer=0,
msgTemplate=null, span=null, nodeId8=e3036cdb, msg=, type=NODE_JOINED,
tstamp=1597245556647], val2=AffinityTopologyVersion
[topVer=4043289110866827658, minorTopVer=0]]] - PASSED{color}
* {color:#013220}IgniteServiceGridTestSuite:
ServiceDeploymentProcessIdSelfTest.requestId[Test event=IgniteBiTuple
[val1=DiscoveryEvent [evtNode=f569d24e-8439-493d-ae79-4cfda46347c3, topVer=0,
msgTemplate=null, span=null, nodeId8=e3036cdb, msg=, type=NODE_JOINED,
tstamp=1597245556647], val2=AffinityTopologyVersion
[topVer=4043289110866827658, minorTopVer=0]]] - PASSED{color}
* {color:#013220}IgniteServiceGridTestSuite:
ServiceDeploymentProcessIdSelfTest.topologyVersion[Test event=IgniteBiTuple
[val1=DiscoveryCustomEvent [customMsg=ServiceChangeBatchRequest
[id=ba70143e371-1060fb3f-d8a1-44a9-8374-2460100ff1f1, reqs=SingletonList
[ServiceUndeploymentRequest []]], affTopVer=null, super=DiscoveryEvent
[evtNode=93bb11d9-6db9-4183-8ca7-fe5bd1fc48d4, topVer=0, msgTemplate=null,
span=null, nodeId8=93bb11d9, msg=null, type=DISCOVERY_CUSTOM_EVT,
tstamp=1597245556647]], val2=AffinityTopologyVersion
[topVer=7712774926676635059, minorTopVer=0]]] - PASSED{color}
{color:#8b}Service Grid (legacy mode){color} [[tests
4|https://ci.ignite.apache.org/viewLog.html?buildId=5535495]]
* {color:#013220}IgniteServiceGridTestSuite:
ServiceDeploymentProcessIdSelfTest.topologyVersion[Test event=IgniteBiTuple
[val1=DiscoveryEvent [evtNode=9dd1727f-7188-4bc8-ba1c-a97a03757d4c, topVer=0,
msgTemplate=null, span=null, nodeId8=febf8310, msg=, type=NODE_JOINED,
tstamp=1597245604529], val2=AffinityTopologyVersion
[topVer=-532717008369219942, minorTopVer=0]]] - PASSED{color}
* {color:#013220}IgniteServiceGridTestSuite:
ServiceDeploymentProcessIdSelfTest.requestId[Test event=IgniteBiTuple
[val1=DiscoveryEvent [evtNode=9dd1727f-7188-4bc8-ba1c-a97a03757d4c, topVer=0,
msgTemplate=null, span=null, nodeId8=febf8310, msg=, type=NODE_JOINED,
tstamp=1597245604529], val2=AffinityTopologyVersion
[topVer=-532717008369219942, minorTopVer=0]]] - PASSED{color}
* {color:#013220}IgniteServiceGridTestSuite:
ServiceDeploymentProcessIdSelfTest.topologyVersion[Test event=IgniteBiTuple
[val1=DiscoveryCustomEvent [customMsg=ServiceChangeBatchRequest
[id=55ccb43e371-68bc7ffc-8b12-455c-9851-4762685df930, reqs=SingletonList
[ServiceUndeploymentRequest []]], affTopVer=null, super=DiscoveryEvent
[evtNode=9f018365-0dde-4888-9dca-ae8e0e62401f, topVer=0, msgTemplate=null,
span=null, nodeId8=9f018365, msg=null, type=DISCOVERY_CUSTOM_EVT,
tstamp=1597245604529]], val2=AffinityTopologyVersion
[topVer=347831288372854690, minorTopVer=0]]] - PASSED{color}
* {color:#013220}IgniteServiceGridTestSuite:
ServiceDeploymentProcessIdSelfTest.requestId[Test event=IgniteBiTuple
[val1=DiscoveryCustomEvent [customMsg=ServiceChangeBatchRequest
[id=55ccb43e371-68bc7ffc-8b12-455c-9851-4762685df930, reqs=SingletonList
[ServiceUndeploymentRequest []]], affTopVer=null, super=DiscoveryEvent
[evtNod
[jira] [Commented] (IGNITE-5038) BinaryMarshaller might need to use context class loader for deserialization
[ https://issues.apache.org/jira/browse/IGNITE-5038?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17176394#comment-17176394 ] Ivan Rakov commented on IGNITE-5038: [~agoncharuk] [~gvvinblade] [~v.pyatkov] Guys, please take a look at new PR: https://github.com/apache/ignite/pull/8146 I've addressed Vlad's and Alex's comments in it. > Also, what happens if I call deserialize() and pass system class loader? Object instance will unmarshalled with system classloader, without using cache (with extra call of Class.forName). > We need to add tests to verify that user class loaders do not leak to the > static cache It's covered by BinaryClassLoaderMultiJvmTest, see BinaryClassLoaderMultiJvmTest#checkClassCacheEmpty. A bit tricky, but should work. I'll groom test code (will add test scenarios description and so on) if TC shows that current patch is viable. > BinaryMarshaller might need to use context class loader for deserialization > --- > > Key: IGNITE-5038 > URL: https://issues.apache.org/jira/browse/IGNITE-5038 > Project: Ignite > Issue Type: Improvement > Components: binary >Affects Versions: 2.0 >Reporter: Dmitry Karachentsev >Assignee: Mirza Aliev >Priority: Major > Labels: features > Attachments: results-compound-20170802.zip, > results-compound-20170808.zip > > Time Spent: 1.5h > Remaining Estimate: 0h > > There is a special use case discussed on the dev list: > http://apache-ignite-developers.2346864.n4.nabble.com/Re-BinaryObjectImpl-deserializeValue-with-specific-ClassLoader-td17126.html#a17224 > According to the use case, BinaryMarshaller might need to try to deserialize > an object using a context class loader if it failed to do so with a custom > classloader (`IgniteConfiguration.getClassLoader()`) or the system one. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (IGNITE-5038) BinaryMarshaller might need to use context class loader for deserialization
[
https://issues.apache.org/jira/browse/IGNITE-5038?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17173116#comment-17173116
]
Alexey Goncharuk commented on IGNITE-5038:
--
[~maliev], a few comments:
* Why is thread-local only set to one value? I think we should use try-finally
to always keep the current behavior.
* We need to add tests to verify that user class loaders do not leak to the
static cache. Also, what happens if I call {{deserialize()}} and pass system
class loader?
[~gvvinblade] can you take a look at the change as well?
> BinaryMarshaller might need to use context class loader for deserialization
> ---
>
> Key: IGNITE-5038
> URL: https://issues.apache.org/jira/browse/IGNITE-5038
> Project: Ignite
> Issue Type: Improvement
> Components: binary
>Affects Versions: 2.0
>Reporter: Dmitry Karachentsev
>Assignee: Mirza Aliev
>Priority: Major
> Labels: features
> Attachments: results-compound-20170802.zip,
> results-compound-20170808.zip
>
> Time Spent: 50m
> Remaining Estimate: 0h
>
> There is a special use case discussed on the dev list:
> http://apache-ignite-developers.2346864.n4.nabble.com/Re-BinaryObjectImpl-deserializeValue-with-specific-ClassLoader-td17126.html#a17224
> According to the use case, BinaryMarshaller might need to try to deserialize
> an object using a context class loader if it failed to do so with a custom
> classloader (`IgniteConfiguration.getClassLoader()`) or the system one.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (IGNITE-5038) BinaryMarshaller might need to use context class loader for deserialization
[ https://issues.apache.org/jira/browse/IGNITE-5038?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17172560#comment-17172560 ] Vladislav Pyatkov commented on IGNITE-5038: --- [~maliev] I left several comments in PR. > BinaryMarshaller might need to use context class loader for deserialization > --- > > Key: IGNITE-5038 > URL: https://issues.apache.org/jira/browse/IGNITE-5038 > Project: Ignite > Issue Type: Improvement > Components: binary >Affects Versions: 2.0 >Reporter: Dmitry Karachentsev >Assignee: Mirza Aliev >Priority: Major > Labels: features > Attachments: results-compound-20170802.zip, > results-compound-20170808.zip > > Time Spent: 50m > Remaining Estimate: 0h > > There is a special use case discussed on the dev list: > http://apache-ignite-developers.2346864.n4.nabble.com/Re-BinaryObjectImpl-deserializeValue-with-specific-ClassLoader-td17126.html#a17224 > According to the use case, BinaryMarshaller might need to try to deserialize > an object using a context class loader if it failed to do so with a custom > classloader (`IgniteConfiguration.getClassLoader()`) or the system one. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (IGNITE-5038) BinaryMarshaller might need to use context class loader for deserialization
[
https://issues.apache.org/jira/browse/IGNITE-5038?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16462911#comment-16462911
]
Vladimir Ozerov commented on IGNITE-5038:
-
[~daradurvs], while being pretty simple at the first glance, this ticket is
rather non-trivial. Main problem - we rely on {{Class}} caching a lot, and this
caching should be avoided carefully when using custom classloader. Current
implementation is not finished yet and is effectively on hold.
With new module system (aka Jigsaw) and drop of Java 7 support, I am not sure
that this fix makes sense in any environment except of Java 8. This is de-facto
standard for now, but with Java 10 release this may change. I am not sure we
should continue with this patch until we understand how it would work in Java
9/10.
> BinaryMarshaller might need to use context class loader for deserialization
> ---
>
> Key: IGNITE-5038
> URL: https://issues.apache.org/jira/browse/IGNITE-5038
> Project: Ignite
> Issue Type: Improvement
> Components: binary
>Affects Versions: 2.0
>Reporter: Dmitry Karachentsev
>Assignee: Vladimir Ozerov
>Priority: Major
> Labels: features
> Fix For: 2.6
>
> Attachments: results-compound-20170802.zip,
> results-compound-20170808.zip
>
>
> There is a special use case discussed on the dev list:
> http://apache-ignite-developers.2346864.n4.nabble.com/Re-BinaryObjectImpl-deserializeValue-with-specific-ClassLoader-td17126.html#a17224
> According to the use case, BinaryMarshaller might need to try to deserialize
> an object using a context class loader if it failed to do so with a custom
> classloader (`IgniteConfiguration.getClassLoader()`) or the system one.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (IGNITE-5038) BinaryMarshaller might need to use context class loader for deserialization
[
https://issues.apache.org/jira/browse/IGNITE-5038?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16457449#comment-16457449
]
Vyacheslav Daradur commented on IGNITE-5038:
Hi, [~vozerov], [~v.pyatkov],
I have a use case where classes descriptions are owned by different
classloaders.
I have to serialize objects myself and use {{IgniteCache}} via
{{withKeepBinary}} mode in this case.
Completion of this task will allow us to use Ignite's serialization via
efficient {{BinaryMarshaller}} and significantly improve performance.
As far as I can see you have prepared the solution, are you going to merge it?
> BinaryMarshaller might need to use context class loader for deserialization
> ---
>
> Key: IGNITE-5038
> URL: https://issues.apache.org/jira/browse/IGNITE-5038
> Project: Ignite
> Issue Type: Improvement
> Components: binary
>Affects Versions: 2.0
>Reporter: Dmitry Karachentsev
>Assignee: Vladimir Ozerov
>Priority: Major
> Labels: features
> Fix For: 2.6
>
> Attachments: results-compound-20170802.zip,
> results-compound-20170808.zip
>
>
> There is a special use case discussed on the dev list:
> http://apache-ignite-developers.2346864.n4.nabble.com/Re-BinaryObjectImpl-deserializeValue-with-specific-ClassLoader-td17126.html#a17224
> According to the use case, BinaryMarshaller might need to try to deserialize
> an object using a context class loader if it failed to do so with a custom
> classloader (`IgniteConfiguration.getClassLoader()`) or the system one.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (IGNITE-5038) BinaryMarshaller might need to use context class loader for deserialization
[ https://issues.apache.org/jira/browse/IGNITE-5038?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16409468#comment-16409468 ] Ryabov Dmitrii commented on IGNITE-5038: [~dpavlov], can you merge it? And change Xmx for *Cache [5]* suite and remove *150 Clients* suite from *run all* build? > BinaryMarshaller might need to use context class loader for deserialization > --- > > Key: IGNITE-5038 > URL: https://issues.apache.org/jira/browse/IGNITE-5038 > Project: Ignite > Issue Type: Improvement > Components: binary >Affects Versions: 2.0 >Reporter: Dmitry Karachentsev >Assignee: Vladimir Ozerov >Priority: Major > Labels: features > Fix For: 2.5 > > Attachments: results-compound-20170802.zip, > results-compound-20170808.zip > > > There is a special use case discussed on the dev list: > http://apache-ignite-developers.2346864.n4.nabble.com/Re-BinaryObjectImpl-deserializeValue-with-specific-ClassLoader-td17126.html#a17224 > According to the use case, BinaryMarshaller might need to try to deserialize > an object using a context class loader if it failed to do so with a custom > classloader (`IgniteConfiguration.getClassLoader()`) or the system one. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (IGNITE-5038) BinaryMarshaller might need to use context class loader for deserialization
[ https://issues.apache.org/jira/browse/IGNITE-5038?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16289129#comment-16289129 ] Ryabov Dmitrii commented on IGNITE-5038: [~v.pyatkov], [~vozerov] what is current status of ticket? Vladimir, what you mean by "problems with "static" context"? > BinaryMarshaller might need to use context class loader for deserialization > --- > > Key: IGNITE-5038 > URL: https://issues.apache.org/jira/browse/IGNITE-5038 > Project: Ignite > Issue Type: Improvement > Components: binary >Affects Versions: 2.0 >Reporter: Dmitry Karachentsev >Assignee: Vladimir Ozerov > Labels: features > Attachments: results-compound-20170802.zip, > results-compound-20170808.zip > > > There is a special use case discussed on the dev list: > http://apache-ignite-developers.2346864.n4.nabble.com/Re-BinaryObjectImpl-deserializeValue-with-specific-ClassLoader-td17126.html#a17224 > According to the use case, BinaryMarshaller might need to try to deserialize > an object using a context class loader if it failed to do so with a custom > classloader (`IgniteConfiguration.getClassLoader()`) or the system one. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (IGNITE-5038) BinaryMarshaller might need to use context class loader for deserialization
[
https://issues.apache.org/jira/browse/IGNITE-5038?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16150380#comment-16150380
]
Vladislav Pyatkov commented on IGNITE-5038:
---
[~vozerov] Then you for detailed review my changes.
I have had accepted your change and too your point of view.
Please look at my final PR again:
https://github.com/apache/ignite/pull/2011/commits
also I passed the patch in TC, look at the run over {{pull/2011/head}}.
> BinaryMarshaller might need to use context class loader for deserialization
> ---
>
> Key: IGNITE-5038
> URL: https://issues.apache.org/jira/browse/IGNITE-5038
> Project: Ignite
> Issue Type: Improvement
> Components: binary
>Affects Versions: 2.0
>Reporter: Dmitry Karachentsev
>Assignee: Vladislav Pyatkov
> Labels: features
> Fix For: 2.3
>
> Attachments: results-compound-20170802.zip,
> results-compound-20170808.zip
>
>
> There is a special use case discussed on the dev list:
> http://apache-ignite-developers.2346864.n4.nabble.com/Re-BinaryObjectImpl-deserializeValue-with-specific-ClassLoader-td17126.html#a17224
> According to the use case, BinaryMarshaller might need to try to deserialize
> an object using a context class loader if it failed to do so with a custom
> classloader (`IgniteConfiguration.getClassLoader()`) or the system one.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (IGNITE-5038) BinaryMarshaller might need to use context class loader for deserialization
[
https://issues.apache.org/jira/browse/IGNITE-5038?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16138113#comment-16138113
]
Vladimir Ozerov commented on IGNITE-5038:
-
[~v.pyatkov], my comments:
1) {{IgniteUtils.forName}} - {{noCache=true}} is ignored if passed classloader
is null
2) {{GridBinaryMarshaller.unmarshal(..., ClassLoader)}} - why {{true}}? Looks
like it should depend on passed classloader
As a whole I am very concerned of passing {{true}} flag all over the code. Why
do we need it? As I understand, in order to understand whether cache should be
used or not, we only need node's classloader and current classloader. Node's
classloader is always somewhere near. Let's try to get rid of {{useCache}} as
much as possible to minimize chance of error. We need clear invariant here.
> BinaryMarshaller might need to use context class loader for deserialization
> ---
>
> Key: IGNITE-5038
> URL: https://issues.apache.org/jira/browse/IGNITE-5038
> Project: Ignite
> Issue Type: New Feature
> Components: cache
>Affects Versions: 2.0
>Reporter: Dmitry Karachentsev
>Assignee: Vladislav Pyatkov
> Labels: features
> Fix For: 2.2
>
> Attachments: results-compound-20170802.zip,
> results-compound-20170808.zip
>
>
> There is a special use case discussed on the dev list:
> http://apache-ignite-developers.2346864.n4.nabble.com/Re-BinaryObjectImpl-deserializeValue-with-specific-ClassLoader-td17126.html#a17224
> According to the use case, BinaryMarshaller might need to try to deserialize
> an object using a context class loader if it failed to do so with a custom
> classloader (`IgniteConfiguration.getClassLoader()`) or the system one.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (IGNITE-5038) BinaryMarshaller might need to use context class loader for deserialization
[ https://issues.apache.org/jira/browse/IGNITE-5038?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16118232#comment-16118232 ] Vladislav Pyatkov commented on IGNITE-5038: --- [~vozerov] Than you for the review. I have applayed your comment and rerun benchmark [^results-compound-20170808.zip]. Could you please, review patch again? > BinaryMarshaller might need to use context class loader for deserialization > --- > > Key: IGNITE-5038 > URL: https://issues.apache.org/jira/browse/IGNITE-5038 > Project: Ignite > Issue Type: New Feature > Components: cache >Affects Versions: 2.0 >Reporter: Dmitry Karachentsev >Assignee: Vladislav Pyatkov > Labels: features > Fix For: 2.2 > > Attachments: results-compound-20170802.zip, > results-compound-20170808.zip > > > There is a special use case discussed on the dev list: > http://apache-ignite-developers.2346864.n4.nabble.com/Re-BinaryObjectImpl-deserializeValue-with-specific-ClassLoader-td17126.html#a17224 > According to the use case, BinaryMarshaller might need to try to deserialize > an object using a context class loader if it failed to do so with a custom > classloader (`IgniteConfiguration.getClassLoader()`) or the system one. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (IGNITE-5038) BinaryMarshaller might need to use context class loader for deserialization
[
https://issues.apache.org/jira/browse/IGNITE-5038?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16116521#comment-16116521
]
Vladimir Ozerov commented on IGNITE-5038:
-
[~v.pyatkov], I understand the solution in general, but there are several
comments from my side at the moment:
1) I am strongly against any {{ClassLoader}} and/or {{Class}} caches.
{{IgniteUtils.weakCache}} must go away. We had too many problems with such
caches before. So just don't.
2) {{IgniteUtils}} doesn't compile on my machine. Are sure you have Java 7
compilation level set in IDE?
3) {{OptimziedObjectStreamRegistry.in}} - if {{useCache == false}}, then we
cannot use {{inPool}}, are we?
> BinaryMarshaller might need to use context class loader for deserialization
> ---
>
> Key: IGNITE-5038
> URL: https://issues.apache.org/jira/browse/IGNITE-5038
> Project: Ignite
> Issue Type: New Feature
> Components: cache
>Affects Versions: 2.0
>Reporter: Dmitry Karachentsev
>Assignee: Vladislav Pyatkov
> Labels: features
> Fix For: 2.2
>
> Attachments: results-compound-20170802.zip
>
>
> There is a special use case discussed on the dev list:
> http://apache-ignite-developers.2346864.n4.nabble.com/Re-BinaryObjectImpl-deserializeValue-with-specific-ClassLoader-td17126.html#a17224
> According to the use case, BinaryMarshaller might need to try to deserialize
> an object using a context class loader if it failed to do so with a custom
> classloader (`IgniteConfiguration.getClassLoader()`) or the system one.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (IGNITE-5038) BinaryMarshaller might need to use context class loader for deserialization
[ https://issues.apache.org/jira/browse/IGNITE-5038?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16110513#comment-16110513 ] Vladislav Pyatkov commented on IGNITE-5038: --- I had prepared a benchmark which demonstrate a performance with custom class loader. Look at the result: [~results-compound-20170802.zip] PR with the benchmark source here: https://github.com/apache/ignite/pull/2375. > BinaryMarshaller might need to use context class loader for deserialization > --- > > Key: IGNITE-5038 > URL: https://issues.apache.org/jira/browse/IGNITE-5038 > Project: Ignite > Issue Type: New Feature > Components: cache >Affects Versions: 2.0 >Reporter: Dmitry Karachentsev >Assignee: Vladislav Pyatkov > Labels: features > Fix For: 2.2 > > Attachments: results-compound-20170802.zip > > > There is a special use case discussed on the dev list: > http://apache-ignite-developers.2346864.n4.nabble.com/Re-BinaryObjectImpl-deserializeValue-with-specific-ClassLoader-td17126.html#a17224 > According to the use case, BinaryMarshaller might need to try to deserialize > an object using a context class loader if it failed to do so with a custom > classloader (`IgniteConfiguration.getClassLoader()`) or the system one. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (IGNITE-5038) BinaryMarshaller might need to use context class loader for deserialization
[
https://issues.apache.org/jira/browse/IGNITE-5038?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16068357#comment-16068357
]
Vladislav Pyatkov commented on IGNITE-5038:
---
[~sboikov],
Thank for review and comment.
I have make new observe and understood what you are wary.
In all cases when class loader was used on BinaryReader, I am checking
{{useCache}}, that should by solve issue with caching fields classes of cache
object.
I have added throw through whole marshallers {{useCache}} flag, because Binary
in some case can use Optimizer and Jdk through it.
> BinaryMarshaller might need to use context class loader for deserialization
> ---
>
> Key: IGNITE-5038
> URL: https://issues.apache.org/jira/browse/IGNITE-5038
> Project: Ignite
> Issue Type: New Feature
> Components: cache
>Affects Versions: 2.0
>Reporter: Dmitry Karachentsev
>Assignee: Vladislav Pyatkov
> Labels: features
> Fix For: 2.1
>
>
> There is a special use case discussed on the dev list:
> http://apache-ignite-developers.2346864.n4.nabble.com/Re-BinaryObjectImpl-deserializeValue-with-specific-ClassLoader-td17126.html#a17224
> According to the use case, BinaryMarshaller might need to try to deserialize
> an object using a context class loader if it failed to do so with a custom
> classloader (`IgniteConfiguration.getClassLoader()`) or the system one.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (IGNITE-5038) BinaryMarshaller might need to use context class loader for deserialization
[ https://issues.apache.org/jira/browse/IGNITE-5038?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16042952#comment-16042952 ] Vladislav Pyatkov commented on IGNITE-5038: --- [~vozerov] I have added some changes and think that will not leaking already. Please, view latest commit into the PR. > BinaryMarshaller might need to use context class loader for deserialization > --- > > Key: IGNITE-5038 > URL: https://issues.apache.org/jira/browse/IGNITE-5038 > Project: Ignite > Issue Type: New Feature > Components: cache >Affects Versions: 2.0 >Reporter: Dmitry Karachentsev >Assignee: Vladislav Pyatkov > Labels: features > Fix For: 2.1 > > > There is a special use case discussed on the dev list: > http://apache-ignite-developers.2346864.n4.nabble.com/Re-BinaryObjectImpl-deserializeValue-with-specific-ClassLoader-td17126.html#a17224 > According to the use case, BinaryMarshaller might need to try to deserialize > an object using a context class loader if it failed to do so with a custom > classloader (`IgniteConfiguration.getClassLoader()`) or the system one. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (IGNITE-5038) BinaryMarshaller might need to use context class loader for deserialization
[
https://issues.apache.org/jira/browse/IGNITE-5038?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16033259#comment-16033259
]
Nick Pordash commented on IGNITE-5038:
--
[~v.pyatkov] Thanks for the feedback, I am not sure what approach makes the
most sense at this time. I'm exploring a few options and will take it back to
the dev list.
[~vozerov] Agree with your #2 comment; however, caching of the deserialized
object can potentially be an issue even in the {{deserialize()}} case. I ran
into this yesterday since I have a service that creates a cache with
{{OnheapCacheEnabled}} and uses domain classes that are provided by a
{{ClassLoader}} created specifically for the service (since the services in
this case support dynamic class deployment). When the service gets reloaded -
and therefore we get a new {{ClassLoader}} - then {{deserialize()}} always
fails. I've had to disable {{OnheapCacheEnabled}} which isn't a big deal at the
moment, but it makes me wonder if there should be a way to configure the cache
so that {{deserialize()}} won't cache the result. IMO {{OnheapCacheEnabled}}
would still be useful in this case as it would avoid unnecessary allocations
copying from off-heap to on-heap with each read. I can take this to the mailing
list for further discussion.
> BinaryMarshaller might need to use context class loader for deserialization
> ---
>
> Key: IGNITE-5038
> URL: https://issues.apache.org/jira/browse/IGNITE-5038
> Project: Ignite
> Issue Type: New Feature
> Components: cache
>Affects Versions: 2.0
>Reporter: Dmitry Karachentsev
>Assignee: Vladislav Pyatkov
> Labels: features
> Fix For: 2.1
>
>
> There is a special use case discussed on the dev list:
> http://apache-ignite-developers.2346864.n4.nabble.com/Re-BinaryObjectImpl-deserializeValue-with-specific-ClassLoader-td17126.html#a17224
> According to the use case, BinaryMarshaller might need to try to deserialize
> an object using a context class loader if it failed to do so with a custom
> classloader (`IgniteConfiguration.getClassLoader()`) or the system one.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (IGNITE-5038) BinaryMarshaller might need to use context class loader for deserialization
[ https://issues.apache.org/jira/browse/IGNITE-5038?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16033062#comment-16033062 ] Vladislav Pyatkov commented on IGNITE-5038: --- [~vozerov] I have make a change. Could you please review it again? > BinaryMarshaller might need to use context class loader for deserialization > --- > > Key: IGNITE-5038 > URL: https://issues.apache.org/jira/browse/IGNITE-5038 > Project: Ignite > Issue Type: New Feature > Components: cache >Affects Versions: 2.0 >Reporter: Dmitry Karachentsev >Assignee: Vladislav Pyatkov > Labels: features > Fix For: 2.1 > > > There is a special use case discussed on the dev list: > http://apache-ignite-developers.2346864.n4.nabble.com/Re-BinaryObjectImpl-deserializeValue-with-specific-ClassLoader-td17126.html#a17224 > According to the use case, BinaryMarshaller might need to try to deserialize > an object using a context class loader if it failed to do so with a custom > classloader (`IgniteConfiguration.getClassLoader()`) or the system one. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (IGNITE-5038) BinaryMarshaller might need to use context class loader for deserialization
[
https://issues.apache.org/jira/browse/IGNITE-5038?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16032671#comment-16032671
]
Vladislav Pyatkov commented on IGNITE-5038:
---
[~npordash] I think you wan something so API to attache custom class loader to
cache instance like:
{code}
IgniteCache withClassLoader(ClassLoader ldr);
{code}
but I don't sure that is are you want.
It will be better if you are create a new issue and offer specific API through
developer's mail list.
[~vozerov] Thank to the review.
I apply your comment nearest time.
> BinaryMarshaller might need to use context class loader for deserialization
> ---
>
> Key: IGNITE-5038
> URL: https://issues.apache.org/jira/browse/IGNITE-5038
> Project: Ignite
> Issue Type: New Feature
> Components: cache
>Affects Versions: 2.0
>Reporter: Dmitry Karachentsev
>Assignee: Vladislav Pyatkov
> Labels: features
> Fix For: 2.1
>
>
> There is a special use case discussed on the dev list:
> http://apache-ignite-developers.2346864.n4.nabble.com/Re-BinaryObjectImpl-deserializeValue-with-specific-ClassLoader-td17126.html#a17224
> According to the use case, BinaryMarshaller might need to try to deserialize
> an object using a context class loader if it failed to do so with a custom
> classloader (`IgniteConfiguration.getClassLoader()`) or the system one.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (IGNITE-5038) BinaryMarshaller might need to use context class loader for deserialization
[ https://issues.apache.org/jira/browse/IGNITE-5038?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16029998#comment-16029998 ] Nick Pordash commented on IGNITE-5038: -- Thanks [~v.pyatkov]! There are really two changes that can be deduced from the dev list discussion: 1) What your PR currently contains 2) Defaulting to Thread.currentThread().getContextClassLoader() in the no-args deserialize methods and if that is null then fallback to the classloader in IgniteConfiguration #1 gives fine-grained control over what classloader is used, which is good since I don't have to fallback on internal Ignite APIs and also the deserialized value is properly cached by BinaryObjectImpl. #2 is about having less surprising behavior when it comes to automatically determining which classloader to use. Without this I still have to work with caches in binary which means if I want to provide a better development experience (f.e. allow working with POJOs) I need to expose custom Ignite and IgniteCache implementations in my APIs so that I can do this classloader detection myself. I'm not sure if this should really be two separate issues/PRs or not or if more discussion is needed around #2. > BinaryMarshaller might need to use context class loader for deserialization > --- > > Key: IGNITE-5038 > URL: https://issues.apache.org/jira/browse/IGNITE-5038 > Project: Ignite > Issue Type: New Feature > Components: cache >Affects Versions: 2.0 >Reporter: Dmitry Karachentsev >Assignee: Vladislav Pyatkov > Labels: features > Fix For: 2.1 > > > There is a special use case discussed on the dev list: > http://apache-ignite-developers.2346864.n4.nabble.com/Re-BinaryObjectImpl-deserializeValue-with-specific-ClassLoader-td17126.html#a17224 > According to the use case, BinaryMarshaller might need to try to deserialize > an object using a context class loader if it failed to do so with a custom > classloader (`IgniteConfiguration.getClassLoader()`) or the system one. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (IGNITE-5038) BinaryMarshaller might need to use context class loader for deserialization
[
https://issues.apache.org/jira/browse/IGNITE-5038?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16026033#comment-16026033
]
Vladislav Pyatkov commented on IGNITE-5038:
---
I heave add ability to customize class loader when deserialize object from
binary.
Added method {{BinaryObject#deserialize(java.lang.ClassLoader)}}.
Lock at my PR:
https://github.com/apache/ignite/pull/2011
Now the fix is testing on TC
> BinaryMarshaller might need to use context class loader for deserialization
> ---
>
> Key: IGNITE-5038
> URL: https://issues.apache.org/jira/browse/IGNITE-5038
> Project: Ignite
> Issue Type: New Feature
> Components: cache
>Affects Versions: 2.0
>Reporter: Dmitry Karachentsev
>Assignee: Vladislav Pyatkov
> Labels: features
> Fix For: 2.1
>
>
> There is a special use case discussed on the dev list:
> http://apache-ignite-developers.2346864.n4.nabble.com/Re-BinaryObjectImpl-deserializeValue-with-specific-ClassLoader-td17126.html#a17224
> According to the use case, BinaryMarshaller might need to try to deserialize
> an object using a context class loader if it failed to do so with a custom
> classloader (`IgniteConfiguration.getClassLoader()`) or the system one.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
