ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Valentin Kulichenko <valentin.kuliche...@gmail.com>
Subject Re: Re[2]: [DISCUSSION] Request for thread unsafe Compute functionality deprecation.
Date Fri, 05 Feb 2021 22:00:47 GMT
Zhenya,

Could you clarify what you mean by "one instance is shared between numerous
of fabric"? What is the exact scenario and what are the implications of
running multiple compute tasks in parallel? A code sample demonstrating the
scenario might be helpful as well.

-Val

On Fri, Feb 5, 2021 at 12:58 PM Vladislav Pyatkov <vldpyatkov@gmail.com>
wrote:

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

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message