ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Anton Vinogradov ...@apache.org>
Subject Re: Ability to check and completely fill transactions on creation
Date Tue, 29 May 2018 12:39:05 GMT
Dmitriy,
I fixed design according to your and Yakov's comments, thanks again for
clear explanation.

>> 1. You use internal API in public event, i.e. you cannot have user
>> accessing to IgniteInternalTx instance through TxEvent.

Event definition changed to
public class TransactionStartedEvent extends EventAdapter {
    private IgniteTransactions tx;
...
}

Not it's 100% public.

>> 2. Throwing runtime errors from listener is not documented and I doubt
if
>> it can be fully supported in the pattern you use in TxLabelTest. After
>> looking at the mentioned test user may think that throwing runtime error
>> when notified on new node join may prohibit new node joining which is
not
>> true. Do you have any example in Ignite when throwing exception from
>> listener is valid and documented.

Test's logic changed to
...
// Label
IgniteTransactions tx = evt.tx();

if (tx.label() == null)
tx.tx().rollback();
...
and
...
// Timeout
Transaction tx = evt.tx().tx();

if (tx.timeout() < 200)
tx.rollback();
...

So, tx will be rollbacked on creation and any commit attempt will cause
TransactionRollbackException

Full code listing available at
https://github.com/apache/ignite/pull/4036/files

Dmitriy, Yakov,
Could you please check and confirm changes?

чт, 24 мая 2018 г. в 16:32, Dmitriy Setrakyan <dsetrakyan@apache.org>:

> Anton, why do you need to *alter* event sub-system to introduce a new
> event? Yakov's issue was that you propagated private interface to public
> API, which is bad of course. Come up with a clean design and it will be
> accepted.
>
> My problem with TransactionValidator is that it only solves a small problem
> for transactions. If we do that, then we will have to add cache validators,
> compute validators, etc, etc, etc. That is why we either should use the
> existing event subsystem or come up with a holistic design that will work
> across the whole project.
>
> D.
>
> On Thu, May 24, 2018 at 1:38 AM, Anton Vinogradov <av@apache.org> wrote:
>
> > Dmitriy,
> >
> > Yakov is against the solution based on event sub-system
> > >> I think that we should think about some other solution instead of
> > altering
> > >> event sub-system.
> >
> > Also, I checked is there any chances to fix all the issues found by Yakov
> > and see that solution becomes overcomplicated in that case.
> > That's why I'm proposing this lightweight solution.
> >
> > As for me it's a good idea to have such validator since that's a common
> > problem at huge deployments when more than one team have access to Ignite
> > cluster and there is no other way to setup tx cretion rules.
> >
> > Yakov,
> >
> > Could you please share your thoughts on that?
> >
> >
> > чт, 24 мая 2018 г. в 8:58, Dmitriy Setrakyan <dsetrakyan@apache.org>:
> >
> > > On Wed, May 23, 2018 at 4:08 AM, Anton Vinogradov <av@apache.org>
> wrote:
> > >
> > > > Dmitriy, Yakov
> > > >
> > > > Are there any objections to updated design taking into account the
> > > comments
> > > > I provided?
> > > >
> > >
> > > Anton, I do not like an additional validator. I think you can
> accomplish
> > > the same with a transaction event. You just need to design it more
> > cleanly,
> > > incorporating the feedback from Yakov.
> > >
> >
>

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