Hi Zhenya, I don't understand your proposal without a patch. I see that you want to mark as @Depricate three methods in IgniteCompute interface, but what will you give instead of?
It seems to me, this interface can invoke some job without contains a class locally. How will this be supported in your mind? On Thu, Jan 28, 2021 at 6:58 PM Ilya Kasnacheev <ilya.kasnach...@gmail.com> wrote: > 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 > > >>>> > > >>>> > > >> > > >> > > >> > > >> > -- Vladislav Pyatkov
