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 Mon, 21 May 2018 15:14:46 GMT
Dmitriy,

Main idea is to restrict transaction creation in case label or timeout are
not set.
But there can be variations, for example label should fit some regexp or
timeout should be between 30 and 5000 ms.

That's not easy to restrict this at user code when you have dozens of
libraries written by different people in one system using one ignite
cluster.
That solution based on idea of TopologyValidator when you have no chances
to use cluster in case something wrong with nodes.
So, any client should have no chances to create transaction not suitable
for this ignite cluster.

пн, 21 мая 2018 г. в 17:48, Dmitriy Setrakyan <dsetrakyan@apache.org>:

> Anton,
>
> The change looks very questionable. We cannot be adding configuration
> validators for every piece of Ignite API. What is it you are trying to
> achieve?
>
> D.
>
> On Mon, May 21, 2018 at 7:22 AM, Anton Vinogradov <av@apache.org> wrote:
>
> > Yakov, thank's for deep check.
> >
> > >> I think that we should think about some other solution instead of
> > altering
> > >> event sub-system.
> >
> > Thank's to your comments now I see that solution is not perfect.
> >
> > How about to create
> >
> > interface TransactionsValidator {
> >    boolean validate(IgniteTransactions tx){
> >       ...
> >    }
> > }
> >
> > and add it's setter to IgniteConfiguration?
> >
> > icfg.setTransactionsValidator(new CustomTransactionsValidator(...));
> >
> > In that case we'll gain easy and proper solution allows to check
> > transaction configuration before real tx creation.
> >
> > It will be necessary to add some getters to IgniteTransactions:
> > - label()
> > - timeout()
> > - concurrency() (optional)
> > - isolation() (optional)
> > - txSize() (optional)
> >
> > Thoughts?
> >
> > пн, 21 мая 2018 г. в 16:31, Yakov Zhdanov <yzhdanov@apache.org>:
> >
> > > Ok, then it probably might have been created before this PR. Anyway, I
> > > would not bother too much about pt 3.
> > >
> > > --Yakov
> > >
> > > 2018-05-21 16:15 GMT+03:00 Dmitry Pavlov <dpavlov.spb@gmail.com>:
> > >
> > > > Hi Yakov,
> > > >
> > > > I've checked this code and IgniteCacheTestSuite6 includes
> TxLabelTest,
> > so
> > > > point 3 can be considered as solved.
> > > >
> > > > Sincerely,
> > > > Dmitriy Pavlov
> > > >
> > > > пн, 21 мая 2018 г. в 16:05, Yakov Zhdanov <yzhdanov@apache.org>:
> > > >
> > > > > Anton, I have objections. Please do not merge unless we agree on
> > > details.
> > > > >
> > > > > 1. You use internal API in public event, i.e. you cannot have user
> > > > > accessing to IgniteInternalTx instance through TxEvent.
> > > > > 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.
> > > > > 3. TxLabelTest is not included into any suite.
> > > > > 4. EVT_TX_STARTED - does not seems to be a proper and descriptive
> > name
> > > > >
> > > > > I think that we should think about some other solution instead of
> > > > altering
> > > > > event sub-system.
> > > > >
> > > > > Also I want to ask everyone in community to request explicit
> approval
> > > > from
> > > > > community members before changing anything in transactional logic.
> > > > >
> > > > > Thanks!
> > > > >
> > > > > --Yakov
> > > > >
> > > >
> > >
> >
>

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