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 <sboi...@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 < > avinogra...@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 <ptupit...@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....@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 < > ptupit...@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 < > > ptupit...@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....@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 <ptupit...@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....@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....@gmail.com > > > >>>>>>> > > > >>>>>>> > > > >>>>>> > > > >>>>> > > > >>>>> > > > >>>>> -- > > > >>>>> Nikolay Izhikov > > > >>>>> nizhikov....@gmail.com > > > >>>>> > > > >>>>> > > > >>>> > > > >>>> > > > >>> > > > >> > > > > > > -- Nikolay Izhikov nizhikov....@gmail.com