Hi Ilya , of course it contains in my PR (i don`t know if it shout be published
before this discussion will be finished).
Little changes from single thread into multiple, for example here [1] will
highlight a problem, or i can just publish my PR.
[1]
https://github.com/apache/ignite/blob/master/modules/core/src/test/java/org/apache/ignite/internal/IgniteExplicitImplicitDeploymentSelfTest.java#L221
>
>>
>>>Hello!
>>>
>>>Do you have some kind of reproducer which demonstrates the issue?
>>>
>>>Regards,
>>>--
>>>Ilya Kasnacheev
>>>
>>>
>>>чт, 28 янв. 2021 г. в 10:32, Zhenya Stanilovsky < arzamas...@mail.ru.invalid
>>>>:
>>>
>>>>
>>>> Hello Igniters !
>>>> In the process of Ignite usage i found that some part of Compute
>>>> functionality are thread unsafe and seems was designed with such
>>>> limitations initially.
>>>> Example : one (client, but it doesn`t matter at all) instance is
>>>> shared between numerous of fabric, all of them calls something like :
>>>> IgniteCompute#execute(ComputeTask<T,R>, T)
>>>> or
>>>> IgniteCompute#execute(java.lang.Class<? extends ComputeTask<T,R>>, T)
>>>> and appropriate «async» methods — what kind of instance will be called is
>>>> nondeterministic for now and as a confirmation of my words — i found no
>>>> tests covered multi thread usage of Computing i also found nothing on
>>>> documentation page [1].
>>>> We have all necessary info for correct processing of such cases:
>>>> from initiator (ignite.compute(...) starter) side we have Class or it
>>>> instance and appropriate class loader which will be wired by class loader
>>>> id from execution side.
>>>> I create a fix and seems all work perfectly well besides one place, this
>>>> functionality :
>>>> /**
>>>> * Executes given task within the cluster group. For step-by-step
>>>> explanation of task execution process
>>>> * refer to {@link ComputeTask} documentation.
>>>> * <p>
>>>> * If task for given name has not been deployed yet, then {@code taskName}
>>>> will be
>>>> * used as task class name to auto-deploy the task (see {@link
>>>> #localDeployTask(Class, ClassLoader)} method).
>>>> */
>>>> public <T, R> R execute(String taskName, T arg) throws IgniteException;
>>>> and attendant
>>>> /**
>>>> * Finds class loader for the given class.
>>>> *
>>>> * @param rsrcName Class name or class alias to find class loader for.
>>>> * @return Deployed class loader, or {@code null} if not deployed.
>>>> */
>>>> public DeploymentResource findResource(String rsrcName);
>>>> is thread unsafe by default, no guarantee that concurrent call of
>>>> localDeployTask and execute will bring expected result.
>>>> My proposal is to deprecate (or probably annotate [2], as a minimal
>>>> — additionally document it) this methods and to append additional :
>>>> public DeploymentResource findResource(String rsrcName, ClassLoader
>>>> clsLdr);
>>>> Only one problem i can observe here, if someone creates new class loaders
>>>> and appropriate class instances in loop (i don`t know the purpose) and
>>>> doesn`t undeploy them then he will get possibly OOM here.
>>>>
>>>> Such approach will give a possibility to use compute in concurrent
>>>> scenario. If there is no objections here i will mark this methods and
>>>> publish my PR, of course with additional tests.
>>>>
>>>> What do you think ?
>>>>
>>>>
>>>> [1]
>>>> https://ignite.apache.org/docs/latest/code-deployment/peer-class-loading
>>>> [2]
>>>> https://jcip.net/annotations/doc/net/jcip/annotations/NotThreadSafe.html
>>>>
>>>>
>>
>>
>>
>>
