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 <dsetrak...@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 <a...@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 <dsetrak...@apache.org>: > > > > > On Wed, May 23, 2018 at 4:08 AM, Anton Vinogradov <a...@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. > > > > > >