Support for starting transaction in another thread IGNITE-4887

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

Support for starting transaction in another thread IGNITE-4887

voipp
Consider thread *Th1* started transaction *Tx1*, done some actions, and is
calling commit (GridNearTxLocal#commit -> commitNearTxLocalAsync). And
concurrently thread *Th2 *is calling the same commit on* Tx1*. Now, if you
look into code :

public IgniteInternalFuture<IgniteInternalTx> commitNearTxLocalAsync() {
    if (log.isDebugEnabled())
        log.debug("Committing near local tx: " + this);

    if (fastFinish()) {
        state(PREPARING);
        state(PREPARED);
        state(COMMITTING);

        cctx.tm().fastFinishTx(this, true);

        state(COMMITTED);

        return new GridFinishedFuture<>((IgniteInternalTx)this);
    }

    final IgniteInternalFuture<?> prepareFut = prepareNearTxLocal();//1

    GridNearTxFinishFuture fut = commitFut;

    if (fut == null &&
        !COMMIT_FUT_UPD.compareAndSet(this, null, fut = new
GridNearTxFinishFuture<>(cctx, this, true)))
        return commitFut;//2

    cctx.mvcc().addFuture(fut, fut.futureId());//3

Both threads would end up on //1 .Then.
*Th1 *would create *commitFut* in //2 and add it to mvcc //3
*Th2 *concurrently would evaluate //2 into true(it sees fut not null now)
and put fut into mvcc //3 which would lead to exception.

So, there is no locking sections (mutex) here

---------- Forwarded message ---------
From: Yakov Zhdanov <[hidden email]>
Date: пн, 3 июл. 2017 г. в 14:16
Subject: Re: Support for starting transaction in another thread IGNITE-4887
To: ALEKSEY KUZNETSOV <[hidden email]>


Alex, what do you mean by "multiple threads committing?". Transaction
should be committed only by one thread and only once.

Can you please provide a test or a code snippet to demonstrate the issue
you mentioned.


--Yakov




Im looking into current implementation of commitNearTxLocalAsync() , and in
case of multiple concurrent threads committing there would be exception
thrown by

cctx.mvcc().addFuture(fut, fut.futureId());

Is it correct behavior ?

--

*Best Regards,*

*Kuznetsov Aleksey*
Reply | Threaded
Open this post in threaded view
|

Re: Support for starting transaction in another thread IGNITE-4887

voipp
Lock is put by cache adapter. Now its clear

пн, 3 июл. 2017 г. в 15:14, ALEKSEY KUZNETSOV <[hidden email]>:

> Consider thread *Th1* started transaction *Tx1*, done some actions, and
> is calling commit (GridNearTxLocal#commit -> commitNearTxLocalAsync). And
> concurrently thread *Th2 *is calling the same commit on* Tx1*. Now, if
> you look into code :
>
> public IgniteInternalFuture<IgniteInternalTx> commitNearTxLocalAsync() {
>     if (log.isDebugEnabled())
>         log.debug("Committing near local tx: " + this);
>
>     if (fastFinish()) {
>         state(PREPARING);
>         state(PREPARED);
>         state(COMMITTING);
>
>         cctx.tm().fastFinishTx(this, true);
>
>         state(COMMITTED);
>
>         return new GridFinishedFuture<>((IgniteInternalTx)this);
>     }
>
>     final IgniteInternalFuture<?> prepareFut = prepareNearTxLocal();//1
>
>     GridNearTxFinishFuture fut = commitFut;
>
>     if (fut == null &&
>         !COMMIT_FUT_UPD.compareAndSet(this, null, fut = new GridNearTxFinishFuture<>(cctx, this, true)))
>         return commitFut;//2
>
>     cctx.mvcc().addFuture(fut, fut.futureId());//3
>
> Both threads would end up on //1 .Then.
> *Th1 *would create *commitFut* in //2 and add it to mvcc //3
> *Th2 *concurrently would evaluate //2 into true(it sees fut not null now)
> and put fut into mvcc //3 which would lead to exception.
>
> So, there is no locking sections (mutex) here
>
> ---------- Forwarded message ---------
> From: Yakov Zhdanov <[hidden email]>
> Date: пн, 3 июл. 2017 г. в 14:16
> Subject: Re: Support for starting transaction in another thread IGNITE-4887
> To: ALEKSEY KUZNETSOV <[hidden email]>
>
>
> Alex, what do you mean by "multiple threads committing?". Transaction
> should be committed only by one thread and only once.
>
> Can you please provide a test or a code snippet to demonstrate the issue
> you mentioned.
>
>
> --Yakov
>
>
>
>
> Im looking into current implementation of commitNearTxLocalAsync() , and
> in case of multiple concurrent threads committing there would be exception
> thrown by
>
> cctx.mvcc().addFuture(fut, fut.futureId());
>
> Is it correct behavior ?
>
> --
>
> *Best Regards,*
>
> *Kuznetsov Aleksey*
>
--

*Best Regards,*

*Kuznetsov Aleksey*
Reply | Threaded
Open this post in threaded view
|

Re: Support for starting transaction in another thread IGNITE-4887

yzhdanov
In reply to this post by voipp
>Consider thread *Th1* started transaction *Tx1*, done some actions, and is
>calling commit (GridNearTxLocal#commit -> commitNearTxLocalAsync). And
>concurrently thread *Th2 *is calling the same commit on* Tx1*.

Alexey, this looks weird to me. IMO, if we talk about proper implementation
you should detect all cases of illegal access (e.g. commit from thread not
owning the transaction).

--Yakov
Reply | Threaded
Open this post in threaded view
|

Re: Support for starting transaction in another thread IGNITE-4887

voipp
I've go a test, which illustrates transaction commit fail when multiple threads try to commit. As i said, it happenes in mvcc stage.

So we should create a special lock mechanism for all transaction methods. In a separate ticket.

пн, 3 июл. 2017 г. в 16:26, Yakov Zhdanov <[hidden email]>:
>Consider thread *Th1* started transaction *Tx1*, done some actions, and is
>calling commit (GridNearTxLocal#commit -> commitNearTxLocalAsync). And
>concurrently thread *Th2 *is calling the same commit on* Tx1*.

Alexey, this looks weird to me. IMO, if we talk about proper implementation
you should detect all cases of illegal access (e.g. commit from thread not
owning the transaction).

--Yakov
--

Best Regards,

Kuznetsov Aleksey

Reply | Threaded
Open this post in threaded view
|

Re: Support for starting transaction in another thread IGNITE-4887

yzhdanov
What separate ticket do you mean here? I think this is the scope of the
original one.

--Yakov

2017-07-03 16:39 GMT+03:00 ALEKSEY KUZNETSOV <[hidden email]>:

> I've go a test, which illustrates transaction commit fail when multiple
> threads try to commit. As i said, it happenes in mvcc stage.
>
> So we should create a special lock mechanism for all transaction methods.
> In a separate ticket.
>
> пн, 3 июл. 2017 г. в 16:26, Yakov Zhdanov <[hidden email]>:
>
>> >Consider thread *Th1* started transaction *Tx1*, done some actions, and
>> is
>> >calling commit (GridNearTxLocal#commit -> commitNearTxLocalAsync). And
>> >concurrently thread *Th2 *is calling the same commit on* Tx1*.
>>
>> Alexey, this looks weird to me. IMO, if we talk about proper
>> implementation
>> you should detect all cases of illegal access (e.g. commit from thread not
>> owning the transaction).
>>
>> --Yakov
>>
> --
>
> *Best Regards,*
>
> *Kuznetsov Aleksey*
>
Reply | Threaded
Open this post in threaded view
|

Re: Support for starting transaction in another thread IGNITE-4887

voipp
It is a global scope) Now multiple threads can commit transaction and it
will lead to exception. It doesn't refer to my ticket only, but for a
current transaction implementation.

пн, 3 июл. 2017 г. в 16:53, Yakov Zhdanov <[hidden email]>:

> What separate ticket do you mean here? I think this is the scope of the
> original one.
>
> --Yakov
>
> 2017-07-03 16:39 GMT+03:00 ALEKSEY KUZNETSOV <[hidden email]>:
>
> > I've go a test, which illustrates transaction commit fail when multiple
> > threads try to commit. As i said, it happenes in mvcc stage.
> >
> > So we should create a special lock mechanism for all transaction methods.
> > In a separate ticket.
> >
> > пн, 3 июл. 2017 г. в 16:26, Yakov Zhdanov <[hidden email]>:
> >
> >> >Consider thread *Th1* started transaction *Tx1*, done some actions, and
> >> is
> >> >calling commit (GridNearTxLocal#commit -> commitNearTxLocalAsync). And
> >> >concurrently thread *Th2 *is calling the same commit on* Tx1*.
> >>
> >> Alexey, this looks weird to me. IMO, if we talk about proper
> >> implementation
> >> you should detect all cases of illegal access (e.g. commit from thread
> not
> >> owning the transaction).
> >>
> >> --Yakov
> >>
> > --
> >
> > *Best Regards,*
> >
> > *Kuznetsov Aleksey*
> >
>
--

*Best Regards,*

*Kuznetsov Aleksey*
Reply | Threaded
Open this post in threaded view
|

Re: Support for starting transaction in another thread IGNITE-4887

yzhdanov
Disagree. Currently transaction is bound to a thread and if user tries to
pass it over to another thread this would be a contract violation.

Your ticket will make inter-thread transitions allowed.

--Yakov
Reply | Threaded
Open this post in threaded view
|

Re: Support for starting transaction in another thread IGNITE-4887

voipp
So i need to put new lock mechanism to* all* transaction operations ? If
so, don't we need additional ticket for this ?(for example sub ticket for
my original one)

пн, 3 июл. 2017 г. в 16:59, Yakov Zhdanov <[hidden email]>:

> Disagree. Currently transaction is bound to a thread and if user tries to
> pass it over to another thread this would be a contract violation.
>
> Your ticket will make inter-thread transitions allowed.
>
> --Yakov
>
--

*Best Regards,*

*Kuznetsov Aleksey*
Reply | Threaded
Open this post in threaded view
|

Re: Support for starting transaction in another thread IGNITE-4887

yzhdanov
Yes, feel free to file a sub-ticket.

--Yakov
Reply | Threaded
Open this post in threaded view
|

Re: Support for starting transaction in another thread IGNITE-4887

voipp
So, desired behavior is the following: when some thread evaluates tx
operation's secured scope, other threads should be locked and then released
with return after first thread has finished evaluating
secured scope ?

пн, 3 июл. 2017 г. в 17:12, Yakov Zhdanov <[hidden email]>:

> Yes, feel free to file a sub-ticket.
>
> --Yakov
>
--

*Best Regards,*

*Kuznetsov Aleksey*