From dev-return-35028-archive-asf-public=cust-asf.ponee.io@ignite.apache.org Tue May 29 19:41:01 2018 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by mx-eu-01.ponee.io (Postfix) with SMTP id D99F0180648 for ; Tue, 29 May 2018 19:41:00 +0200 (CEST) Received: (qmail 59544 invoked by uid 500); 29 May 2018 17:40:59 -0000 Mailing-List: contact dev-help@ignite.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@ignite.apache.org Delivered-To: mailing list dev@ignite.apache.org Received: (qmail 59532 invoked by uid 99); 29 May 2018 17:40:59 -0000 Received: from mail-relay.apache.org (HELO mailrelay2-lw-us.apache.org) (207.244.88.137) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 29 May 2018 17:40:59 +0000 Received: from mail-oi0-f44.google.com (mail-oi0-f44.google.com [209.85.218.44]) by mailrelay2-lw-us.apache.org (ASF Mail Server at mailrelay2-lw-us.apache.org) with ESMTPSA id D5CFA2972 for ; Tue, 29 May 2018 17:40:58 +0000 (UTC) Received: by mail-oi0-f44.google.com with SMTP id l22-v6so6084687oib.4 for ; Tue, 29 May 2018 10:40:58 -0700 (PDT) X-Gm-Message-State: ALKqPwdduse1grpf3aZdJjgrMmbXwlVA18vWiUsy8tUCzbnIdRLYy9hf bwHbAc3f8uzamiCzahgaPypsPYakBc20PHKGbDQNyA== X-Google-Smtp-Source: ADUXVKJWHVndc83qGH9/K/p5DifqbNHelUL0+9QdcP9KFWByENjfVqir9iMROk/aK+htcQkgSlXXM11XojmSOO54kLE= X-Received: by 2002:aca:4344:: with SMTP id q65-v6mr104158oia.140.1527615657888; Tue, 29 May 2018 10:40:57 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:ac9:390a:0:0:0:0:0 with HTTP; Tue, 29 May 2018 10:40:17 -0700 (PDT) In-Reply-To: References: From: Dmitriy Setrakyan Date: Tue, 29 May 2018 10:40:17 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: Ability to check and completely fill transactions on creation To: dev Content-Type: multipart/alternative; boundary="000000000000a46b91056d5bbcf0" --000000000000a46b91056d5bbcf0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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 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 err= or > >> 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 =3D evt.tx(); > > if (tx.label() =3D=3D null) > tx.tx().rollback(); > ... > and > ... > // Timeout > Transaction tx =3D 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? > > =D1=87=D1=82, 24 =D0=BC=D0=B0=D1=8F 2018 =D0=B3. =D0=B2 16:32, Dmitriy Se= trakyan : > > > 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 publi= c > > 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 wo= rk > > across the whole project. > > > > D. > > > > On Thu, May 24, 2018 at 1:38 AM, Anton Vinogradov 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 comm= on > > > 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? > > > > > > > > > =D1=87=D1=82, 24 =D0=BC=D0=B0=D1=8F 2018 =D0=B3. =D0=B2 8:58, Dmitriy= Setrakyan : > > > > > > > On Wed, May 23, 2018 at 4:08 AM, Anton Vinogradov > > wrote: > > > > > > > > > Dmitriy, Yakov > > > > > > > > > > Are there any objections to updated design taking into account th= e > > > > 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. > > > > > > > > > > --000000000000a46b91056d5bbcf0--