Re: Proposal of new event QUERY_EXECUTION_EVENT
Юрий, can you take a look? JIRA - https://issues.apache.org/jira/browse/IGNITE-13450 PR - https://github.com/apache/ignite/pull/8252 вт, 15 сент. 2020 г. в 08:59, Dmitrii Ryabov : > > Ok, I created a ticket [1]. > > [1] https://issues.apache.org/jira/browse/IGNITE-13450 > > пн, 14 сент. 2020 г. в 14:59, Юрий : > > > > Dmitrii, seems you are right and we can go with new separate event > > > > пн, 7 сент. 2020 г. в 23:53, Dmitrii Ryabov : > > > > > Any objections to create a separate event, which will be fired before > > > executing a query? > > > > > > ср, 2 сент. 2020 г. в 22:33, Dmitrii Ryabov : > > > > > > > > I agree with Max, we need to add a separate event for starting query > > > > execution, and EVT_CACHE_QUERY_EXECUTED shouldn't be deprecated, > > > > because it is another case - it is fired when cache query was > > > > successfully finished. > > > > > > > > > Would the event notification be synchronous? In which thread? > > > > As Max said, synchronicity depends on implementation. As I see, we > > > > don't use a separate thread for any record calls. > > > > > > > > > What happens in case the event listener fails? > > > > Exceptions are logged by `U.error(...)` call. Errors are thrown out. > > > > > > > > > Should we discuss this within this topic? > > > > I suggest to separate adding a new event and configuring existing > > > > events. > > > > > > > > пн, 20 июл. 2020 г. в 14:37, Max Timonin : > > > > > > > > > > Looks like EVT_CACHE_QUERY_EXECUTED just works for different use > > > > > cases: > > > > > 1. it relates to a specific cache (Event for SQL queries looks > > > different as > > > > > it could contain multiple caches or none of them); > > > > > 2. Also the EVT_CACHE_QUERY_EXECUTED event fires multiple times for > > > > > distributed queries, see GridMapQueryExecutor class (For SQL query > > > > > it's > > > > > required to fire once independently on how many nodes are affected). > > > > > > > > > > So there are different patterns for events. I think > > > > > EVT_CACHE_QUERY_EXECUTED should not be deprecated or changed. > > > > > > > > > > > What happens in case the event listener fails? > > > > > > Would the event notification be synchronous? > > > > > It depends on how other events are implemented. As I see for the > > > > > EVT_CACHE_QUERY_EXECUTED event - it's synchronous, and listener errors > > > > > aren't handled. > > > > > > > > > > I think these questions are related to GridEventStorageManager as it > > > just > > > > > provides an API for recording events. Internal implementations (sync > > > > > / async, error handling) is not related to an event, is it? > > > > > > > > > > > I have some doubts about provide text of a query even with > > > > > hidden arguments, probably it should be configured due to it could > > > > > lead > > > > > to security leak > > > > > Currently event EVT_CACHE_QUERY_EXECUTED provides a sql clause without > > > > > limitations. If we're going to provide some restrictions it will > > > require > > > > > additional investigation. I see at least 2 configurations here: > > > > > 1. Ignite can be configured to hide clase, params only or nothing for > > > all > > > > > listeners; > > > > > 2. Only authorized listeners can subscribe to the event. > > > > > > > > > > Should we discuss this within this topic? > > > > > > > > > > On Mon, Jul 20, 2020 at 1:55 PM Юрий > > > wrote: > > > > > > > > > > > In my opinion existing events EVT_CACHE_QUERY_EXECUTION_STARTED > > > should be > > > > > > deprecated and added two new for start and for end of queries which > > > should > > > > > > cover all SQL query types. > > > > > > I have some doubts about provide text of a query even with hidden > > > > > > arguments, probably it should be configured due to it could lead to > > > > > > security leak > > > > > > > > > > > > пн, 20 июл. 2020 г.
Re: Proposal of new event QUERY_EXECUTION_EVENT
Ok, I created a ticket [1]. [1] https://issues.apache.org/jira/browse/IGNITE-13450 пн, 14 сент. 2020 г. в 14:59, Юрий : > > Dmitrii, seems you are right and we can go with new separate event > > пн, 7 сент. 2020 г. в 23:53, Dmitrii Ryabov : > > > Any objections to create a separate event, which will be fired before > > executing a query? > > > > ср, 2 сент. 2020 г. в 22:33, Dmitrii Ryabov : > > > > > > I agree with Max, we need to add a separate event for starting query > > > execution, and EVT_CACHE_QUERY_EXECUTED shouldn't be deprecated, > > > because it is another case - it is fired when cache query was > > > successfully finished. > > > > > > > Would the event notification be synchronous? In which thread? > > > As Max said, synchronicity depends on implementation. As I see, we > > > don't use a separate thread for any record calls. > > > > > > > What happens in case the event listener fails? > > > Exceptions are logged by `U.error(...)` call. Errors are thrown out. > > > > > > > Should we discuss this within this topic? > > > I suggest to separate adding a new event and configuring existing events. > > > > > > пн, 20 июл. 2020 г. в 14:37, Max Timonin : > > > > > > > > Looks like EVT_CACHE_QUERY_EXECUTED just works for different use cases: > > > > 1. it relates to a specific cache (Event for SQL queries looks > > different as > > > > it could contain multiple caches or none of them); > > > > 2. Also the EVT_CACHE_QUERY_EXECUTED event fires multiple times for > > > > distributed queries, see GridMapQueryExecutor class (For SQL query it's > > > > required to fire once independently on how many nodes are affected). > > > > > > > > So there are different patterns for events. I think > > > > EVT_CACHE_QUERY_EXECUTED should not be deprecated or changed. > > > > > > > > > What happens in case the event listener fails? > > > > > Would the event notification be synchronous? > > > > It depends on how other events are implemented. As I see for the > > > > EVT_CACHE_QUERY_EXECUTED event - it's synchronous, and listener errors > > > > aren't handled. > > > > > > > > I think these questions are related to GridEventStorageManager as it > > just > > > > provides an API for recording events. Internal implementations (sync > > > > / async, error handling) is not related to an event, is it? > > > > > > > > > I have some doubts about provide text of a query even with > > > > hidden arguments, probably it should be configured due to it could lead > > > > to security leak > > > > Currently event EVT_CACHE_QUERY_EXECUTED provides a sql clause without > > > > limitations. If we're going to provide some restrictions it will > > require > > > > additional investigation. I see at least 2 configurations here: > > > > 1. Ignite can be configured to hide clase, params only or nothing for > > all > > > > listeners; > > > > 2. Only authorized listeners can subscribe to the event. > > > > > > > > Should we discuss this within this topic? > > > > > > > > On Mon, Jul 20, 2020 at 1:55 PM Юрий > > wrote: > > > > > > > > > In my opinion existing events EVT_CACHE_QUERY_EXECUTION_STARTED > > should be > > > > > deprecated and added two new for start and for end of queries which > > should > > > > > cover all SQL query types. > > > > > I have some doubts about provide text of a query even with hidden > > > > > arguments, probably it should be configured due to it could lead to > > > > > security leak > > > > > > > > > > пн, 20 июл. 2020 г. в 12:49, Stanislav Lukyanov < > > stanlukya...@gmail.com>: > > > > > > > > > > > Maksim, > > > > > > > > > > > > Can we change the EVT_CACHE_QUERY_EXECUTED to fire earlier? Or > > should > > > > > > there be an EVT_CACHE_QUERY_EXECUTION_STARTED for the query start, > > while > > > > > > the old event would continue to work for query finish? > > > > > > I think the new event needs to either reuse the old one, or be its > > mirror > > > > > > for the query start. It also means that we probably should resolve > > the > >
Re: Proposal of new event QUERY_EXECUTION_EVENT
Any objections to create a separate event, which will be fired before executing a query? ср, 2 сент. 2020 г. в 22:33, Dmitrii Ryabov : > > I agree with Max, we need to add a separate event for starting query > execution, and EVT_CACHE_QUERY_EXECUTED shouldn't be deprecated, > because it is another case - it is fired when cache query was > successfully finished. > > > Would the event notification be synchronous? In which thread? > As Max said, synchronicity depends on implementation. As I see, we > don't use a separate thread for any record calls. > > > What happens in case the event listener fails? > Exceptions are logged by `U.error(...)` call. Errors are thrown out. > > > Should we discuss this within this topic? > I suggest to separate adding a new event and configuring existing events. > > пн, 20 июл. 2020 г. в 14:37, Max Timonin : > > > > Looks like EVT_CACHE_QUERY_EXECUTED just works for different use cases: > > 1. it relates to a specific cache (Event for SQL queries looks different as > > it could contain multiple caches or none of them); > > 2. Also the EVT_CACHE_QUERY_EXECUTED event fires multiple times for > > distributed queries, see GridMapQueryExecutor class (For SQL query it's > > required to fire once independently on how many nodes are affected). > > > > So there are different patterns for events. I think > > EVT_CACHE_QUERY_EXECUTED should not be deprecated or changed. > > > > > What happens in case the event listener fails? > > > Would the event notification be synchronous? > > It depends on how other events are implemented. As I see for the > > EVT_CACHE_QUERY_EXECUTED event - it's synchronous, and listener errors > > aren't handled. > > > > I think these questions are related to GridEventStorageManager as it just > > provides an API for recording events. Internal implementations (sync > > / async, error handling) is not related to an event, is it? > > > > > I have some doubts about provide text of a query even with > > hidden arguments, probably it should be configured due to it could lead > > to security leak > > Currently event EVT_CACHE_QUERY_EXECUTED provides a sql clause without > > limitations. If we're going to provide some restrictions it will require > > additional investigation. I see at least 2 configurations here: > > 1. Ignite can be configured to hide clase, params only or nothing for all > > listeners; > > 2. Only authorized listeners can subscribe to the event. > > > > Should we discuss this within this topic? > > > > On Mon, Jul 20, 2020 at 1:55 PM Юрий wrote: > > > > > In my opinion existing events EVT_CACHE_QUERY_EXECUTION_STARTED should be > > > deprecated and added two new for start and for end of queries which should > > > cover all SQL query types. > > > I have some doubts about provide text of a query even with hidden > > > arguments, probably it should be configured due to it could lead to > > > security leak > > > > > > пн, 20 июл. 2020 г. в 12:49, Stanislav Lukyanov : > > > > > > > Maksim, > > > > > > > > Can we change the EVT_CACHE_QUERY_EXECUTED to fire earlier? Or should > > > > there be an EVT_CACHE_QUERY_EXECUTION_STARTED for the query start, while > > > > the old event would continue to work for query finish? > > > > I think the new event needs to either reuse the old one, or be its > > > > mirror > > > > for the query start. It also means that we probably should resolve the > > > > issues you've listed. > > > > > > > > Would the event notification be synchronous? In which thread? > > > Asynchronous > > > > is generally preferred - would it work? > > > > > > > > What happens in case the event listener fails? > > > > > > > > Thanks, > > > > Stan > > > > > > > > > On 16 Jul 2020, at 18:49, Denis Magda wrote: > > > > > > > > > > Taras, Yury, Ivan, > > > > > > > > > > Could you please join this thread and share your thoughts? Do we > > > already > > > > > have any plans on tracking of the DDL and DML queries? > > > > > > > > > > - > > > > > Denis > > > > > > > > > > > > > > > On Wed, Jul 15, 2020 at 12:09 AM Max Timonin > > > > > wrote: > > > > > > > > > >> Hi Denis, thanks for the answer! > > > > &
Re: Proposal of new event QUERY_EXECUTION_EVENT
I agree with Max, we need to add a separate event for starting query execution, and EVT_CACHE_QUERY_EXECUTED shouldn't be deprecated, because it is another case - it is fired when cache query was successfully finished. > Would the event notification be synchronous? In which thread? As Max said, synchronicity depends on implementation. As I see, we don't use a separate thread for any record calls. > What happens in case the event listener fails? Exceptions are logged by `U.error(...)` call. Errors are thrown out. > Should we discuss this within this topic? I suggest to separate adding a new event and configuring existing events. пн, 20 июл. 2020 г. в 14:37, Max Timonin : > > Looks like EVT_CACHE_QUERY_EXECUTED just works for different use cases: > 1. it relates to a specific cache (Event for SQL queries looks different as > it could contain multiple caches or none of them); > 2. Also the EVT_CACHE_QUERY_EXECUTED event fires multiple times for > distributed queries, see GridMapQueryExecutor class (For SQL query it's > required to fire once independently on how many nodes are affected). > > So there are different patterns for events. I think > EVT_CACHE_QUERY_EXECUTED should not be deprecated or changed. > > > What happens in case the event listener fails? > > Would the event notification be synchronous? > It depends on how other events are implemented. As I see for the > EVT_CACHE_QUERY_EXECUTED event - it's synchronous, and listener errors > aren't handled. > > I think these questions are related to GridEventStorageManager as it just > provides an API for recording events. Internal implementations (sync > / async, error handling) is not related to an event, is it? > > > I have some doubts about provide text of a query even with > hidden arguments, probably it should be configured due to it could lead > to security leak > Currently event EVT_CACHE_QUERY_EXECUTED provides a sql clause without > limitations. If we're going to provide some restrictions it will require > additional investigation. I see at least 2 configurations here: > 1. Ignite can be configured to hide clase, params only or nothing for all > listeners; > 2. Only authorized listeners can subscribe to the event. > > Should we discuss this within this topic? > > On Mon, Jul 20, 2020 at 1:55 PM Юрий wrote: > > > In my opinion existing events EVT_CACHE_QUERY_EXECUTION_STARTED should be > > deprecated and added two new for start and for end of queries which should > > cover all SQL query types. > > I have some doubts about provide text of a query even with hidden > > arguments, probably it should be configured due to it could lead to > > security leak > > > > пн, 20 июл. 2020 г. в 12:49, Stanislav Lukyanov : > > > > > Maksim, > > > > > > Can we change the EVT_CACHE_QUERY_EXECUTED to fire earlier? Or should > > > there be an EVT_CACHE_QUERY_EXECUTION_STARTED for the query start, while > > > the old event would continue to work for query finish? > > > I think the new event needs to either reuse the old one, or be its mirror > > > for the query start. It also means that we probably should resolve the > > > issues you've listed. > > > > > > Would the event notification be synchronous? In which thread? > > Asynchronous > > > is generally preferred - would it work? > > > > > > What happens in case the event listener fails? > > > > > > Thanks, > > > Stan > > > > > > > On 16 Jul 2020, at 18:49, Denis Magda wrote: > > > > > > > > Taras, Yury, Ivan, > > > > > > > > Could you please join this thread and share your thoughts? Do we > > already > > > > have any plans on tracking of the DDL and DML queries? > > > > > > > > - > > > > Denis > > > > > > > > > > > > On Wed, Jul 15, 2020 at 12:09 AM Max Timonin > > > > wrote: > > > > > > > >> Hi Denis, thanks for the answer! > > > >> > > > >> We already checked EVT_CACHE_QUERY_EXECUTED and found that it works > > > only in > > > >> cases: > > > >> 1. Scan queries and Select queries (common pattern is access to cache > > > >> data); > > > >> 2. This event triggers only if query execution succeeds, in case of > > > failure > > > >> while execution this event won't fire. > > > >> > > > >> Our additional requirements are to protocol queries: > > > >> 1. that aren't cache related (for example, alter user); > > > >> 2. that relate to multiple caches (while EVT_CACHE_QUERY_EXECUTED have > > > >> field cacheName related to specific cache); > > > >> 3. we need to protocol also DDL and DML queries. > > > >> > > > >> Regards, > > > >> Maksim > > > >> > > > >> On Tue, Jul 14, 2020 at 10:20 PM Denis Magda > > wrote: > > > >> > > > >>> Hi Max, > > > >>> > > > >>> Could you check if the EVT_CACHE_QUERY_EXECUTED event is what you're > > > >>> looking for? > > > >>> > > > >>> > > > >> > > > > > https://www.gridgain.com/docs/latest/developers-guide/events/events#cache-query-events > > > >>> > > > >>> - > > > >>> Denis > > > >>> > > > >>> > > > >>> On Fri, Jul 10, 2020 at 3:54 AM Max Timonin > > > > > >>> wrote: > > > >>> > > >
Re: [DISCUSSION] Improvement in control.sh configuration.
Hello! Nikolay, yes, but currently, we can't declare user attributes through control.sh. пн, 16 мар. 2020 г., 12:06 Nikolay Izhikov : > Hello, Mikhail. > > Can you, please, double-check link to the ticket? > > For now it’s a IGNITE-12049 Add user attributes to thin clients which is > resolved. > > > 16 марта 2020 г., в 12:04, Mikhail Petrov > написал(а): > > > > Hello, Igniters. > > > > I'd like to propose a small improvement in control.sh configuration, > which is to add the ability to pass parameters needed for cluster > connection via a special properties file located in the same directory as > control.sh script. > > > > The connection parameters in the proposed properties file can be > considered as defaults and overwritten by those that are specified via the > command line (current approach). > > > > The proposal can help to avoid unnecessary repeating of cluster > connection configuration. Also, some connection parameters (e.g. user > attributes [1]) cannot be passed through the command line due to its > limited length. > > > > WDYT? > > > > [1] https://issues.apache.org/jira/browse/IGNITE-12049 > > > >
Re: Add user attributes to thin clients
Ok, if no one have objections, I will restrict a map by strings only.
чт, 23 янв. 2020 г., 17:20 Pavel Tupitsyn :
> > Well, my understanding was a binary object with compact footer = false
> is totally standalone entity and can be read without any external metadata
> Not exactly: type name and field names are unknown, you only have typeId,
> field ids and positions.
> There is one more Marshaller "mode": UNREGISTERED_TYPE_ID. In this mode,
> full type name is included in the binary object.
> However, field names are still missing and the class needs to be present to
> deserialize.
> That's why arbitrary objects should not be allowed in the handshake: in
> most cases they can't be decoded.
>
> On Thu, Jan 23, 2020 at 2:27 PM Dmitrii Ryabov
> wrote:
>
> > Alexei, yes, compactFooter is used only in 1 place.
> >
> > ```
> >
> > BinaryWriterExImpl.marshal0() {
> >
> > BinaryClassDescriptor desc = ctx.descriptorForClass(cls);
> >
> > // descriptor transportation fails here
> >
> > ...
> >
> > desc.write(obj, this); // compactFooter here
> >
> > }
> >
> > ```
> >
> > чт, 23 янв. 2020 г., 14:15 Pavel Tupitsyn :
> >
> > > > If we support only strings it will be necessary to encode binary
> values
> > > to
> > > > something like BASE64 which is not sounds good from usability side
> > >
> > > There should be no need to put binary values to attributes. What's the
> > use
> > > case?
> > >
> > > On Thu, Jan 23, 2020 at 2:08 PM Alexei Scherbakov <
> > > alexey.scherbak...@gmail.com> wrote:
> > >
> > > > > Footer is checked in postWrite - much later class descriptor check.
> > > >
> > > > Well, my understanding was a binary object with compact footer =
> false
> > is
> > > > totally standalone entity and can be read without any external
> > metadata.
> > > > Dmitrii Ryabov can you double check ?
> > > >
> > > > If we support only strings it will be necessary to encode binary
> values
> > > to
> > > > something like BASE64 which is not sounds good from usability side.
> > > >
> > > >
> > > >
> > > > чт, 23 янв. 2020 г. в 13:26, Nikita Amelchev :
> > > >
> > > > > +1 for the hardcoded String type only
> > > > >
> > > > > чт, 23 янв. 2020 г. в 13:15, Pavel Tupitsyn >:
> > > > > >
> > > > > > - Cross-platform binary objects are totally possible, all those
> > thin
> > > > > > clients support them.
> > > > > > - User attributes can be useful, no objections here
> > > > > >
> > > > > > However, I don't think we should allow arbitrary objects in user
> > > > > attributes.
> > > > > > Let's make them string only, much less to worry about.
> > > > > >
> > > > > > And using attributes for authentication still seems weird and
> > dirty.
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > On Thu, Jan 23, 2020 at 12:40 PM Dmitrii Ryabov <
> > > somefire...@gmail.com
> > > > >
> > > > > > wrote:
> > > > > >
> > > > > > > > Even if compact footer is disabled ?
> > > > > > > Footer is checked in postWrite - much later class descriptor
> > check.
> > > > > > >
> > > > > > > чт, 23 янв. 2020 г., 12:23 Alexei Scherbakov <
> > > > > alexey.scherbak...@gmail.com
> > > > > > > >:
> > > > > > >
> > > > > > > > чт, 23 янв. 2020 г. в 12:17, Dmitrii Ryabov <
> > > somefire...@gmail.com
> > > > >:
> > > > > > > >
> > > > > > > > > > The protocol must be language-agnostic. If we add some
> > > features
> > > > > > > there,
> > > > > > > > > let's make sure they are usable from anywhere.
> > > > > > > > >
> > > > > > > > > That's why I want to allow primitives only. Any language
> can
> > > send
> > > > > > > numbers
> > > > > > > > > and strings.
> > > > > > > > >
Re: Add user attributes to thin clients
Alexei, yes, compactFooter is used only in 1 place.
```
BinaryWriterExImpl.marshal0() {
BinaryClassDescriptor desc = ctx.descriptorForClass(cls);
// descriptor transportation fails here
...
desc.write(obj, this); // compactFooter here
}
```
чт, 23 янв. 2020 г., 14:15 Pavel Tupitsyn :
> > If we support only strings it will be necessary to encode binary values
> to
> > something like BASE64 which is not sounds good from usability side
>
> There should be no need to put binary values to attributes. What's the use
> case?
>
> On Thu, Jan 23, 2020 at 2:08 PM Alexei Scherbakov <
> alexey.scherbak...@gmail.com> wrote:
>
> > > Footer is checked in postWrite - much later class descriptor check.
> >
> > Well, my understanding was a binary object with compact footer = false is
> > totally standalone entity and can be read without any external metadata.
> > Dmitrii Ryabov can you double check ?
> >
> > If we support only strings it will be necessary to encode binary values
> to
> > something like BASE64 which is not sounds good from usability side.
> >
> >
> >
> > чт, 23 янв. 2020 г. в 13:26, Nikita Amelchev :
> >
> > > +1 for the hardcoded String type only
> > >
> > > чт, 23 янв. 2020 г. в 13:15, Pavel Tupitsyn :
> > > >
> > > > - Cross-platform binary objects are totally possible, all those thin
> > > > clients support them.
> > > > - User attributes can be useful, no objections here
> > > >
> > > > However, I don't think we should allow arbitrary objects in user
> > > attributes.
> > > > Let's make them string only, much less to worry about.
> > > >
> > > > And using attributes for authentication still seems weird and dirty.
> > > >
> > > >
> > > >
> > > >
> > > > On Thu, Jan 23, 2020 at 12:40 PM Dmitrii Ryabov <
> somefire...@gmail.com
> > >
> > > > wrote:
> > > >
> > > > > > Even if compact footer is disabled ?
> > > > > Footer is checked in postWrite - much later class descriptor check.
> > > > >
> > > > > чт, 23 янв. 2020 г., 12:23 Alexei Scherbakov <
> > > alexey.scherbak...@gmail.com
> > > > > >:
> > > > >
> > > > > > чт, 23 янв. 2020 г. в 12:17, Dmitrii Ryabov <
> somefire...@gmail.com
> > >:
> > > > > >
> > > > > > > > The protocol must be language-agnostic. If we add some
> features
> > > > > there,
> > > > > > > let's make sure they are usable from anywhere.
> > > > > > >
> > > > > > > That's why I want to allow primitives only. Any language can
> send
> > > > > numbers
> > > > > > > and strings.
> > > > > > >
> > > > > >
> > > > > > In general it's possible to have cross-platform complex data
> > > structures,
> > > > > > for example see protobuf.
> > > > > >
> > > > > >
> > > > > > >
> > > > > > > Binary marshaller, before packing object to byte[], will try to
> > use
> > > > > > > discovery processor and send message containing class
> descriptor.
> > > But
> > > > > > thin
> > > > > > > clients don't have discovery. Furthermore, if we write binary
> > > > > marshaller
> > > > > > > without class descriptor synchronization, we can get objects
> with
> > > > > > different
> > > > > > > class versions and corresponding exceptions.
> > > > > > >
> > > > > >
> > > > > > Even if compact footer is disabled ?
> > > > > >
> > > > > >
> > > > > > >
> > > > > > > But we can say users to declare their classes in
> > > > > > > META-INF/classnames.properties and current binary marshaller
> will
> > > works
> > > > > > > good.
> > > > > > >
> > > > > >
> > > > > > This approach doesn't looks like cross-platform.
> > > > > >
> > > > > >
> > > > > > >
> > > > > > > чт, 23 янв. 2020 г., 12:13 Alex Plehanov <
> > plehanov.a...@gmail.com
> > &
Re: Add user attributes to thin clients
> Even if compact footer is disabled ? Footer is checked in postWrite - much later class descriptor check. чт, 23 янв. 2020 г., 12:23 Alexei Scherbakov : > чт, 23 янв. 2020 г. в 12:17, Dmitrii Ryabov : > > > > The protocol must be language-agnostic. If we add some features there, > > let's make sure they are usable from anywhere. > > > > That's why I want to allow primitives only. Any language can send numbers > > and strings. > > > > In general it's possible to have cross-platform complex data structures, > for example see protobuf. > > > > > > Binary marshaller, before packing object to byte[], will try to use > > discovery processor and send message containing class descriptor. But > thin > > clients don't have discovery. Furthermore, if we write binary marshaller > > without class descriptor synchronization, we can get objects with > different > > class versions and corresponding exceptions. > > > > Even if compact footer is disabled ? > > > > > > But we can say users to declare their classes in > > META-INF/classnames.properties and current binary marshaller will works > > good. > > > > This approach doesn't looks like cross-platform. > > > > > > чт, 23 янв. 2020 г., 12:13 Alex Plehanov : > > > > > Hello, > > > > > > User attributes also (besides authentication) can be used to pass some > > info > > > about an application that uses a client and then display this > information > > > in monitoring tools. Other vendors use such approach (Oracle DB, for > > > example, have DBMS_APPLICATION_INFO package, PostgreeSQL have > > > application_name connection property and application information > > available > > > later in system views). > > > > > > About allowed data types: we should definitely limit attribute types to > > > only primitive types. Thin client binary marshaller can't send > > information > > > about custom types before the handshake. > > > > > > > > > ср, 22 янв. 2020 г. в 21:39, Pavel Tupitsyn : > > > > > > > I've looked through the PR more closely, trying to understand the use > > > case, > > > > and there are some Java-specific things going on (left a comment). > > > > Please keep in mind that we have thin clients in Python, Node.js, > C++, > > > C#. > > > > The protocol must be language-agnostic. If we add some features > there, > > > > let's make sure they are usable from anywhere. > > > > > > > > On Wed, Jan 22, 2020 at 9:21 PM Pavel Tupitsyn > > > > > wrote: > > > > > > > > > The approach with UserAttributes map looks dirty to me and raises > > > > > questions: > > > > > > > > > > - Why is UserAttributes property related to authentication? > > > > > - UserAttributes name implies that users can put there anything > they > > > > want, > > > > > but what for? What are those additional use cases? > > > > > > > > > > I think we should focus on a specific problem at hand and avoid > > > > > unnecessary future-proofing. > > > > > What are current and potential future custom authenticators and > what > > > kind > > > > > of data do they need? > > > > > > > > > > On Wed, Jan 22, 2020 at 6:28 PM Nikita Amelchev < > > nsamelc...@gmail.com> > > > > > wrote: > > > > > > > > > >> I think we should add this. It will provide an extra level of > > > security. > > > > >> This approach is used in many products, for example in AWS (MFA). > > [1] > > > > >> > > > > >> [1] > > > > >> > > > > > > > > > > https://docs.aws.amazon.com/general/latest/gr/aws-sec-cred-types.html#multi-factor-authentication > > > > >> > > > > >> ср, 22 янв. 2020 г. в 18:13, Andrey Kuznetsov >: > > > > >> > > > > > >> > Hi, Pavel! > > > > >> > > > > > >> > Sometimes single authentication factor is not enough. Attributes > > > > >> proposed > > > > >> > allow to add extra factors flexibly. > > > > >> > > > > > >> > ср, 22 янв. 2020 г., 17:39 Pavel Tupitsyn >: > > > > >> > > > >
Re: Add user attributes to thin clients
> The protocol must be language-agnostic. If we add some features there, let's make sure they are usable from anywhere. That's why I want to allow primitives only. Any language can send numbers and strings. Binary marshaller, before packing object to byte[], will try to use discovery processor and send message containing class descriptor. But thin clients don't have discovery. Furthermore, if we write binary marshaller without class descriptor synchronization, we can get objects with different class versions and corresponding exceptions. But we can say users to declare their classes in META-INF/classnames.properties and current binary marshaller will works good. чт, 23 янв. 2020 г., 12:13 Alex Plehanov : > Hello, > > User attributes also (besides authentication) can be used to pass some info > about an application that uses a client and then display this information > in monitoring tools. Other vendors use such approach (Oracle DB, for > example, have DBMS_APPLICATION_INFO package, PostgreeSQL have > application_name connection property and application information available > later in system views). > > About allowed data types: we should definitely limit attribute types to > only primitive types. Thin client binary marshaller can't send information > about custom types before the handshake. > > > ср, 22 янв. 2020 г. в 21:39, Pavel Tupitsyn : > > > I've looked through the PR more closely, trying to understand the use > case, > > and there are some Java-specific things going on (left a comment). > > Please keep in mind that we have thin clients in Python, Node.js, C++, > C#. > > The protocol must be language-agnostic. If we add some features there, > > let's make sure they are usable from anywhere. > > > > On Wed, Jan 22, 2020 at 9:21 PM Pavel Tupitsyn > > wrote: > > > > > The approach with UserAttributes map looks dirty to me and raises > > > questions: > > > > > > - Why is UserAttributes property related to authentication? > > > - UserAttributes name implies that users can put there anything they > > want, > > > but what for? What are those additional use cases? > > > > > > I think we should focus on a specific problem at hand and avoid > > > unnecessary future-proofing. > > > What are current and potential future custom authenticators and what > kind > > > of data do they need? > > > > > > On Wed, Jan 22, 2020 at 6:28 PM Nikita Amelchev > > > wrote: > > > > > >> I think we should add this. It will provide an extra level of > security. > > >> This approach is used in many products, for example in AWS (MFA). [1] > > >> > > >> [1] > > >> > > > https://docs.aws.amazon.com/general/latest/gr/aws-sec-cred-types.html#multi-factor-authentication > > >> > > >> ср, 22 янв. 2020 г. в 18:13, Andrey Kuznetsov : > > >> > > > >> > Hi, Pavel! > > >> > > > >> > Sometimes single authentication factor is not enough. Attributes > > >> proposed > > >> > allow to add extra factors flexibly. > > >> > > > >> > ср, 22 янв. 2020 г., 17:39 Pavel Tupitsyn : > > >> > > > >> > > Token can be sent instead of a password (like git works with > GitHub > > >> > > tokens). > > >> > > > > >> > > For now I don't see a reason to include attributes into the > > handshake > > >> > > message. > > >> > > > > >> > > On Wed, Jan 22, 2020 at 5:32 PM Ilya Kasnacheev < > > >> ilya.kasnach...@gmail.com > > >> > > > > > >> > > wrote: > > >> > > > > >> > > > Hello! > > >> > > > > > >> > > > One does not send security certificate as attribute. The only > way > > to > > >> > > obtain > > >> > > > peer security certificate is to ask SSL engine to provide it. > > >> > > > > > >> > > > Nevertheless, I can see how it can be useful with e.g. Kerberos, > > >> which is > > >> > > > token-based IIRC. > > >> > > > > > >> > > > Regards, > > >> > > > -- > > >> > > > Ilya Kasnacheev > > >> > > > > > >> > > > > > >> > > > ср, 22 янв. 2020 г. в 17:20, Dmitrii Ryabov < > > somefire...@gmail.com > > >> >
Re: Add user attributes to thin clients
This map is something like user object from `SecurityCredentials`. Sometimes login and password are not enough for security checks. For example, we can send security certificate and validate it inside authenticator. ср, 22 янв. 2020 г., 17:16 Igor Sapego : > Hi Dmitrii, > > Can you please explain your use case? > I'm not sure I'm getting what is the motivation of this change. > > Best Regards, > Igor > > > On Wed, Jan 22, 2020 at 5:11 PM Pavel Tupitsyn > wrote: > > > Hi Dmitrii, > > > > Honestly, I could not grasp the problem, can you explain it in more > detail? > > What do we solve by adding a map with arbitrary stuff to the client > > protocol handshake? > > > > On Wed, Jan 22, 2020 at 5:02 PM Dmitrii Ryabov > > wrote: > > > > > Hello, Igniters! > > > > > > I want to add the possibility of sending user defined attributes from > > thin > > > clients. And check them inside custom authenticator during handshake > [1]. > > > > > > There is an issue in hardcoded binary writer for JDBC and > `IgniteClient`. > > > This writer searches for a classes in the JDK and > > > META-INF/classnames.properties, and tries to sync notdeclared classes > > with > > > cluster. But fails because current classloading uses discovery. > > > > > > I'd like to keep this writer and allow only primitive types and > `String` > > > for user attributes to prevent unexpected fails. I think it is better > > than > > > changing writer to one with heavy classloading. > > > > > > Is it ok to restrict thin attributes to primitives and 'String'? > > > > > > [1] https://issues.apache.org/jira/browse/IGNITE-12049 > > > > > >
Add user attributes to thin clients
Hello, Igniters! I want to add the possibility of sending user defined attributes from thin clients. And check them inside custom authenticator during handshake [1]. There is an issue in hardcoded binary writer for JDBC and `IgniteClient`. This writer searches for a classes in the JDK and META-INF/classnames.properties, and tries to sync notdeclared classes with cluster. But fails because current classloading uses discovery. I'd like to keep this writer and allow only primitive types and `String` for user attributes to prevent unexpected fails. I think it is better than changing writer to one with heavy classloading. Is it ok to restrict thin attributes to primitives and 'String'? [1] https://issues.apache.org/jira/browse/IGNITE-12049
Re: [MTCGA]: new failures in builds [4627134] needs to be handled
> IMHO, I don't think that it is a good practice to have a commit for a ticket which is already resolved I think it's ok for hotfix. TC Bot commented ticket with latest runAll visa. чт, 26 сент. 2019 г., 13:19 Вячеслав Коптилин : > Hello Dmitrii, > > > I used the same ticket [1] in PR [2] name > IMHO, I don't think that it is a good practice to have a commit for a > ticket which is already resolved. If I am not mistaken in that case you > cannot get a visa from TC Bot. > > Anyway, tests successfully passed. Pavel could you please take a look at > the PR and merge the fix if you have no objection. > > Thanks, > S. > > чт, 26 сент. 2019 г. в 12:10, Dmitrii Ryabov : > > > I used same ticket [1] in PR [2] name. Tests [3] are ok, I didn't found > > stable failures. > > > > Pavel, can you take a look? > > > > [1] - https://issues.apache.org/jira/browse/IGNITE-11094 > > [2] - https://github.com/apache/ignite/pull/6910 > > [3] - > > > > > https://ci.ignite.apache.org/viewLog.html?buildId=4634438&buildTypeId=IgniteTests24Java8_RunAll > > > > ср, 25 сент. 2019 г., 17:54 Вячеслав Коптилин >: > > > > > Hi Dmitrii, > > > > > > > I prepared fix, waiting for TC results. > > > Thank you a lot! > > > > > > Could you please share the JIRA ticket? > > > > > > Thanks, > > > S. > > > > > > > > > ср, 25 сент. 2019 г. в 17:46, Dmitrii Ryabov : > > > > > > > Bug looks strange. I prepared fix, waiting for TC results. > > > > > > > > > >
Re: [MTCGA]: new failures in builds [4627134] needs to be handled
I used same ticket [1] in PR [2] name. Tests [3] are ok, I didn't found stable failures. Pavel, can you take a look? [1] - https://issues.apache.org/jira/browse/IGNITE-11094 [2] - https://github.com/apache/ignite/pull/6910 [3] - https://ci.ignite.apache.org/viewLog.html?buildId=4634438&buildTypeId=IgniteTests24Java8_RunAll ср, 25 сент. 2019 г., 17:54 Вячеслав Коптилин : > Hi Dmitrii, > > > I prepared fix, waiting for TC results. > Thank you a lot! > > Could you please share the JIRA ticket? > > Thanks, > S. > > > ср, 25 сент. 2019 г. в 17:46, Dmitrii Ryabov : > > > Bug looks strange. I prepared fix, waiting for TC results. > > >
Re: [MTCGA]: new failures in builds [4627134] needs to be handled
Bug looks strange. I prepared fix, waiting for TC results.
Re: Update Apache Zookeeper dependency
Pavel, I described changes in the ticket. ср, 11 сент. 2019 г., 15:10 Dmitrii Ryabov : > Ok, I'll do it. > > ср, 11 сент. 2019 г., 14:23 Pavel Kovalenko : > >> Hi Dmitrii, >> >> Thank you for contribution. I'll take a look at it. Could you please >> describe in the ticket what changes have you done besides version update >> and why they are needed? >> >> ср, 11 сент. 2019 г. в 14:06, Dmitrii Ryabov : >> >> > Hello, Igniters! >> > >> > I want to update Apachee Zookeeper version from 3.4.13 to 3.5.5 and >> Apache >> > Curator from 2.9.1 to 4.2.0 as part of IGNITE-11094 [1]. Any objections? >> > >> > [1] https://issues.apache.org/jira/browse/IGNITE-11094 >> > >> >
Re: Update Apache Zookeeper dependency
Ok, I'll do it. ср, 11 сент. 2019 г., 14:23 Pavel Kovalenko : > Hi Dmitrii, > > Thank you for contribution. I'll take a look at it. Could you please > describe in the ticket what changes have you done besides version update > and why they are needed? > > ср, 11 сент. 2019 г. в 14:06, Dmitrii Ryabov : > > > Hello, Igniters! > > > > I want to update Apachee Zookeeper version from 3.4.13 to 3.5.5 and > Apache > > Curator from 2.9.1 to 4.2.0 as part of IGNITE-11094 [1]. Any objections? > > > > [1] https://issues.apache.org/jira/browse/IGNITE-11094 > > >
Update Apache Zookeeper dependency
Hello, Igniters! I want to update Apachee Zookeeper version from 3.4.13 to 3.5.5 and Apache Curator from 2.9.1 to 4.2.0 as part of IGNITE-11094 [1]. Any objections? [1] https://issues.apache.org/jira/browse/IGNITE-11094
Re: ipFinder configuration for Samples
Yakov, I checked hazelcast - it uses multicast by default. чт, 28 февр. 2019 г., 14:31 Stanislav Lukyanov : > Yakov, > > Our product is for engineers sitting in one office. > Companies which use Ignite have many engineers running Ignite on their > laptops the same way > as companies that make changes to Ignite. > “Could it be because of the multicast?” is one of the questions I ask > every time I see a question on SO > or user list, and I would be happy not to. > > The activity of changing the examples from multicast to local addresses > was started and stopped multiple times, > but AFAIR most of the community was in favor of this change. > > Moreover, our default configuration (when discoSpi is not set– such as in > default-config.xml) > uses multicast. I’d change that to a localhost VmIPFinder. > > If we want an easy start of the examples (even though I think each user > would want to use examples with multicast > exactly once, if ever) let’s add multiple configs or a switch to > ignite.sh. I believe it’s better than showing multicast as default. > > Regarding the address list – AFAIR Ignite tries to scan the multicast > group first. Only after that it > will try to connect to addresses from the list. So, the list is a bit > misleading. > > Finally, if Hazelcast uses multicast by default – well, probably they > should turn that off as well. > > My 2 cents, > Stan > > From: Yakov Zhdanov > Sent: 28 февраля 2019 г. 12:53 > To: dev@ignite.apache.org > Subject: Re: ipFinder configuration for Samples > > Guys, > > I remember we did the opposite change some time ago - switched VM IP finder > to multicast. That was done for user being able to start cluster spanning > multiple machines using examples configuration. With this change you > removed all the working samples for starting really distributed > environment. > > What was the problem? Multicast IP finder has the list of addresses that we > try to connect to on start. As far as clashes - it seems it affects only > engineers sitting in one office, but they can set up env var to override > default mcast group. The primary goal for switching to multicast was to > allow people new to Ignite to quickly start the cluster. > > As far as I remember hazelcast has multicast enabled by default. Can > anybody check this? > > --Yakov > >
Re: ipFinder configuration for Samples
Hello, Igniters! Code is ready and reviewed, tests are passed. Can we make final decision about this change? Do we really need it? [1] Pros: * Multicast ipFinder adds some instability when several persons try & debug samples or evaluate a new Ignite version at the same local network. * Speedup for node start. * Same change was made for test framework [2]. [1] https://issues.apache.org/jira/browse/IGNITE-6826?focusedCommentId=16752473&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16752473 [2] https://issues.apache.org/jira/browse/IGNITE-10555 пт, 3 нояб. 2017 г., 11:14 Alexey Popov : > I've created https://issues.apache.org/jira/browse/IGNITE-6826 > > Thanks, > Alexey > > > > -- > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/ >
Re: Default failure handler was changed for tests
Yes, currently handler handles exception in the same way as 'GridAbstractTest', so, it will work from any thread. пт, 11 янв. 2019 г., 15:18 Ilya Lantukh ilant...@gridgain.com: > Dmitry, > > It doesn't make sense to run that test now because the root cause of it's > failure had been fixed. > > You should verify that getting an unhandled exception in any system thread > leads to the failure of currently running test. > > On Fri, Jan 11, 2019 at 12:16 PM Dmitrii Ryabov > wrote: > >> Ilya, can you check your test on current implementation [1]? >> >> [1] https://github.com/apache/ignite/pull/5662 >> >> 10 дек. 2018 г. 17:10 пользователь "Dmitriy Pavlov" >> написал: >> >> Reverted. >> >> https://issues.apache.org/jira/browse/IGNITE-8227 reopened >> >> пн, 10 дек. 2018 г. в 16:23, Dmitriy Pavlov : >> >> >> > Anton, I was expecting that you revert, because you wanted to do it. >> > >> > Provided that I agree that fix could be reverted because of both >> > functional and style possible improvements, does not mean I believe it >> is >> > the only option and it should be reverted. >> > >> > Even if I agree to revert doesn't mean all community agrees, so >> reverting >> > just 1 minute after writing to dev list would be strange. I believe we >> > should be courteous enough to give a couple of days for people to come >> and >> > give feedback. >> > >> > So if you have a spare minute, please go ahead. If not, I can do it >> later. >> > >> > пн, 10 дек. 2018 г. в 14:23, Anton Vinogradov : >> > >> >> Dmitriy, >> >> >> >> You confirmed that fix should be reverted and reworked last Friday. >> >> Why it still not reverted? >> >> >> >> On Mon, Dec 10, 2018 at 12:46 AM Dmitrii Ryabov > > >> >> wrote: >> >> >> >> > Agree, it is reasonable to revert. >> >> > пт, 7 дек. 2018 г. в 18:44, Dmitriy Pavlov : >> >> > > >> >> > > Hi Ilya, >> >> > > >> >> > > thank you for noticing. >> >> > > >> >> > > Calling to fail is equal to re-throw, >> >> > > >> >> > > throw new AssertionFailedError(message); >> >> > > >> >> > > So, yes, for now it is absolutely valid reason to revert and rework >> >> fix >> >> > > >> >> > > - as Nikolay suggested to reduce method override ocurrences. >> >> > > - and with transferring this exception into GridAbstractTest and >> >> > > correctly failing test. >> >> > > >> >> > > Sincerely, >> >> > > Dmitriy Pavlov >> >> > > >> >> > > >> >> > > пт, 7 дек. 2018 г. в 18:38, Ilya Lantukh : >> >> > > >> >> > > > Unfortunately, this FailureHandler doesn't seem to work. I wrote >> a >> >> test >> >> > > > that reproduces a bug and should fail. It prints the following >> text >> >> > into >> >> > > > log, but the test still passes "successfully": >> >> > > > >> >> > > > [2018-12-07 >> >> > > > >> >> > > > >> >> > >> >> >> 18:28:23,800][ERROR][sys-stripe-1-#345%recovery.GridPointInTimeRecoveryCacheNoAffinityExchangeTest1%][IgniteTestResources] >> >> > > > Critical system error detected. Will be handled accordingly to >> >> > configured >> >> > > > handler [hnd=TestFailingFailureHandler [], >> failureCtx=FailureContext >> >> > > > [type=CRITICAL_ERROR, err=java.lang.IllegalStateException: >> Unable to >> >> > find >> >> > > > consistentId by UUID >> [nodeId=80dd2ec6-1913-4a5c-a839-630315c3, >> >> > > > topVer=AffinityTopologyVersion [topVer=12, minorTopVer=0 >> >> > > > java.lang.IllegalStateException: Unable to find consistentId by >> UUID >> >> > > > [nodeId=80dd2ec6-1913-4a5c-a839-630315c3, >> >> > > > topVer=AffinityTopologyVersion [topVer=12, minorTopVer=0]] >> >> > > > at >> >> > > > >> >> > > >
Re: Default failure handler was changed for tests
Ilya, can you check your test on current implementation [1]? [1] https://github.com/apache/ignite/pull/5662 10 дек. 2018 г. 17:10 пользователь "Dmitriy Pavlov" написал: Reverted. https://issues.apache.org/jira/browse/IGNITE-8227 reopened пн, 10 дек. 2018 г. в 16:23, Dmitriy Pavlov : > Anton, I was expecting that you revert, because you wanted to do it. > > Provided that I agree that fix could be reverted because of both > functional and style possible improvements, does not mean I believe it is > the only option and it should be reverted. > > Even if I agree to revert doesn't mean all community agrees, so reverting > just 1 minute after writing to dev list would be strange. I believe we > should be courteous enough to give a couple of days for people to come and > give feedback. > > So if you have a spare minute, please go ahead. If not, I can do it later. > > пн, 10 дек. 2018 г. в 14:23, Anton Vinogradov : > >> Dmitriy, >> >> You confirmed that fix should be reverted and reworked last Friday. >> Why it still not reverted? >> >> On Mon, Dec 10, 2018 at 12:46 AM Dmitrii Ryabov >> wrote: >> >> > Agree, it is reasonable to revert. >> > пт, 7 дек. 2018 г. в 18:44, Dmitriy Pavlov : >> > > >> > > Hi Ilya, >> > > >> > > thank you for noticing. >> > > >> > > Calling to fail is equal to re-throw, >> > > >> > > throw new AssertionFailedError(message); >> > > >> > > So, yes, for now it is absolutely valid reason to revert and rework >> fix >> > > >> > > - as Nikolay suggested to reduce method override ocurrences. >> > > - and with transferring this exception into GridAbstractTest and >> > > correctly failing test. >> > > >> > > Sincerely, >> > > Dmitriy Pavlov >> > > >> > > >> > > пт, 7 дек. 2018 г. в 18:38, Ilya Lantukh : >> > > >> > > > Unfortunately, this FailureHandler doesn't seem to work. I wrote a >> test >> > > > that reproduces a bug and should fail. It prints the following text >> > into >> > > > log, but the test still passes "successfully": >> > > > >> > > > [2018-12-07 >> > > > >> > > > >> > >> 18:28:23,800][ERROR][sys-stripe-1-#345%recovery.GridPointInTimeRecoveryCacheNoAffinityExchangeTest1%][IgniteTestResources] >> > > > Critical system error detected. Will be handled accordingly to >> > configured >> > > > handler [hnd=TestFailingFailureHandler [], failureCtx=FailureContext >> > > > [type=CRITICAL_ERROR, err=java.lang.IllegalStateException: Unable to >> > find >> > > > consistentId by UUID [nodeId=80dd2ec6-1913-4a5c-a839-630315c3, >> > > > topVer=AffinityTopologyVersion [topVer=12, minorTopVer=0 >> > > > java.lang.IllegalStateException: Unable to find consistentId by UUID >> > > > [nodeId=80dd2ec6-1913-4a5c-a839-630315c3, >> > > > topVer=AffinityTopologyVersion [topVer=12, minorTopVer=0]] >> > > > at >> > > > >> > > > >> > >> org.apache.ignite.internal.managers.discovery.ConsistentIdMapper.mapToCompactId(ConsistentIdMapper.java:62) >> > > > at >> > > > >> > > > >> > >> org.apache.ignite.internal.managers.discovery.ConsistentIdMapper.mapToCompactIds(ConsistentIdMapper.java:123) >> > > > at >> > > > >> > > > >> > >> org.apache.ignite.internal.processors.cache.transactions.IgniteTxManager.newTxRecord(IgniteTxManager.java:2507) >> > > > at >> > > > >> > > > >> > >> org.apache.ignite.internal.processors.cache.transactions.IgniteTxManager.logTxRecord(IgniteTxManager.java:2483) >> > > > at >> > > > >> > > > >> > >> org.apache.ignite.internal.processors.cache.transactions.IgniteTxAdapter.state(IgniteTxAdapter.java:1226) >> > > > at >> > > > >> > > > >> > >> org.apache.ignite.internal.processors.cache.transactions.IgniteTxAdapter.state(IgniteTxAdapter.java:1054) >> > > > at >> > > > >> > > > >> > >> org.apache.ignite.internal.processors.cache.transactions.IgniteTxHandler.startRemoteTx(IgniteTxHand
Re: Set 'TcpDiscoveryVmIpFinder' as default IP finder for tests instead of 'TcpDiscoveryMulticastIpFinder'
Hi, Anton, can you review ticket for examples [1]? [1] https://issues.apache.org/jira/browse/IGNITE-6826 пн, 24 дек. 2018 г., 15:23 Anton Vinogradov a...@apache.org: > Folks, > > The important thing here is that 99% of "final static" ip-finders were > removed. (~800 pcs.) > Non-static, mostly, kept as is since removal may or cause a test failure. > (~130 pcs.) > > In case someone interested in the continuation of cleanup, please feel free > to create an issue and provide PR, I'm ready to review it. > > On Mon, Dec 24, 2018 at 3:04 PM Vyacheslav Daradur > wrote: > > > The second step [2] "removing related boilerplate in tests" - has been > > done. It is expected that a bit memory will be released at tests TC > > agents, which could not be cleaned by GC because of static final > > fields. > > > > Guys, please, do not generate a new one ) > > > > [1] https://issues.apache.org/jira/browse/IGNITE-10715 > > > > On Tue, Dec 18, 2018 at 6:29 PM Anton Vinogradov wrote: > > > > > > Folks, > > > > > > Now I see 5-10% speedup at all TC suites right after the merge. > > > Great fix! > > > > > > > > > On Mon, Dec 17, 2018 at 2:39 PM Vyacheslav Daradur < > daradu...@gmail.com> > > > wrote: > > > > > > > Andrey Mashenkov, at first sight, I have not seen any problems with > > > > .NET platform. > > > > > > > > I believe we need carefully configure ports in platform's examples, > > > > additional actions should not be required. > > > > > > > > On Mon, Dec 17, 2018 at 2:35 PM Vyacheslav Daradur < > > daradu...@gmail.com> > > > > wrote: > > > > > > > > > > The task [1] is done. > > > > > > > > > > 'TcpDiscoveryVmIpFinder' is default IP finder in tests now. > > > > > > > > > > The IP finder is initialized on per tests class level to avoid > hidden > > > > > affecting among tests, it means the test methods in the common test > > > > > class will use the same IP finder. > > > > > > > > > > You don't need to set up 'TcpDiscoveryVmIpFinder' in your tests > > > > > explicitly anymore, also task [2] has been filled to remove related > > > > > boilerplate if nobody minds. > > > > > > > > > > [1] https://issues.apache.org/jira/browse/IGNITE-10555 > > > > > [2] https://issues.apache.org/jira/browse/IGNITE-10715 > > > > > > > > > > > > > > > On Wed, Dec 5, 2018 at 7:17 PM Dmitriy Pavlov > > > > wrote: > > > > > > > > > > > > ++1 > > > > > > > > > > > > ср, 5 дек. 2018 г. в 18:38, Denis Mekhanikov < > > dmekhani...@gmail.com>: > > > > > > > > > > > > > Andrey, > > > > > > > > > > > > > > Multi-JVM tests may also use a static IP finder, but it should > > use > > > > some > > > > > > > specific port range instead of being shared. > > > > > > > Something like 127.0.0.1:48500..48509 would do. > > > > > > > > > > > > > > Denis > > > > > > > > > > > > > > ср, 5 дек. 2018 г. в 18:34, Vyacheslav Daradur < > > daradu...@gmail.com > > > > >: > > > > > > > > > > > > > > > I filled a task [1]. > > > > > > > > > > > > > > > > >> Slava, do you think Platforms tests can be fixed as well > or > > one > > > > more > > > > > > > > ticket > > > > > > > > should be created? > > > > > > > > > > > > > > > > I'll try to fix them within one ticket, it should be > > investigated > > > > a bit > > > > > > > > deeper. > > > > > > > > > > > > > > > > I'll inform about the task's progress in this thread later. > > > > > > > > > > > > > > > > Thanks! > > > > > > > > > > > > > > > > [1] https://issues.apache.org/jira/browse/IGNITE-10555 > > > > > > > > On Wed, Dec 5, 2018 at 6:28 PM Andrey Mashenkov > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > Slava, > > > > > > > > > +1 for your proposal. > > > > > > > > > Is there any ticket for this? > > > > > > > > > > > > > > > > > > Denis, > > > > > > > > > I've just read in nabble thread you suggest to allow > > multicast > > > > finder > > > > > > > for > > > > > > > > > multiJVM tests > > > > > > > > > and I'd think we shouldn't use multicast in test at all > > (excepts > > > > > > > > multicast > > > > > > > > > Ip finder self tests of course), > > > > > > > > > but e.g. add an assertion to force user to create ipfinder > > > > properly. > > > > > > > > > > > > > > > > > > > > > > > > > > > Also, we have a ticket for similar issue in 'examples' > > module. > > > > > > > > > Seems, there are some issues with Platforms module > > integration. > > > > > > > > > Slava, do you think Platforms tests can be fixed as well or > > one > > > > more > > > > > > > > ticket > > > > > > > > > should be created? > > > > > > > > > > > > > > > > > > [1] https://issues.apache.org/jira/browse/IGNITE-6826 > > > > > > > > > > > > > > > > > > On Wed, Dec 5, 2018 at 5:55 PM Denis Mekhanikov < > > > > dmekhani...@gmail.com > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > Slava, > > > > > > > > > > > > > > > > > > > > These are exactly my thoughts, so I fully support you > here. > > > > > > > > > > I already wrote about it: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
Re: Apache Ignite TeamCity Bot - muted tests
Dmitriy,
with ignore annotations we will need to check forgotten tests
manually. Thats bad, but we will save some machine time in common
runs.
пн, 17 дек. 2018 г. в 17:06, Dmitriy Pavlov :
>
> Investigation parameters: Can leave as empty
> Unmute: Manual (don't use auto because of flaky tests)
> Text: https://issues.apache.org/jira/browse/IGNITE-N
>
> But anyway, I hope in the nearest future best way to mute test will be to
> ignore it using:
> @IgniteIgnore
> or @Ignore
> or Assume.that()
>
> In this case, we will not need TeamCity mutes anymore.
>
> I'm not sure which way of ignoring will be best.
>
> пн, 17 дек. 2018 г. в 17:02, Anton Vinogradov :
>
> > Dmitrii,
> >
> > My question was mostly about how to mute test properly.
> >
> >
> > On Mon, Dec 17, 2018 at 4:27 PM Dmitrii Ryabov
> > wrote:
> >
> > > Anton,
> > >
> > > In the "Mute" column you can find link to the test on TeamCity. On
> > > this page you can see test details like run history and current mutes.
> > > Here you should be able to unmute existing mutes.
> > > пн, 17 дек. 2018 г. в 16:21, Anton Vinogradov :
> > > >
> > > > Great feature!
> > > >
> > > > Could you please explain how to have a deal with it?
> > > > How should I mite/unmute tests for now?
> > > >
> > > > On Fri, Dec 14, 2018 at 6:50 PM Dmitriy Pavlov
> > > wrote:
> > > >
> > > > > We have IgniteIgnore, so we can use it. It seems behavior is the
> > same.
> > > We
> > > > > need just to understand in which case we can get issue ID from TC
> > REST
> > > (for
> > > > > the Bot).
> > > > >
> > > > > пт, 14 дек. 2018 г. в 17:51, Vyacheslav Daradur > >:
> > > > >
> > > > > > Dmitry Pavlov,
> > > > > >
> > > > > > As far as I know, we are able to use following construction to
> > ignore
> > > > > test:
> > > > > > Assume.assumeTrue("link-to-issue", false);
> > > > > >
> > > > > > On Fri, Dec 14, 2018 at 5:44 PM Dmitriy Pavlov > >
> > > > > wrote:
> > > > > > >
> > > > > > > Thank you, Dmitrii,
> > > > > > >
> > > > > > > I've started unmuting tests starting from yesterday.
> > > > > > >
> > > > > > > Now muted test count was decreased from 3540 to 3408
> > > > > > >
> > > > > >
> > > > >
> > >
> > https://ci.ignite.apache.org/project.html?projectId=IgniteTests24Java8&tab=mutedProblems&branch_IgniteTests24Java8=%3Cdefault%3E
> > > > > > >
> > > > > > >
> > > > > > > Probably we could automate this activity by auto-unmute, but
> > > hopefully,
> > > > > > > JUnit4 & @Ignore opportunity will come earlier.
> > > > > > >
> > > > > > > Sincerely,
> > > > > > > Dmitriy Pavlov
> > > > > > >
> > > > > > > пт, 14 дек. 2018 г. в 17:24, Dmitrii Ryabov <
> > somefire...@gmail.com
> > > >:
> > > > > > >
> > > > > > > > Hello, Igniters!
> > > > > > > >
> > > > > > > > Our Bot got new functionality - it shows tests muted on
> > TeamCity
> > > and
> > > > > > their
> > > > > > > > ticket status [1].
> > > > > > > >
> > > > > > > > So, we can see forgotten mutes for resolved tickets now.
> > > > > > > >
> > > > > > > > Igniters with rights to unmute tests on TC - please, check this
> > > page
> > > > > > > > periodically for forgotten mutes.
> > > > > > > >
> > > > > > > > [1] https://mtcga.gridgain.com/mutes.html
> > > > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > --
> > > > > > Best Regards, Vyacheslav D.
> > > > > >
> > > > >
> > >
> >
Re: Apache Ignite TeamCity Bot - muted tests
Anton,
In the "Mute" column you can find link to the test on TeamCity. On
this page you can see test details like run history and current mutes.
Here you should be able to unmute existing mutes.
пн, 17 дек. 2018 г. в 16:21, Anton Vinogradov :
>
> Great feature!
>
> Could you please explain how to have a deal with it?
> How should I mite/unmute tests for now?
>
> On Fri, Dec 14, 2018 at 6:50 PM Dmitriy Pavlov wrote:
>
> > We have IgniteIgnore, so we can use it. It seems behavior is the same. We
> > need just to understand in which case we can get issue ID from TC REST (for
> > the Bot).
> >
> > пт, 14 дек. 2018 г. в 17:51, Vyacheslav Daradur :
> >
> > > Dmitry Pavlov,
> > >
> > > As far as I know, we are able to use following construction to ignore
> > test:
> > > Assume.assumeTrue("link-to-issue", false);
> > >
> > > On Fri, Dec 14, 2018 at 5:44 PM Dmitriy Pavlov
> > wrote:
> > > >
> > > > Thank you, Dmitrii,
> > > >
> > > > I've started unmuting tests starting from yesterday.
> > > >
> > > > Now muted test count was decreased from 3540 to 3408
> > > >
> > >
> > https://ci.ignite.apache.org/project.html?projectId=IgniteTests24Java8&tab=mutedProblems&branch_IgniteTests24Java8=%3Cdefault%3E
> > > >
> > > >
> > > > Probably we could automate this activity by auto-unmute, but hopefully,
> > > > JUnit4 & @Ignore opportunity will come earlier.
> > > >
> > > > Sincerely,
> > > > Dmitriy Pavlov
> > > >
> > > > пт, 14 дек. 2018 г. в 17:24, Dmitrii Ryabov :
> > > >
> > > > > Hello, Igniters!
> > > > >
> > > > > Our Bot got new functionality - it shows tests muted on TeamCity and
> > > their
> > > > > ticket status [1].
> > > > >
> > > > > So, we can see forgotten mutes for resolved tickets now.
> > > > >
> > > > > Igniters with rights to unmute tests on TC - please, check this page
> > > > > periodically for forgotten mutes.
> > > > >
> > > > > [1] https://mtcga.gridgain.com/mutes.html
> > > > >
> > >
> > >
> > >
> > > --
> > > Best Regards, Vyacheslav D.
> > >
> >
Apache Ignite TeamCity Bot - muted tests
Hello, Igniters! Our Bot got new functionality - it shows tests muted on TeamCity and their ticket status [1]. So, we can see forgotten mutes for resolved tickets now. Igniters with rights to unmute tests on TC - please, check this page periodically for forgotten mutes. [1] https://mtcga.gridgain.com/mutes.html
Re: Default failure handler was changed for tests
gradov wrote: > > > > > >> We stop, for now, then you will chill a > > > >> little bit, then you will have an absolutely fantastic weekend, and > > then > > > on > > > >> Monday, Dec 10 we will continue this discussion in a positive and > > > >> constructive manner. > > > Agree > > > > > > On Thu, Dec 6, 2018 at 3:55 PM Nikolay Izhikov > > > wrote: > > > > > > > Anton. > > > > > > > > I discussed this fix privately with Dmitriy Pavlov. > > > > > > > > 1. We had NoOpHandler for ALL tests before this merge. > > > > 2. Dmitry Ryabov will remove all copypasted code soon. > > > > > > > > So, this fix make things better. > > > > > > > > I think we shouldn't revert it. > > > > > > > > I think we should continue work to turn off NoOpHandler in all tests. > > > > > > > > Dmitriy Pavlov, can you do it, as a committer of this patch? > > > > > > > > On 12/6/18 3:02 PM, Anton Vinogradov wrote: > > > > >>> I still hope Anton will do the first bunch of tests research to > > > > > demonstrate > > > > >>> the idea. > > > > > > > > > > Dmitriy, > > > > > Just want to remind you that we already spend time here because of > > > > > unacceptable code merge situation. > > > > > Such merges should NEVER happen again. > > > > > Please, next time make sure that code you merge has no massive > > > > duplication > > > > > and fixes without proper reason investigation. > > > > > Committer always MUST be ready to explain each symbol inside code he > > > > merged. > > > > > The situation when you have no clue why it written this way > > > unacceptable. > > > > > > > > > > Feel free to start a discussion at private in case you have some > > > > objections. > > > > > But, hope you agree and will help us to solve the issue instead. > > > > > > > > > > Dmitrii, > > > > >>> Anton, I mean `copy-paste reduce` ticket. I'll try to describe the > > > > > reasons for > > > > >>> no-op in tests. Then, we can create tickets to fix this cases if > > > > needed. > > > > > > > > > > In case no-one will be ready to start a proper fix (investigate why > > > every > > > > > no-op required and create tickets for each problem) before Friday > > > > evening, > > > > > the code will be rolled back. > > > > > Simple no-op is better that same but overcomplicated. > > > > > > > > > > On Thu, Dec 6, 2018 at 2:14 PM Dmitrii Ryabov > > > > > > wrote: > > > > > > > > > >> Anton, I mean `copy-paste reduce` ticket. I'll try to describe > > reasons > > > > for > > > > >> no-op in tests. Then, we can create tickets to fix this cases if > > > needed. > > > > >> > > > > >> чт, 6 дек. 2018 г., 13:53 Dmitriy Pavlov dpav...@apache.org: > > > > >> > > > > >>> BTW, No-Op or StopNode-FailTest in case of a deep investigation > > will > > > > >> always > > > > >>> require to understand what test does and what it tests. > > > > >>> > > > > >>> So we can get a positive outcome from this research if we agree to > > > add > > > > >>> - a small description to each test about the reason for existing of > > > > this > > > > >>> test, > > > > >>> - what is the expected behavior of the product in the test, and how > > > it > > > > is > > > > >>> checked? > > > > >>> - failure handler influence, etc. > > > > >>> > > > > >>> I still hope Anton will do the first bunch of tests research to > > > > >> demonstrate > > > > >>> the idea. > > > > >>> > > > > >>> чт, 6 дек. 2018 г. в 13:39, Anton Vinogradov : > > > > >>> > > > > >>>> Dmitrii, > > > > >>>> > > > > >>>>>> I agree with Nikolay's solution. If no one minds, I'll create > > > ti
Re: Default failure handler was changed for tests
Anton, I mean `copy-paste reduce` ticket. I'll try to describe reasons for no-op in tests. Then, we can create tickets to fix this cases if needed. чт, 6 дек. 2018 г., 13:53 Dmitriy Pavlov dpav...@apache.org: > BTW, No-Op or StopNode-FailTest in case of a deep investigation will always > require to understand what test does and what it tests. > > So we can get a positive outcome from this research if we agree to add > - a small description to each test about the reason for existing of this > test, > - what is the expected behavior of the product in the test, and how it is > checked? > - failure handler influence, etc. > > I still hope Anton will do the first bunch of tests research to demonstrate > the idea. > > чт, 6 дек. 2018 г. в 13:39, Anton Vinogradov : > > > Dmitrii, > > > > >> I agree with Nikolay's solution. If no one minds, I'll create ticket > for > > >> appropriate changes and recheck issues. > > Do you mean 'copy-paste reduce' ticket or check/fix of all tests with > no-op > > to have a proper handler? > > > > Just want to make sure that copy-paste minimization is not the final > step. > > > > On Thu, Dec 6, 2018 at 1:24 PM Павлухин Иван > wrote: > > > > > Dmitrii Ryabov, > > > > > > Your comments sounds reasonable to me. Marker base class approach > > > looks good to me so far. > > > > > > P.S. I had even worse name in mind 'StopGaps' =) > > > чт, 6 дек. 2018 г. в 13:08, Dmitrii Ryabov : > > > > > > > > Ivan, I think `Workarounds` class isn't good idea, because it looks > > like > > > we > > > > create stable workarounds, which will never be fixed. > > > > > > > > I agree with Nikolay's solution. If no one minds, I'll create ticket > > for > > > > appropriate changes and recheck issues. > > > > > > > > чт, 6 дек. 2018 г., 12:17 Anton Vinogradov a...@apache.org: > > > > > > > > > Folks, thank's everyone for solution research. > > > > > I'm ok with Nikolay approach in case that's not a final step. > > > > > > > > > > On Thu, Dec 6, 2018 at 12:11 PM Павлухин Иван > > > > wrote: > > > > > > > > > > > Nikolay, > > > > > > > > > > > > I meant "not expensive" by "cheap". And I meant that it is good > > that > > > > > > it cheap =). And I said it to contrast with "expensive" ~100 > tests > > > > > > investigation. And if we agree (mostly I would like an opinion > from > > > > > > Dmitriy Ryabov as an original author) on a way how to improve the > > > > > > patch then let's do it. > > > > > > чт, 6 дек. 2018 г. в 10:41, Nikolay Izhikov >: > > > > > > > > > > > > > > Dmitriy Ryabov, Dmitriy Pavlov, sorry. > > > > > > > > > > > > > > Of course it should be "NOT to blame author". > > > > > > > > > > > > > > Sorry, one more time. > > > > > > > > > > > > > > чт, 6 дек. 2018 г., 10:40 Dmitriy Pavlov dpav...@apache.org: > > > > > > > > > > > > > > > I hope you've misprinted here > > > > > > > > > I'm here to blame the author. > > > > > > > > > > > > > > > > We can blame code but never coders. > > > > > > > > > > > > > > > > Please see https://discourse.pi-hole.net/faq - has > absolutely > > > > > nothing > > > > > > in > > > > > > > > common with Apache Guides, but says the same things. It is a > > > > > practical > > > > > > > > necessity to maintain a friendly atmosphere. > > > > > > > > > > > > > > > > чт, 6 дек. 2018 г. в 10:31, Nikolay Izhikov < > > nizhi...@apache.org > > > >: > > > > > > > > > > > > > > > > > Ivan. > > > > > > > > > > > > > > > > > > > 1. Accept the patch and bring an improvement to Ignite > (and > > > > > create > > > > > > a> > > > > > > > > > ticket for further investigation). > > > > > > > > > > >
Re: Default failure handler was changed for tests
nything (anyone) what can guarantee > it. > > > > > > So, I personally prefer an option 1 against 2 because I believe > > that > > > > > > it is good if the system "can make a progress". > > > > > > > > > > > > [1] https://github.com/apache/ignite/pull/5584/files > > > > > > [2] https://github.com/apache/ignite/pull/5586/files > > > > > > ср, 5 дек. 2018 г. в 21:22, Nikolay Izhikov >: > > > > > > > > > > > > > > Dmitriy. > > > > > > > > > > > > > > > The closest analog to Noop handler is mute of test failure. > > > > > > > > By this commit, we had unmuted (possible) failures in > > > > > ~5-~100=~49900 > > > > > > > > > > > > > > tests, and we’re still concerned about style or minor details > if > > > > no-op > > > > > was > > > > > > > copy-pasted, aren’t we? > > > > > > > > > > > > > > Can you explain this idea a bit more? > > > > > > > I don't understand what is unmuted by discussed commit. > > > > > > > > > > > > > > ср, 5 дек. 2018 г. в 20:40, Nikolay Izhikov < > nizhi...@apache.org > > >: > > > > > > > > > > > > > > > > Thanks, as an improvement to the code, this may be better. > > > > > > > > > > > > > > > > I can prepare a full patch for NoOp handler. > > > > > > > > What do you think? > > > > > > > > > > > > > > > > Anton Vinogradov, do you agree with this approach? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > ср, 5 дек. 2018 г. в 20:33, Dmitriy Pavlov < > dpav...@apache.org > > >: > > > > > > > > > > > > > > > > > Thanks, as an improvement to the code, this may be better. > > But > > > > > still, it > > > > > > > > > is > > > > > > > > > not a reason to revert. And Anton mentioned something with > > better > > > > > > > > > exception > > > > > > > > > handling/logging. Probably we will see an implementation as > > well. > > > > > > > > > > > > > > > > > > This case here is a big thing related to The Apache Way, - > > and > > > > I'll > > > > > > > > > explain > > > > > > > > > why it makes me switched into fight-mode - until we stop > this > > > > > nonsense. If > > > > > > > > > PMCs (at least) are aware of patterns and anti-patterns in > > the > > > > > community, > > > > > > > > > we will succeed as a project much more as with (only) > perfect > > > > code. > > > > > > > > > > > > > > > > > > The closest analog to Noop handler is mute of test failure. > > By > > > > this > > > > > > > > > commit, > > > > > > > > > we had unmuted (possible) failures in ~5-~100=~49900 > > tests, > > > > > and we’re > > > > > > > > > still concerned about style or minor details if no-op was > > > > > copy-pasted, > > > > > > > > > aren’t we? > > > > > > > > > > > > > > > > > > To everyone arguing about the number of tests we are > allowed > > to > > > > > have with > > > > > > > > > no-op: please visit this page > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://ci.ignite.apache.org/project.html?projectId=IgniteTests24Java8&tab=mutedProblems&branch_IgniteTests24Java8=__all_branches__ > > > > > > > > > > > > > > > > > > It says: Muted tests: 3154. Are there any disagreements > > here? Why > > > > > there > > > > > > > > > are > > > > > > > > > no insistent disagreement/not happy PMCs with absolutely > > > > > unconditionally > > > > > > >
Re: Default failure handler was changed for tests
Anton, I think wrapping every disconnecting node with try-catch will be less readable than no-op handler. ср, 5 дек. 2018 г., 9:26 Dmitriy Pavlov dpav...@apache.org: > Folks let me remind you that Dmitry changed default of ALL tests from noop > to a meaningful handler. So we should start every message here from saying > thank you to Dmitry. > > Please review remaining tests and remove noop where possible. > > вт, 4 дек. 2018 г., 23:48 Andrey Mashenkov : > > > Hi all, > > > > Really, why noop? > > > > If you expect failure handler should be triggered, you can override > default > > one and rise some flag, which can be checked in test. > > This will make test clearer. > > > > With noop, you'll get previous unwanted behavior, that you are trying to > > improve, isnt'it? > > > > 4 дек. 2018 г. 23:25 пользователь "Anton Vinogradov" > > написал: > > > > And you have to check the reason of failure inside the try-catch block, > of > > course. > > In case found not equals to expected then test should rethrow the > > exception. > > > > > > вт, 4 дек. 2018 г. в 23:21, Anton Vinogradov : > > > > > > > Dmitrii, > > > > > > The solution is not clear to me. > > > In case you expect the failure then a correct case is to wrap it with > > > try-catch block instead of no-op failure handler usage. > > > > > > вт, 4 дек. 2018 г. в 21:41, Dmitrii Ryabov : > > > > > >> Anton, > > >> > > >> Tests in these classes check fail cases when we expect critical > > >> failure like node stop or exception thrown. Such tests trigger failure > > >> handler and it fails test when everything goes as it should go. That's > > >> why we need no-op handler here. > > >> вт, 4 дек. 2018 г. в 20:06, Dmitriy Pavlov : > > >> > > > >> > Hi Igniters, > > >> > > > >> > BTW, if you find in any of your tests it does't need an old value of > > >> > handler (=NoOp), feel free to remove it. > > >> > > > >> > Sincerely, > > >> > Dmitriy Pavlov > > >> > > > >> > вт, 4 дек. 2018 г. в 20:02, Anton Vinogradov : > > >> > > > >> > > Dmitrii, > > >> > > > > >> > > Could you please explain the reason of explicit set of 100+ > > >> > > NoOpFailureHandlers? > > >> > > > > >> > > > > >> > > вт, 4 дек. 2018 г. в 19:12, Dmitrii Ryabov >: > > >> > > > > >> > > > Hello, Igniters! > > >> > > > > > >> > > > Today the test framework's default no-op failure handler was > > >> changed to > > >> > > the > > >> > > > handler, which stops the node and fails the test. > > >> > > > > > >> > > > Over 100 tests kept no-op failure handler by overrided > > >> > > > `getFailureHandler()` method. > > >> > > > > > >> > > > If you'll found a problem or something unexpected - write here > or > > >> in the > > >> > > > ticket [1]. > > >> > > > > > >> > > > [1] https://issues.apache.org/jira/browse/IGNITE-8227 > > >> > > > > > >> > > > > >> > > > > > >
Re: Default failure handler was changed for tests
Anton, Tests in these classes check fail cases when we expect critical failure like node stop or exception thrown. Such tests trigger failure handler and it fails test when everything goes as it should go. That's why we need no-op handler here. вт, 4 дек. 2018 г. в 20:06, Dmitriy Pavlov : > > Hi Igniters, > > BTW, if you find in any of your tests it does't need an old value of > handler (=NoOp), feel free to remove it. > > Sincerely, > Dmitriy Pavlov > > вт, 4 дек. 2018 г. в 20:02, Anton Vinogradov : > > > Dmitrii, > > > > Could you please explain the reason of explicit set of 100+ > > NoOpFailureHandlers? > > > > > > вт, 4 дек. 2018 г. в 19:12, Dmitrii Ryabov : > > > > > Hello, Igniters! > > > > > > Today the test framework's default no-op failure handler was changed to > > the > > > handler, which stops the node and fails the test. > > > > > > Over 100 tests kept no-op failure handler by overrided > > > `getFailureHandler()` method. > > > > > > If you'll found a problem or something unexpected - write here or in the > > > ticket [1]. > > > > > > [1] https://issues.apache.org/jira/browse/IGNITE-8227 > > > > >
Default failure handler was changed for tests
Hello, Igniters! Today the test framework's default no-op failure handler was changed to the handler, which stops the node and fails the test. Over 100 tests kept no-op failure handler by overrided `getFailureHandler()` method. If you'll found a problem or something unexpected - write here or in the ticket [1]. [1] https://issues.apache.org/jira/browse/IGNITE-8227
IEP-29 IGNITE-10165 Gather query stats and expose interfaces to access it
Hello, Igniters, I want to do IGNITE-10165 (Gather query stats and expose interfaces to access it) [1], if you don't mind. Yury, Vladimir, did you planned something about statistics implementation? Also, can you point out in which sessions should be available option to turn on/off stats gathering? This also mentioned in the IEP-29 [2] and IGNITE-10166 [3]. [1] https://issues.apache.org/jira/browse/IGNITE-10165 [2] https://cwiki.apache.org/confluence/display/IGNITE/IEP-29%3A+SQL+management+and+monitoring [3] https://issues.apache.org/jira/browse/IGNITE-10166
Re: [VOTE] Creation dedicated list for github notifiacations
0 вт, 27 нояб. 2018 г. в 02:33, Alexey Kuznetsov : > > +1 > Do not forget notification from GitBox too! > > On Tue, Nov 27, 2018 at 2:20 AM Zhenya wrote: > > > +1, already make it by filers. > > > > > This was discussed already [1]. > > > > > > So, I want to complete this discussion with moving outside dev-list > > > GitHub-notification to dedicated list. > > > > > > Please start voting. > > > > > > +1 - to accept this change. > > > 0 - you don't care. > > > -1 - to decline this change. > > > > > > This vote will go for 72 hours. > > > > > > [1] > > > > > http://apache-ignite-developers.2346864.n4.nabble.com/Time-to-remove-automated-messages-from-the-devlist-td37484i20.html > > > > > -- > Alexey Kuznetsov
Re: Apache Ignite 2.7. Last Mile
Yes, revert both. вт, 20 нояб. 2018 г., 11:52 Vladimir Ozerov voze...@gridgain.com: > +1 for reverting both. > > On Tue, Nov 20, 2018 at 9:43 AM Nikolay Izhikov > wrote: > > > Hello, Dmitrii. > > > > I see 2 tickets for this improvement: > > > > IGNITE-602 - [Test] GridToStringBuilder is vulnerable for > > StackOverflowError caused by infinite recursion [1] > > IGNITE-9209 - GridDistributedTxMapping.toString() returns broken string > [2] > > > > Should we revert both commits? > > > > [1] https://github.com/apache/ignite/commit/d67c5bf > > [2] https://github.com/apache/ignite/commit/9bb9c04 > > > > > > В Пн, 19/11/2018 в 13:36 +0300, Dmitrii Ryabov пишет: > > > I agree to revert and make fix for 2.8. So, we will have more time to > > test > > > it. > > > > > > пн, 19 нояб. 2018 г., 10:53 Vladimir Ozerov voze...@gridgain.com: > > > > > > > +1 for revert. > > > > > > > > On Sun, Nov 18, 2018 at 11:31 PM Dmitriy Pavlov > > > > wrote: > > > > > > > > > I personally don't mind. > > > > > > > > > > But I would like Dmitry Ryabov and Alexey Goncharuck share their > > > > > > > > opinions. > > > > > > > > > > вс, 18 нояб. 2018 г., 20:43 Nikolay Izhikov : > > > > > > > > > > > Yes, I think so. > > > > > > > > > > > > вс, 18 нояб. 2018 г., 20:34 Denis Magda dma...@apache.org: > > > > > > > > > > > > > Sounds good to me. Are we starting the vote then? > > > > > > > > > > > > > > Denis > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Sun, Nov 18, 2018 at 8:25 AM Nikolay Izhikov < > > nizhi...@apache.org > > > > > > > wrote: > > > > > > > > > > > > > > > Hello, Igniters. > > > > > > > > > > > > > > > > This issue is the only ticket that blocks 2.7 release. > > > > > > > > > > > > > > > > I looked at IGNITE-602 PR and GridToStringBuilder. > > > > > > > > The code looks complicated for me. > > > > > > > > And it's not obvious for me how to fix this issue in a short > > period > > > > > > > > > > of > > > > > > > > time. > > > > > > > > Especially, code deals with recursion and other things that > can > > > > > > > > lead > > > > > to > > > > > > > > very dangerous errors. > > > > > > > > > > > > > > > > Let's revert this patch and fix it in calmly. > > > > > > > > Also, we need additional tests for it. > > > > > > > > > > > > > > > > В Пт, 16/11/2018 в 17:57 +0300, Dmitrii Ryabov пишет: > > > > > > > > > Ok, I'll check the issue. > > > > > > > > > пт, 16 нояб. 2018 г. в 17:52, Alexey Goncharuk < > > > > > > > > > > > > > > > > alexey.goncha...@gmail.com>: > > > > > > > > > > > > > > > > > > > > Igniters, > > > > > > > > > > > > > > > > > > > > I've just found that S.toString() implementation is > broken > > in > > > > > > > > > > > > > > > > ignite-2.7 and master [1]. It leads to a message > > > > > > > > > > Wrapper [p=Parent [a=0]Child [b=0, super=]] > > > > > > > > > > being formed instead of > > > > > > > > > > Wrapper [p=Child [b=0, super=Parent [a=0]]] > > > > > > > > > > for classes with inheritance that use > > > > > > > > S.toString(SomeClass.class, > > > > > > > > this, super.toString()) embedded to some other object. > > > > > > > > > > > > > > > > > > > > Dmitrii Ryabov, I've reverted two commits related to > > IGNITE-602 > > > > > > > > > > and > > > > > > > > IGNITE-9209 tickets locally and it fixes the issue. Can you > > take a > > > >
Re: Apache Ignite 2.7. Last Mile
I agree to revert and make fix for 2.8. So, we will have more time to test it. пн, 19 нояб. 2018 г., 10:53 Vladimir Ozerov voze...@gridgain.com: > +1 for revert. > > On Sun, Nov 18, 2018 at 11:31 PM Dmitriy Pavlov > wrote: > > > I personally don't mind. > > > > But I would like Dmitry Ryabov and Alexey Goncharuck share their > opinions. > > > > вс, 18 нояб. 2018 г., 20:43 Nikolay Izhikov : > > > > > Yes, I think so. > > > > > > вс, 18 нояб. 2018 г., 20:34 Denis Magda dma...@apache.org: > > > > > > > Sounds good to me. Are we starting the vote then? > > > > > > > > Denis > > > > > > > > > > > > > > > > On Sun, Nov 18, 2018 at 8:25 AM Nikolay Izhikov > > > > > wrote: > > > > > > > > > Hello, Igniters. > > > > > > > > > > This issue is the only ticket that blocks 2.7 release. > > > > > > > > > > I looked at IGNITE-602 PR and GridToStringBuilder. > > > > > The code looks complicated for me. > > > > > And it's not obvious for me how to fix this issue in a short period > > of > > > > > time. > > > > > Especially, code deals with recursion and other things that can > lead > > to > > > > > very dangerous errors. > > > > > > > > > > Let's revert this patch and fix it in calmly. > > > > > Also, we need additional tests for it. > > > > > > > > > > В Пт, 16/11/2018 в 17:57 +0300, Dmitrii Ryabov пишет: > > > > > > Ok, I'll check the issue. > > > > > > пт, 16 нояб. 2018 г. в 17:52, Alexey Goncharuk < > > > > > alexey.goncha...@gmail.com>: > > > > > > > > > > > > > > Igniters, > > > > > > > > > > > > > > I've just found that S.toString() implementation is broken in > > > > > ignite-2.7 and master [1]. It leads to a message > > > > > > > Wrapper [p=Parent [a=0]Child [b=0, super=]] > > > > > > > being formed instead of > > > > > > > Wrapper [p=Child [b=0, super=Parent [a=0]]] > > > > > > > for classes with inheritance that use > S.toString(SomeClass.class, > > > > > this, super.toString()) embedded to some other object. > > > > > > > > > > > > > > Dmitrii Ryabov, I've reverted two commits related to IGNITE-602 > > and > > > > > IGNITE-9209 tickets locally and it fixes the issue. Can you take a > > look > > > > at > > > > > the issue? > > > > > > > > > > > > > > I think this regression essentially makes our logs unreadable > in > > > some > > > > > cases and I would like to get it fixed in ignite-2.7 or revert both > > > > commits > > > > > from the release. > > > > > > > > > > > > > > [1] https://issues.apache.org/jira/browse/IGNITE-10301 > > > > > > > > > > > > > > пт, 9 нояб. 2018 г. в 09:22, Nikolay Izhikov < > > nizhi...@apache.org > > > >: > > > > > > > > > > > > > > > > Hello, Igniters. > > > > > > > > > > > > > > > > We still have 5 tickets for 2.7: > > > > > > > > > > > > > > > > IGNITE-10052Andrew Mashenkov Restart node during TX > causes > > > > > vacuum error. > > > > > > > > IGNITE-10170Unassigned .NET: > > Services.ServicesTestAsync > > > > > fails > > > > > > > > IGNITE-10196Maxim Pudov Remove kafka-clients-*-test > > > > > dependency > > > > > > > > IGNITE-10154Andrey Gura Critical worker liveness > check > > > > > configuration is non-trivial and inconsistent > > > > > > > > IGNITE-9996 Nikolay Izhikov Investigate possible > > performance > > > > > drop in FSYNC mode for ignite-2.7 compared to ignite-2.6 > > > > > > > > > > > > > > > > > > > > > > > > В Чт, 08/11/2018 в 14:25 +0300, Nikolay Izhikov пишет: > > > > > > > > > I'm OK with this. > > > > > > > > > > > > > > &
Re: Apache Ignite 2.7. Last Mile
Ok, I'll check the issue. пт, 16 нояб. 2018 г. в 17:52, Alexey Goncharuk : > > Igniters, > > I've just found that S.toString() implementation is broken in ignite-2.7 and > master [1]. It leads to a message > Wrapper [p=Parent [a=0]Child [b=0, super=]] > being formed instead of > Wrapper [p=Child [b=0, super=Parent [a=0]]] > for classes with inheritance that use S.toString(SomeClass.class, this, > super.toString()) embedded to some other object. > > Dmitrii Ryabov, I've reverted two commits related to IGNITE-602 and > IGNITE-9209 tickets locally and it fixes the issue. Can you take a look at > the issue? > > I think this regression essentially makes our logs unreadable in some cases > and I would like to get it fixed in ignite-2.7 or revert both commits from > the release. > > [1] https://issues.apache.org/jira/browse/IGNITE-10301 > > пт, 9 нояб. 2018 г. в 09:22, Nikolay Izhikov : >> >> Hello, Igniters. >> >> We still have 5 tickets for 2.7: >> >> IGNITE-10052Andrew Mashenkov Restart node during TX causes vacuum error. >> IGNITE-10170Unassigned .NET: Services.ServicesTestAsync fails >> IGNITE-10196Maxim Pudov Remove kafka-clients-*-test dependency >> IGNITE-10154Andrey Gura Critical worker liveness check >> configuration is non-trivial and inconsistent >> IGNITE-9996 Nikolay Izhikov Investigate possible performance drop in >> FSYNC mode for ignite-2.7 compared to ignite-2.6 >> >> >> В Чт, 08/11/2018 в 14:25 +0300, Nikolay Izhikov пишет: >> > I'm OK with this. >> > >> > чт, 8 нояб. 2018 г., 13:44 Andrey Gura ag...@apache.org: >> > > Long, long way to release :) >> > > >> > > Guys, we have a breaking change in Ignite 2.7 so we must add >> > > IGNITE-10154 [1] fix to the release. >> > > >> > > [1] https://issues.apache.org/jira/browse/IGNITE-10154 >> > > On Tue, Nov 6, 2018 at 6:30 PM Igor Sapego wrote: >> > > > >> > > > Guys, >> > > > >> > > > I've found the following issue: [1]. It is quite local (only affects >> > > > Ignite C++ Linux build system) but quite critical too. I think it >> > > > should be included in 2.7. >> > > > >> > > > What do you think? >> > > > >> > > > [1] - https://issues.apache.org/jira/browse/IGNITE-10147 >> > > > >> > > > Best Regards, >> > > > Igor >> > > > >> > > > >> > > > On Tue, Oct 30, 2018 at 4:35 PM Вячеслав Коптилин >> > > > >> > > > wrote: >> > > > >> > > > > Hello Nikolay, Igniters, >> > > > > >> > > > > It seems that we lost the following commit that should be included in >> > > > > 'ignite-2.7' branch >> > > > > (It looks like the change was not accidentally cherry-picked from >> > > > > 'master' >> > > > > to 'ignite-2.7') >> > > > > - >> > > > > >> > > > > https://github.com/apache/ignite/commit/6e0ff06f8e309657a16c94da605348d9c3b804ad >> > > > > >> > > > > The most important part is the change introduced into >> > > > > GridDhtAtomicCache, >> > > > > the fix prevents NullPointerException during cache updates under some >> > > > > circumstances. >> > > > > So, I propose including the fix into ignite-2.7, at least the change >> > > > > of >> > > > > GridDhtAtomicCache. >> > > > > >> > > > > Thanks, >> > > > > Slava. >> > > > > >> > > > > пн, 29 окт. 2018 г. в 11:20, Nikolay Izhikov : >> > > > > >> > > > > > Hello, guys. >> > > > > > >> > > > > > For today we have 11 tickets mapped to 2.7 >> > > > > > >> > > > > > IGNITE-10010 Alexey Goncharuk Node halted if second node was >> > > > > > stopped, >> > > > > then >> > > > > > cache destroyed, then second node returned >> > > > > > IGNITE-10015 Alexey Goncharuk Sporadic JVM crash due to restart >> > > > > > nodes >> > > > > > IGNITE-10013 Unassigned Node restart may lead to NPE in >> > > > > > GridDhtPartit
Re: Unreliable checks in tests for string presence in GridStringLogger contents
> waitForCondition(lsnr::check, timeout);
Agree, it is more convenient to use.
пт, 26 окт. 2018 г. в 13:01, Pavel Pereslegin :
>
> Nikita,
> personally, I don’t like that "check()" throws an AssertionError, but in
> the case of a composite listener, it will indicate which of the conditions
> did not work.
> Btw, your case can be solved with custom listener, but I think it's good
> improvement, let's do it.
>
> чт, 25 окт. 2018 г. в 21:31, Andrey Kuznetsov :
>
> > Nikita,
> >
> > I like your suggestion. It looks more expressive for me than existing
> > throwing version.
> >
> > чт, 25 окт. 2018 г. в 21:07, Nikita Amelchev :
> >
> > > Hi, Igniters.
> > >
> > > I suggest improving new listening test logger.
> > >
> > > I found usage case when needs wait for conditions for test duration
> > > optimization.
> > > For example, that messages A and B will be logged.
> > >
> > > For now, LogListener.check() doesn't return checking result as boolean.
> > > It throws the exception if conditions fail. Code for this case:
> > >
> > > waitForCondition(() -> {
> > > try {
> > >lsnr.check();
> > >
> > >return true;
> > > }
> > > catch (AssertionError ignored) {
> > >return false;
> > > }
> > > }, timeout);
> > >
> > > For code readability, I suggest make LogListener.check() with boolean
> > type:
> > >
> > > waitForCondition(lsnr::check, timeout);
> > >
> > > Also, it's more understandable when we write explicit assert in tests:
> > > assertTrue("Fail reason.", lsnr.check());
> > > ср, 23 мая 2018 г. в 14:36, Andrey Kuznetsov :
> > > >
> > > > Thanks, Vyacheslav.
> > > >
> > > > Created the issue [1] based on your idea.
> > > >
> > > > [1] https://issues.apache.org/jira/browse/IGNITE-8570
> > > >
> > > >
> > > > 2018-05-23 12:41 GMT+03:00 Vyacheslav Daradur :
> > > >
> > > > > Hi, Andrey, I have faced this problem too.
> > > > >
> > > > > I'd suggest introducing new logger for tests instead of extending API
> > > > > of *GridStringLogger*.
> > > > >
> > > > > The new logger should be some kind of *listened*, for example with
> > the
> > > > > folowing API:
> > > > >
> > > > > void addListener(String pattern, CountDownLatch latch);
> > > > > void addListener(IgniteInClosure lsnr);
> > > > >
> > > > > This approach reduces memory load in comparison with
> > > *GridStringLogger*.
> > > > >
> > > > > Just for example these should demonstrate my idea, *listened logger*
> > -
> > > > > [1], *listener* - [2]:
> > > > >
> > > > > [1] https://github.com/apache/ignite/blob/master/modules/
> > > > >
> > >
> > compatibility/src/test/java/org/apache/ignite/compatibility/testframework/
> > > > > junits/logger/ListenedGridTestLog4jLogger.java
> > > > > [2] https://github.com/apache/ignite/blob/master/modules/
> > > > >
> > >
> > compatibility/src/test/java/org/apache/ignite/compatibility/testframework/
> > > > > junits/IgniteCompatibilityAbstractTest.java#L304
> > > > >
> > > > >
> > > > >
> > > > --
> > > > Best regards,
> > > > Andrey Kuznetsov.
> > >
> > >
> > >
> > > --
> > > Best wishes,
> > > Amelchev Nikita
> > >
> >
> >
> > --
> > Best regards,
> > Andrey Kuznetsov.
> >
Re: Switching to real FailureHandler in tests
I tried to replace default no-op handler by handler stopping node and
failing the test.
I've returned the no-op handler in many classes because critical
situations are expected behavior. But PR still have a lot of failed
tests and suites. In some tests, I can't understand a failure reason.
I'm not finished to check failures, but after several RunAll runs, I
see new flaky tests (1 or 2 fails) appeared because of new handler.
I think we should keep no-op handler as default, but add new handler
for a few classes, where critical situations aren't expected.
пт, 21 сент. 2018 г. в 17:03, Andrey Kuznetsov :
>
> Thanks to all for participating the discussion.
>
> I've updated [1]: now it requires new handler from [2] for completion.
>
> [1] https://issues.apache.org/jira/browse/IGNITE-9660
> [2] https://issues.apache.org/jira/browse/IGNITE-8227
>
> чт, 20 сент. 2018 г. в 21:56, Vladimir Ozerov :
>
> > Stop node handler is not very good choice. Some test will continue work as
> > usual even if some node failed. E.g. SQL queries with backups may continue
> > function in some cases, especially if these are test with REPLICATED cache.
> >
> > New test-scope handler looks like a better candidate to me.
> >
> > чт, 20 сент. 2018 г. в 21:22, Andrey Kuznetsov :
> >
> > > I meant the first comment in [1]. We are to decide first whether we'll do
> > > it or not.
> > >
> > > [1] https://issues.apache.org/jira/browse/IGNITE-8227
> > > <
> > >
> > https://issues.apache.org/jira/browse/IGNITE-8227?focusedCommentId=16435298&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16435298
> > > >
> > >
> > > чт, 20 сент. 2018 г. в 21:18, Dmitriy Pavlov :
> > >
> > > > Sorry, incomplete message.
> > > >
> > > > Why do you think there is no consensus?
> > > >
> > > > I have no clue what can be a reason for another approach.
> > > > By default failure handler should fail all test.
> > > >
> > > > Failure handlers test will be always a minority of tests, so fail
> > handler
> > > > call is something abnormal.
> > > >
> > > > чт, 20 сент. 2018 г. в 21:15, Dmitriy Pavlov :
> > > >
> > > > > Why do you think there is no consensus?
> > > > >
> > > > > I have no clue that by default failure handler should fail all test.
> > > > >
> > > > > чт, 20 сент. 2018 г. в 21:10, Andrey Kuznetsov :
> > > > >
> > > > >> I've created [1] to address this.
> > > > >>
> > > > >> Dmitriy, I like your idea of creating special test-scope handler.
> > But
> > > > >> there
> > > > >> is no consensus about it, so I don't want to rely on that potential
> > > > >> handler
> > > > >> right now. We can switch to it later, of course.
> > > > >>
> > > > >> [1] https://issues.apache.org/jira/browse/IGNITE-9660
> > > > >>
> > > > >> чт, 20 сент. 2018 г. в 20:03, Maxim Muzafarov :
> > > > >>
> > > > >> > Andrey,
> > > > >> >
> > > > >> > I like your idea.
> > > > >> >
> > > > >> > After changing the default node failure handler to the new one we
> > > > should
> > > > >> > carefully review the whole new test failures. For instance,
> > calling
> > > > this
> > > > >> > method in tests should not lead test to the node being stopped:
> > > > >> >
> > > > >> > FOR TEST ONLY!!!
> > > > >> > TcpDiscoverySpi#simulateNodeFailure
> > > > >> >
> > > > >> > BTW, I would like to remove this method at all from production
> > code.
> > > > >> >
> > > > >> > On Thu, 20 Sep 2018 at 19:43 Dmitriy Pavlov <
> > dpavlov@gmail.com>
> > > > >> wrote:
> > > > >> >
> > > > >> > > But the totally ideal situation would be finding a way to fail
> > the
> > > > >> test
> > > > >> > by
> > > > >> > > default, not only stopping a node.
> > > > >> > >
> > > > >> > > Some time ago I've created
> > > > >> > > https://issues.apache.org/jira/browse/IGNITE-8227 to
> > > > >> > > find out a way to do so.
> > > > >> > >
> > > > >> > > чт, 20 сент. 2018 г. в 19:40, Dmitriy Pavlov <
> > > dpavlov@gmail.com
> > > > >:
> > > > >> > >
> > > > >> > > > ++1
> > > > >> > > >
> > > > >> > > > чт, 20 сент. 2018 г. в 19:39, Andrey Kuznetsov <
> > > stku...@gmail.com
> > > > >:
> > > > >> > > >
> > > > >> > > >> Igniters,
> > > > >> > > >>
> > > > >> > > >> While running tests I see a lot of ignored critical failures
> > > > >> caused by
> > > > >> > > >> {{NoOpFailureHandler}} that we use by default. In some tests,
> > > of
> > > > >> > cource,
> > > > >> > > >> critical failures are the part of normal workflow, but in the
> > > > >> majority
> > > > >> > > of
> > > > >> > > >> tests they indicate bugs. By using {{NoOpFailureHandler}} we
> > > just
> > > > >> hide
> > > > >> > > >> these bugs from ourselves.
> > > > >> > > >>
> > > > >> > > >> What do you think about changing default handler to
> > > > >> > > >> {{StopNodeFailureHandler}} at {{GridAbstractTest}} level?
> > This
> > > > >> could
> > > > >> > be
> > > > >> > > >> overridden in subclasses.
> > > > >> > > >>
> > > > >> > > >> --
> > > > >> > > >> Best regards,
> > > > >> > > >> Andrey Kuznetsov.
> > > > >> > > >>
> >
Re: Abbreviation code-style requirement.
+1 to > leave abbreviations for common words with single meaning. For example, group -> grp, transaction -> tx, context -> ctx. And optional for the multi-words. ср, 17 окт. 2018 г. в 14:54, Ilya Lantukh : > + 1 from me to make abbreviations optional. > > On Wed, Oct 17, 2018 at 1:00 PM Sergey Antonov > wrote: > > > + 1 > > > > But, I think that we must leave abbreviations for common words with > single > > meaning. For example, group -> grp, transaction -> tx, context -> ctx. > > > > ср, 17 окт. 2018 г. в 12:46, Alexey Zinoviev : > > > > > + 1 > > > I dislike the current list of abbreviations. It gives me a pain to > > support > > > code with unclear variables naming, also I agree that we should avoid > > crazy > > > Java camel long naming like > > > FactoryBuildingCrazyAffinityCallerForComibingInSpace but instead that > we > > > make shorter clear concepts like /counter/, /vertex/, /collection/ and > > etc > > > > > > > > > > > > -- > > > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/ > > > > > > > > > -- > > BR, Sergey Antonov > > > > > -- > Best regards, > Ilya >
Apache Ignite TeamCity Bot - wiki page available
Hello, Igniters! You've all seen the messages with new failures from MTCGA.Bot. But the bot can also be used to analyze TeamCity runs and to print tests analysis results in the JIRA ticket. Now you can learn how to work with the bot at the wiki page [1]. [1] https://cwiki.apache.org/confluence/display/IGNITE/Apache+Ignite+Teamcity+Bot
Re: TeamCity Helper's wiki page
Thank you, I'll do it today.
2018-09-24 18:19 GMT+03:00 Dmitriy Pavlov :
> Awesome, thanks! Added you to list of contributors.
>
> Feel free to create a new page under https://cwiki.apache.
> org/confluence/display/IGNITE/Testing+and+benchmarking
>
> Sincerely,
> Dmitriy Pavlov
>
> пн, 24 сент. 2018 г. в 15:39, Dmitrii Ryabov :
>
>> I signed up. My username - somefire.
>>
>> 2018-09-24 15:08 GMT+03:00 Dmitriy Pavlov :
>>
>>> Dmitrii,
>>>
>>> could you please sign up to the wiki and share your ID. I will set
>>> necessary permissions.
>>>
>>> Sincerely,
>>> Dmitriy Pavlov
>>>
>>> пн, 24 сент. 2018 г. в 14:57, Dmitrii Ryabov :
>>>
>>> > Hello, Igniters!
>>> >
>>> > I propose to create page about TeamCity Helper on the wiki and move
>>> here
>>> > information about Helper from the MTCGA page.
>>> >
>>> > Also, I want to add instruction about how to use TCH to comment JIRA:
>>> >
>>> > 1. Start "Run All" build for PR on TeamCity.
>>> >
>>> > 2. Check failures and fix possible blockers.
>>> > * Go to https://mtcga.gridgain.com/
>>> > * Fill the form "Check branch/PR" with branch from TeamCity
>>> > ("pull//head"). Leave base branch empty and press
>>> > "Latest" button to compare with the latest master.
>>> >
>>> > 3. Comment JIRA ticket with TeamCity Helper message containing analisys
>>> > results:
>>> > * Go to https://mtcga.gridgain.com/services.html
>>> > * Fill the form "Notify JIRA" with branch from TeamCity (or
>>> > "" only). If PR is named according to contributing
>>> > guide (name starts with "IGNITE-"), you can leave ticket field empty.
>>> > Otherwise - enter JIRA ticket number.
>>> >
>>>
>>
>>
Re: TeamCity Helper's wiki page
I signed up. My username - somefire.
2018-09-24 15:08 GMT+03:00 Dmitriy Pavlov :
> Dmitrii,
>
> could you please sign up to the wiki and share your ID. I will set
> necessary permissions.
>
> Sincerely,
> Dmitriy Pavlov
>
> пн, 24 сент. 2018 г. в 14:57, Dmitrii Ryabov :
>
> > Hello, Igniters!
> >
> > I propose to create page about TeamCity Helper on the wiki and move here
> > information about Helper from the MTCGA page.
> >
> > Also, I want to add instruction about how to use TCH to comment JIRA:
> >
> > 1. Start "Run All" build for PR on TeamCity.
> >
> > 2. Check failures and fix possible blockers.
> > * Go to https://mtcga.gridgain.com/
> > * Fill the form "Check branch/PR" with branch from TeamCity
> > ("pull//head"). Leave base branch empty and press
> > "Latest" button to compare with the latest master.
> >
> > 3. Comment JIRA ticket with TeamCity Helper message containing analisys
> > results:
> > * Go to https://mtcga.gridgain.com/services.html
> > * Fill the form "Notify JIRA" with branch from TeamCity (or
> > "" only). If PR is named according to contributing
> > guide (name starts with "IGNITE-"), you can leave ticket field empty.
> > Otherwise - enter JIRA ticket number.
> >
>
Include TeamCity Helper into contribution process
Hello, Igniters! I propose to update the "How to Contribute" page to include TeamCity Helper's JIRA commenting functionality into contribution process. This will speedup review process by highlighting new failures, timeouts and othe critical failures in the JIRA ticket. For example, we can replace next text on the "How to Contribute" page * Once tests are passed, the pull request can be reviewed and merged by a committer. Move a corresponding JIRA ticket to "Patch Available" state by clicking on "Submit Patch" button and let the community know that you're ready for review. replace to * Once tests are passed, use TeamCity Helper(https://mtcga.gridgain.com/) to analyze tests and comment(https://mtcga.gridgain.com/services.html) JIRA ticket with the results. * Fix found failures (possible blockers) and rerun tests. * Restart suites with timeout if you sure that they failed not because of PR. * Once TeamCity Helper has shown the absence of possible blockers, the pull request can be reviewed and merged by a committer. Move a corresponding JIRA ticket to "Patch Available" state by clicking on "Submit Patch" button and let the community know that you're ready for review.
TeamCity Helper's wiki page
Hello, Igniters!
I propose to create page about TeamCity Helper on the wiki and move here
information about Helper from the MTCGA page.
Also, I want to add instruction about how to use TCH to comment JIRA:
1. Start "Run All" build for PR on TeamCity.
2. Check failures and fix possible blockers.
* Go to https://mtcga.gridgain.com/
* Fill the form "Check branch/PR" with branch from TeamCity
("pull//head"). Leave base branch empty and press
"Latest" button to compare with the latest master.
3. Comment JIRA ticket with TeamCity Helper message containing analisys
results:
* Go to https://mtcga.gridgain.com/services.html
* Fill the form "Notify JIRA" with branch from TeamCity (or
"" only). If PR is named according to contributing
guide (name starts with "IGNITE-"), you can leave ticket field empty.
Otherwise - enter JIRA ticket number.
Re: [GitHub] SomeFire commented on a change in pull request #14: Performance
Hi, Dmitiy,
I created ticket [1] 3 days ago.
[1] https://issues.apache.org/jira/browse/INFRA-17032
2018-09-19 20:51 GMT+03:00 Dmitriy Pavlov :
> Hi Dmitriy Ryabov,
>
> could you please create and share INFRA ticket to disable comment
> forwarding to the dev list?
>
> Sincerely,
> Dmitriy Pavlov
>
> ср, 19 сент. 2018 г. в 19:02, GitBox :
>
> > SomeFire commented on a change in pull request #14: Performance
> > URL:
> > https://github.com/apache/ignite-teamcity-bot/pull/14#
> discussion_r218863424
> >
> >
> >
> > ##
> > File path:
> > ignite-tc-helper-web/src/main/java/org/apache/ignite/ci/
> issue/IssueDetector.java
> > ##
> > @@ -386,15 +386,17 @@ private void checkFailures() {
> > private void checkFailuresEx() {
> > int buildsToQry =
> > EventTemplates.templates.stream().mapToInt(
> EventTemplate::cntEvents).max().getAsInt();
> >
> > +ExecutorService executor =
> > MoreExecutors.newDirectExecutorService();
> > +
> >
> > GetTrackedBranchTestResults.getTrackedBranchTestFailures(
> FullQueryParams.DEFAULT_BRANCH_NAME,
> > -false, buildsToQry, backgroundOpsTcHelper,
> > backgroundOpsCreds);
> > +false, buildsToQry, backgroundOpsTcHelper,
> > backgroundOpsCreds, executor);
> >
> > TestFailuresSummary failures =
> >
> > GetTrackedBranchTestResults.getTrackedBranchTestFailures(
> FullQueryParams.DEFAULT_BRANCH_NAME,
> > false,
> > 1,
> > backgroundOpsTcHelper,
> > -backgroundOpsCreds);
> > +backgroundOpsCreds, executor);
> >
> > Review comment:
> >Move executor to a new line.
> >
> >
> > This is an automated message from the Apache Git Service.
> > To respond to the message, please log on GitHub and use the
> > URL above to go to the specific comment.
> >
> > For queries about this service, please contact Infrastructure at:
> > us...@infra.apache.org
> >
> >
> > With regards,
> > Apache Git Services
> >
>
Re: [MTCGA]: new failures in builds [1871897] needs to be handled
Hi, Dmitriy, I checked 7618 and previous commits: test fails locally starting from 7618. It fails because `cache.get()` remembers deactivated state and doesn't check current state. 2018-09-20 18:41 GMT+03:00 Dmitriy Pavlov : > Hi, > > IgniteStandByClusterTest seems to fail, Dmitriy G., Ivan, would it be > reasonable to revert commit? > > Dmitriy Ryabov, is it related to recent fix or is it a standalone problem? > > Sincerely, > Dmitriy Pavlov > > пн, 17 сент. 2018 г. в 18:45, Dmitrii Ryabov : > > > Looks like problem I had described in the ticket. > > > > > > https://issues.apache.org/jira/browse/IGNITE-7618? > focusedCommentId=16506923&page=com.atlassian.jira. > plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16506923 > > > > 2018-09-15 12:01 GMT+03:00 Dmitriy Pavlov : > > > > > Dmitriy G, Ivan B, > > > > > > could you please double-check if failure is not coming from > > > https://issues.apache.org/jira/browse/IGNITE-7618 > > > > > > Sincerely, > > > Dmitriy Pavlov > > > > > > сб, 15 сент. 2018 г. в 5:42, : > > > > > > > Hi Ignite Developer, > > > > > > > > I am MTCGA.Bot, and I've detected some issue on TeamCity to be > > addressed. > > > > I hope you can help. > > > > > > > > *New test failure in master IgniteStandByClusterTest.testSimple > > > > https://ci.ignite.apache.org/project.html?projectId= > > > IgniteTests24Java8&testNameId=1332314705000986815&branch=% > > > 3Cdefault%3E&tab=testDetails > > > > Changes may led to failure were done by > > > > - bessonov.ip > > > > http://ci.ignite.apache.org/viewModification.html?modId= > > > 831651&personal=false > > > > > > > > - If your changes can led to this failure(s), please create > > issue > > > > with label MakeTeamCityGreenAgain and assign it to you. > > > > -- If you have fix, please set ticket to PA state and write > to > > > dev > > > > list fix is ready > > > > -- For case fix will require some time please mute test and > set > > > > label Muted_Test to issue > > > > - If you know which change caused failure please contact > change > > > > author directly > > > > - If you don't know which change caused failure please send > > > > message to dev list to find out > > > > Should you have any questions please contact dev@ignite.apache.org > > > > Best Regards, > > > > MTCGA.Bot > > > > Notification generated at Sat Sep 15 05:42:21 MSK 2018 > > > > > > > > > >
***UNCHECKED*** TeamCity Bot can comment ticket with PR failures
Hello, Igniters! If you din't saw the "Apache Ignite TeamCity Bot: Features overview" webinar, you should know: Ignite TC Bot can detect failures appeared in the Pull Request and comment JIRA ticket with this failures. If PR has no new failures - bot will comment about the absence of such failures. Unfortunately, the process is not automated yet, so you need to start it manually. You can do it on services page [1] of Ignite TeamCity Helper. Just write the PR number and press the "Notify" button. Example: Branch [4741] Ticket [9026]. You can skip ticket number if PR title starts with "IGNITE-". To learn more about TC Bot see wiki [2] or ask on devlist. [1] https://mtcga.gridgain.com/services.html [2] https://cwiki.apache.org/confluence/display/IGNITE/Make+Teamcity+Green+Again#MakeTeamcityGreenAgain-MTCGABot
Re: [MTCGA]: new failures in builds [1871897] needs to be handled
Looks like problem I had described in the ticket. https://issues.apache.org/jira/browse/IGNITE-7618?focusedCommentId=16506923&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16506923 2018-09-15 12:01 GMT+03:00 Dmitriy Pavlov : > Dmitriy G, Ivan B, > > could you please double-check if failure is not coming from > https://issues.apache.org/jira/browse/IGNITE-7618 > > Sincerely, > Dmitriy Pavlov > > сб, 15 сент. 2018 г. в 5:42, : > > > Hi Ignite Developer, > > > > I am MTCGA.Bot, and I've detected some issue on TeamCity to be addressed. > > I hope you can help. > > > > *New test failure in master IgniteStandByClusterTest.testSimple > > https://ci.ignite.apache.org/project.html?projectId= > IgniteTests24Java8&testNameId=1332314705000986815&branch=% > 3Cdefault%3E&tab=testDetails > > Changes may led to failure were done by > > - bessonov.ip > > http://ci.ignite.apache.org/viewModification.html?modId= > 831651&personal=false > > > > - If your changes can led to this failure(s), please create issue > > with label MakeTeamCityGreenAgain and assign it to you. > > -- If you have fix, please set ticket to PA state and write to > dev > > list fix is ready > > -- For case fix will require some time please mute test and set > > label Muted_Test to issue > > - If you know which change caused failure please contact change > > author directly > > - If you don't know which change caused failure please send > > message to dev list to find out > > Should you have any questions please contact dev@ignite.apache.org > > Best Regards, > > MTCGA.Bot > > Notification generated at Sat Sep 15 05:42:21 MSK 2018 > > >
Re: TeamCity Helper shows new failures in PRs
Hi, Dmitriy, Webinar is good, but we need a page on the wiki (or "help" in the bot) for new contributors. After that, I think we should add mention about helper to the contributing guide. 2018-09-14 9:43 GMT+03:00 Nikolay Izhikov : > Hello, Dmitriy. > > Webinar is a great idea, let's have it. > > В Пт, 14/09/2018 в 09:39 +0300, Dmitriy Pavlov пишет: > > Hi Folks, > > > > There is no description of this feature yet. But I think we should come > > back to the idea of running a webinar. > > > > Please share your vision. If Igniters will attend I send an invitation to > > the beginning of next week. > > > > I hope in the nearest time we will have a few more features to speak > about. > > > > Sincerely, > > Dmitriy Pavlov > > > > чт, 13 сент. 2018 г. в 22:58, Nikolay Izhikov : > > > > > Hello, Dmitrii. > > > > > > Can you give a link to detailed description of this feature? > > > > > > Actually, I wonder if someone except bit developers know how to use it > > > > > > чт, 13 сент. 2018 г., 22:46 Dmitrii Ryabov : > > > > > > > Hello, Igniters! > > > > > > > > We've improved tests analysis in the TeamCity Helper. > > > > > > > > Now, when you view PR analysis, you can see a table with possible > > > > > > blockers > > > > - failed tests and suites, that most likely appeared in the pull > request. > > > > >
TeamCity Helper shows new failures in PRs
Hello, Igniters! We've improved tests analysis in the TeamCity Helper. Now, when you view PR analysis, you can see a table with possible blockers - failed tests and suites, that most likely appeared in the pull request.
Re: Workflow improvement
Yep, it's working. Thank you. 2018-09-13 19:09 GMT+03:00 Dmitriy Pavlov : > Hi Dmitriy, > > Sure, I agree with extra permissions assignment. Done. > > Could you please check if you can manage your builds now? > > Sincerely, > Dmitriy Pavlov > > чт, 13 сент. 2018 г. в 16:05, Dmitrii Ryabov : > > > Hi, Dmitriy, > > > > Can you give me rights to stop my builds on TeamCity? Working on the > TCH, I > > run a lot of builds, and I don't like to ask other people to stop builds > > too often. > > > > 2018-09-10 23:53 GMT+03:00 Dmitrii Ryabov : > > > > > Hi, Dmitriy, > > > > > > I made PRs in my fork for test purposes. Real PRs were made to the > GitHub > > > mirror, and one of them is already merged by D. Govorukhin. PR with > > GitHub > > > statuses [1] is ready for review. PR with JIRA comment will be ready > in a > > > few days. > > > > > > [1] https://github.com/apache/ignite-teamcity-bot/pull/5 > > > > > > 2018-09-10 23:00 GMT+03:00 Dmitriy Pavlov : > > > > > >> Hi Dmitrii, > > >> > > >> I deployed change with blockers summary of failures at the top of PR > > >> result > > >> page. The Bot is migrating entries now, I hope it will be done in 1-4 > > >> hours. > > >> > > >> I noticed your PRs are created in your own fork, not in > > >> https://github.com/apache/ignite-teamcity-bot > > >> Could you please create unmerged PR in GitHub mirror so it could be > > merged > > >> after reviewing by the apply-pr script. > > >> > > >> Sincerely, > > >> Dmitriy Pavlov > > >> > > >> пн, 3 сент. 2018 г. в 18:20, Dmitrii Ryabov : > > >> > > >> > Hi, Dmitriy, > > >> > > > >> > I created a table with possible blockers [1] for the page with the > > >> latest > > >> > build analysis. Anton K. has reviewed it. Can you check and merge > it? > > >> > > > >> > Also, I created a button to notify PR about analisis results [2]. I > > used > > >> > GitHub statuses (example [3]), but to work it needs a token with > push > > >> > permission. As Apache PMC, can you acquire such token? I think > > statuses > > >> are > > >> > much more notable than comments. > > >> > > > >> > [1] https://issues.apache.org/jira/browse/IGNITE-9376 > > >> > [2] https://issues.apache.org/jira/browse/IGNITE-9377 > > >> > [3] https://github.com/SomeFire/ignite-teamcity-bot/pulls > > >> > > > >> > 2018-08-25 1:04 GMT+03:00 Dmitriy Pavlov : > > >> > > > >> > > Not sure I understood what bot statistic reduction means. > > >> > > > > >> > > As far as I understood, you also mean locating failure > automatically > > >> with > > >> > > re-runs on specific git revisions (hashes). I found it will be a > > good > > >> > > contribution to TC Bot for both flaky and non-flaky failures > > >> detection. > > >> > E.g > > >> > > failure occurred after 10 commits merged to master. How to find > out > > >> bad > > >> > > commit? Automatic location (analog of bisecting, but using TC > test) > > >> will > > >> > be > > >> > > really helpful. But it is not a simple contribution. > > >> > > > > >> > > пт, 24 авг. 2018 г. в 18:35, Dmitrii Ryabov < > somefire...@gmail.com > > >: > > >> > > > > >> > > > Hi, Dmitriy, > > >> > > > > > >> > > > Good, I'll create tickets for this steps. > > >> > > > > > >> > > > Stable suites are a good idea, but also we need to do some > actions > > >> > when a > > >> > > > flaky test will appear in stable suite of master branch run. To > > find > > >> > out > > >> > > a > > >> > > > guilty commit, we should run previous commits on empty > TeamCity's > > >> > queue. > > >> > > > This runs will reduce bot's statistics for the latest commits. > > >> > > > > > >> > > > 2018-08-24 16:32 GMT+03:00 Dmitriy Pavlov < > dpavlov@gmail.com > > >: > > >> >
Re: Workflow improvement
Hi, Dmitriy, Can you give me rights to stop my builds on TeamCity? Working on the TCH, I run a lot of builds, and I don't like to ask other people to stop builds too often. 2018-09-10 23:53 GMT+03:00 Dmitrii Ryabov : > Hi, Dmitriy, > > I made PRs in my fork for test purposes. Real PRs were made to the GitHub > mirror, and one of them is already merged by D. Govorukhin. PR with GitHub > statuses [1] is ready for review. PR with JIRA comment will be ready in a > few days. > > [1] https://github.com/apache/ignite-teamcity-bot/pull/5 > > 2018-09-10 23:00 GMT+03:00 Dmitriy Pavlov : > >> Hi Dmitrii, >> >> I deployed change with blockers summary of failures at the top of PR >> result >> page. The Bot is migrating entries now, I hope it will be done in 1-4 >> hours. >> >> I noticed your PRs are created in your own fork, not in >> https://github.com/apache/ignite-teamcity-bot >> Could you please create unmerged PR in GitHub mirror so it could be merged >> after reviewing by the apply-pr script. >> >> Sincerely, >> Dmitriy Pavlov >> >> пн, 3 сент. 2018 г. в 18:20, Dmitrii Ryabov : >> >> > Hi, Dmitriy, >> > >> > I created a table with possible blockers [1] for the page with the >> latest >> > build analysis. Anton K. has reviewed it. Can you check and merge it? >> > >> > Also, I created a button to notify PR about analisis results [2]. I used >> > GitHub statuses (example [3]), but to work it needs a token with push >> > permission. As Apache PMC, can you acquire such token? I think statuses >> are >> > much more notable than comments. >> > >> > [1] https://issues.apache.org/jira/browse/IGNITE-9376 >> > [2] https://issues.apache.org/jira/browse/IGNITE-9377 >> > [3] https://github.com/SomeFire/ignite-teamcity-bot/pulls >> > >> > 2018-08-25 1:04 GMT+03:00 Dmitriy Pavlov : >> > >> > > Not sure I understood what bot statistic reduction means. >> > > >> > > As far as I understood, you also mean locating failure automatically >> with >> > > re-runs on specific git revisions (hashes). I found it will be a good >> > > contribution to TC Bot for both flaky and non-flaky failures >> detection. >> > E.g >> > > failure occurred after 10 commits merged to master. How to find out >> bad >> > > commit? Automatic location (analog of bisecting, but using TC test) >> will >> > be >> > > really helpful. But it is not a simple contribution. >> > > >> > > пт, 24 авг. 2018 г. в 18:35, Dmitrii Ryabov : >> > > >> > > > Hi, Dmitriy, >> > > > >> > > > Good, I'll create tickets for this steps. >> > > > >> > > > Stable suites are a good idea, but also we need to do some actions >> > when a >> > > > flaky test will appear in stable suite of master branch run. To find >> > out >> > > a >> > > > guilty commit, we should run previous commits on empty TeamCity's >> > queue. >> > > > This runs will reduce bot's statistics for the latest commits. >> > > > >> > > > 2018-08-24 16:32 GMT+03:00 Dmitriy Pavlov : >> > > > >> > > > > Hi Dmitriy, >> > > > > >> > > > > Sounds like a plan ;) I totally agree. >> > > > > Just one proposal: I would like to avoid hiding any test >> failures. 2 >> > > > > separate tables named 'Possible Blockers' and 'Other failures' >> should >> > > be >> > > > > completely clear. Comment to PR can contain only first category. >> > > > > >> > > > > We had a private discussion with Anton K. and he proposed a very >> > > > > interesting thing, I would like to bring it here. We can add >> > > > configuration >> > > > > into TC bot and mark some tests and some suites as 'Known Stable' >> It >> > > > means >> > > > > that suite or test failure should be considered as a blocker for >> > merge >> > > > > every time it fails, even if fail rate is non-zero. Very first >> list >> > of >> > > > such >> > > > > suites are >> > > > > - Build Apache Ignite >> > > > > - License check >> > > > > And tests: >> > > > > - .Net API Parity c
Re: Workflow improvement
Hi, Dmitriy, I made PRs in my fork for test purposes. Real PRs were made to the GitHub mirror, and one of them is already merged by D. Govorukhin. PR with GitHub statuses [1] is ready for review. PR with JIRA comment will be ready in a few days. [1] https://github.com/apache/ignite-teamcity-bot/pull/5 2018-09-10 23:00 GMT+03:00 Dmitriy Pavlov : > Hi Dmitrii, > > I deployed change with blockers summary of failures at the top of PR result > page. The Bot is migrating entries now, I hope it will be done in 1-4 > hours. > > I noticed your PRs are created in your own fork, not in > https://github.com/apache/ignite-teamcity-bot > Could you please create unmerged PR in GitHub mirror so it could be merged > after reviewing by the apply-pr script. > > Sincerely, > Dmitriy Pavlov > > пн, 3 сент. 2018 г. в 18:20, Dmitrii Ryabov : > > > Hi, Dmitriy, > > > > I created a table with possible blockers [1] for the page with the latest > > build analysis. Anton K. has reviewed it. Can you check and merge it? > > > > Also, I created a button to notify PR about analisis results [2]. I used > > GitHub statuses (example [3]), but to work it needs a token with push > > permission. As Apache PMC, can you acquire such token? I think statuses > are > > much more notable than comments. > > > > [1] https://issues.apache.org/jira/browse/IGNITE-9376 > > [2] https://issues.apache.org/jira/browse/IGNITE-9377 > > [3] https://github.com/SomeFire/ignite-teamcity-bot/pulls > > > > 2018-08-25 1:04 GMT+03:00 Dmitriy Pavlov : > > > > > Not sure I understood what bot statistic reduction means. > > > > > > As far as I understood, you also mean locating failure automatically > with > > > re-runs on specific git revisions (hashes). I found it will be a good > > > contribution to TC Bot for both flaky and non-flaky failures detection. > > E.g > > > failure occurred after 10 commits merged to master. How to find out bad > > > commit? Automatic location (analog of bisecting, but using TC test) > will > > be > > > really helpful. But it is not a simple contribution. > > > > > > пт, 24 авг. 2018 г. в 18:35, Dmitrii Ryabov : > > > > > > > Hi, Dmitriy, > > > > > > > > Good, I'll create tickets for this steps. > > > > > > > > Stable suites are a good idea, but also we need to do some actions > > when a > > > > flaky test will appear in stable suite of master branch run. To find > > out > > > a > > > > guilty commit, we should run previous commits on empty TeamCity's > > queue. > > > > This runs will reduce bot's statistics for the latest commits. > > > > > > > > 2018-08-24 16:32 GMT+03:00 Dmitriy Pavlov : > > > > > > > > > Hi Dmitriy, > > > > > > > > > > Sounds like a plan ;) I totally agree. > > > > > Just one proposal: I would like to avoid hiding any test failures. > 2 > > > > > separate tables named 'Possible Blockers' and 'Other failures' > should > > > be > > > > > completely clear. Comment to PR can contain only first category. > > > > > > > > > > We had a private discussion with Anton K. and he proposed a very > > > > > interesting thing, I would like to bring it here. We can add > > > > configuration > > > > > into TC bot and mark some tests and some suites as 'Known Stable' > It > > > > means > > > > > that suite or test failure should be considered as a blocker for > > merge > > > > > every time it fails, even if fail rate is non-zero. Very first list > > of > > > > such > > > > > suites are > > > > > - Build Apache Ignite > > > > > - License check > > > > > And tests: > > > > > - .Net API Parity check > > > > > Please share your vision. > > > > > > > > > > Meanwhile, I've updated the TC bot. > > > > > - Underlying Apache Ignite DB was refactored to use lower > partitions > > > > count, > > > > > so restart should be faster. > > > > > - Now 100 builds are saved as our failure rate statistics basis. > > > > Flakiness > > > > > border was not changed, so more test now will be considered as > flaky. > > > > > - Now 4 builds required to be failed or timed out in a row before > > > > > notif
Relevance of savepoints support inside of Ignite Transactions
Hi, Igniters! I have an implementation for savepoints support inside of transactions, which was reviewed by Boikov a year ago. It allows us to rollback piece of the transaction, not whole tx. Current restrictions: * Doesn't work with near caches. * MVCC should be disabled. MVCC was merged recently. So, I want to know – is ticket actual now? JIRA - https://issues.apache.org/jira/browse/IGNITE-4188 Design - https://docs.google.com/document/d/1IxTEVrjY44nsyZUUW1f0sGwuzahiPhy5nV-vFS20FZE/edit?usp=sharing PR - https://github.com/apache/ignite/pull/1815 Upsource - https://reviews.ignite.apache.org/ignite/review/IGNT-CR-186 Discussion - http://apache-ignite-developers.2346864.n4.nabble.com/TX-savepoints-td12041.html
Re: Workflow improvement
Hi, Dmitriy, I created a table with possible blockers [1] for the page with the latest build analysis. Anton K. has reviewed it. Can you check and merge it? Also, I created a button to notify PR about analisis results [2]. I used GitHub statuses (example [3]), but to work it needs a token with push permission. As Apache PMC, can you acquire such token? I think statuses are much more notable than comments. [1] https://issues.apache.org/jira/browse/IGNITE-9376 [2] https://issues.apache.org/jira/browse/IGNITE-9377 [3] https://github.com/SomeFire/ignite-teamcity-bot/pulls 2018-08-25 1:04 GMT+03:00 Dmitriy Pavlov : > Not sure I understood what bot statistic reduction means. > > As far as I understood, you also mean locating failure automatically with > re-runs on specific git revisions (hashes). I found it will be a good > contribution to TC Bot for both flaky and non-flaky failures detection. E.g > failure occurred after 10 commits merged to master. How to find out bad > commit? Automatic location (analog of bisecting, but using TC test) will be > really helpful. But it is not a simple contribution. > > пт, 24 авг. 2018 г. в 18:35, Dmitrii Ryabov : > > > Hi, Dmitriy, > > > > Good, I'll create tickets for this steps. > > > > Stable suites are a good idea, but also we need to do some actions when a > > flaky test will appear in stable suite of master branch run. To find out > a > > guilty commit, we should run previous commits on empty TeamCity's queue. > > This runs will reduce bot's statistics for the latest commits. > > > > 2018-08-24 16:32 GMT+03:00 Dmitriy Pavlov : > > > > > Hi Dmitriy, > > > > > > Sounds like a plan ;) I totally agree. > > > Just one proposal: I would like to avoid hiding any test failures. 2 > > > separate tables named 'Possible Blockers' and 'Other failures' should > be > > > completely clear. Comment to PR can contain only first category. > > > > > > We had a private discussion with Anton K. and he proposed a very > > > interesting thing, I would like to bring it here. We can add > > configuration > > > into TC bot and mark some tests and some suites as 'Known Stable' It > > means > > > that suite or test failure should be considered as a blocker for merge > > > every time it fails, even if fail rate is non-zero. Very first list of > > such > > > suites are > > > - Build Apache Ignite > > > - License check > > > And tests: > > > - .Net API Parity check > > > Please share your vision. > > > > > > Meanwhile, I've updated the TC bot. > > > - Underlying Apache Ignite DB was refactored to use lower partitions > > count, > > > so restart should be faster. > > > - Now 100 builds are saved as our failure rate statistics basis. > > Flakiness > > > border was not changed, so more test now will be considered as flaky. > > > - Now 4 builds required to be failed or timed out in a row before > > > notification is sent. > > > > > > Sincerely, > > > Dmitriy Pavlov > > > > > > чт, 23 авг. 2018 г. в 17:09, Dmitrii Ryabov : > > > > > > > Hi, Dmitriy, > > > > > > > > I propose the next steps: > > > > > > > > 1. Show current 0% tests in a separate table at the top of the > analysis > > > > results page. Thus, we'll see most suspicious (new or very flaky) > tests > > > > firstly. Or we can hide other tests under "More >>" button, like top > > long > > > > running tests. > > > > 2. Create a button by clicking on which the info about 0% tests will > be > > > > written in the PR. > > > > 3. Replace button by webhook for TeamCity (for Run All), which will > > start > > > > analysis on TCH and write results in the PR. > > > > 4. ... > > > > > > > > What do you think? > > > > > > > > > > > > 2018-08-22 8:55 GMT+03:00 Dmitrii Ryabov : > > > > > > > > > I think we should check not N last runs, but all runs in last N > days. > > > > > > > > > > A simple rule to detect flaky fails by hands - get test history > > ordered > > > > > by TEST_STATUS_DESC and check its date. As I see, we can get list > of > > > > > failures from TC. We don't need to check successfull runs, because > > they > > > > are > > > > > uninformative for our needs. > > > > >
Re: Bots on dev list
> 2. MTCGA messages should mention the commiter and authors of the "bad" commit. But this is not always possible. 2018-08-30 13:04 GMT+03:00 Dmitrii Ryabov : > Vladimir, your idea is good, but... > 1. Who should be mentioned in messages like PR or JIRA ticket creation? As > I see, only these messages clutter up the nabble. > 2. MTCGA messages should mention the commiter and authors of the "bad" > commit. > 2. Comments and changes in PR, tickets, and reviews already are sent to > the users, who subscribed to them. > > 2018-08-30 12:19 GMT+03:00 Vladimir Ozerov : > >> This is somewhat controversial question. Some people has filters, some >> doesn't. And it s true that at the moment it is hard to find pieces of >> human communication in tons of automated e-mails (JIRA, GitHub, TC bot). >> If to put JIRA and GitHub aside (let's do not mix things), personally >> content of TC bot looks not very good. I doubt that contributors known how >> to react to it. If we want contributors to take actions on specific >> problem, we should send emails directly to them, not to everyone. >> >> For me good solution looks as follows: >> 1) Create separate channel for bot messages >> 2) "To" field should contain this channel and e-mails of specific people >> >> Thoughts? >> >> On Thu, Aug 30, 2018 at 10:41 AM Denis Mekhanikov >> wrote: >> >> > I guess, nobody uses it because it's flooded by bots. >> > It may be useful to lookup old threads for people, who are not >> subscribed >> > to the mailing list from the beginning of time. >> > >> > Denis >> > >> > чт, 30 авг. 2018 г. в 3:39, Dmitriy Setrakyan : >> > >> > > Is anyone in the community using or was using Nabble for the dev list >> > > communication? Personally, I am subscribed to the dev list and use >> > filters >> > > in my email client. >> > > >> > > D. >> > > >> > > On Wed, Aug 29, 2018 at 3:40 AM, Denis Mekhanikov < >> dmekhani...@gmail.com >> > > >> > > wrote: >> > > >> > > > Guys, >> > > > >> > > > Yep, I use filters on my mail account. But the portal is impossible >> to >> > > use. >> > > > When you subscribe to the dev list for the first time, you don't >> have >> > any >> > > > history on your email, >> > > > and the archive is polluted with messages, sent by bots. >> > > > >> > > > Some view on Nabble, that doesn't contain any automatically >> generated >> > > > messages, would help here. >> > > > >> > > > Denis >> > > > >> > > > ср, 29 авг. 2018 г. в 12:26, Dmitrii Ryabov > >: >> > > > >> > > > > Modern mail services allow users to filter messages. You can >> easily >> > > > filter >> > > > > out bot messages. >> > > > > >> > > > > 2018-08-29 11:48 GMT+03:00 Denis Mekhanikov < >> dmekhani...@gmail.com>: >> > > > > >> > > > > > Igniters, >> > > > > > >> > > > > > We have a lot of threads, created by bots on the dev list. >> > > > > > Currently messages are sent by JIRA, GitHub and MTCGA bots. >> Maybe, >> > > some >> > > > > > others too, but these are the most active. >> > > > > > >> > > > > > Take a look at this page: >> > > > > > http://apache-ignite-developers.2346864.n4.nabble. >> > > > > > com/Apache-Ignite-Developers-f1i35.html >> > > > > > It's hard to find actual discussions in this mess. I'd like to >> see >> > > > > > something like what we have on the users list: >> > > > > > http://apache-ignite-users.70518.x6.nabble.com/ >> > > > > > >> > > > > > It doesn't seem necessary to me to mix discussions with this >> > cryptic >> > > > flow >> > > > > > of information. >> > > > > > Can we create a separate mailing list for bots? >> > > > > > >> > > > > > Denis >> > > > > > >> > > > > >> > > > >> > > >> > >> > >
Re: Bots on dev list
Vladimir, your idea is good, but... 1. Who should be mentioned in messages like PR or JIRA ticket creation? As I see, only these messages clutter up the nabble. 2. MTCGA messages should mention the commiter and authors of the "bad" commit. 2. Comments and changes in PR, tickets, and reviews already are sent to the users, who subscribed to them. 2018-08-30 12:19 GMT+03:00 Vladimir Ozerov : > This is somewhat controversial question. Some people has filters, some > doesn't. And it s true that at the moment it is hard to find pieces of > human communication in tons of automated e-mails (JIRA, GitHub, TC bot). > If to put JIRA and GitHub aside (let's do not mix things), personally > content of TC bot looks not very good. I doubt that contributors known how > to react to it. If we want contributors to take actions on specific > problem, we should send emails directly to them, not to everyone. > > For me good solution looks as follows: > 1) Create separate channel for bot messages > 2) "To" field should contain this channel and e-mails of specific people > > Thoughts? > > On Thu, Aug 30, 2018 at 10:41 AM Denis Mekhanikov > wrote: > > > I guess, nobody uses it because it's flooded by bots. > > It may be useful to lookup old threads for people, who are not subscribed > > to the mailing list from the beginning of time. > > > > Denis > > > > чт, 30 авг. 2018 г. в 3:39, Dmitriy Setrakyan : > > > > > Is anyone in the community using or was using Nabble for the dev list > > > communication? Personally, I am subscribed to the dev list and use > > filters > > > in my email client. > > > > > > D. > > > > > > On Wed, Aug 29, 2018 at 3:40 AM, Denis Mekhanikov < > dmekhani...@gmail.com > > > > > > wrote: > > > > > > > Guys, > > > > > > > > Yep, I use filters on my mail account. But the portal is impossible > to > > > use. > > > > When you subscribe to the dev list for the first time, you don't have > > any > > > > history on your email, > > > > and the archive is polluted with messages, sent by bots. > > > > > > > > Some view on Nabble, that doesn't contain any automatically generated > > > > messages, would help here. > > > > > > > > Denis > > > > > > > > ср, 29 авг. 2018 г. в 12:26, Dmitrii Ryabov : > > > > > > > > > Modern mail services allow users to filter messages. You can > easily > > > > filter > > > > > out bot messages. > > > > > > > > > > 2018-08-29 11:48 GMT+03:00 Denis Mekhanikov >: > > > > > > > > > > > Igniters, > > > > > > > > > > > > We have a lot of threads, created by bots on the dev list. > > > > > > Currently messages are sent by JIRA, GitHub and MTCGA bots. > Maybe, > > > some > > > > > > others too, but these are the most active. > > > > > > > > > > > > Take a look at this page: > > > > > > http://apache-ignite-developers.2346864.n4.nabble. > > > > > > com/Apache-Ignite-Developers-f1i35.html > > > > > > It's hard to find actual discussions in this mess. I'd like to > see > > > > > > something like what we have on the users list: > > > > > > http://apache-ignite-users.70518.x6.nabble.com/ > > > > > > > > > > > > It doesn't seem necessary to me to mix discussions with this > > cryptic > > > > flow > > > > > > of information. > > > > > > Can we create a separate mailing list for bots? > > > > > > > > > > > > Denis > > > > > > > > > > > > > > > > > > > > >
Re: TeamCity's security certificate expired
Thank you. 2018-08-29 13:33 GMT+03:00 Alexey Goncharuk : > Certificates are updated, TC is up and running. > > ср, 29 авг. 2018 г. в 12:55, Dmitrii Ryabov : > > > Hi, Igniters! > > > > Who can refresh TeamCity's security certificate? > > >
TeamCity's security certificate expired
Hi, Igniters! Who can refresh TeamCity's security certificate?
Re: Bots on dev list
Modern mail services allow users to filter messages. You can easily filter out bot messages. 2018-08-29 11:48 GMT+03:00 Denis Mekhanikov : > Igniters, > > We have a lot of threads, created by bots on the dev list. > Currently messages are sent by JIRA, GitHub and MTCGA bots. Maybe, some > others too, but these are the most active. > > Take a look at this page: > http://apache-ignite-developers.2346864.n4.nabble. > com/Apache-Ignite-Developers-f1i35.html > It's hard to find actual discussions in this mess. I'd like to see > something like what we have on the users list: > http://apache-ignite-users.70518.x6.nabble.com/ > > It doesn't seem necessary to me to mix discussions with this cryptic flow > of information. > Can we create a separate mailing list for bots? > > Denis >
Re: Workflow improvement
Hi, Dmitriy, Good, I'll create tickets for this steps. Stable suites are a good idea, but also we need to do some actions when a flaky test will appear in stable suite of master branch run. To find out a guilty commit, we should run previous commits on empty TeamCity's queue. This runs will reduce bot's statistics for the latest commits. 2018-08-24 16:32 GMT+03:00 Dmitriy Pavlov : > Hi Dmitriy, > > Sounds like a plan ;) I totally agree. > Just one proposal: I would like to avoid hiding any test failures. 2 > separate tables named 'Possible Blockers' and 'Other failures' should be > completely clear. Comment to PR can contain only first category. > > We had a private discussion with Anton K. and he proposed a very > interesting thing, I would like to bring it here. We can add configuration > into TC bot and mark some tests and some suites as 'Known Stable' It means > that suite or test failure should be considered as a blocker for merge > every time it fails, even if fail rate is non-zero. Very first list of such > suites are > - Build Apache Ignite > - License check > And tests: > - .Net API Parity check > Please share your vision. > > Meanwhile, I've updated the TC bot. > - Underlying Apache Ignite DB was refactored to use lower partitions count, > so restart should be faster. > - Now 100 builds are saved as our failure rate statistics basis. Flakiness > border was not changed, so more test now will be considered as flaky. > - Now 4 builds required to be failed or timed out in a row before > notification is sent. > > Sincerely, > Dmitriy Pavlov > > чт, 23 авг. 2018 г. в 17:09, Dmitrii Ryabov : > > > Hi, Dmitriy, > > > > I propose the next steps: > > > > 1. Show current 0% tests in a separate table at the top of the analysis > > results page. Thus, we'll see most suspicious (new or very flaky) tests > > firstly. Or we can hide other tests under "More >>" button, like top long > > running tests. > > 2. Create a button by clicking on which the info about 0% tests will be > > written in the PR. > > 3. Replace button by webhook for TeamCity (for Run All), which will start > > analysis on TCH and write results in the PR. > > 4. ... > > > > What do you think? > > > > > > 2018-08-22 8:55 GMT+03:00 Dmitrii Ryabov : > > > > > I think we should check not N last runs, but all runs in last N days. > > > > > > A simple rule to detect flaky fails by hands - get test history ordered > > > by TEST_STATUS_DESC and check its date. As I see, we can get list of > > > failures from TC. We don't need to check successfull runs, because they > > are > > > uninformative for our needs. > > > > > > 2018-08-21 20:24 GMT+03:00 Dmitriy Pavlov : > > > > > >> Hi Dmitriy, > > >> > > >> The Bot is able to detect a frequent change of test status, but > > currently > > >> only 50 last runs count. Same is true for the failure rate. > > >> > > >> This value can be easily changed to 70 or 100, moreover, the auto > > >> trigger feature gives us much more builds. > > >> > > >> We can improve these rules. We can add not only status change, but > > status > > >> change without any code changes. We can somehow save this data in > > RunStat > > >> class. Let's create a better rule, and later we can code it. > > >> > > >> Sincerely, > > >> Dmitriy Pavlov > > >> > > >> вт, 21 авг. 2018 г. в 19:22, Dmitrii Ryabov : > > >> > > >> > I think plugin will be more pretty looking, but comments can contain > > any > > >> > information, so they can be more usefull. I agree with your idea to > > >> create > > >> > bot instead of plugin. > > >> > > > >> > As for fail rate - I'm not sure it is working as you describe. > > >> > I'm looking on my runAll [1]. There is > > >> > `IgniteCacheGroupsTest.testCacheApiTxReplicated` > > >> > in `Cache 3` suite with fail rate = 0.0%. But it is flaky and fails > in > > >> > master branch [2]. > > >> > > > >> > [1] > > >> > > > >> > https://mtcga.gridgain.com/pr.html?serverId=apache&suiteId=I > > >> gniteTests24Java8_RunAll&branchForTc=pull%2F4519%2Fhead&action=Latest > > >> > [2] > > >> > > > >> > https://ci.ignite.apache.org/project.html?
Re: Workflow improvement
Hi, Dmitriy, I propose the next steps: 1. Show current 0% tests in a separate table at the top of the analysis results page. Thus, we'll see most suspicious (new or very flaky) tests firstly. Or we can hide other tests under "More >>" button, like top long running tests. 2. Create a button by clicking on which the info about 0% tests will be written in the PR. 3. Replace button by webhook for TeamCity (for Run All), which will start analysis on TCH and write results in the PR. 4. ... What do you think? 2018-08-22 8:55 GMT+03:00 Dmitrii Ryabov : > I think we should check not N last runs, but all runs in last N days. > > A simple rule to detect flaky fails by hands - get test history ordered > by TEST_STATUS_DESC and check its date. As I see, we can get list of > failures from TC. We don't need to check successfull runs, because they are > uninformative for our needs. > > 2018-08-21 20:24 GMT+03:00 Dmitriy Pavlov : > >> Hi Dmitriy, >> >> The Bot is able to detect a frequent change of test status, but currently >> only 50 last runs count. Same is true for the failure rate. >> >> This value can be easily changed to 70 or 100, moreover, the auto >> trigger feature gives us much more builds. >> >> We can improve these rules. We can add not only status change, but status >> change without any code changes. We can somehow save this data in RunStat >> class. Let's create a better rule, and later we can code it. >> >> Sincerely, >> Dmitriy Pavlov >> >> вт, 21 авг. 2018 г. в 19:22, Dmitrii Ryabov : >> >> > I think plugin will be more pretty looking, but comments can contain any >> > information, so they can be more usefull. I agree with your idea to >> create >> > bot instead of plugin. >> > >> > As for fail rate - I'm not sure it is working as you describe. >> > I'm looking on my runAll [1]. There is >> > `IgniteCacheGroupsTest.testCacheApiTxReplicated` >> > in `Cache 3` suite with fail rate = 0.0%. But it is flaky and fails in >> > master branch [2]. >> > >> > [1] >> > >> > https://mtcga.gridgain.com/pr.html?serverId=apache&suiteId=I >> gniteTests24Java8_RunAll&branchForTc=pull%2F4519%2Fhead&action=Latest >> > [2] >> > >> > https://ci.ignite.apache.org/project.html?projectId=IgniteTe >> sts24Java8&buildTypeId=&tab=testDetails&testNameId=5628470 >> 782089555961&order=TEST_STATUS_DESC&branch_IgniteTests >> 24Java8=__all_branches__&itemsCount=50 >> > >> > 2018-08-21 18:00 GMT+03:00 Dmitriy Pavlov : >> > >> > > Hi Dmitrii, >> > > >> > > I'm not sure we're able to install Github apps to Apache mirrors. >> > > >> > > The simplest solution, what can be as efficient as a plugin, is fake >> > MTCGA >> > > bot account in Github, which will provide PR comments using Github >> > program >> > > interface. What do you think? >> > > >> > > A new test failure can be identified by the Ignite TC Bot by master >> > recent >> > > fail rate = 0.0%. The same rule can be applied to timed out suites. >> > > >> > > Sincerely, >> > > Dmitriy Pavlov >> > > >> > > вт, 21 авг. 2018 г. в 16:16, Dmitrii Ryabov : >> > > >> > > > Hello, Igniters! >> > > > >> > > > I want to suggest improvement for TeamCity Helper [1] – we need an >> easy >> > > way >> > > > to get list of failed tests that don’t fall in the master branch. >> These >> > > > tests should: >> > > > * fail in the PR >> > > > * not fail in the master >> > > > * not be flaky. >> > > > >> > > > Also, I want to suggest to create a GitHub plugin, which will >> notify PR >> > > if >> > > > it has such tests. PR will have a marker, which allows/prohibits >> merge. >> > > > This marker will be shown near PR conflicts. >> > > > >> > > > Allowing marker will be shown in case: >> > > > * no new fails. >> > > > >> > > > Prohibiting marker will be shown in cases: >> > > > * new fails – tests must be fixed. >> > > > * new timed out test suite – suite should be restarted or tests >> must be >> > > > fixed. >> > > > * runAll wasn’t launched – tests must be launched. >> > > > >> > > > This will make test checks much faster and easier. Also, this will >> > > decrease >> > > > the number of merges with new failed tests made by inattention to >> the >> > > > tests. >> > > > >> > > > Further, we can expand the plugin by adding new checks, showing PR >> > > quality. >> > > > >> > > > [1] https://github.com/apache/ignite-teamcity-bot >> > > > >> > > >> > >> > >
Re: Workflow improvement
I think we should check not N last runs, but all runs in last N days. A simple rule to detect flaky fails by hands - get test history ordered by TEST_STATUS_DESC and check its date. As I see, we can get list of failures from TC. We don't need to check successfull runs, because they are uninformative for our needs. 2018-08-21 20:24 GMT+03:00 Dmitriy Pavlov : > Hi Dmitriy, > > The Bot is able to detect a frequent change of test status, but currently > only 50 last runs count. Same is true for the failure rate. > > This value can be easily changed to 70 or 100, moreover, the auto > trigger feature gives us much more builds. > > We can improve these rules. We can add not only status change, but status > change without any code changes. We can somehow save this data in RunStat > class. Let's create a better rule, and later we can code it. > > Sincerely, > Dmitriy Pavlov > > вт, 21 авг. 2018 г. в 19:22, Dmitrii Ryabov : > > > I think plugin will be more pretty looking, but comments can contain any > > information, so they can be more usefull. I agree with your idea to > create > > bot instead of plugin. > > > > As for fail rate - I'm not sure it is working as you describe. > > I'm looking on my runAll [1]. There is > > `IgniteCacheGroupsTest.testCacheApiTxReplicated` > > in `Cache 3` suite with fail rate = 0.0%. But it is flaky and fails in > > master branch [2]. > > > > [1] > > > > https://mtcga.gridgain.com/pr.html?serverId=apache&suiteId= > IgniteTests24Java8_RunAll&branchForTc=pull%2F4519%2Fhead&action=Latest > > [2] > > > > https://ci.ignite.apache.org/project.html?projectId=IgniteTests24Java8&; > buildTypeId=&tab=testDetails&testNameId=5628470782089555961&order= > TEST_STATUS_DESC&branch_IgniteTests24Java8=__all_branches__&itemsCount=50 > > > > 2018-08-21 18:00 GMT+03:00 Dmitriy Pavlov : > > > > > Hi Dmitrii, > > > > > > I'm not sure we're able to install Github apps to Apache mirrors. > > > > > > The simplest solution, what can be as efficient as a plugin, is fake > > MTCGA > > > bot account in Github, which will provide PR comments using Github > > program > > > interface. What do you think? > > > > > > A new test failure can be identified by the Ignite TC Bot by master > > recent > > > fail rate = 0.0%. The same rule can be applied to timed out suites. > > > > > > Sincerely, > > > Dmitriy Pavlov > > > > > > вт, 21 авг. 2018 г. в 16:16, Dmitrii Ryabov : > > > > > > > Hello, Igniters! > > > > > > > > I want to suggest improvement for TeamCity Helper [1] – we need an > easy > > > way > > > > to get list of failed tests that don’t fall in the master branch. > These > > > > tests should: > > > > * fail in the PR > > > > * not fail in the master > > > > * not be flaky. > > > > > > > > Also, I want to suggest to create a GitHub plugin, which will notify > PR > > > if > > > > it has such tests. PR will have a marker, which allows/prohibits > merge. > > > > This marker will be shown near PR conflicts. > > > > > > > > Allowing marker will be shown in case: > > > > * no new fails. > > > > > > > > Prohibiting marker will be shown in cases: > > > > * new fails – tests must be fixed. > > > > * new timed out test suite – suite should be restarted or tests must > be > > > > fixed. > > > > * runAll wasn’t launched – tests must be launched. > > > > > > > > This will make test checks much faster and easier. Also, this will > > > decrease > > > > the number of merges with new failed tests made by inattention to the > > > > tests. > > > > > > > > Further, we can expand the plugin by adding new checks, showing PR > > > quality. > > > > > > > > [1] https://github.com/apache/ignite-teamcity-bot > > > > > > > > > >
Re: Workflow improvement
I think plugin will be more pretty looking, but comments can contain any information, so they can be more usefull. I agree with your idea to create bot instead of plugin. As for fail rate - I'm not sure it is working as you describe. I'm looking on my runAll [1]. There is `IgniteCacheGroupsTest.testCacheApiTxReplicated` in `Cache 3` suite with fail rate = 0.0%. But it is flaky and fails in master branch [2]. [1] https://mtcga.gridgain.com/pr.html?serverId=apache&suiteId=IgniteTests24Java8_RunAll&branchForTc=pull%2F4519%2Fhead&action=Latest [2] https://ci.ignite.apache.org/project.html?projectId=IgniteTests24Java8&buildTypeId=&tab=testDetails&testNameId=5628470782089555961&order=TEST_STATUS_DESC&branch_IgniteTests24Java8=__all_branches__&itemsCount=50 2018-08-21 18:00 GMT+03:00 Dmitriy Pavlov : > Hi Dmitrii, > > I'm not sure we're able to install Github apps to Apache mirrors. > > The simplest solution, what can be as efficient as a plugin, is fake MTCGA > bot account in Github, which will provide PR comments using Github program > interface. What do you think? > > A new test failure can be identified by the Ignite TC Bot by master recent > fail rate = 0.0%. The same rule can be applied to timed out suites. > > Sincerely, > Dmitriy Pavlov > > вт, 21 авг. 2018 г. в 16:16, Dmitrii Ryabov : > > > Hello, Igniters! > > > > I want to suggest improvement for TeamCity Helper [1] – we need an easy > way > > to get list of failed tests that don’t fall in the master branch. These > > tests should: > > * fail in the PR > > * not fail in the master > > * not be flaky. > > > > Also, I want to suggest to create a GitHub plugin, which will notify PR > if > > it has such tests. PR will have a marker, which allows/prohibits merge. > > This marker will be shown near PR conflicts. > > > > Allowing marker will be shown in case: > > * no new fails. > > > > Prohibiting marker will be shown in cases: > > * new fails – tests must be fixed. > > * new timed out test suite – suite should be restarted or tests must be > > fixed. > > * runAll wasn’t launched – tests must be launched. > > > > This will make test checks much faster and easier. Also, this will > decrease > > the number of merges with new failed tests made by inattention to the > > tests. > > > > Further, we can expand the plugin by adding new checks, showing PR > quality. > > > > [1] https://github.com/apache/ignite-teamcity-bot > > >
Workflow improvement
Hello, Igniters! I want to suggest improvement for TeamCity Helper [1] – we need an easy way to get list of failed tests that don’t fall in the master branch. These tests should: * fail in the PR * not fail in the master * not be flaky. Also, I want to suggest to create a GitHub plugin, which will notify PR if it has such tests. PR will have a marker, which allows/prohibits merge. This marker will be shown near PR conflicts. Allowing marker will be shown in case: * no new fails. Prohibiting marker will be shown in cases: * new fails – tests must be fixed. * new timed out test suite – suite should be restarted or tests must be fixed. * runAll wasn’t launched – tests must be launched. This will make test checks much faster and easier. Also, this will decrease the number of merges with new failed tests made by inattention to the tests. Further, we can expand the plugin by adding new checks, showing PR quality. [1] https://github.com/apache/ignite-teamcity-bot
Re: IP finder in tests
We have ticket [1] for this change. [1] https://issues.apache.org/jira/browse/IGNITE-6826 2018-08-01 15:58 GMT+03:00 Alexey Zinoviev : > Great, sometimes I made it manually in local tests. It will be great to > change the situation. > > 2018-08-01 18:22 GMT+06:00 Stanislav Lukyanov : > > > +1. > > > > I don’t see why do we need to fallback to multicast for multi-JVM – let’s > > just set 127.0.0.1:47500..47509 by default, > > it’ll be enough for most (if not all) tests. > > > > Stan > > > > From: Dmitriy Pavlov > > Sent: 1 августа 2018 г. 14:21 > > To: dev@ignite.apache.org > > Subject: Re: IP finder in tests > > > > Hi Denis, > > > > Thank you for bringing the question here. > > > > I totally support this change. > > > > Sincerely, > > Dmitriy Pavlov > > > > ср, 1 авг. 2018 г. в 13:23, Denis Mekhanikov : > > > > > Igniters, > > > > > > Almost every test in Ignite project has the following pattern: shared > > > *TcpDiscoveryVmIpFinder > > > *is defined as a field of a test class, which is then used in discovery > > > configuration in *getConfiguration(...)* method. There are more than > 700 > > > test classes with this setup. > > > > > > But for some reason *TcpDiscoveryMulticastIpFinder *is used in tests by > > > default. I don't think, that it should be used in tests at all, since > > > external nodes may accidentally affect test results. > > > > > > The only case, where it makes sense is multi JVM tests. Shared static > IP > > > finder is not applicable there, since nodes are run in different JVMs > and > > > cannot share the same IP finder object. > > > > > > I would like to change the default IP finder to a shared > > > *TcpDiscoveryVmIpFinder. > > > *In cases, when *GridAbstractTest#isMultiJvm() *returns *true*, we > could > > > fall back to multicast. > > > > > > What do you think? > > > > > > Denis > > > > > > > >
Re: Deprecating LOCAL cache
Vladimir, do we have a ticket for cross-cache transactions with LOCAL cache? I can't find it. In my task I met this situation in tests and I want to fail such tests with link to this ticket. 2018-07-25 12:55 GMT+03:00 Vladimir Ozerov : > Dima, > > LOCAL cache adds very little value to the product. It doesn't support > cross-cache transactions, consumes a lot of memory, much slower than any > widely-used concurrent hash map. Let's go the same way as Java - mark LOCAL > cache as "deprecated for removal", and then remove it in 3.0. > > On Wed, Jul 25, 2018 at 12:10 PM Dmitrii Ryabov > wrote: > > > +1 to make LOCAL as filtered PARTITIONED cache. I think it would be much > > easier and faster than fixing all bugs. > > > > 2018-07-25 11:51 GMT+03:00 Dmitriy Setrakyan : > > > > > I would stay away from deprecating such huge pieces as a whole LOCAL > > cache. > > > In retrospect, we should probably not even have LOCAL caches, but now I > > am > > > certain that it is used by many users. > > > > > > I would do one of the following, whichever one is easier: > > > > > >- Fix the issues found with LOCAL caches, including persistence > > support > > >- Implement LOCAL caches as PARTITIONED caches over the local node. > In > > >this case, we would have to hide any distribution-related config > from > > >users, like affinity function, for example. > > > > > > D. > > > > > > On Wed, Jul 25, 2018 at 9:05 AM, Valentin Kulichenko < > > > valentin.kuliche...@gmail.com> wrote: > > > > > > > It sounds like the main drawback of LOCAL cache is that it's > > implemented > > > > separately and therefore has to be maintained separately. If that's > the > > > > only issue, why not keep LOCAL cache mode on public API, but > implement > > it > > > > as a PARTITIONED cache with a node filter forcefully set? That's > > similar > > > to > > > > what we do with REPLICATED caches which are actually PARTITIONED with > > > > infinite number of backups. > > > > > > > > This way we fix the issues described by Stan and don't have to > > deprecate > > > > anything. > > > > > > > > -Val > > > > > > > > On Wed, Jul 25, 2018 at 12:53 AM Stanislav Lukyanov < > > > > stanlukya...@gmail.com> > > > > wrote: > > > > > > > > > Hi Igniters, > > > > > > > > > > I’d like to start a discussion about the deprecation of the LOCAL > > > caches. > > > > > > > > > > LOCAL caches are an edge-case functionality > > > > > I haven’t done any formal analysis, but from my experience LOCAL > > caches > > > > > are needed very rarely, if ever. > > > > > I think most usages of LOCAL caches I’ve seen were misuses: the > users > > > > > actually needed a simple HashMap, or an actual PARTITIONED cache. > > > > > > > > > > LOCAL caches are easy to implement on top of PARTITIONED > > > > > If one requires a LOCAL cache (which is itself questionable, as > > > discussed > > > > > above) it is quite easy to implement one on top of PARTITIONED > cache. > > > > > A node filter of form `node -> node.id().equals(localNodeId)` is > > > enough > > > > > to make the cache to be stored on the node that created it. > > > > > Locality of access to the cache (i.e. making it unavailable from > > other > > > > > nodes) can be achieved on the application level. > > > > > > > > > > LOCAL caches are hard to maintain > > > > > A quick look at the open issues mentioning “local cache” suggests > > that > > > > > this is a corner case for implementation of many Ignite features: > > > > > > > > > > https://issues.apache.org/jira/issues/?jql=text%20~%20% > > > > 22local%20cache%22%20and%20%20project%20%3D%20IGNITE% > > > > 20and%20status%20%3D%20open > > > > > In particular, a recent SO question brought up the fact that LOCAL > > > caches > > > > > don’t support native persistence: > > > > > > > > > > https://stackoverflow.com/questions/51511892/how-to- > > > > configure-persistent-storage-for-apache-ignite-cache > > > > > Having to ask ourselves “how does it play with LOCAL caches” every > > time > > > > we > > > > > write any code in Ignite seems way to much for the benefits we gain > > > from > > > > it. > > > > > > > > > > Proposal > > > > > Let’s deprecate LOCAL caches in 2.x and remove them in 3.0. > > > > > As a part of deprecation let’s do the following: > > > > > - Put @Deprecated on the CacheMode.LOCAL > > > > > - Print a warning every time a LOCAL cache is created > > > > > - Remove all mentions of LOCAL caches from readme.io, if any, > except > > > for > > > > > the page about cache modes > > > > > - On the page about cache modes explain that LOCAL is deprecated > and > > > can > > > > > be replaced with a PARTITIONED cache with a node filter > > > > > > > > > > Thanks, > > > > > Stan > > > > > > > > > > > > > > >
Re: Deprecating LOCAL cache
Vladimir, do we have a ticket for cross-cache transactions with LOCAL cache? I can't find it. In my task I met this situation in tests and I want to fail such tests with link to this ticket. Also documentation in readme.io say nothing about cross-cache failures with LOCAL cache. ср, 25 июл. 2018 г., 13:55 Stanislav Lukyanov : > To me it also raises a question of having REPLICATED :) > But that might seem too radical, so let’s decide on the LOCAL first. > > I don’t mind keeping LOCAL as an alias for PARTITIONED with a default node > filter, > but I don’t see much benefit in it. Why to have a shortcut for something > used by 1% (or less?) when there is a way to do the same with a few more > lines? > Compatibility is a reason to keep things as they are, but that’s why I > suggest to only remove it in 3.0 when we’ll have a chance to do some > cleaning. > > Stan > > From: Valentin Kulichenko > Sent: 25 июля 2018 г. 11:09 > To: dev@ignite.apache.org > Subject: Re: Deprecating LOCAL cache > > It sounds like the main drawback of LOCAL cache is that it's implemented > separately and therefore has to be maintained separately. If that's the > only issue, why not keep LOCAL cache mode on public API, but implement it > as a PARTITIONED cache with a node filter forcefully set? That's similar to > what we do with REPLICATED caches which are actually PARTITIONED with > infinite number of backups. > > This way we fix the issues described by Stan and don't have to deprecate > anything. > > -Val > > On Wed, Jul 25, 2018 at 12:53 AM Stanislav Lukyanov < > stanlukya...@gmail.com> > wrote: > > > Hi Igniters, > > > > I’d like to start a discussion about the deprecation of the LOCAL caches. > > > > LOCAL caches are an edge-case functionality > > I haven’t done any formal analysis, but from my experience LOCAL caches > > are needed very rarely, if ever. > > I think most usages of LOCAL caches I’ve seen were misuses: the users > > actually needed a simple HashMap, or an actual PARTITIONED cache. > > > > LOCAL caches are easy to implement on top of PARTITIONED > > If one requires a LOCAL cache (which is itself questionable, as discussed > > above) it is quite easy to implement one on top of PARTITIONED cache. > > A node filter of form `node -> node.id().equals(localNodeId)` is enough > > to make the cache to be stored on the node that created it. > > Locality of access to the cache (i.e. making it unavailable from other > > nodes) can be achieved on the application level. > > > > LOCAL caches are hard to maintain > > A quick look at the open issues mentioning “local cache” suggests that > > this is a corner case for implementation of many Ignite features: > > > > > https://issues.apache.org/jira/issues/?jql=text%20~%20%22local%20cache%22%20and%20%20project%20%3D%20IGNITE%20and%20status%20%3D%20open > > In particular, a recent SO question brought up the fact that LOCAL caches > > don’t support native persistence: > > > > > https://stackoverflow.com/questions/51511892/how-to-configure-persistent-storage-for-apache-ignite-cache > > Having to ask ourselves “how does it play with LOCAL caches” every time > we > > write any code in Ignite seems way to much for the benefits we gain from > it. > > > > Proposal > > Let’s deprecate LOCAL caches in 2.x and remove them in 3.0. > > As a part of deprecation let’s do the following: > > - Put @Deprecated on the CacheMode.LOCAL > > - Print a warning every time a LOCAL cache is created > > - Remove all mentions of LOCAL caches from readme.io, if any, except for > > the page about cache modes > > - On the page about cache modes explain that LOCAL is deprecated and can > > be replaced with a PARTITIONED cache with a node filter > > > > Thanks, > > Stan > > > >
Re: Deprecating LOCAL cache
+1 to make LOCAL as filtered PARTITIONED cache. I think it would be much easier and faster than fixing all bugs. 2018-07-25 11:51 GMT+03:00 Dmitriy Setrakyan : > I would stay away from deprecating such huge pieces as a whole LOCAL cache. > In retrospect, we should probably not even have LOCAL caches, but now I am > certain that it is used by many users. > > I would do one of the following, whichever one is easier: > >- Fix the issues found with LOCAL caches, including persistence support >- Implement LOCAL caches as PARTITIONED caches over the local node. In >this case, we would have to hide any distribution-related config from >users, like affinity function, for example. > > D. > > On Wed, Jul 25, 2018 at 9:05 AM, Valentin Kulichenko < > valentin.kuliche...@gmail.com> wrote: > > > It sounds like the main drawback of LOCAL cache is that it's implemented > > separately and therefore has to be maintained separately. If that's the > > only issue, why not keep LOCAL cache mode on public API, but implement it > > as a PARTITIONED cache with a node filter forcefully set? That's similar > to > > what we do with REPLICATED caches which are actually PARTITIONED with > > infinite number of backups. > > > > This way we fix the issues described by Stan and don't have to deprecate > > anything. > > > > -Val > > > > On Wed, Jul 25, 2018 at 12:53 AM Stanislav Lukyanov < > > stanlukya...@gmail.com> > > wrote: > > > > > Hi Igniters, > > > > > > I’d like to start a discussion about the deprecation of the LOCAL > caches. > > > > > > LOCAL caches are an edge-case functionality > > > I haven’t done any formal analysis, but from my experience LOCAL caches > > > are needed very rarely, if ever. > > > I think most usages of LOCAL caches I’ve seen were misuses: the users > > > actually needed a simple HashMap, or an actual PARTITIONED cache. > > > > > > LOCAL caches are easy to implement on top of PARTITIONED > > > If one requires a LOCAL cache (which is itself questionable, as > discussed > > > above) it is quite easy to implement one on top of PARTITIONED cache. > > > A node filter of form `node -> node.id().equals(localNodeId)` is > enough > > > to make the cache to be stored on the node that created it. > > > Locality of access to the cache (i.e. making it unavailable from other > > > nodes) can be achieved on the application level. > > > > > > LOCAL caches are hard to maintain > > > A quick look at the open issues mentioning “local cache” suggests that > > > this is a corner case for implementation of many Ignite features: > > > > > > https://issues.apache.org/jira/issues/?jql=text%20~%20% > > 22local%20cache%22%20and%20%20project%20%3D%20IGNITE% > > 20and%20status%20%3D%20open > > > In particular, a recent SO question brought up the fact that LOCAL > caches > > > don’t support native persistence: > > > > > > https://stackoverflow.com/questions/51511892/how-to- > > configure-persistent-storage-for-apache-ignite-cache > > > Having to ask ourselves “how does it play with LOCAL caches” every time > > we > > > write any code in Ignite seems way to much for the benefits we gain > from > > it. > > > > > > Proposal > > > Let’s deprecate LOCAL caches in 2.x and remove them in 3.0. > > > As a part of deprecation let’s do the following: > > > - Put @Deprecated on the CacheMode.LOCAL > > > - Print a warning every time a LOCAL cache is created > > > - Remove all mentions of LOCAL caches from readme.io, if any, except > for > > > the page about cache modes > > > - On the page about cache modes explain that LOCAL is deprecated and > can > > > be replaced with a PARTITIONED cache with a node filter > > > > > > Thanks, > > > Stan > > > > > >
Re: IGNITE-4188, savepoints with atomic cache?
Ok, go #1. I'll create ticket for defaults change and make appropriate changes in current PR with link to new ticket. 2018-06-14 10:56 GMT+03:00 Dmitry Karachentsev : > I would vote for 1 or 3 (this I like more), but not 2 as such breaking > change involves more pain to our users. Let it be in 3.0 and included in > migration guide, I don't see much drawbacks here. > > Thanks! > > 13.06.2018 19:20, Ivan Rakov пишет: > > +1 to Eduard. >> >> It's a reasonable change, but we can't just break working code of all the >> guys that access atomic caches in their transactions. If we try, we will >> end up with another emergency release. >> >> Best Regards, >> Ivan Rakov >> >> On 13.06.2018 19:13, Eduard Shangareev wrote: >> >>> Guys, >>> >>> I believe, that it's not the case when we should change default >>> behaviour. >>> So, #1 and make it default in 3.0. >>> >>> On Wed, Jun 13, 2018 at 6:46 PM, Dmitrii Ryabov >>> wrote: >>> >>> Vote for #2. I think no one will change this defaults in configuration in >>>> #1. >>>> >>>> 2018-06-13 18:29 GMT+03:00 Anton Vinogradov : >>>> >>>> Vote for #2 since it can shed light on hidden bug at production. >>>>> >>>>> ср, 13 июн. 2018 г. в 18:10, Alexey Goncharuk < >>>>> >>>> alexey.goncha...@gmail.com >>>> >>>>> : >>>>>> Igniters, >>>>>> >>>>>> Bumping up this discussion. The fix has been implemented and it is >>>>>> fine >>>>>> from the technical point of view, but since the fix did not make it to >>>>>> >>>>> the >>>>> >>>>>> Ignite 2.0, the implemented fix [1] now will be a breaking change for >>>>>> current Ignite users. >>>>>> >>>>>> I see the following options: >>>>>> 1) Have the fix merged, but do not change the defaults - atomic caches >>>>>> >>>>> will >>>>> >>>>>> still be allowed in transactions by default and only configuration >>>>>> >>>>> change >>>> >>>>> will make Ignite throw exceptions in this case >>>>>> 2) Have the fix merged as is and describe this change in the release >>>>>> >>>>> notes >>>>> >>>>>> 3) Postpone the fix until Ignite 3.0 >>>>>> >>>>>> I would vote for option #1 and change only the defaults in Ignite 3.0. >>>>>> >>>>>> Thoughts? >>>>>> >>>>>> [1] https://issues.apache.org/jira/browse/IGNITE-2313 >>>>>> >>>>>> ср, 5 апр. 2017 г. в 22:53, Дмитрий Рябов : >>>>>> >>>>>> IGNITE-2313 done, can you review it? >>>>>>> >>>>>>> PR: https://github.com/apache/ignite/pull/1709/files >>>>>>> JIRA: https://issues.apache.org/jira/browse/IGNITE-2313 >>>>>>> CI: http://ci.ignite.apache.org/viewType.html?buildTypeId= >>>>>>> IgniteTests_RatJavadoc&branch_IgniteTests=pull%2F1709% >>>>>>> 2Fhead&tab=buildTypeStatusDiv >>>>>>> >>>>>>> 2017-03-29 20:58 GMT+03:00 Denis Magda : >>>>>>> >>>>>>> Sorry, I get lost in tickets. >>>>>>>> >>>>>>>> Yes, IGNITE-2313 has to be completed in 2.0 if we want to makes >>>>>>>> >>>>>>> this >>>> >>>>> change. >>>>>>>> >>>>>>>> — >>>>>>>> Denis >>>>>>>> >>>>>>>> On Mar 29, 2017, at 2:12 AM, Дмитрий Рябов < >>>>>>>>> >>>>>>>> somefire...@gmail.com> >>>> >>>>> wrote: >>>>>>>> >>>>>>>>> Savepoints marked for 2.1, exceptions for 2.0. Do you want me to >>>>>>>>> >>>>>>>> make >>>>> >>>>>> exceptions first? >>>>>>>>> >>>>>>>>> 2017-03-29 11:24 GMT+03:00 Дмитрий Рябов >>>>>>>> >>>>>>>> : >>>&
Re: IGNITE-4188, savepoints with atomic cache?
Vote for #2. I think no one will change this defaults in configuration in #1. 2018-06-13 18:29 GMT+03:00 Anton Vinogradov : > Vote for #2 since it can shed light on hidden bug at production. > > ср, 13 июн. 2018 г. в 18:10, Alexey Goncharuk >: > > > Igniters, > > > > Bumping up this discussion. The fix has been implemented and it is fine > > from the technical point of view, but since the fix did not make it to > the > > Ignite 2.0, the implemented fix [1] now will be a breaking change for > > current Ignite users. > > > > I see the following options: > > 1) Have the fix merged, but do not change the defaults - atomic caches > will > > still be allowed in transactions by default and only configuration change > > will make Ignite throw exceptions in this case > > 2) Have the fix merged as is and describe this change in the release > notes > > 3) Postpone the fix until Ignite 3.0 > > > > I would vote for option #1 and change only the defaults in Ignite 3.0. > > > > Thoughts? > > > > [1] https://issues.apache.org/jira/browse/IGNITE-2313 > > > > ср, 5 апр. 2017 г. в 22:53, Дмитрий Рябов : > > > > > IGNITE-2313 done, can you review it? > > > > > > PR: https://github.com/apache/ignite/pull/1709/files > > > JIRA: https://issues.apache.org/jira/browse/IGNITE-2313 > > > CI: http://ci.ignite.apache.org/viewType.html?buildTypeId= > > > IgniteTests_RatJavadoc&branch_IgniteTests=pull%2F1709% > > > 2Fhead&tab=buildTypeStatusDiv > > > > > > 2017-03-29 20:58 GMT+03:00 Denis Magda : > > > > > > > Sorry, I get lost in tickets. > > > > > > > > Yes, IGNITE-2313 has to be completed in 2.0 if we want to makes this > > > > change. > > > > > > > > — > > > > Denis > > > > > > > > > On Mar 29, 2017, at 2:12 AM, Дмитрий Рябов > > > > wrote: > > > > > > > > > > Savepoints marked for 2.1, exceptions for 2.0. Do you want me to > make > > > > > exceptions first? > > > > > > > > > > 2017-03-29 11:24 GMT+03:00 Дмитрий Рябов : > > > > > > > > > >> Finish savepoints or flag&exceptions for atomic operations? > > > > >> Not sure about savepoints. Exceptions - yes. > https://issues.apache. > > > > >> org/jira/browse/IGNITE-2313 isn't it? > > > > >> > > > > >> 2017-03-29 2:12 GMT+03:00 Denis Magda : > > > > >> > > > > >>> If we want to make the exception based approach the default one > > then > > > > the > > > > >>> task has to be released in 2.0. > > > > >>> > > > > >>> Dmitriy Ryabov, do you think you can finish it (dev, review, QA) > by > > > the > > > > >>> code freeze data (April 14)? > > > > >>> > > > > >>> — > > > > >>> Denis > > > > >>> > > > > On Mar 28, 2017, at 11:57 AM, Dmitriy Setrakyan < > > > > dsetrak...@apache.org> > > > > >>> wrote: > > > > > > > > On Tue, Mar 28, 2017 at 11:54 AM, Sergi Vladykin < > > > > >>> sergi.vlady...@gmail.com> > > > > wrote: > > > > > > > > > I think updating an Atomic cache from within a transaction > > > perfectly > > > > >>> makes > > > > > sense. For example for some kind of operations logging and so > > > forth. > > > > >>> Still > > > > > I agree that this can be error prone and forbidden by default. > I > > > > agree > > > > >>> with > > > > > Yakov that by default we should throw an exception and have > some > > > kind > > > > >>> of > > > > > flag (on cache or on TX?) to be able to explicitly enable this > > > > >>> behavior. > > > > > > > > > > > > > > > > > Agree, this sounds like a good idea. > > > > >>> > > > > >>> > > > > >> > > > > > > > > > > > > > >
