ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Николай Ижиков <nizhikov....@gmail.com>
Subject Re: .Net client call tx,close in other thread
Date Wed, 26 Jul 2017 14:06:08 GMT
Hello, Semyon.

Thank you for your clarification.
I removed check for threadId from my pull request.

2017-07-26 16:52 GMT+03:00 Semyon Boikov <sboikov@gridgain.com>:

> Hi,
>
> Currently tx is AutoCloseable and 'close' should be always called by
> interface contract.
>
> Currently Transaction is single-threaded and should not be used by multiple
> concurrent threads. But if you call commitAsync and then call 'close' from
> commitAsync listener this is correct usage and we should not prohibit
> calling 'close' from another thread (such assert if it was added should be
> removed).
>
> Thanks
> Semyon
>
> On Wed, Jul 26, 2017 at 12:48 PM, Anton Vinogradov <
> avinogradov@gridgain.com
> > wrote:
>
> > close() for Transaction is a method inherited from AutoCloseable.
> >
> > It used to rollback tx in case it was not rollbacked or commited inside
> try
> > section
> >
> > here's the code of close:
> >
> > @Override public void close() throws IgniteCheckedException {
> >         TransactionState state = state();
> >
> >        if (state != ROLLING_BACK && state != ROLLED_BACK && state
!=
> > COMMITTING && state != COMMITTED)
> >             rollback();
> > ...
> >
> > I see no reason to call close manually or at another thread.
> >
> > On Wed, Jul 26, 2017 at 12:41 PM, Pavel Tupitsyn <ptupitsyn@apache.org>
> > wrote:
> >
> > > We still need to confirm the correct behavior:
> > > 1) Should it be possible to call Transaction.close() from a different
> > > thread?
> > > 2) Do we need to call close() after commit()? What about commitAsync()?
> > > What if exception happens?
> > >
> > > Can someone clarify this?
> > >
> > > On Tue, Jul 25, 2017 at 11:08 PM, Nikolay Izhikov <
> > nizhikov.dev@gmail.com>
> > > wrote:
> > >
> > > > Pavel.
> > > >
> > > > You will fix .Net client so I don't need to change threadId checks in
> > my
> > > > pull request.
> > > > Is that correct?
> > > >
> > > > 25.07.2017 20:19, Pavel Tupitsyn пишет:
> > > >
> > > > Further investigation shows that platform code performs unnecessary
> > > close()
> > > >> calls for committed & rolled back transactions (sync and async).
> > > >> Ticket filed: https://issues.apache.org/jira/browse/IGNITE-5834
> > > >>
> > > >> I'll fix this tomorrow and it should resolve your issue.
> > > >>
> > > >> Thanks,
> > > >> Pavel
> > > >>
> > > >> On Tue, Jul 25, 2017 at 7:56 PM, Pavel Tupitsyn <
> ptupitsyn@apache.org
> > >
> > > >> wrote:
> > > >>
> > > >> Anyway, disallowing close() from another thread would be a breaking
> > > >>> change, we can't do that.
> > > >>>
> > > >>> On Tue, Jul 25, 2017 at 7:36 PM, Pavel Tupitsyn <
> > ptupitsyn@apache.org>
> > > >>> wrote:
> > > >>>
> > > >>> I've reproduced the issue, it happens because in .NET we auto-close
> > the
> > > >>>> transaction
> > > >>>> on commit/rollback. This involves two things:
> > > >>>> * release .NET reference to Java object
> > > >>>> * call Transaction.close()
> > > >>>>
> > > >>>> With sync methods this is trivial, but with
> > commitAsync/rollbackAsync
> > > we
> > > >>>> have to call close()
> > > >>>> in IgniteFuture listener, which is called in a different thread.
> > > >>>>
> > > >>>> I think we need to be able to close() a transaction from any
> thread.
> > > >>>> Otherwise I don't see a non-blocking way to deal with
> > > >>>> commitAsync/rollbackAsync.
> > > >>>>
> > > >>>> Thanks,
> > > >>>> Pavel
> > > >>>>
> > > >>>> On Tue, Jul 25, 2017 at 6:10 PM, Николай Ижиков
<
> > > nizhikov.dev@gmail.com
> > > >>>> >
> > > >>>> wrote:
> > > >>>>
> > > >>>> Hi, Pavel
> > > >>>>>
> > > >>>>> You can check my pull request.
> > > >>>>> https://github.com/apache/ignite/pull/2334
> > > >>>>>
> > > >>>>> For now I return checks so it has to fail on .Net test
suite
> > > >>>>>
> > > >>>>>
> > > >>>>> 2017-07-25 17:16 GMT+03:00 Pavel Tupitsyn <ptupitsyn@apache.org
> >:
> > > >>>>>
> > > >>>>> Hi Николай,
> > > >>>>>>
> > > >>>>>> .NET test in question (TestTxStateAndExceptions) does
not do any
> > > >>>>>> multithreading,
> > > >>>>>> everything is called from a single thread.
> > > >>>>>>
> > > >>>>>> I see that the latest .NET run of your pull request
on TeamCity
> > > >>>>>>
> > > >>>>> finished
> > > >>>>>
> > > >>>>>> successfully:
> > > >>>>>> http://ci.ignite.apache.org/viewLog.html?buildId=738277
> > > >>>>>>
> > > >>>>>> Do you still have a problem with this? If yes, how
can I
> reproduce
> > > it?
> > > >>>>>> Can you prepare a branch where the test fails, so
I can run it
> > > >>>>>> locally?
> > > >>>>>>
> > > >>>>>> Thanks,
> > > >>>>>> Pavel
> > > >>>>>>
> > > >>>>>> On Tue, Jul 25, 2017 at 4:47 PM, Николай Ижиков
<
> > > >>>>>>
> > > >>>>> nizhikov.dev@gmail.com>
> > > >>>>>
> > > >>>>>> wrote:
> > > >>>>>>
> > > >>>>>> Hello, Igniters.
> > > >>>>>>>
> > > >>>>>>> I working on issue https://issues.apache.org/
> > > jira/browse/IGNITE-5712
> > > >>>>>>> I found that .Net client perform transaction
> > > operation(`tx.close()`)
> > > >>>>>>>
> > > >>>>>> in
> > > >>>>>
> > > >>>>>> thread that not owns transaction.
> > > >>>>>>>
> > > >>>>>>> So I can't include checks for threadId in my pull
request.
> > > >>>>>>>
> > > >>>>>>> Is it a bug or a desirable behabiour?
> > > >>>>>>>
> > > >>>>>>> With my new check I got following stack trace:
> > > >>>>>>>
> > > >>>>>>> Check:
> > > >>>>>>>
> > > >>>>>>> `assert (threadId() == Thread.currentThread().getId());`
> > > >>>>>>>
> > > >>>>>>> Exception:
> > > >>>>>>>
> > > >>>>>>> Test(s) failed. System.AggregateException : One
or more errors
> > > >>>>>>>
> > > >>>>>> occurred.
> > > >>>>>
> > > >>>>>> ----> Apache.Ignite.Core.Common.IgniteException
: Java
> exception
> > > >>>>>>>
> > > >>>>>> occurred
> > > >>>>>>
> > > >>>>>>> [class=java.lang.AssertionError, message=] ---->
> > > >>>>>>> Apache.Ignite.Core.Common.JavaException :
> > java.lang.AssertionError
> > > >>>>>>>
> > > >>>>>> at
> > > >>>>>
> > > >>>>>> org.apache.ignite.internal.processors.cache.transactions.
> > > >>>>>>> TransactionProxyImpl.enter0(TransactionProxyImpl.java:113)
> > > >>>>>>> at
> > > >>>>>>> org.apache.ignite.internal.processors.cache.transactions.
> > > >>>>>>> TransactionProxyImpl.enter(TransactionProxyImpl.java:106)
> > > >>>>>>> at
> > > >>>>>>> org.apache.ignite.internal.processors.cache.transactions.
> > > >>>>>>> TransactionProxyImpl.close(TransactionProxyImpl.java:317)
> > > >>>>>>> at
> > > >>>>>>> org.apache.ignite.internal.processors.platform.transactions.
> > > >>>>>>> PlatformTransactions.txClose(PlatformTransactions.java:136)
> > > >>>>>>> at
> > > >>>>>>> org.apache.ignite.internal.processors.platform.transactions.
> > > >>>>>>> PlatformTransactions.processInLongOutLong(PlatformTransactio
> > > >>>>>>>
> > > >>>>>> ns.java:178)
> > > >>>>>
> > > >>>>>> at
> > > >>>>>>> org.apache.ignite.internal.processors.platform.PlatformTarge
> > > >>>>>>>
> > > >>>>>> tProxyImpl.
> > > >>>>>
> > > >>>>>> inLongOutLong(PlatformTargetProxyImpl.java:53)
> > > >>>>>>> at System.Threading.Tasks.Task.ThrowIfExceptional(Boolean
> > > >>>>>>> includeTaskCanceledExceptions) at
> System.Threading.Tasks.Task.Wa
> > > >>>>>>>
> > > >>>>>> it(Int32
> > > >>>>>
> > > >>>>>> millisecondsTimeout, CancellationToken cancellationToken)
at
> > > >>>>>>> System.Threading.Tasks.Task.Wait() at
> > > >>>>>>> Apache.Ignite.Core.Tests.Cache.CacheAbstractTransactionalTest
> > > >>>>>>> .TestTxStateAndExceptions()
> > > >>>>>>> in
> > > >>>>>>> c:\BuildAgent\work\820be461cd64b574\modules\
> > > platforms\dotnet\Apache.
> > > >>>>>>> Ignite.Core.Tests\Cache\CacheAbstractTransactionalTest.cs:line
> > > >>>>>>> 510 --IgniteException at
> > > >>>>>>> Apache.Ignite.Core.Impl.Unmanaged.UnmanagedCallbacks.
> Error(Void*
> > > >>>>>>>
> > > >>>>>> target,
> > > >>>>>
> > > >>>>>> Int32 errType, SByte* errClsChars, Int32 errClsCharsLen,
SByte*
> > > >>>>>>> errMsgChars, Int32 errMsgCharsLen, SByte* stackTraceChars,
> Int32
> > > >>>>>>> stackTraceCharsLen, Void* errData, Int32 errDataLen)
in
> > > >>>>>>> c:\BuildAgent\work\820be461cd64b574\modules\
> > > platforms\dotnet\Apache.
> > > >>>>>>> Ignite.Core\Impl\Unmanaged\UnmanagedCallbacks.cs:line
> > > >>>>>>> 1066 at
> > > >>>>>>> Apache.Ignite.Core.Impl.Unmanaged.IgniteJniNativeMethods.
> > > >>>>>>> TargetInLongOutLong(Void*
> > > >>>>>>> ctx, Void* target, Int32 opType, Int64 val) at
> > > >>>>>>> Apache.Ignite.Core.Impl.Unmanaged.UnmanagedUtils.TargetInLon
> > > >>>>>>>
> > > >>>>>> gOutLong(
> > > >>>>>
> > > >>>>>> IUnmanagedTarget
> > > >>>>>>> target, Int32 opType, Int64 memPtr) in
> > > >>>>>>> c:\BuildAgent\work\820be461cd64b574\modules\
> > > platforms\dotnet\Apache.
> > > >>>>>>> Ignite.Core\Impl\Unmanaged\UnmanagedUtils.cs:line
> > > >>>>>>> 429 at Apache.Ignite.Core.Impl.PlatformTarget.DoOutInOp(Int32
> > > type,
> > > >>>>>>>
> > > >>>>>> Int64
> > > >>>>>>
> > > >>>>>>> val) in
> > > >>>>>>> c:\BuildAgent\work\820be461cd64b574\modules\
> > > platforms\dotnet\Apache.
> > > >>>>>>> Ignite.Core\Impl\PlatformTarget.cs:line
> > > >>>>>>> 717 at
> > > >>>>>>> Apache.Ignite.Core.Impl.Transactions.TransactionsImpl.
> > > >>>>>>> TxClose(TransactionImpl
> > > >>>>>>> tx) in
> > > >>>>>>> c:\BuildAgent\work\820be461cd64b574\modules\
> > > platforms\dotnet\Apache.
> > > >>>>>>> Ignite.Core\Impl\Transactions\TransactionsImpl.cs:line
> > > >>>>>>> 216 at Apache.Ignite.Core.Impl.Transactions.TransactionImpl.
> > > Close()
> > > >>>>>>>
> > > >>>>>> in
> > > >>>>>
> > > >>>>>> c:\BuildAgent\work\820be461cd64b574\modules\
> > > platforms\dotnet\Apache.
> > > >>>>>>> Ignite.Core\Impl\Transactions\TransactionImpl.cs:line
> > > >>>>>>> 442 at
> > > >>>>>>> Apache.Ignite.Core.Impl.Transactions.TransactionImpl.<
> > > >>>>>>> CloseWhenComplete>b__d(Task
> > > >>>>>>> x) in
> > > >>>>>>> c:\BuildAgent\work\820be461cd64b574\modules\
> > > platforms\dotnet\Apache.
> > > >>>>>>> Ignite.Core\Impl\Transactions\TransactionImpl.cs:line
> > > >>>>>>> 460 at System.Threading.Tasks.Task.Execute() --JavaException
> > > -------
> > > >>>>>>> Stderr: ------- Test started:
> > > >>>>>>> CacheAbstractTransactionalTest.TestTxStateAndExceptions
Test
> > > >>>>>>>
> > > >>>>>> finished:
> > > >>>>>
> > > >>>>>> CacheAbstractTransactionalTest.TestTxStateAndExceptions
> > > >>>>>>> --
> > > >>>>>>> Nikolay Izhikov
> > > >>>>>>> NIzhikov.dev@gmail.com
> > > >>>>>>>
> > > >>>>>>>
> > > >>>>>>
> > > >>>>>
> > > >>>>>
> > > >>>>> --
> > > >>>>> Nikolay Izhikov
> > > >>>>> NIzhikov.dev@gmail.com
> > > >>>>>
> > > >>>>>
> > > >>>>
> > > >>>>
> > > >>>
> > > >>
> > >
> >
>



-- 
Nikolay Izhikov
NIzhikov.dev@gmail.com

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