Dmitriy, Yakov
Are there any objections to updated design taking into account the comments
I provided?
пн, 21 мая 2018 г. в 18:49, Anton Vinogradov <a...@apache.org>:
> One more case is to analize and log tx's creators info without tx creation
> restriction.
> This is very important feature on huge system trial period when you may
> want to check who creates tx, tx content and configuration.
> For example you may want to log stacktrace for any tx without meta or with
> empty timeout.
> This will allow you to find a team responsible for that and make sure that
> they will fix their code.
>
> пн, 21 мая 2018 г. в 18:14, Anton Vinogradov <a...@apache.org>:
>
>> 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 <dsetrak...@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 <a...@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 <yzhda...@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....@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 <yzhda...@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
>>> > > > >
>>> > > >
>>> > >
>>> >
>>>
>>
