Re: [DISCUSSION] Code style. Variable abbrevations
Val.
> But then you said that common sense doesn't work. Code review IS common sense
> for the most part though.
Code review must catch errors like "No validation in the world prevent me from
typing manually "mess" instead of «msg»"
But, when one reviewer prefers `qry` and another requires `query` it confusing
and stressful.
> I'm not against enforced rules per se, but in this case, we are clearly
> struggling to come up with a rule that could work
Agree. Discussion shows that different opinions exist in community.
I’m relying on rules written in wiki.
If someone wants to change it - please go ahead with vote.
But, silently ignore rules created by ourselves is simply ridiculous.
> So many abbreviations simply make it look cryptic and weird -- this might
> easily confuse a newcomer.
Not agreed, but this seems to be a matter of taste.
> It's also clear why we're struggling. Just one example: we have the rule to
> abbreviate "exception" as "e».
+1 I’m also don’t like these changes :)
Seems we have to do it only for catch block variables.
Will fix.
> 23 июня 2021 г., в 07:24, Ivan Pavlukhina написал(а):
>
> Folks,
>
> I suppose we should rollout voting regarding abbreviations in Ignite 3.
>
> BTW, are you actually aware of any other project defining something similar
> to our abbreviations?
>
> Sent from my iPhone
>
>> On 23 Jun 2021, at 14:53, Nikita Ivanov wrote:
>>
>> What's worked for countless companies is the combination of the following:
>> 1. Well defined rules (abbreviations, code styles & documentation, code
>> organization, etc., etc.) for common/frequent use cases.
>> 2. Some basic tooling (wherever possible) plus a strong culture of code
>> reviews.
>> 3. And... common sense! None of the rules in (1) are axiomatic but rather a
>> guidance that requires common sense.
>>
>> Code review culture takes time and organizational discipline to become
>> effective & productive. Rules alone or some fiat enforcement is unlikely to
>> work.
>> --
>> Nikita Ivanov
>>
>>
>>
>> On Tue, Jun 22, 2021 at 4:44 PM Valentin Kulichenko <
>> valentin.kuliche...@gmail.com> wrote:
>>
>>> Nikolay,
>>>
>>> I'm confused. Konstantin gave you an example of a bad abbreviation that
>>> cannot be caught by automated validation, and you referred to the code
>>> review as a solution. But then you said that common sense doesn't work.
>>> Code review IS common sense for the most part though.
>>>
>>> Either way, I'm not against enforced rules per se, but in this case, we are
>>> clearly struggling to come up with a rule that could work. This and several
>>> previous discussions show that.
>>>
>>> It's also clear why we're struggling. Just one example: we have the rule to
>>> abbreviate "exception" as "e". This is perfectly fine for the following
>>> line:
>>>
>>> catch (Exception e) {}
>>>
>>> But then I see the following in your PR:
>>>
>>> boolean isE = false;
>>>
>>> I'm ready to argue that this is not an example of good code :) How would
>>> you propose to fix that? It's really hard to set up generic rules like that
>>> -- common sense is required.
>>>
>>> Also, in general, the impression I get from your PR is that the code
>>> becomes less readable. So many abbreviations simply make it look cryptic
>>> and weird -- this might easily confuse a newcomer. And honestly, I have no
>>> idea how to formalize this.
>>>
>>> What is the exact problem that you're trying to solve? Is it that we have a
>>> rule that is not followed because it's too complicated? In that case, I
>>> propose to cancel the rule and then try to come up with more general
>>> guidelines for contributors and reviewers. Again -- common sense is our
>>> friend here.
>>>
>>> WDYT?
>>>
>>> -Val
>>>
>>> On Tue, Jun 22, 2021 at 12:04 AM Nikolay Izhikov
>>> wrote:
>>>
Hello, Pavel
> But it doesn't mean that now we need to use only full names.
> Abbreviations may be used e.g. for local variables with short scope.
> Here we must follow common sense and existing practices of effective
naming.
> It shouldn't be a cargo cult as it is now.
But «common sense» has a very different definition for each community
member.
So, it seems, your proposal will end with different naming styles through
codebase.
Moreover, one reviewer will require «first» naming style and another
«second» because of personal common sense.
Personally, I believe strict automatically checked code style rules are
better than common sense rules.
Meanwhile, please, keep in mind that abbreviation rules is mandatory.
And this discussion shown there are many Ignite developers who want to
keep it.
If someone wants to change or make it optional - please, start a vote.
> 21 июня 2021 г., в 21:45, Pavel Kovalenko
написал(а):
>
>>> But do you have a plan what should we do with the subject?
>
> I think we need to just
Re: IGNITE-14812: Statistics
Hi, > Taras, can you, please, describe the features that was implemented in this merge? > How users supposed to use them? > Do we have plans to document? Sure. Alexander Belyak will describe and file ticket to documentation. On 23.06.2021 9:27, Nikolay Izhikov wrote: Hello, Taras. Thanks for feedback. AFAIK and as long as I can remember Ignite project try to minimize external dependencies usage and adds new external dependency only when there is no other way out. Does it mean we have to incapsulate every external library we want to use? Taras, can you, please, describe the features that was implemented in this merge? How users supposed to use them? Do we have plans to document? 23 июня 2021 г., в 09:21, Taras Ledkov написал(а): Hi, We have discussed BCrypt include/add dependency here [1]. AFAIK and as long as I can remember Ignite project try to minimize external dependencies usage and adds new external dependency only when there is no other way out. [1]. http://apache-ignite-developers.2346864.n4.nabble.com/Username-password-authentication-for-thin-clients-tp26058p26954.html On 23.06.2021 3:08, Valentin Kulichenko wrote: Dmitry, As the PMC chair, would you mind contacting legal regarding the matter? This is not the only example of such code (e.g. [1]), so we should look into this asap. [1] https://github.com/apache/ignite/blob/master/modules/core/src/main/java/org/mindrot/BCrypt.java As for this particular commit, can HLL be added as a dependency instead? Is there any particular reason to include the source code? @Sasha Belyak , can you please chime in? -Val On Tue, Jun 22, 2021 at 8:10 AM Dmitry Pavlov wrote: Hi Nikolai, thank you for noticing. I guess it's not about license, but about Intellectual Property (IP) ownership. AFAIK, Apache License 2.0 is here and AL 2.0 is definetely allowed to be used in the codebase for an Apache project ( https://www.apache.org/legal/resolved.html) But licenese and IP owner are 2 sligthly different things. E.g at the end of any website you can find: Copyright © 2021 The Apache Software Foundation, Licensed under the Apache License, Version 2.0. Incubated projects are mandated to transfer IP to the ASF. And I'm not aware of any exceptions. In this PR there are 5 classes which licenses with AL 2.0, but IP owner is 3rd party company. I'm a bit concerned about having such code in the project. I'd rather reverted it until we have approval from experts at mailing list: legal-disc...@apache.org Sincerely, Dmitriy Pavlov On 2021/06/22 14:56:49, Nikolay Izhikov wrote: Hello, Igniters. Recently huge commit was merged [1]. Taras, Alexander, can you, please, explain what is purpose of the commit? What feature it implemented? Looked inside the ticket and found no explanation. Description is "Add statistics collection and usage.» Do we have plans to document this new feature? Also, I see very strange license in added files [2] ``` * Copyright 2013 Aggregate Knowledge, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); ``` Is it OK to have those copyright inside ASF codebase? Is it some kind of external dependency we adopted as part of the code? Why do we need it? [1] https://github.com/apache/ignite/commit/503a98495433e1d0cf84f8be8c1e2adc57034fbb [2] https://github.com/apache/ignite/blob/master/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/stat/hll/serialization/IHLLMetadata.java -- Taras Ledkov Mail-To: tled...@gridgain.com -- Taras Ledkov Mail-To: tled...@gridgain.com
Re: IEP-75 Thin Client MsgPack Serialization for 3.0
Igniters, Looks like there are no objections and we can accept the proposal. I will close it tomorrow and move on to the thin client protocol itself. On Fri, Jun 18, 2021 at 12:10 PM Ivan Daschinsky wrote: > >> To make it fair. Ignite uses thread-local reusable buffers, see [1]. > I know, but PooledMessageBufferOutput is not about thread-local, isn't it? > > I'm not against about MsgPack, I'm for fair and not biased comparison. > > I suppose that MsgPack is an ideal candidate for thin client binary > protocol, not only for serializing some user data. > I am analyzed some tarantool connectors [1] [2] [3] and found msgpack based > protocol is a really good idea. > Also there is realy super fast and just 1 header library with liberal BSD-2 > licence for C -- msgpuck [4]. It used in tarantool itself > and in [1] and is stable and bullet proof. > > [1] -- https://github.com/igorcoding/asynctnt > [2] -- https://github.com/tarantool/tarantool-python/ > [3] -- https://github.com/tarantool/go-tarantool > [4] -- https://github.com/rtsisyk/msgpuck > > пт, 18 июн. 2021 г. в 11:44, Pavel Tupitsyn : > > > Ivan, > > > > > why do you use PooledMessageBufferOutput in benchmarks? > > > > To make it fair. Ignite uses thread-local reusable buffers, see [1]. > > > > > > > why packer from msgpack-core show better performance than > > > BinaryWriter. And I suppose that benchmark is not quite fair. > > > > MsgPack writes and reads less bytes, so it should be faster. > > Benchmark is not 100% fair, there are some small extra things that > > BinaryWriter does. > > > > However: > > 1. I don't think we care about super-precise benchmarks here, just the > > ballpark. > > 2. We are discussing the format, not the implementation. > > > > Important takeaway is: > > The format does not prevent someone from implementing it efficiently. > > > > > > > > [1] > > > > > https://github.com/apache/ignite/blob/master/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryWriterExImpl.java#L101 > > > > On Thu, Jun 17, 2021 at 10:40 PM Ivan Daschinsky > > wrote: > > > > > Andrey, here we discuss serialization format, as far as I understand. > > > Current implementation of ignite binary object serialization can be > > > rewritten. > > > If we do not care about fast (O(1)) field lookup, about schema > validation > > > and so on, msgpack is a really good option. It is also good for client > > > binary protocol, i.e. > > > tarantool uses it. > > > > > > >> Binarilizable interface forces user to write serialization code > > > I am talking about speed comparison. You can see from Pavel's data, > > > jackson-msgpack shows a pathetic performance comparing with a ignite's > > > default binary marshaller. If you want really fast serialization -- the > > > only option is to write code by yourself or use code generation. > Default > > > packer from msgpack-core java package is similar to BinaryWriter. So I > am > > > wondering why packer from msgpack-core show better performance than > > > BinaryWriter. And I suppose that benchmark is not quite fair. > > > > > > > > > чт, 17 июн. 2021 г. в 22:19, Andrey Mashenkov < > > andrey.mashen...@gmail.com > > > >: > > > > > > > Ivan, thankd for clarification. > > > > > > > > Binarilizable interface forces user to write serialization code. We > can > > > > support this or similar interface. > > > > But I'd like Ignite has some default serializer in addition. It can > be > > > also > > > > useful e.g. in compute for param and result serialization. > > > > > > > > BinaryObjectBuider requires an Ignite node for object construction, > but > > > we > > > > are looking for a detached builder and won't care about schemas. > > > > > > > > AFAIR, BinaryObject creates an objectReader on every single field > read > > > > operation. > > > > So, BO solution produces a lot of garbage and BO has noticable > overhead > > > > which affects the object footprint. > > > > > > > > чт, 17 июн. 2021 г., 21:41 Ivan Daschinsky : > > > > > > > > > >> Double checked -- there is not any links to PR either in IEP or > in > > > > jira > > > > > issue > > > > > Sorry, there is a link in IEP, but not in jira ticket. > > > > > > > > > > чт, 17 июн. 2021 г. в 21:39, Ivan Daschinsky >: > > > > > > > > > > > Andrey, > > > > > > >> arbitrary object graph > > > > > > Also, that is not true, msgpack format doesn't handle circular > > > graphs. > > > > > > Think about msgpack as binary json. You couldn't understand full > > > > > structure > > > > > > of message if you didn't deserialize it fully before, maps and > > arrays > > > > are > > > > > > serialized just as contiguos chunks > > > > > > of values/kv-pairs. Msgpack is a really dumb and simple format. > > > > > > > > > > > > Also, as for me, I cannot understand why current ignite > > serialization > > > > > > (BinaryObjectBuilder or Binarilizable) is slower than raw message > > > pack > > > > > > serializer. > > > > > > I suppose that this is an issue and we should investigate it. > > >
Re: IEP-75 Thin Client MsgPack Serialization for 3.0
Pavel, let me clarify one thing. 1. If this proposal is about binary protocol, then there is no objection I suppose. 2. If this proposal about serialization of key-value, there are some uncertainties, especially about complex objects. In this case this proposal needs more work. ср, 23 июн. 2021 г. в 14:07, Pavel Tupitsyn : > Igniters, > > Looks like there are no objections and we can accept the proposal. > I will close it tomorrow and move on to the thin client protocol itself. > > On Fri, Jun 18, 2021 at 12:10 PM Ivan Daschinsky > wrote: > > > >> To make it fair. Ignite uses thread-local reusable buffers, see [1]. > > I know, but PooledMessageBufferOutput is not about thread-local, isn't > it? > > > > I'm not against about MsgPack, I'm for fair and not biased comparison. > > > > I suppose that MsgPack is an ideal candidate for thin client binary > > protocol, not only for serializing some user data. > > I am analyzed some tarantool connectors [1] [2] [3] and found msgpack > based > > protocol is a really good idea. > > Also there is realy super fast and just 1 header library with liberal > BSD-2 > > licence for C -- msgpuck [4]. It used in tarantool itself > > and in [1] and is stable and bullet proof. > > > > [1] -- https://github.com/igorcoding/asynctnt > > [2] -- https://github.com/tarantool/tarantool-python/ > > [3] -- https://github.com/tarantool/go-tarantool > > [4] -- https://github.com/rtsisyk/msgpuck > > > > пт, 18 июн. 2021 г. в 11:44, Pavel Tupitsyn : > > > > > Ivan, > > > > > > > why do you use PooledMessageBufferOutput in benchmarks? > > > > > > To make it fair. Ignite uses thread-local reusable buffers, see [1]. > > > > > > > > > > why packer from msgpack-core show better performance than > > > > BinaryWriter. And I suppose that benchmark is not quite fair. > > > > > > MsgPack writes and reads less bytes, so it should be faster. > > > Benchmark is not 100% fair, there are some small extra things that > > > BinaryWriter does. > > > > > > However: > > > 1. I don't think we care about super-precise benchmarks here, just the > > > ballpark. > > > 2. We are discussing the format, not the implementation. > > > > > > Important takeaway is: > > > The format does not prevent someone from implementing it efficiently. > > > > > > > > > > > > [1] > > > > > > > > > https://github.com/apache/ignite/blob/master/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryWriterExImpl.java#L101 > > > > > > On Thu, Jun 17, 2021 at 10:40 PM Ivan Daschinsky > > > wrote: > > > > > > > Andrey, here we discuss serialization format, as far as I understand. > > > > Current implementation of ignite binary object serialization can be > > > > rewritten. > > > > If we do not care about fast (O(1)) field lookup, about schema > > validation > > > > and so on, msgpack is a really good option. It is also good for > client > > > > binary protocol, i.e. > > > > tarantool uses it. > > > > > > > > >> Binarilizable interface forces user to write serialization code > > > > I am talking about speed comparison. You can see from Pavel's data, > > > > jackson-msgpack shows a pathetic performance comparing with a > ignite's > > > > default binary marshaller. If you want really fast serialization -- > the > > > > only option is to write code by yourself or use code generation. > > Default > > > > packer from msgpack-core java package is similar to BinaryWriter. So > I > > am > > > > wondering why packer from msgpack-core show better performance than > > > > BinaryWriter. And I suppose that benchmark is not quite fair. > > > > > > > > > > > > чт, 17 июн. 2021 г. в 22:19, Andrey Mashenkov < > > > andrey.mashen...@gmail.com > > > > >: > > > > > > > > > Ivan, thankd for clarification. > > > > > > > > > > Binarilizable interface forces user to write serialization code. We > > can > > > > > support this or similar interface. > > > > > But I'd like Ignite has some default serializer in addition. It can > > be > > > > also > > > > > useful e.g. in compute for param and result serialization. > > > > > > > > > > BinaryObjectBuider requires an Ignite node for object construction, > > but > > > > we > > > > > are looking for a detached builder and won't care about schemas. > > > > > > > > > > AFAIR, BinaryObject creates an objectReader on every single field > > read > > > > > operation. > > > > > So, BO solution produces a lot of garbage and BO has noticable > > overhead > > > > > which affects the object footprint. > > > > > > > > > > чт, 17 июн. 2021 г., 21:41 Ivan Daschinsky : > > > > > > > > > > > >> Double checked -- there is not any links to PR either in IEP > or > > in > > > > > jira > > > > > > issue > > > > > > Sorry, there is a link in IEP, but not in jira ticket. > > > > > > > > > > > > чт, 17 июн. 2021 г. в 21:39, Ivan Daschinsky < > ivanda...@gmail.com > > >: > > > > > > > > > > > > > Andrey, > > > > > > > >> arbitrary object graph > > > > > > > Also, that is not true, msgpack format doesn't handle circular > > > > graphs
Re: [DISCUSSION] Brand new distributed environment testing framework ready to be reviewed/merged.
Folks, Here's the video [1] that explains the proposal in detail. Feel free to ask questions here. [1] https://www.youtube.com/watch?v=f-i9COU5uAQ (in Russian) On Tue, Jun 8, 2021 at 2:51 PM Anton Vinogradov wrote: > Igniters, > Let me present a framework, we developed, that allows automating Apache > Ignite testing on a real cluster. > The framework was initially presented at Ignite Summit. > > In brief, > The framework allows automating operations with any applications on a real > cluster using ssh in a form of a python test. > > Features: > - Ignite nodes can be started/stopped on a Docker or a real cluster with > any custom configuration > - Any AI version is supported (released or compiled from sources) > - Ignite forks are also supported «out of the box» > - Any other application execution is also possible, eg. we implemented > starters for Spark and Zookeeper > - Cluster can be managed using the control.sh, we made this a part of the > test API > - Custom Java applications can be executed remotely with/without a > built-in Ignite node or a Thin client > - Any ssh command can be executed remotely, and the result will be > available locally (at the python test) > - Network can be broken by iptables change to check communication issues > - Tests can be executed in parallel when cluster size bigger than tests > requirements > > As a result, seems, we may automate any testing scenario we can even > imagine. > > Framework based on Ducktape [1] library from Kafka team, that's why we > called it Ducktests. > > The Ducktests were developed during work on IEP-56 [2] and already were > used during the work on IEP-45 [3]. > IEP-45 measurement results examples [4.1] [4.2] were demonstrated at the > HighLoad conference last month. > > Code available at PR-9117 [5] and ready to be reviewed/merged. > Feel free to ask questions or make proposals. > > [1] https://ducktape-docs.readthedocs.io/en/latest/index.html > [2] > https://cwiki.apache.org/confluence/display/IGNITE/IEP-56%3A+Distributed+environment+tests > [3] > https://cwiki.apache.org/confluence/display/IGNITE/IEP-45%3A+Crash+Recovery+Speed-Up > [4.1] https://youtu.be/UZsvCNjbkww?t=1065 (in Russian) > [4.2] https://youtu.be/UZsvCNjbkww?t=1609 (in Russian) > [5] https://github.com/apache/ignite/pull/9117 >
Re: IEP-75 Thin Client MsgPack Serialization for 3.0
Ivan, Yes, this proposal is about the binary protocol - the way we serialize primitive values, the foundation for the thin client protocol. Key-value API still has some uncertainty around it, we'll discuss it separately. On Wed, Jun 23, 2021 at 2:31 PM Ivan Daschinsky wrote: > Pavel, let me clarify one thing. > > 1. If this proposal is about binary protocol, then there is no objection I > suppose. > 2. If this proposal about serialization of key-value, there are some > uncertainties, especially about complex objects. In this case this proposal > needs more work. > > ср, 23 июн. 2021 г. в 14:07, Pavel Tupitsyn : > > > Igniters, > > > > Looks like there are no objections and we can accept the proposal. > > I will close it tomorrow and move on to the thin client protocol itself. > > > > On Fri, Jun 18, 2021 at 12:10 PM Ivan Daschinsky > > wrote: > > > > > >> To make it fair. Ignite uses thread-local reusable buffers, see [1]. > > > I know, but PooledMessageBufferOutput is not about thread-local, isn't > > it? > > > > > > I'm not against about MsgPack, I'm for fair and not biased comparison. > > > > > > I suppose that MsgPack is an ideal candidate for thin client binary > > > protocol, not only for serializing some user data. > > > I am analyzed some tarantool connectors [1] [2] [3] and found msgpack > > based > > > protocol is a really good idea. > > > Also there is realy super fast and just 1 header library with liberal > > BSD-2 > > > licence for C -- msgpuck [4]. It used in tarantool itself > > > and in [1] and is stable and bullet proof. > > > > > > [1] -- https://github.com/igorcoding/asynctnt > > > [2] -- https://github.com/tarantool/tarantool-python/ > > > [3] -- https://github.com/tarantool/go-tarantool > > > [4] -- https://github.com/rtsisyk/msgpuck > > > > > > пт, 18 июн. 2021 г. в 11:44, Pavel Tupitsyn : > > > > > > > Ivan, > > > > > > > > > why do you use PooledMessageBufferOutput in benchmarks? > > > > > > > > To make it fair. Ignite uses thread-local reusable buffers, see [1]. > > > > > > > > > > > > > why packer from msgpack-core show better performance than > > > > > BinaryWriter. And I suppose that benchmark is not quite fair. > > > > > > > > MsgPack writes and reads less bytes, so it should be faster. > > > > Benchmark is not 100% fair, there are some small extra things that > > > > BinaryWriter does. > > > > > > > > However: > > > > 1. I don't think we care about super-precise benchmarks here, just > the > > > > ballpark. > > > > 2. We are discussing the format, not the implementation. > > > > > > > > Important takeaway is: > > > > The format does not prevent someone from implementing it efficiently. > > > > > > > > > > > > > > > > [1] > > > > > > > > > > > > > > https://github.com/apache/ignite/blob/master/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryWriterExImpl.java#L101 > > > > > > > > On Thu, Jun 17, 2021 at 10:40 PM Ivan Daschinsky < > ivanda...@gmail.com> > > > > wrote: > > > > > > > > > Andrey, here we discuss serialization format, as far as I > understand. > > > > > Current implementation of ignite binary object serialization can be > > > > > rewritten. > > > > > If we do not care about fast (O(1)) field lookup, about schema > > > validation > > > > > and so on, msgpack is a really good option. It is also good for > > client > > > > > binary protocol, i.e. > > > > > tarantool uses it. > > > > > > > > > > >> Binarilizable interface forces user to write serialization code > > > > > I am talking about speed comparison. You can see from Pavel's data, > > > > > jackson-msgpack shows a pathetic performance comparing with a > > ignite's > > > > > default binary marshaller. If you want really fast serialization -- > > the > > > > > only option is to write code by yourself or use code generation. > > > Default > > > > > packer from msgpack-core java package is similar to BinaryWriter. > So > > I > > > am > > > > > wondering why packer from msgpack-core show better performance than > > > > > BinaryWriter. And I suppose that benchmark is not quite fair. > > > > > > > > > > > > > > > чт, 17 июн. 2021 г. в 22:19, Andrey Mashenkov < > > > > andrey.mashen...@gmail.com > > > > > >: > > > > > > > > > > > Ivan, thankd for clarification. > > > > > > > > > > > > Binarilizable interface forces user to write serialization code. > We > > > can > > > > > > support this or similar interface. > > > > > > But I'd like Ignite has some default serializer in addition. It > can > > > be > > > > > also > > > > > > useful e.g. in compute for param and result serialization. > > > > > > > > > > > > BinaryObjectBuider requires an Ignite node for object > construction, > > > but > > > > > we > > > > > > are looking for a detached builder and won't care about schemas. > > > > > > > > > > > > AFAIR, BinaryObject creates an objectReader on every single field > > > read > > > > > > operation. > > > > > > So, BO solution produces a lot of garbage and BO has noticable > > > overhead > > > > > >
Re: IGNITE-14812: Statistics
Taras, It is true that we try to minimize dependencies, but there are some anyway. I think it's perfectly fine to add this library as a dependency. Will you be able to do this asap? It is surely better than reverting the commit :) -Val On Wed, Jun 23, 2021 at 12:32 AM Taras Ledkov wrote: > Hi, > > > Taras, can you, please, describe the features that was implemented in > this merge? > > How users supposed to use them? > > Do we have plans to document? > > Sure. Alexander Belyak will describe and file ticket to documentation. > > On 23.06.2021 9:27, Nikolay Izhikov wrote: > > Hello, Taras. > > > > Thanks for feedback. > > > >> AFAIK and as long as I can remember Ignite project try to minimize > external dependencies usage and adds new external dependency only when > there is no other way out. > > Does it mean we have to incapsulate every external library we want to > use? > > > > Taras, can you, please, describe the features that was implemented in > this merge? > > How users supposed to use them? > > Do we have plans to document? > > > > > >> 23 июня 2021 г., в 09:21, Taras Ledkov > написал(а): > >> > >> Hi, > >> > >> We have discussed BCrypt include/add dependency here [1]. > >> AFAIK and as long as I can remember Ignite project try to minimize > external dependencies usage > >> and adds new external dependency only when there is no other way out. > >> > >> [1]. > http://apache-ignite-developers.2346864.n4.nabble.com/Username-password-authentication-for-thin-clients-tp26058p26954.html > >> > >> On 23.06.2021 3:08, Valentin Kulichenko wrote: > >>> Dmitry, > >>> > >>> As the PMC chair, would you mind contacting legal regarding the matter? > >>> This is not the only example of such code (e.g. [1]), so we should look > >>> into this asap. > >>> > >>> [1] > >>> > https://github.com/apache/ignite/blob/master/modules/core/src/main/java/org/mindrot/BCrypt.java > >>> > >>> As for this particular commit, can HLL be added as a dependency > instead? Is > >>> there any particular reason to include the source code? @Sasha Belyak > >>> , can you please chime in? > >>> > >>> -Val > >>> > >>> On Tue, Jun 22, 2021 at 8:10 AM Dmitry Pavlov > wrote: > >>> > Hi Nikolai, > > thank you for noticing. I guess it's not about license, but about > Intellectual Property (IP) ownership. > > AFAIK, Apache License 2.0 is here and AL 2.0 is definetely allowed to > be > used in the codebase for an Apache project ( > https://www.apache.org/legal/resolved.html) > > But licenese and IP owner are 2 sligthly different things. E.g at the > end > of any website you can find: > Copyright © 2021 The Apache Software Foundation, Licensed under the > Apache > License, Version 2.0. > > Incubated projects are mandated to transfer IP to the ASF. And I'm not > aware of any exceptions. > > In this PR there are 5 classes which licenses with AL 2.0, but IP > owner is > 3rd party company. > > I'm a bit concerned about having such code in the project. I'd rather > reverted it until we have approval from experts at mailing list: > legal-disc...@apache.org > > Sincerely, > Dmitriy Pavlov > > On 2021/06/22 14:56:49, Nikolay Izhikov wrote: > > Hello, Igniters. > > > > Recently huge commit was merged [1]. > > > > Taras, Alexander, can you, please, explain what is purpose of the > commit? > > What feature it implemented? > > > > Looked inside the ticket and found no explanation. > > Description is "Add statistics collection and usage.» > > > > Do we have plans to document this new feature? > > > > Also, I see very strange license in added files [2] > > > > ``` > > * Copyright 2013 Aggregate Knowledge, Inc. > > * > > * Licensed under the Apache License, Version 2.0 (the "License"); > > ``` > > > > Is it OK to have those copyright inside ASF codebase? > > Is it some kind of external dependency we adopted as part of the > code? > > Why do we need it? > > > > [1] > > https://github.com/apache/ignite/commit/503a98495433e1d0cf84f8be8c1e2adc57034fbb > > [2] > > https://github.com/apache/ignite/blob/master/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/stat/hll/serialization/IHLLMetadata.java > > >> -- > >> Taras Ledkov > >> Mail-To: tled...@gridgain.com > >> > -- > Taras Ledkov > Mail-To: tled...@gridgain.com > >
Re: [DISCUSSION] Brand new distributed environment testing framework ready to be reviewed/merged.
Congrats, Anton, that's a valuable contribution! I attended your session at the Ignite Summit and wonder if you should share that recording with an English-speaking part of the community? - Denis On Wed, Jun 23, 2021 at 7:37 AM Anton Vinogradov wrote: > Folks, > > Here's the video [1] that explains the proposal in detail. > Feel free to ask questions here. > > [1] https://www.youtube.com/watch?v=f-i9COU5uAQ (in Russian) > > On Tue, Jun 8, 2021 at 2:51 PM Anton Vinogradov wrote: > > > Igniters, > > Let me present a framework, we developed, that allows automating Apache > > Ignite testing on a real cluster. > > The framework was initially presented at Ignite Summit. > > > > In brief, > > The framework allows automating operations with any applications on a > real > > cluster using ssh in a form of a python test. > > > > Features: > > - Ignite nodes can be started/stopped on a Docker or a real cluster with > > any custom configuration > > - Any AI version is supported (released or compiled from sources) > > - Ignite forks are also supported «out of the box» > > - Any other application execution is also possible, eg. we implemented > > starters for Spark and Zookeeper > > - Cluster can be managed using the control.sh, we made this a part of the > > test API > > - Custom Java applications can be executed remotely with/without a > > built-in Ignite node or a Thin client > > - Any ssh command can be executed remotely, and the result will be > > available locally (at the python test) > > - Network can be broken by iptables change to check communication issues > > - Tests can be executed in parallel when cluster size bigger than tests > > requirements > > > > As a result, seems, we may automate any testing scenario we can even > > imagine. > > > > Framework based on Ducktape [1] library from Kafka team, that's why we > > called it Ducktests. > > > > The Ducktests were developed during work on IEP-56 [2] and already were > > used during the work on IEP-45 [3]. > > IEP-45 measurement results examples [4.1] [4.2] were demonstrated at the > > HighLoad conference last month. > > > > Code available at PR-9117 [5] and ready to be reviewed/merged. > > Feel free to ask questions or make proposals. > > > > [1] https://ducktape-docs.readthedocs.io/en/latest/index.html > > [2] > > > https://cwiki.apache.org/confluence/display/IGNITE/IEP-56%3A+Distributed+environment+tests > > [3] > > > https://cwiki.apache.org/confluence/display/IGNITE/IEP-45%3A+Crash+Recovery+Speed-Up > > [4.1] https://youtu.be/UZsvCNjbkww?t=1065 (in Russian) > > [4.2] https://youtu.be/UZsvCNjbkww?t=1609 (in Russian) > > [5] https://github.com/apache/ignite/pull/9117 > > >
Re: IGNITE-14812: Statistics
AFAIK, it is enough to include mention of this library in NOTICE, please see here [1][2][3] [1] -- https://www.apache.org/legal/src-headers.html#3party [2] -- https://www.apache.org/legal/src-headers.html#notice [3] -- https://www.apache.org/legal/resolved.html ср, 23 июн. 2021 г. в 17:36, Valentin Kulichenko < valentin.kuliche...@gmail.com>: > Taras, > > It is true that we try to minimize dependencies, but there are some anyway. > I think it's perfectly fine to add this library as a dependency. > > Will you be able to do this asap? It is surely better than reverting the > commit :) > > -Val > > On Wed, Jun 23, 2021 at 12:32 AM Taras Ledkov > wrote: > > > Hi, > > > > > Taras, can you, please, describe the features that was implemented in > > this merge? > > > How users supposed to use them? > > > Do we have plans to document? > > > > Sure. Alexander Belyak will describe and file ticket to documentation. > > > > On 23.06.2021 9:27, Nikolay Izhikov wrote: > > > Hello, Taras. > > > > > > Thanks for feedback. > > > > > >> AFAIK and as long as I can remember Ignite project try to minimize > > external dependencies usage and adds new external dependency only when > > there is no other way out. > > > Does it mean we have to incapsulate every external library we want to > > use? > > > > > > Taras, can you, please, describe the features that was implemented in > > this merge? > > > How users supposed to use them? > > > Do we have plans to document? > > > > > > > > >> 23 июня 2021 г., в 09:21, Taras Ledkov > > написал(а): > > >> > > >> Hi, > > >> > > >> We have discussed BCrypt include/add dependency here [1]. > > >> AFAIK and as long as I can remember Ignite project try to minimize > > external dependencies usage > > >> and adds new external dependency only when there is no other way out. > > >> > > >> [1]. > > > http://apache-ignite-developers.2346864.n4.nabble.com/Username-password-authentication-for-thin-clients-tp26058p26954.html > > >> > > >> On 23.06.2021 3:08, Valentin Kulichenko wrote: > > >>> Dmitry, > > >>> > > >>> As the PMC chair, would you mind contacting legal regarding the > matter? > > >>> This is not the only example of such code (e.g. [1]), so we should > look > > >>> into this asap. > > >>> > > >>> [1] > > >>> > > > https://github.com/apache/ignite/blob/master/modules/core/src/main/java/org/mindrot/BCrypt.java > > >>> > > >>> As for this particular commit, can HLL be added as a dependency > > instead? Is > > >>> there any particular reason to include the source code? @Sasha Belyak > > >>> , can you please chime in? > > >>> > > >>> -Val > > >>> > > >>> On Tue, Jun 22, 2021 at 8:10 AM Dmitry Pavlov > > wrote: > > >>> > > Hi Nikolai, > > > > thank you for noticing. I guess it's not about license, but about > > Intellectual Property (IP) ownership. > > > > AFAIK, Apache License 2.0 is here and AL 2.0 is definetely allowed > to > > be > > used in the codebase for an Apache project ( > > https://www.apache.org/legal/resolved.html) > > > > But licenese and IP owner are 2 sligthly different things. E.g at > the > > end > > of any website you can find: > > Copyright © 2021 The Apache Software Foundation, Licensed under the > > Apache > > License, Version 2.0. > > > > Incubated projects are mandated to transfer IP to the ASF. And I'm > not > > aware of any exceptions. > > > > In this PR there are 5 classes which licenses with AL 2.0, but IP > > owner is > > 3rd party company. > > > > I'm a bit concerned about having such code in the project. I'd > rather > > reverted it until we have approval from experts at mailing list: > > legal-disc...@apache.org > > > > Sincerely, > > Dmitriy Pavlov > > > > On 2021/06/22 14:56:49, Nikolay Izhikov > wrote: > > > Hello, Igniters. > > > > > > Recently huge commit was merged [1]. > > > > > > Taras, Alexander, can you, please, explain what is purpose of the > > commit? > > > What feature it implemented? > > > > > > Looked inside the ticket and found no explanation. > > > Description is "Add statistics collection and usage.» > > > > > > Do we have plans to document this new feature? > > > > > > Also, I see very strange license in added files [2] > > > > > > ``` > > > * Copyright 2013 Aggregate Knowledge, Inc. > > > * > > > * Licensed under the Apache License, Version 2.0 (the "License"); > > > ``` > > > > > > Is it OK to have those copyright inside ASF codebase? > > > Is it some kind of external dependency we adopted as part of the > > code? > > > Why do we need it? > > > > > > [1] > > > > > https://github.com/apache/ignite/commit/503a98495433e1d0cf84f8be8c1e2adc57034fbb > > > [2] > > > > > https://github.com/apache/ignite/blob/master/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query
