Hello! Please publish it. I don't see why not.
Regards, -- Ilya Kasnacheev чт, 28 янв. 2021 г. в 14:28, Zhenya Stanilovsky <arzamas...@mail.ru.invalid >: > > > 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 > >>>> > >>>> > >> > >> > >> > >>
