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 Fri, 01 Jun 2018 13:20:42 GMT
Dmitriy,

In that case there will be no chances to listen only tx creation events
without slowing down the system on other tx events creation and filtering.
All events are processed at same thread where tx changes the state, so, we
have to have the way to decrease potential slowdown.

I made it similar to
 public static final int[] EVTS_CACHE = {
        EVT_CACHE_ENTRY_CREATED,
        EVT_CACHE_ENTRY_DESTROYED,
        EVT_CACHE_OBJECT_PUT,
        EVT_CACHE_OBJECT_READ,
        EVT_CACHE_OBJECT_REMOVED,
        EVT_CACHE_OBJECT_LOCKED,
        EVT_CACHE_OBJECT_UNLOCKED,
        EVT_CACHE_OBJECT_EXPIRED
    };


чт, 31 мая 2018 г. в 20:48, Dmitriy Setrakyan <dsetrakyan@apache.org>:

> Anton,
>
> Why not just have one transaction event: EVT_TX_STATE_CHANGED?
>
> D.
>
> On Thu, May 31, 2018 at 9:10 AM, Anton Vinogradov <av@apache.org> wrote:
>
> > Dmitriy,
> >
> > Thanks for your comments!
> >
> > I've updated design to have
> >
> > public class TransactionStateChangedEvent extends EventAdapter {
> >     private Transaction tx;
> > }
> >
> > also I specified following set of possible events
> >
> > public static final int[] EVTS_TX = {
> > EVT_TX_STARTED,
> > EVT_TX_COMMITTED,
> > EVT_TX_ROLLED_BACK,
> > EVT_TX_SUSPENDED,
> > EVT_TX_RESUMED
> > };
> >
> > It contains most of reasonable tx states changes.
> > Additional events can be added later if necessary.
> >
> >
> > Tx label() available only at EVT_TX_STARTED because it is not propagated
> to
> > remote nodes, but
> >
> > - xid()
> > - nodeId()
> > - threadId()
> > - startTime()
> > - isolation()
> > - concurrency()
> > - implicit()
> > - isInvalidate()
> > - state()
> > - timeout()
> >
> > now available at any tx state change event.
> >
> >
> > As usual, full code listing available at
> > https://github.com/apache/ignite/pull/4036/files
> >
> >
> > вт, 29 мая 2018 г. в 20:41, Dmitriy Setrakyan <dsetrakyan@apache.org>:
> >
> > > Anton,
> > >
> > > We cannot have TransactionStartedEvent without having events for all
> > other
> > > transaction states, like TransactionPreparedEvent,
> > > TransactionCommittedEvent, etc. Considering this, I sill do not like
> the
> > > design, as we would have to create many extra event classes.
> > >
> > > Instead, I would suggest that you create TransactionStateChangeEvent,
> > which
> > > would have previous and new transaction state and would cover all state
> > > changes, not just the start of the transaction. This will make the
> design
> > > consistent and thorough.
> > >
> > > D.
> > >
> > > On Tue, May 29, 2018 at 5:39 AM, Anton Vinogradov <av@apache.org>
> wrote:
> > >
> > > > 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